chore: remove deprecated apis on superset, get_or_create_table, sqlla…

…b_viz (#24375)
apache · Jun 14, 2023 · c69634d · c69634d
1 parent 62b4564
commit c69634d
Show file tree

Hide file tree

Showing 6 changed files with 2 additions and 230 deletions.
diff --git 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
@@ -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
@@ -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
@@ -49,26 +49,18 @@
 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
 from superset.dashboards.permalink.exceptions import DashboardPermalinkGetFailedError
 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.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 @@
  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 @@ 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/<int:database_id>/<table_name>/<schema>/")
  @event_logger.log_this

diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py
@@ -1497,8 +1497,6 @@ def test_sql_lab_permissions(self):
  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
@@ -22,7 +22,6 @@
 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 @@ 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")