From 4fdf230a568b014309357b9d691c30fe1a50b32f Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Wed, 4 May 2022 14:55:52 +0100 Subject: [PATCH] feat: deprecate /superset/extra_table_metadata migrate to api v1 (#19921) * feat: deprecate /superset/extra_table_metadata migrate to api v1 * use can_read to table_extra_metadata * troubleshoot sqlite * fix test * fix test * fix test * fix frontend test on sqllab --- .../src/SqlLab/actions/sqlLab.js | 2 +- .../src/SqlLab/actions/sqlLab.test.js | 4 +- .../DndFilterSelect.tsx | 2 +- .../AdhocFilterControl/index.jsx | 2 +- superset/constants.py | 1 + superset/databases/api.py | 68 ++++++++++++++++++- superset/databases/schemas.py | 6 ++ superset/db_engine_specs/base.py | 2 +- superset/db_engine_specs/bigquery.py | 2 +- superset/db_engine_specs/gsheets.py | 2 +- superset/db_engine_specs/presto.py | 4 +- superset/views/core.py | 6 ++ .../integration_tests/databases/api_tests.py | 64 ++++++++++++++++- 13 files changed, 152 insertions(+), 13 deletions(-) diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 41717dd174..e4d4db17e9 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -1045,7 +1045,7 @@ function getTableMetadata(table, query, dispatch) { function getTableExtendedMetadata(table, query, dispatch) { return SupersetClient.get({ endpoint: encodeURI( - `/superset/extra_table_metadata/${query.dbId}/` + + `/api/v1/database/${query.dbId}/table_extra/` + `${encodeURIComponent(table.name)}/${encodeURIComponent( table.schema, )}/`, diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js index 5b20f52335..f2d56caee4 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js @@ -415,10 +415,10 @@ describe('async actions', () => { fetchMock.delete(updateTableSchemaEndpoint, {}); fetchMock.post(updateTableSchemaEndpoint, JSON.stringify({ id: 1 })); - const getTableMetadataEndpoint = 'glob:*/api/v1/database/*'; + const getTableMetadataEndpoint = 'glob:**/api/v1/database/*/table/*/*/'; fetchMock.get(getTableMetadataEndpoint, {}); const getExtraTableMetadataEndpoint = - 'glob:*/superset/extra_table_metadata/*'; + 'glob:**/api/v1/database/*/table_extra/*/*/'; fetchMock.get(getExtraTableMetadataEndpoint, {}); let isFeatureEnabledMock; diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx index bb59b17b0c..4a08c214de 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndFilterSelect.tsx @@ -147,7 +147,7 @@ export const DndFilterSelect = (props: DndFilterSelectProps) => { if (!isSqllabView && dbId && name && schema) { SupersetClient.get({ - endpoint: `/superset/extra_table_metadata/${dbId}/${name}/${schema}/`, + endpoint: `/api/v1/database/${dbId}/table_extra/${name}/${schema}/`, }) .then(({ json }: { json: Record }) => { if (json && json.partitions) { diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx index 44f04b1b45..876eca1e75 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterControl/index.jsx @@ -137,7 +137,7 @@ class AdhocFilterControl extends React.Component { if (!isSqllabView && dbId && name && schema) { SupersetClient.get({ - endpoint: `/superset/extra_table_metadata/${dbId}/${name}/${schema}/`, + endpoint: `/api/v1/database/${dbId}/table_extra/${name}/${schema}/`, }) .then(({ json }) => { if (json && json.partitions) { diff --git a/superset/constants.py b/superset/constants.py index 8399aa457a..821562256a 100644 --- a/superset/constants.py +++ b/superset/constants.py @@ -112,6 +112,7 @@ MODEL_API_RW_METHOD_PERMISSION_MAP = { "schemas": "read", "select_star": "read", "table_metadata": "read", + "table_extra_metadata": "read", "test_connection": "read", "validate_parameters": "read", "favorite_status": "read", diff --git a/superset/databases/api.py b/superset/databases/api.py index 6ec470119c..3833edab55 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -63,6 +63,7 @@ from superset.databases.schemas import ( get_export_ids_schema, SchemasResponseSchema, SelectStarResponseSchema, + TableExtraMetadataResponseSchema, TableMetadataResponseSchema, ) from superset.databases.utils import get_table_metadata @@ -71,7 +72,7 @@ from superset.errors import ErrorLevel, SupersetError, SupersetErrorType from superset.extensions import security_manager from superset.models.core import Database from superset.superset_typing import FlaskResponse -from superset.utils.core import error_msg_from_exception +from superset.utils.core import error_msg_from_exception, parse_js_uri_path_item from superset.views.base_api import ( BaseSupersetModelRestApi, requires_form_data, @@ -89,6 +90,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): RouteMethod.EXPORT, RouteMethod.IMPORT, "table_metadata", + "table_extra_metadata", "select_star", "schemas", "test_connection", @@ -197,6 +199,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): DatabaseRelatedObjectsResponse, DatabaseTestConnectionSchema, DatabaseValidateParametersSchema, + TableExtraMetadataResponseSchema, TableMetadataResponseSchema, SelectStarResponseSchema, SchemasResponseSchema, @@ -527,6 +530,69 @@ class DatabaseRestApi(BaseSupersetModelRestApi): self.incr_stats("success", self.table_metadata.__name__) return self.response(200, **table_info) + @expose("//table_extra///", methods=["GET"]) + @protect() + @check_datasource_access + @safe + @statsd_metrics + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".table_extra_metadata", + log_to_statsd=False, + ) + def table_extra_metadata( + self, database: Database, table_name: str, schema_name: str + ) -> FlaskResponse: + """Table schema info + --- + get: + summary: >- + Get table extra metadata + description: >- + Response depends on each DB engine spec normally focused on partitions + parameters: + - in: path + schema: + type: integer + name: pk + description: The database id + - in: path + schema: + type: string + name: table_name + description: Table name + - in: path + schema: + type: string + name: schema_name + description: Table schema + responses: + 200: + description: Table extra metadata information + content: + application/json: + schema: + $ref: "#/components/schemas/TableExtraMetadataResponseSchema" + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + self.incr_stats("init", self.table_metadata.__name__) + + parsed_schema = parse_js_uri_path_item(schema_name, eval_undefined=True) + table_name = parse_js_uri_path_item(table_name) # type: ignore + payload = database.db_engine_spec.extra_table_metadata( + database, table_name, parsed_schema + ) + return self.response(200, **payload) + @expose("//select_star//", methods=["GET"]) @expose("//select_star///", methods=["GET"]) @protect() diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index dd60e87d16..e28b704017 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -527,6 +527,12 @@ class TableMetadataResponseSchema(Schema): selectStar = fields.String(description="SQL select star") +class TableExtraMetadataResponseSchema(Schema): + metadata = fields.Dict() + partitions = fields.Dict() + clustering = fields.Dict() + + class SelectStarResponseSchema(Schema): result = fields.String(description="SQL select star") diff --git a/superset/db_engine_specs/base.py b/superset/db_engine_specs/base.py index 564367372e..c31b0a7229 100644 --- a/superset/db_engine_specs/base.py +++ b/superset/db_engine_specs/base.py @@ -641,7 +641,7 @@ class BaseEngineSpec: # pylint: disable=too-many-public-methods cls, database: "Database", table_name: str, - schema_name: str, + schema_name: Optional[str], ) -> Dict[str, Any]: """ Returns engine-specific table metadata diff --git a/superset/db_engine_specs/bigquery.py b/superset/db_engine_specs/bigquery.py index d844b342ce..daac53d3ab 100644 --- a/superset/db_engine_specs/bigquery.py +++ b/superset/db_engine_specs/bigquery.py @@ -268,7 +268,7 @@ class BigQueryEngineSpec(BaseEngineSpec): @classmethod def extra_table_metadata( - cls, database: "Database", table_name: str, schema_name: str + cls, database: "Database", table_name: str, schema_name: Optional[str] ) -> Dict[str, Any]: indexes = database.get_indexes(table_name, schema_name) if not indexes: diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py index 94c4cf424b..ba13389e26 100644 --- a/superset/db_engine_specs/gsheets.py +++ b/superset/db_engine_specs/gsheets.py @@ -97,7 +97,7 @@ class GSheetsEngineSpec(SqliteEngineSpec): cls, database: "Database", table_name: str, - schema_name: str, + schema_name: Optional[str], ) -> Dict[str, Any]: engine = cls.get_engine(database, schema=schema_name) with closing(engine.raw_connection()) as conn: diff --git a/superset/db_engine_specs/presto.py b/superset/db_engine_specs/presto.py index dc540ffff2..8fa4d2794a 100644 --- a/superset/db_engine_specs/presto.py +++ b/superset/db_engine_specs/presto.py @@ -899,7 +899,7 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho @classmethod def extra_table_metadata( - cls, database: "Database", table_name: str, schema_name: str + cls, database: "Database", table_name: str, schema_name: Optional[str] ) -> Dict[str, Any]: metadata = {} @@ -931,7 +931,7 @@ class PrestoEngineSpec(BaseEngineSpec): # pylint: disable=too-many-public-metho @classmethod def get_create_view( - cls, database: "Database", schema: str, table: str + cls, database: "Database", schema: Optional[str], table: str ) -> Optional[str]: """ Return a CREATE VIEW statement, or `None` if not a view. diff --git a/superset/views/core.py b/superset/views/core.py index 478cd93a73..2855dd0e60 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -2154,6 +2154,12 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods def extra_table_metadata( # pylint: disable=no-self-use self, database_id: int, table_name: str, schema: str ) -> FlaskResponse: + logger.warning( + "%s.extra_table_metadata " + "This API endpoint is deprecated and will be removed in version 3.0.0", + self.__class__.__name__, + ) + parsed_schema = utils.parse_js_uri_path_item(schema, eval_undefined=True) table_name = utils.parse_js_uri_path_item(table_name) # type: ignore mydb = db.session.query(Database).filter_by(id=database_id).one() diff --git a/tests/integration_tests/databases/api_tests.py b/tests/integration_tests/databases/api_tests.py index 70640728ac..a4ddbfd711 100644 --- a/tests/integration_tests/databases/api_tests.py +++ b/tests/integration_tests/databases/api_tests.py @@ -775,10 +775,30 @@ class TestDatabaseApi(SupersetTestCase): Database API: Test get invalid table from table metadata """ example_db = get_example_database() - uri = f"api/v1/database/{example_db.id}/wrong_table/null/" + uri = f"api/v1/database/{example_db.id}/table/wrong_table/null/" self.login(username="admin") rv = self.client.get(uri) - self.assertEqual(rv.status_code, 404) + data = json.loads(rv.data.decode("utf-8")) + if example_db.backend == "sqlite": + self.assertEqual(rv.status_code, 200) + self.assertEqual( + data, + { + "columns": [], + "comment": None, + "foreignKeys": [], + "indexes": [], + "name": "wrong_table", + "primaryKey": {"constrained_columns": None, "name": None}, + "selectStar": "SELECT\nFROM wrong_table\nLIMIT 100\nOFFSET 0", + }, + ) + elif example_db.backend == "mysql": + self.assertEqual(rv.status_code, 422) + self.assertEqual(data, {"message": "`wrong_table`"}) + else: + self.assertEqual(rv.status_code, 422) + self.assertEqual(data, {"message": "wrong_table"}) def test_get_table_metadata_no_db_permission(self): """ @@ -790,6 +810,46 @@ class TestDatabaseApi(SupersetTestCase): rv = self.client.get(uri) self.assertEqual(rv.status_code, 404) + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") + def test_get_table_extra_metadata(self): + """ + Database API: Test get table extra metadata info + """ + example_db = get_example_database() + self.login(username="admin") + uri = f"api/v1/database/{example_db.id}/table_extra/birth_names/null/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(response, {}) + + def test_get_invalid_database_table_extra_metadata(self): + """ + Database API: Test get invalid database from table extra metadata + """ + database_id = 1000 + self.login(username="admin") + uri = f"api/v1/database/{database_id}/table_extra/some_table/some_schema/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + uri = "api/v1/database/some_database/table_extra/some_table/some_schema/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_get_invalid_table_table_extra_metadata(self): + """ + Database API: Test get invalid table from table extra metadata + """ + example_db = get_example_database() + uri = f"api/v1/database/{example_db.id}/table_extra/wrong_table/null/" + self.login(username="admin") + rv = self.client.get(uri) + data = json.loads(rv.data.decode("utf-8")) + + self.assertEqual(rv.status_code, 200) + self.assertEqual(data, {}) + @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") def test_get_select_star(self): """