From 671461d0d03ccc7ac1d7bef9cf1a5d9c1f39fdeb Mon Sep 17 00:00:00 2001 From: Daniel Vaz Gaspar Date: Wed, 29 Jul 2020 09:33:15 +0100 Subject: [PATCH] feat(api): database schemas migration to new API (#10436) * fix(log): log crashes if expired or not authenticated * fix lint and rison * add tests * more tests * perm fix * fix test not found * JS lint * fix Jest test --- .../components/TableSelector_spec.jsx | 4 +- .../src/components/TableSelector.jsx | 7 +- superset/databases/api.py | 79 +++++++++++++++++-- superset/databases/schemas.py | 9 +++ superset/views/core.py | 3 + tests/database_api_tests.py | 71 ++++++++++++++--- 6 files changed, 153 insertions(+), 20 deletions(-) diff --git a/superset-frontend/spec/javascripts/components/TableSelector_spec.jsx b/superset-frontend/spec/javascripts/components/TableSelector_spec.jsx index 35fb9bf5d2..b78c2149d5 100644 --- a/superset-frontend/spec/javascripts/components/TableSelector_spec.jsx +++ b/superset-frontend/spec/javascripts/components/TableSelector_spec.jsx @@ -196,13 +196,13 @@ describe('TableSelector', () => { }); describe('fetchSchemas', () => { - const FETCH_SCHEMAS_GLOB = 'glob:*/superset/schemas/*/*/'; + const FETCH_SCHEMAS_GLOB = 'glob:*/api/v1/database/*/schemas/?q=(force:!*)'; afterEach(fetchMock.resetHistory); afterAll(fetchMock.reset); it('should fetch schema options', () => { const schemaOptions = { - schemas: ['main', 'erf', 'superset'], + result: ['main', 'erf', 'superset'], }; fetchMock.get(FETCH_SCHEMAS_GLOB, schemaOptions, { overwriteRoutes: true, diff --git a/superset-frontend/src/components/TableSelector.jsx b/superset-frontend/src/components/TableSelector.jsx index 634ac38fbe..922fdd652f 100644 --- a/superset-frontend/src/components/TableSelector.jsx +++ b/superset-frontend/src/components/TableSelector.jsx @@ -188,10 +188,13 @@ export default class TableSelector extends React.PureComponent { const actualDbId = dbId || this.props.dbId; if (actualDbId) { this.setState({ schemaLoading: true }); - const endpoint = `/superset/schemas/${actualDbId}/${forceRefresh}/`; + const queryParams = rison.encode({ + force: Boolean(forceRefresh), + }); + const endpoint = `/api/v1/database/${actualDbId}/schemas/?q=${queryParams}`; return SupersetClient.get({ endpoint }) .then(({ json }) => { - const schemaOptions = json.schemas.map(s => ({ + const schemaOptions = json.result.map(s => ({ value: s, label: s, title: s, diff --git a/superset/databases/api.py b/superset/databases/api.py index 34607a11a5..02a0eb8b6f 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -18,15 +18,18 @@ from typing import Any, Dict, List, Optional from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface -from sqlalchemy.exc import NoSuchTableError, SQLAlchemyError +from sqlalchemy.exc import NoSuchTableError, OperationalError, SQLAlchemyError -from superset import event_logger, security_manager +from superset import event_logger from superset.databases.decorators import check_datasource_access from superset.databases.schemas import ( + database_schemas_query_schema, DatabaseSchemaResponseSchema, + SchemasResponseSchema, SelectStarResponseSchema, TableMetadataResponseSchema, ) +from superset.extensions import security_manager from superset.models.core import Database from superset.typing import FlaskResponse from superset.utils.core import error_msg_from_exception @@ -125,9 +128,16 @@ def get_table_metadata( class DatabaseRestApi(BaseSupersetModelRestApi): datamodel = SQLAInterface(Database) - include_route_methods = {"get_list", "table_metadata", "select_star", "schemas"} + include_route_methods = { + "all_schemas", + "get_list", + "table_metadata", + "select_star", + "schemas", + } class_permission_name = "DatabaseView" method_permission_name = { + "all_schemas": "list", "get_list": "list", "table_metadata": "list", "select_star": "list", @@ -154,25 +164,83 @@ class DatabaseRestApi(BaseSupersetModelRestApi): "backend", "function_names", ] + list_select_columns = list_columns + ["extra", "sqlalchemy_uri", "password"] # Removes the local limit for the page size max_page_size = -1 validators_columns = {"sqlalchemy_uri": sqlalchemy_uri_validator} - openapi_spec_tag = "Database" apispec_parameter_schemas = { + "database_schemas_query_schema": database_schemas_query_schema, "get_schemas_schema": get_schemas_schema, } + openapi_spec_tag = "Database" openapi_spec_component_schemas = ( DatabaseSchemaResponseSchema, TableMetadataResponseSchema, SelectStarResponseSchema, + SchemasResponseSchema, ) + @expose("//schemas/") + @protect() + @safe + @rison(database_schemas_query_schema) + @statsd_metrics + def schemas(self, pk: int, **kwargs: Any) -> FlaskResponse: + """ Get all schemas from a database + --- + get: + description: Get all schemas from a database + parameters: + - in: path + schema: + type: integer + name: pk + description: The database id + - in: query + name: q + content: + application/json: + schema: + $ref: '#/components/schemas/database_schemas_query_schema' + responses: + 200: + description: A List of all schemas from the database + content: + application/json: + schema: + $ref: "#/components/schemas/SchemasResponseSchema" + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 500: + $ref: '#/components/responses/500' + """ + database = self.datamodel.get(pk, self._base_filters) + if not database: + return self.response_404() + try: + schemas = database.get_all_schema_names( + cache=database.schema_cache_enabled, + cache_timeout=database.schema_cache_timeout, + force=kwargs["rison"].get("force", False), + ) + schemas = security_manager.get_schemas_accessible_by_user(database, schemas) + return self.response(200, result=schemas) + except OperationalError: + return self.response( + 500, message="There was an error connecting to the database" + ) + @expose("//table///", methods=["GET"]) @protect() @check_datasource_access @safe @event_logger.log_this + @statsd_metrics def table_metadata( self, database: Database, table_name: str, schema_name: str ) -> FlaskResponse: @@ -229,6 +297,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): @check_datasource_access @safe @event_logger.log_this + @statsd_metrics def select_star( self, database: Database, table_name: str, schema_name: Optional[str] = None ) -> FlaskResponse: @@ -286,7 +355,7 @@ class DatabaseRestApi(BaseSupersetModelRestApi): @safe @statsd_metrics @rison(get_schemas_schema) - def schemas(self, **kwargs: Any) -> FlaskResponse: + def all_schemas(self, **kwargs: Any) -> FlaskResponse: """Get all schemas --- get: diff --git a/superset/databases/schemas.py b/superset/databases/schemas.py index db2fab6f2a..3ef948fe7a 100644 --- a/superset/databases/schemas.py +++ b/superset/databases/schemas.py @@ -16,6 +16,11 @@ # under the License. from marshmallow import fields, Schema +database_schemas_query_schema = { + "type": "object", + "properties": {"force": {"type": "boolean"}}, +} + class TableMetadataOptionsResponseSchema(Schema): deferrable = fields.Bool() @@ -79,6 +84,10 @@ class SelectStarResponseSchema(Schema): result = fields.String(description="SQL select star") +class SchemasResponseSchema(Schema): + result = fields.List(fields.String(description="A database schema name")) + + class DatabaseSchemaObjectResponseSchema(Schema): value = fields.String(description="Schema name") text = fields.String(description="Schema display name") diff --git a/superset/views/core.py b/superset/views/core.py index e4f6be3d16..f42bb6c877 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -887,6 +887,9 @@ class Superset(BaseSupersetView): # pylint: disable=too-many-public-methods def schemas( # pylint: disable=no-self-use self, db_id: int, force_refresh: str = "false" ) -> FlaskResponse: + logger.warning( + "This API endpoint is deprecated and will be removed in version 1.0.0" + ) db_id = int(db_id) database = db.session.query(models.Database).get(db_id) if database: diff --git a/tests/database_api_tests.py b/tests/database_api_tests.py index 9e869bafd4..b1cef86f65 100644 --- a/tests/database_api_tests.py +++ b/tests/database_api_tests.py @@ -34,7 +34,7 @@ from .base_tests import SupersetTestCase class TestDatabaseApi(SupersetTestCase): def test_get_items(self): """ - Database API: Test get items + Database API: Test get items """ self.login(username="admin") uri = "api/v1/database/" @@ -63,6 +63,9 @@ class TestDatabaseApi(SupersetTestCase): self.assertEqual(list(response["result"][0].keys()), expected_columns) def test_get_items_filter(self): + """ + Database API: Test get items with filter + """ fake_db = ( db.session.query(Database).filter_by(database_name="fake_db_100").one() ) @@ -92,7 +95,7 @@ class TestDatabaseApi(SupersetTestCase): def test_get_items_not_allowed(self): """ - Database API: Test get items not allowed + Database API: Test get items not allowed """ self.login(username="gamma") uri = f"api/v1/database/" @@ -103,7 +106,7 @@ class TestDatabaseApi(SupersetTestCase): def test_get_table_metadata(self): """ - Database API: Test get table metadata info + Database API: Test get table metadata info """ example_db = get_example_database() self.login(username="admin") @@ -117,7 +120,7 @@ class TestDatabaseApi(SupersetTestCase): def test_get_invalid_database_table_metadata(self): """ - Database API: Test get invalid database from table metadata + Database API: Test get invalid database from table metadata """ database_id = 1000 self.login(username="admin") @@ -131,7 +134,7 @@ class TestDatabaseApi(SupersetTestCase): def test_get_invalid_table_table_metadata(self): """ - Database API: Test get invalid table from table metadata + 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/" @@ -141,7 +144,7 @@ class TestDatabaseApi(SupersetTestCase): def test_get_table_metadata_no_db_permission(self): """ - Database API: Test get table metadata from not permitted db + Database API: Test get table metadata from not permitted db """ self.login(username="gamma") example_db = get_example_database() @@ -151,7 +154,7 @@ class TestDatabaseApi(SupersetTestCase): def test_get_select_star(self): """ - Database API: Test get select star + Database API: Test get select star """ self.login(username="admin") example_db = get_example_database() @@ -163,7 +166,7 @@ class TestDatabaseApi(SupersetTestCase): def test_get_select_star_not_allowed(self): """ - Database API: Test get select star not allowed + Database API: Test get select star not allowed """ self.login(username="gamma") example_db = get_example_database() @@ -173,7 +176,7 @@ class TestDatabaseApi(SupersetTestCase): def test_get_select_star_datasource_access(self): """ - Database API: Test get select star with datasource access + Database API: Test get select star with datasource access """ session = db.session table = SqlaTable( @@ -201,7 +204,7 @@ class TestDatabaseApi(SupersetTestCase): def test_get_select_star_not_found_database(self): """ - Database API: Test get select star not found database + Database API: Test get select star not found database """ self.login(username="admin") max_id = db.session.query(func.max(Database.id)).scalar() @@ -211,7 +214,7 @@ class TestDatabaseApi(SupersetTestCase): def test_get_select_star_not_found_table(self): """ - Database API: Test get select star not found database + Database API: Test get select star not found database """ self.login(username="admin") example_db = get_example_database() @@ -223,6 +226,9 @@ class TestDatabaseApi(SupersetTestCase): self.assertEqual(rv.status_code, 404) def test_schemas(self): + """ + Database API: Test get select star not found database + """ self.login("admin") dbs = db.session.query(Database).all() schemas = [] @@ -254,8 +260,51 @@ class TestDatabaseApi(SupersetTestCase): @mock.patch("superset.security_manager.get_schemas_accessible_by_user") def test_schemas_no_access(self, mock_get_schemas_accessible_by_user): + """ + Database API: Test all schemas with no access + """ mock_get_schemas_accessible_by_user.return_value = [] self.login("admin") rv = self.client.get("api/v1/database/schemas/") response = json.loads(rv.data.decode("utf-8")) self.assertEqual(0, response["count"]) + + def test_database_schemas(self): + """ + Database API: Test database schemas + """ + self.login("admin") + database = db.session.query(Database).first() + schemas = database.get_all_schema_names() + + rv = self.client.get(f"api/v1/database/{database.id}/schemas/") + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(schemas, response["result"]) + + rv = self.client.get( + f"api/v1/database/{database.id}/schemas/?q={prison.dumps({'force': True})}" + ) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(schemas, response["result"]) + + def test_database_schemas_not_found(self): + """ + Database API: Test database schemas not found + """ + self.logout() + self.login(username="gamma") + example_db = get_example_database() + uri = f"api/v1/database/{example_db.id}/schemas/" + rv = self.client.get(uri) + self.assertEqual(rv.status_code, 404) + + def test_database_schemas_invalid_query(self): + """ + Database API: Test database schemas with invalid query + """ + self.login("admin") + database = db.session.query(Database).first() + rv = self.client.get( + f"api/v1/database/{database.id}/schemas/?q={prison.dumps({'force': 'nop'})}" + ) + self.assertEqual(rv.status_code, 400)