Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove deprecated apis on superset, get_or_create_table, sqllab_viz #24375

Merged
merged 13 commits into from
Jun 14, 2023
Merged
1 change: 0 additions & 1 deletion RESOURCES/STANDARD_ROLES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand Down
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
2 changes: 0 additions & 2 deletions superset/security/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
129 changes: 1 addition & 128 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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 (
Expand Down Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions tests/integration_tests/security_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
97 changes: 0 additions & 97 deletions tests/integration_tests/sqllab_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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")
Expand Down