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 api /superset/tables/<int:db_id>/... #24342

Merged
merged 8 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion RESOURCES/STANDARD_ROLES.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
|can sync druid source on Superset|:heavy_check_mark:|O|O|O|
|can explore on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
|can fave slices on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
|can tables on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
|can slice json on Superset|:heavy_check_mark:|:heavy_check_mark:|:heavy_check_mark:|O|
|can approve on Superset|:heavy_check_mark:|O|O|O|
|can explore json 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 @@ -33,6 +33,7 @@ assists people when migrating to a new version.

### Breaking Changes

- [24342](https://github.com/apache/superset/pull/24342): Removed deprecated API `/superset/tables/<int:db_id>/<schema>/...`
- [24266](https://github.com/apache/superset/pull/24266) Remove the `ENABLE_ACCESS_REQUEST` config parameter and the associated request/approval workflows.
- [24330](https://github.com/apache/superset/pull/24330) Removes `getUiOverrideRegistry` from `ExtensionsRegistry`.
- [23933](https://github.com/apache/superset/pull/23933) Removes the deprecated Multiple Line Charts.
Expand Down
108 changes: 0 additions & 108 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import simplejson as json
from flask import abort, flash, g, redirect, render_template, request, Response
from flask_appbuilder import expose
from flask_appbuilder.models.sqla.interface import SQLAInterface
from flask_appbuilder.security.decorators import (
has_access,
has_access_api,
Expand All @@ -39,7 +38,6 @@
from flask_babel import gettext as __, lazy_gettext as _
from sqlalchemy import and_, or_
from sqlalchemy.exc import DBAPIError, NoSuchModuleError, SQLAlchemyError
from sqlalchemy.orm import lazyload, load_only

from superset import (
app,
Expand Down Expand Up @@ -73,7 +71,6 @@
from superset.dashboards.permalink.exceptions import DashboardPermalinkGetFailedError
from superset.databases.commands.exceptions import DatabaseInvalidError
from superset.databases.dao import DatabaseDAO
from superset.databases.filters import DatabaseFilter
from superset.databases.utils import make_url_safe
from superset.datasets.commands.exceptions import DatasetNotFoundError
from superset.datasource.dao import DatasourceDAO
Expand Down Expand Up @@ -945,111 +942,6 @@ def save_or_overwrite_slice(

return json_success(json.dumps(response))

@api
@has_access_api
@event_logger.log_this
@expose("/tables/<int:db_id>/<schema>/")
@expose("/tables/<int:db_id>/<schema>/<force_refresh>/")
@deprecated(new_target="api/v1/database/<int:pk>/tables/")
def tables( # pylint: disable=no-self-use
self,
db_id: int,
schema: str,
force_refresh: str = "false",
) -> FlaskResponse:
"""Endpoint to fetch the list of tables for given database"""

force_refresh_parsed = force_refresh.lower() == "true"
schema_parsed = utils.parse_js_uri_path_item(schema, eval_undefined=True)

if not schema_parsed:
return json_error_response(_("Schema undefined"), status=422)

# Guarantees database filtering by security access
database = (
DatabaseFilter("id", SQLAInterface(Database, db.session))
.apply(
db.session.query(Database),
None,
)
.filter_by(id=db_id)
.one_or_none()
)

if not database:
return json_error_response(
__("Database not found: %(id)s", id=db_id), status=404
)

try:
tables = security_manager.get_datasources_accessible_by_user(
database=database,
schema=schema_parsed,
datasource_names=sorted(
utils.DatasourceName(*datasource_name)
for datasource_name in database.get_all_table_names_in_schema(
schema=schema_parsed,
force=force_refresh_parsed,
cache=database.table_cache_enabled,
cache_timeout=database.table_cache_timeout,
)
),
)

views = security_manager.get_datasources_accessible_by_user(
database=database,
schema=schema_parsed,
datasource_names=sorted(
utils.DatasourceName(*datasource_name)
for datasource_name in database.get_all_view_names_in_schema(
schema=schema_parsed,
force=force_refresh_parsed,
cache=database.table_cache_enabled,
cache_timeout=database.table_cache_timeout,
)
),
)
except SupersetException as ex:
return json_error_response(ex.message, ex.status)

extra_dict_by_name = {
table.name: table.extra_dict
for table in (
db.session.query(SqlaTable)
.filter(
SqlaTable.database_id == database.id,
SqlaTable.schema == schema_parsed,
)
.options(
load_only(SqlaTable.schema, SqlaTable.table_name, SqlaTable.extra),
lazyload(SqlaTable.columns),
lazyload(SqlaTable.metrics),
)
).all()
}

options = sorted(
[
{
"value": table.table,
"type": "table",
"extra": extra_dict_by_name.get(table.table, None),
}
for table in tables
]
+ [
{
"value": view.table,
"type": "view",
}
for view in views
],
key=lambda item: item["value"],
)

payload = {"tableLength": len(tables) + len(views), "options": options}
return json_success(json.dumps(payload))

@api
@has_access_api
@event_logger.log_this
Expand Down
79 changes: 0 additions & 79 deletions tests/integration_tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,85 +151,6 @@ def test_viz_cache_key(self):

self.assertEqual(cache_key_with_groupby, viz.cache_key(qobj))

def test_get_superset_tables_not_allowed(self):
example_db = superset.utils.database.get_example_database()
schema_name = self.default_schema_backend_map[example_db.backend]
self.login(username="gamma")
uri = f"superset/tables/{example_db.id}/{schema_name}/"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)

@pytest.mark.usefixtures("load_energy_table_with_slice")
def test_get_superset_tables_allowed(self):
session = db.session
table_name = "energy_usage"
role_name = "dummy_role"
self.logout()
self.login(username="gamma")
gamma_user = security_manager.find_user(username="gamma")
security_manager.add_role(role_name)
dummy_role = security_manager.find_role(role_name)
gamma_user.roles.append(dummy_role)

tbl_id = self.table_ids.get(table_name)
table = db.session.query(SqlaTable).filter(SqlaTable.id == tbl_id).first()
table_perm = table.perm

security_manager.add_permission_role(
dummy_role,
security_manager.find_permission_view_menu("datasource_access", table_perm),
)

session.commit()

example_db = utils.get_example_database()
schema_name = self.default_schema_backend_map[example_db.backend]
uri = f"superset/tables/{example_db.id}/{schema_name}/"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 200)

# cleanup
gamma_user = security_manager.find_user(username="gamma")
gamma_user.roles.remove(security_manager.find_role(role_name))
session.commit()

@pytest.mark.usefixtures("load_energy_table_with_slice")
def test_get_superset_tables_not_allowed_with_out_permissions(self):
session = db.session
role_name = "dummy_role_no_table_access"
self.logout()
self.login(username="gamma")
gamma_user = security_manager.find_user(username="gamma")
security_manager.add_role(role_name)
dummy_role = security_manager.find_role(role_name)
gamma_user.roles.append(dummy_role)

session.commit()

example_db = utils.get_example_database()
schema_name = self.default_schema_backend_map[example_db.backend]
uri = f"superset/tables/{example_db.id}/{schema_name}/"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)

# cleanup
gamma_user = security_manager.find_user(username="gamma")
gamma_user.roles.remove(security_manager.find_role(role_name))
session.commit()

def test_get_superset_tables_database_not_found(self):
self.login(username="admin")
uri = f"superset/tables/invalid/public/"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 404)

def test_get_superset_tables_schema_undefined(self):
example_db = superset.utils.database.get_example_database()
self.login(username="gamma")
uri = f"superset/tables/{example_db.id}/undefined/"
rv = self.client.get(uri)
self.assertEqual(rv.status_code, 422)

def test_admin_only_menu_views(self):
def assert_admin_view_menus_in(role_name, assert_func):
role = security_manager.find_role(role_name)
Expand Down