From c69634df279054de0eb145bdcc796cb8fdd26dd8 Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Wed, 14 Jun 2023 08:48:29 +0100 Subject: [PATCH] chore: remove deprecated apis on superset, get_or_create_table, sqllab_viz (#24375) --- RESOURCES/STANDARD_ROLES.md | 1 - UPDATING.md | 1 + superset/security/manager.py | 2 - superset/views/core.py | 129 +--------------------- tests/integration_tests/security_tests.py | 2 - tests/integration_tests/sqllab_tests.py | 97 ---------------- 6 files changed, 2 insertions(+), 230 deletions(-) diff --git a/RESOURCES/STANDARD_ROLES.md b/RESOURCES/STANDARD_ROLES.md index 27af7bba2f..cbef53abbe 100644 --- a/RESOURCES/STANDARD_ROLES.md +++ b/RESOURCES/STANDARD_ROLES.md @@ -64,7 +64,6 @@ |can user slices on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can favstar on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can import dashboards on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| -|can sqllab viz on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:| |can schemas on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| |can sqllab history on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:| |can publish on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| diff --git a/UPDATING.md b/UPDATING.md index 72c9f7d6bf..3db0ab6a4b 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -34,6 +34,7 @@ assists people when migrating to a new version. ### Breaking Changes +- [24375](https://github.com/apache/superset/pull/24375): Removed deprecated API `/superset/get_or_create_table/...`, `/superset/sqllab_viz` - [24360](https://github.com/apache/superset/pull/24360): Removed deprecated APIs `/superset/stop_query/...`, `/superset/queries/...`, `/superset/search_queries` - [24353](https://github.com/apache/superset/pull/24353): Removed deprecated APIs `/copy_dash/int:dashboard_id/`, `/save_dash/int:dashboard_id/`, `/add_slices/int:dashboard_id/`. - [24198](https://github.com/apache/superset/pull/24198) The FAB views `User Registrations` and `User's Statistics` have been changed to Admin only. To re-enable them for non-admin users, please add the following perms to your custom role: `menu access on User's Statistics` and `menu access on User Registrations`. diff --git a/superset/security/manager.py b/superset/security/manager.py index 41e522f3ea..1e01bc7417 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -238,8 +238,6 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods ("can_estimate_query_cost", "SQL Lab"), ("can_export_csv", "SQLLab"), ("can_sqllab_history", "Superset"), - ("can_sqllab_viz", "Superset"), - ("can_sqllab_table_viz", "Superset"), # Deprecated permission remove on 3.0.0 ("can_sqllab", "Superset"), ("can_test_conn", "Superset"), # Deprecated permission remove on 3.0.0 ("can_activate", "TabStateView"), diff --git a/superset/views/core.py b/superset/views/core.py index 8483733730..782fad4978 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -49,12 +49,7 @@ from superset.charts.commands.exceptions import ChartNotFoundError from superset.charts.dao import ChartDAO from superset.common.chart_data import ChartDataResultFormat, ChartDataResultType from superset.connectors.base.models import BaseDatasource -from superset.connectors.sqla.models import ( - AnnotationDatasource, - SqlaTable, - SqlMetric, - TableColumn, -) +from superset.connectors.sqla.models import AnnotationDatasource, SqlaTable from superset.dashboards.commands.exceptions import DashboardAccessDeniedError from superset.dashboards.commands.importers.v0 import ImportDashboardsCommand from superset.dashboards.permalink.commands.get import GetDashboardPermalinkCommand @@ -62,13 +57,10 @@ from superset.dashboards.permalink.exceptions import DashboardPermalinkGetFailed from superset.databases.dao import DatabaseDAO from superset.datasets.commands.exceptions import DatasetNotFoundError from superset.datasource.dao import DatasourceDAO -from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.exceptions import ( CacheLoadError, DatabaseNotFound, - SupersetErrorException, SupersetException, - SupersetGenericErrorException, SupersetSecurityException, ) from superset.explore.form_data.commands.create import CreateFormDataCommand @@ -82,7 +74,6 @@ from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.models.sql_lab import Query, TabState from superset.models.user_attributes import UserAttribute -from superset.sql_parse import ParsedQuery from superset.superset_typing import FlaskResponse from superset.tasks.async_queries import load_explore_json_into_cache from superset.utils import core as utils @@ -100,9 +91,7 @@ from superset.views.base import ( get_error_msg, handle_api_exception, json_error_response, - json_errors_response, json_success, - validate_sqlatable, ) from superset.views.log.dao import LogDAO from superset.views.utils import ( @@ -1290,122 +1279,6 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods def log(self) -> FlaskResponse: # pylint: disable=no-self-use return Response(status=200) - @has_access - @expose("/get_or_create_table/", methods=("POST",)) - @event_logger.log_this - @deprecated(new_target="api/v1/dataset/get_or_create/") - def sqllab_table_viz(self) -> FlaskResponse: # pylint: disable=no-self-use - """Gets or creates a table object with attributes passed to the API. - - It expects the json with params: - * datasourceName - e.g. table name, required - * dbId - database id, required - * schema - table schema, optional - * templateParams - params for the Jinja templating syntax, optional - :return: Response - """ - data = json.loads(request.form["data"]) - table_name = data["datasourceName"] - database_id = data["dbId"] - table = ( - db.session.query(SqlaTable) - .filter_by(database_id=database_id, table_name=table_name) - .one_or_none() - ) - if not table: - # Create table if doesn't exist. - with db.session.no_autoflush: - table = SqlaTable(table_name=table_name, owners=[g.user]) - table.database_id = database_id - table.database = ( - db.session.query(Database).filter_by(id=database_id).one() - ) - table.schema = data.get("schema") - table.template_params = data.get("templateParams") - # needed for the table validation. - # fn can be deleted when this endpoint is removed - validate_sqlatable(table) - - db.session.add(table) - table.fetch_metadata() - db.session.commit() - - return json_success(json.dumps({"table_id": table.id})) - - @has_access - @expose("/sqllab_viz/", methods=("POST",)) - @event_logger.log_this - @deprecated(new_target="api/v1/dataset/") - def sqllab_viz(self) -> FlaskResponse: # pylint: disable=no-self-use - data = json.loads(request.form["data"]) - try: - table_name = data["datasourceName"] - database_id = data["dbId"] - except KeyError as ex: - raise SupersetGenericErrorException( - __( - "One or more required fields are missing in the request. Please try " - "again, and if the problem persists contact your administrator." - ), - status=400, - ) from ex - database = db.session.query(Database).get(database_id) - if not database: - raise SupersetErrorException( - SupersetError( - message=__("The database was not found."), - error_type=SupersetErrorType.DATABASE_NOT_FOUND_ERROR, - level=ErrorLevel.ERROR, - ), - status=404, - ) - table = ( - db.session.query(SqlaTable) - .filter_by(database_id=database_id, table_name=table_name) - .one_or_none() - ) - - if table: - return json_errors_response( - [ - SupersetError( - message=f"Dataset [{table_name}] already exists", - error_type=SupersetErrorType.GENERIC_BACKEND_ERROR, - level=ErrorLevel.WARNING, - ) - ], - status=422, - ) - - table = SqlaTable(table_name=table_name, owners=[g.user]) - table.database = database - table.schema = data.get("schema") - table.template_params = data.get("templateParams") - table.is_sqllab_view = True - table.sql = ParsedQuery(data.get("sql")).stripped() - db.session.add(table) - cols = [] - for config_ in data.get("columns"): - column_name = config_.get("column_name") or config_.get("name") - col = TableColumn( - column_name=column_name, - filterable=True, - groupby=True, - is_dttm=config_.get("is_dttm", False), - type=config_.get("type", False), - ) - cols.append(col) - - table.columns = cols - table.metrics = [SqlMetric(metric_name="count", expression="count(*)")] - db.session.commit() - - return json_success( - json.dumps( - {"table_id": table.id, "data": sanitize_datasource_data(table.data)} - ) - ) - @has_access @expose("/extra_table_metadata////") @event_logger.log_this diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index e5b376dc24..e7b85498a5 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1497,8 +1497,6 @@ class TestRolePermission(SupersetTestCase): self.assertIn(("can_csv", "Superset"), sql_lab_set) self.assertIn(("can_read", "Database"), sql_lab_set) self.assertIn(("can_read", "SavedQuery"), sql_lab_set) - self.assertIn(("can_sqllab_viz", "Superset"), sql_lab_set) - self.assertIn(("can_sqllab_table_viz", "Superset"), sql_lab_set) self.assertIn(("can_sqllab", "Superset"), sql_lab_set) self.assertIn(("menu_access", "SQL Lab"), sql_lab_set) diff --git a/tests/integration_tests/sqllab_tests.py b/tests/integration_tests/sqllab_tests.py index 6880fb2325..fbab4d98d2 100644 --- a/tests/integration_tests/sqllab_tests.py +++ b/tests/integration_tests/sqllab_tests.py @@ -22,7 +22,6 @@ from datetime import datetime import pytest from celery.exceptions import SoftTimeLimitExceeded from parameterized import parameterized -from random import random from unittest import mock import prison @@ -358,102 +357,6 @@ class TestSqlLab(SupersetTestCase): self.assertEqual(len(data), results.size) self.assertEqual(len(cols), len(results.columns)) - def test_sqllab_viz(self): - self.login("admin") - examples_dbid = get_example_database().id - payload = { - "chartType": "dist_bar", - "datasourceName": f"test_viz_flow_table_{random()}", - "schema": "superset", - "columns": [ - { - "is_dttm": False, - "type": "STRING", - "column_name": f"viz_type_{random()}", - }, - { - "is_dttm": False, - "type": "OBJECT", - "column_name": f"ccount_{random()}", - }, - ], - "sql": """\ - SELECT * - FROM birth_names - LIMIT 10""", - "dbId": examples_dbid, - } - data = {"data": json.dumps(payload)} - resp = self.get_json_resp("/superset/sqllab_viz/", data=data) - self.assertIn("table_id", resp) - - # ensure owner is set correctly - table_id = resp["table_id"] - table = db.session.query(SqlaTable).filter_by(id=table_id).one() - self.assertEqual([owner.username for owner in table.owners], ["admin"]) - view_menu = security_manager.find_view_menu(table.get_perm()) - assert view_menu is not None - - # Cleanup - db.session.delete(table) - db.session.commit() - - def test_sqllab_viz_bad_payload(self): - self.login("admin") - payload = { - "chartType": "dist_bar", - "schema": "superset", - "columns": [ - { - "is_dttm": False, - "type": "STRING", - "column_name": f"viz_type_{random()}", - }, - { - "is_dttm": False, - "type": "OBJECT", - "column_name": f"ccount_{random()}", - }, - ], - "sql": """\ - SELECT * - FROM birth_names - LIMIT 10""", - } - data = {"data": json.dumps(payload)} - url = "/superset/sqllab_viz/" - response = self.client.post(url, data=data, follow_redirects=True) - assert response.status_code == 400 - - def test_sqllab_table_viz(self): - self.login("admin") - examples_db = get_example_database() - with examples_db.get_sqla_engine_with_context() as engine: - engine.execute("DROP TABLE IF EXISTS test_sqllab_table_viz") - engine.execute("CREATE TABLE test_sqllab_table_viz AS SELECT 2 as col") - - examples_dbid = examples_db.id - - payload = { - "datasourceName": "test_sqllab_table_viz", - "columns": [], - "dbId": examples_dbid, - } - - data = {"data": json.dumps(payload)} - resp = self.get_json_resp("/superset/get_or_create_table/", data=data) - self.assertIn("table_id", resp) - - # ensure owner is set correctly - table_id = resp["table_id"] - table = db.session.query(SqlaTable).filter_by(id=table_id).one() - self.assertEqual([owner.username for owner in table.owners], ["admin"]) - db.session.delete(table) - - with get_example_database().get_sqla_engine_with_context() as engine: - engine.execute("DROP TABLE test_sqllab_table_viz") - db.session.commit() - @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_sql_limit(self): self.login("admin")