From cb7cccc38d58919e711504e77a06738db1d05d85 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 13 Jun 2023 07:42:08 +0100 Subject: [PATCH 1/7] chore: remove deprecated apis on superset, get_or_create_table, sqllab_viz --- superset/security/manager.py | 2 - superset/views/core.py | 116 ---------------------- tests/integration_tests/security_tests.py | 1 - tests/integration_tests/sqllab_tests.py | 97 ------------------ 4 files changed, 216 deletions(-) diff --git a/superset/security/manager.py b/superset/security/manager.py index 0df9ad03f77d6..838da81227c0b 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -237,8 +237,6 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods ("can_export_csv", "SQLLab"), ("can_sql_json", "Superset"), # Deprecated permission remove on 3.0.0 ("can_sqllab_history", "Superset"), - ("can_sqllab_viz", "Superset"), - ("can_sqllab_table_viz", "Superset"), # Deprecated permission remove on 3.0.0 ("can_sqllab", "Superset"), ("can_stop_query", "Superset"), # Deprecated permission remove on 3.0.0 ("can_test_conn", "Superset"), # Deprecated permission remove on 3.0.0 diff --git a/superset/views/core.py b/superset/views/core.py index d9f9fd61b8adf..10cb378d890c4 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -1563,122 +1563,6 @@ def dashboard_permalink( # pylint: disable=no-self-use 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 ec16541389f8e..dd1c141ca28c3 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1501,7 +1501,6 @@ def test_sql_lab_permissions(self): self.assertIn(("can_read", "Database"), sql_lab_set) self.assertIn(("can_read", "SavedQuery"), sql_lab_set) self.assertIn(("can_sql_json", "Superset"), 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) diff --git a/tests/integration_tests/sqllab_tests.py b/tests/integration_tests/sqllab_tests.py index e9892b1d36c4b..f31a11010b6a1 100644 --- a/tests/integration_tests/sqllab_tests.py +++ b/tests/integration_tests/sqllab_tests.py @@ -23,7 +23,6 @@ import pytest from celery.exceptions import SoftTimeLimitExceeded from parameterized import parameterized -from random import random from unittest import mock import prison @@ -485,102 +484,6 @@ def test_pa_conversion_dict(self): 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") From f360a7cc9c0d1b6782bc7aa4d4f908fad819e483 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 13 Jun 2023 07:44:13 +0100 Subject: [PATCH 2/7] chore: remove deprecated apis on superset, get_or_create_table, sqllab_viz --- RESOURCES/STANDARD_ROLES.md | 1 - UPDATING.md | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/RESOURCES/STANDARD_ROLES.md b/RESOURCES/STANDARD_ROLES.md index 7e804ca75f26f..245b23977b8f1 100644 --- a/RESOURCES/STANDARD_ROLES.md +++ b/RESOURCES/STANDARD_ROLES.md @@ -66,7 +66,6 @@ |can estimate query cost 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 search queries on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:| -|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 copy dash on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O| diff --git a/UPDATING.md b/UPDATING.md index 7ec7a2651840f..0fbd2763399e9 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -34,6 +34,7 @@ assists people when migrating to a new version. ### Breaking Changes +- [---](https://github.com/apache/superset/pull/---): Removed deprecated API `/superset/get_or_create_table/...`, `/superset/sqllab_viz` - [24345](https://github.com/apache/superset/pull/24345) Converts `ENABLE_BROAD_ACTIVITY_ACCESS` and `MENU_HIDE_USER_INFO` into feature flags and changes the value of `ENABLE_BROAD_ACTIVITY_ACCESS` to `False` as it's more secure. - [24342](https://github.com/apache/superset/pull/24342): Removed deprecated API `/superset/tables///...` - [24335](https://github.com/apache/superset/pull/24335): Removed deprecated API `/superset/filter////` From 5bee0a8d59bd20d0d38e32d35d377883759d6947 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 13 Jun 2023 07:48:27 +0100 Subject: [PATCH 3/7] fix lint --- superset/views/core.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index 10cb378d890c4..ce88040bf8e05 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -57,12 +57,7 @@ from superset.common.chart_data import ChartDataResultFormat, ChartDataResultType from superset.common.db_query_status import QueryStatus 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.constants import QUERY_EARLY_CANCEL_KEY from superset.dashboards.commands.exceptions import DashboardAccessDeniedError from superset.dashboards.commands.importers.v0 import ImportDashboardsCommand @@ -83,7 +78,6 @@ SupersetCancelQueryException, SupersetErrorException, SupersetException, - SupersetGenericErrorException, SupersetSecurityException, SupersetTimeoutException, ) @@ -141,7 +135,6 @@ json_error_response, json_errors_response, json_success, - validate_sqlatable, ) from superset.views.log.dao import LogDAO from superset.views.sql_lab.schemas import SqlJsonPayloadSchema From 5c001ecda78a1665b0b1ce163b37082a2fb0e75a Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 13 Jun 2023 08:43:57 +0100 Subject: [PATCH 4/7] fix tests --- tests/integration_tests/security_tests.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py index dd1c141ca28c3..05070cab63f5e 100644 --- a/tests/integration_tests/security_tests.py +++ b/tests/integration_tests/security_tests.py @@ -1501,7 +1501,6 @@ def test_sql_lab_permissions(self): self.assertIn(("can_read", "Database"), sql_lab_set) self.assertIn(("can_read", "SavedQuery"), sql_lab_set) self.assertIn(("can_sql_json", "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) From 263100a41975e5cc04ba8fb50d0daa40001ff5ef Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 13 Jun 2023 08:57:22 +0100 Subject: [PATCH 5/7] update updating with PR number --- UPDATING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPDATING.md b/UPDATING.md index 0fbd2763399e9..45daaa85f5200 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -34,7 +34,7 @@ assists people when migrating to a new version. ### Breaking Changes -- [---](https://github.com/apache/superset/pull/---): Removed deprecated API `/superset/get_or_create_table/...`, `/superset/sqllab_viz` +- [24375](https://github.com/apache/superset/pull/24375): Removed deprecated API `/superset/get_or_create_table/...`, `/superset/sqllab_viz` - [24345](https://github.com/apache/superset/pull/24345) Converts `ENABLE_BROAD_ACTIVITY_ACCESS` and `MENU_HIDE_USER_INFO` into feature flags and changes the value of `ENABLE_BROAD_ACTIVITY_ACCESS` to `False` as it's more secure. - [24342](https://github.com/apache/superset/pull/24342): Removed deprecated API `/superset/tables///...` - [24335](https://github.com/apache/superset/pull/24335): Removed deprecated API `/superset/filter////` From 74a03bc8c6147a4bdda411cfffc4348d6f3f2d9b Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 13 Jun 2023 11:52:52 +0100 Subject: [PATCH 6/7] fix lint --- superset/views/core.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index a6164f1fbbbda..b86ababf291c6 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -62,12 +62,10 @@ 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, SupersetCancelQueryException, - SupersetErrorException, SupersetException, SupersetSecurityException, ) @@ -82,7 +80,6 @@ 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 @@ -101,7 +98,6 @@ get_error_msg, handle_api_exception, json_error_response, - json_errors_response, json_success, ) from superset.views.log.dao import LogDAO From e32e0ebb660ccd0fa4a7932a45ef54e3529ea720 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 13 Jun 2023 16:25:58 +0100 Subject: [PATCH 7/7] fix lint --- superset/views/core.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/superset/views/core.py b/superset/views/core.py index 4c6de07b419a7..782fad4978b8f 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -50,13 +50,6 @@ from superset.common.chart_data import ChartDataResultFormat, ChartDataResultType from superset.connectors.base.models import BaseDatasource from superset.connectors.sqla.models import AnnotationDatasource, SqlaTable -from superset.constants import QUERY_EARLY_CANCEL_KEY -from superset.connectors.sqla.models import ( - AnnotationDatasource, - SqlaTable, - SqlMetric, - TableColumn, -) from superset.dashboards.commands.exceptions import DashboardAccessDeniedError from superset.dashboards.commands.importers.v0 import ImportDashboardsCommand from superset.dashboards.permalink.commands.get import GetDashboardPermalinkCommand @@ -67,8 +60,6 @@ from superset.exceptions import ( CacheLoadError, DatabaseNotFound, - SupersetCancelQueryException, - SupersetErrorException, SupersetException, SupersetSecurityException, )