From f7d9959f5e33bd07e34f4657209761443301a175 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Wed, 25 Jan 2023 21:39:15 -0800 Subject: [PATCH 1/7] Add datasource api file --- superset/datasource/api.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 superset/datasource/api.py diff --git a/superset/datasource/api.py b/superset/datasource/api.py new file mode 100644 index 0000000000000..018c2349eedfc --- /dev/null +++ b/superset/datasource/api.py @@ -0,0 +1,33 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import logging + +from flask_appbuilder.api import BaseApi, expose, protect, safe + +from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP + +logger = logging.getLogger(__name__) + + +class DatasourceRestApi(BaseApi): + include_route_methods = {} + method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP + allow_browser_login = True + class_permission_name = "Datasource" + resource_name = "datasource" + openapi_spec_tag = "Datasources" + From f0c9ecdeb9b46f8b1c578b0fe4b4657592173d68 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 26 Jan 2023 19:57:48 -0800 Subject: [PATCH 2/7] Migrate to new endpoint --- .../explore/_skip.AdhocFilters.test.ts | 4 +- .../index.tsx | 6 +- superset/datasource/api.py | 52 ++++++- superset/initialization/__init__.py | 2 + superset/security/manager.py | 6 +- superset/views/core.py | 1 + tests/integration_tests/conftest.py | 75 +++++----- tests/integration_tests/core_tests.py | 12 +- .../integration_tests/datasource/__init__.py | 16 ++ .../integration_tests/datasource/api_tests.py | 137 ++++++++++++++++++ 10 files changed, 258 insertions(+), 53 deletions(-) create mode 100644 tests/integration_tests/datasource/__init__.py create mode 100644 tests/integration_tests/datasource/api_tests.py diff --git a/superset-frontend/cypress-base/cypress/integration/explore/_skip.AdhocFilters.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/_skip.AdhocFilters.test.ts index 8eb12a95b14f7..5eb9e3a066daf 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/_skip.AdhocFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/_skip.AdhocFilters.test.ts @@ -18,7 +18,9 @@ */ describe.skip('AdhocFilters', () => { beforeEach(() => { - cy.intercept('GET', '/superset/filter/table/*/name').as('filterValues'); + cy.intercept( + 'GET', '/api/v1/datasource/table/*/column/name/values' + ).as('filterValues'); cy.intercept('POST', '/superset/explore_json/**').as('postJson'); cy.intercept('GET', '/superset/explore_json/**').as('getJson'); cy.visitChartByName('Boys'); // a table chart diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx index c15123ef3498b..32fcd1bdec9ea 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx @@ -408,13 +408,13 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC = props => { controller.abort(); } setLoadingComparatorSuggestions(true); - SupersetClient.get({ + SupersetClient.get({ signal, - endpoint: `/superset/filter/${datasource.type}/${datasource.id}/${col}/`, + endpoint: `/api/v1/datasource/${datasource.type}/${datasource.id}/column/${col}/values/`, }) .then(({ json }) => { setSuggestions( - json.map((suggestion: null | number | boolean | string) => ({ + json.result.map((suggestion: null | number | boolean | string) => ({ value: suggestion, label: optionLabel(suggestion), })), diff --git a/superset/datasource/api.py b/superset/datasource/api.py index 018c2349eedfc..6f16d8cd41cc9 100644 --- a/superset/datasource/api.py +++ b/superset/datasource/api.py @@ -18,16 +18,66 @@ from flask_appbuilder.api import BaseApi, expose, protect, safe +from superset import app, db, event_logger from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP +from superset.dao.exceptions import DatasourceNotFound, DatasourceTypeNotSupportedError +from superset.datasource.dao import DatasourceDAO +from superset.exceptions import SupersetSecurityException +from superset.superset_typing import FlaskResponse +from superset.utils.core import apply_max_row_limit, DatasourceType logger = logging.getLogger(__name__) class DatasourceRestApi(BaseApi): - include_route_methods = {} + include_route_methods = {"get_column_values"} method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP allow_browser_login = True class_permission_name = "Datasource" resource_name = "datasource" openapi_spec_tag = "Datasources" + @expose( + "///column//values/", + methods=["GET"], + ) + @protect() + @safe + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" + f".get_column_values", + log_to_statsd=False, + ) + def get_column_values( + self, datasource_type: str, datasource_id: int, column_name: str + ) -> FlaskResponse: + try: + datasource = DatasourceDAO.get_datasource( + db.session, DatasourceType(datasource_type), datasource_id + ) + datasource.raise_for_access() + except ValueError: + return self.response( + 400, message=f"Invalid datasource type: {datasource_type}" + ) + except DatasourceTypeNotSupportedError as ex: + return self.response(400, message=ex.message) + except DatasourceNotFound as ex: + return self.response(404, message=ex.message) + except SupersetSecurityException as ex: + return self.response(403, message=ex.message) + + row_limit = apply_max_row_limit(app.config["FILTER_SELECT_ROW_LIMIT"]) + try: + payload = datasource.values_for_column( + column_name=column_name, limit=row_limit + ) + return self.response(200, result=payload) + except NotImplementedError: + return self.response( + 400, + message=( + "Unable to get column values for " + f"datasource type: {datasource_type}" + ) + ) diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 7e37781502313..057517b49a591 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -137,6 +137,7 @@ def init_views(self) -> None: from superset.datasets.api import DatasetRestApi from superset.datasets.columns.api import DatasetColumnsRestApi from superset.datasets.metrics.api import DatasetMetricRestApi + from superset.datasource.api import DatasourceRestApi from superset.embedded.api import EmbeddedDashboardRestApi from superset.embedded.view import EmbeddedView from superset.explore.api import ExploreRestApi @@ -207,6 +208,7 @@ def init_views(self) -> None: appbuilder.add_api(DatasetRestApi) appbuilder.add_api(DatasetColumnsRestApi) appbuilder.add_api(DatasetMetricRestApi) + appbuilder.add_api(DatasourceRestApi) appbuilder.add_api(EmbeddedDashboardRestApi) appbuilder.add_api(ExploreRestApi) appbuilder.add_api(ExploreFormDataRestApi) diff --git a/superset/security/manager.py b/superset/security/manager.py index 9fe07ec16c6fd..b1be13b782701 100644 --- a/superset/security/manager.py +++ b/superset/security/manager.py @@ -406,8 +406,10 @@ def get_datasource_access_error_msg(datasource: "BaseDatasource") -> str: :returns: The error message """ - return f"""This endpoint requires the datasource {datasource.name}, database or - `all_datasource_access` permission""" + return ( + f"This endpoint requires the datasource {datasource.name}, " + "database or `all_datasource_access` permission" + ) @staticmethod def get_datasource_access_link( # pylint: disable=unused-argument diff --git a/superset/views/core.py b/superset/views/core.py index d65023d600ba0..1a3d08c92446f 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -997,6 +997,7 @@ def explore( @has_access_api @event_logger.log_this @expose("/filter////") + @deprecated() def filter( # pylint: disable=no-self-use self, datasource_type: str, datasource_id: int, column: str ) -> FlaskResponse: diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py index 5c132381b1930..048a62ea25c64 100644 --- a/tests/integration_tests/conftest.py +++ b/tests/integration_tests/conftest.py @@ -293,45 +293,48 @@ def wrapper(*args, **kwargs): def virtual_dataset(): from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn - dataset = SqlaTable( - table_name="virtual_dataset", - sql=( - "SELECT 0 as col1, 'a' as col2, 1.0 as col3, NULL as col4, '2000-01-01 00:00:00' as col5 " - "UNION ALL " - "SELECT 1, 'b', 1.1, NULL, '2000-01-02 00:00:00' " - "UNION ALL " - "SELECT 2 as col1, 'c' as col2, 1.2, NULL, '2000-01-03 00:00:00' " - "UNION ALL " - "SELECT 3 as col1, 'd' as col2, 1.3, NULL, '2000-01-04 00:00:00' " - "UNION ALL " - "SELECT 4 as col1, 'e' as col2, 1.4, NULL, '2000-01-05 00:00:00' " - "UNION ALL " - "SELECT 5 as col1, 'f' as col2, 1.5, NULL, '2000-01-06 00:00:00' " - "UNION ALL " - "SELECT 6 as col1, 'g' as col2, 1.6, NULL, '2000-01-07 00:00:00' " - "UNION ALL " - "SELECT 7 as col1, 'h' as col2, 1.7, NULL, '2000-01-08 00:00:00' " - "UNION ALL " - "SELECT 8 as col1, 'i' as col2, 1.8, NULL, '2000-01-09 00:00:00' " - "UNION ALL " - "SELECT 9 as col1, 'j' as col2, 1.9, NULL, '2000-01-10 00:00:00' " - ), - database=get_example_database(), - ) - TableColumn(column_name="col1", type="INTEGER", table=dataset) - TableColumn(column_name="col2", type="VARCHAR(255)", table=dataset) - TableColumn(column_name="col3", type="DECIMAL(4,2)", table=dataset) - TableColumn(column_name="col4", type="VARCHAR(255)", table=dataset) - # Different database dialect datetime type is not consistent, so temporarily use varchar - TableColumn(column_name="col5", type="VARCHAR(255)", table=dataset) - - SqlMetric(metric_name="count", expression="count(*)", table=dataset) - db.session.merge(dataset) + with app.app_context(): + dataset = SqlaTable( + table_name="virtual_dataset", + sql=( + "SELECT 0 as col1, 'a' as col2, 1.0 as col3, NULL as col4, '2000-01-01 00:00:00' as col5 " + "UNION ALL " + "SELECT 1, 'b', 1.1, NULL, '2000-01-02 00:00:00' " + "UNION ALL " + "SELECT 2 as col1, 'c' as col2, 1.2, NULL, '2000-01-03 00:00:00' " + "UNION ALL " + "SELECT 3 as col1, 'd' as col2, 1.3, NULL, '2000-01-04 00:00:00' " + "UNION ALL " + "SELECT 4 as col1, 'e' as col2, 1.4, NULL, '2000-01-05 00:00:00' " + "UNION ALL " + "SELECT 5 as col1, 'f' as col2, 1.5, NULL, '2000-01-06 00:00:00' " + "UNION ALL " + "SELECT 6 as col1, 'g' as col2, 1.6, NULL, '2000-01-07 00:00:00' " + "UNION ALL " + "SELECT 7 as col1, 'h' as col2, 1.7, NULL, '2000-01-08 00:00:00' " + "UNION ALL " + "SELECT 8 as col1, 'i' as col2, 1.8, NULL, '2000-01-09 00:00:00' " + "UNION ALL " + "SELECT 9 as col1, 'j' as col2, 1.9, NULL, '2000-01-10 00:00:00' " + ), + database=get_example_database(), + ) + TableColumn(column_name="col1", type="INTEGER", table=dataset) + TableColumn(column_name="col2", type="VARCHAR(255)", table=dataset) + TableColumn(column_name="col3", type="DECIMAL(4,2)", table=dataset) + TableColumn(column_name="col4", type="VARCHAR(255)", table=dataset) + # Different database dialect datetime type is not consistent, so temporarily use varchar + TableColumn(column_name="col5", type="VARCHAR(255)", table=dataset) + + SqlMetric(metric_name="count", expression="count(*)", table=dataset) + db.session.add(dataset) + db.session.commit() yield dataset - db.session.delete(dataset) - db.session.commit() + with app.app_context(): + db.session.delete(dataset) + db.session.commit() @pytest.fixture diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py index 1b35d81f8394a..deedfb5e5d786 100644 --- a/tests/integration_tests/core_tests.py +++ b/tests/integration_tests/core_tests.py @@ -325,21 +325,13 @@ def test_save_slice(self): @pytest.mark.usefixtures("load_energy_table_with_slice") def test_filter_endpoint(self): self.login(username="admin") - slice_name = "Energy Sankey" - slice_id = self.get_slice(slice_name, db.session).id - db.session.commit() tbl_id = self.table_ids.get("energy_usage") table = db.session.query(SqlaTable).filter(SqlaTable.id == tbl_id) table.filter_select_enabled = True - url = ( - "/superset/filter/table/{}/target/?viz_type=sankey&groupby=source" - "&metric=sum__value&flt_col_0=source&flt_op_0=in&flt_eq_0=&" - "slice_id={}&datasource_name=energy_usage&" - "datasource_id=1&datasource_type=table" - ) + url = "/superset/filter/table/{}/target/" # Changing name - resp = self.get_resp(url.format(tbl_id, slice_id)) + resp = self.get_resp(url.format(tbl_id)) assert len(resp) > 0 assert "energy_target0" in resp diff --git a/tests/integration_tests/datasource/__init__.py b/tests/integration_tests/datasource/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/integration_tests/datasource/__init__.py @@ -0,0 +1,16 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. diff --git a/tests/integration_tests/datasource/api_tests.py b/tests/integration_tests/datasource/api_tests.py new file mode 100644 index 0000000000000..0a89eaccf7db5 --- /dev/null +++ b/tests/integration_tests/datasource/api_tests.py @@ -0,0 +1,137 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import json +from unittest.mock import Mock, patch + +import pytest + +from superset import db, security_manager +from superset.connectors.sqla.models import SqlaTable +from superset.dao.exceptions import DatasourceTypeNotSupportedError +from tests.integration_tests.base_tests import SupersetTestCase + + +class TestDatasourceApi(SupersetTestCase): + def get_virtual_dataset(self): + return ( + db.session.query(SqlaTable) + .filter(SqlaTable.table_name == "virtual_dataset") + .one() + ) + + @pytest.mark.usefixtures("virtual_dataset") + def test_get_column_values_ints(self): + self.login(username="admin") + table = self.get_virtual_dataset() + rv = self.client.get(f"api/v1/datasource/table/{table.id}/column/col1/values/") + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + for val in range(10): + assert val in response["result"] + + @pytest.mark.usefixtures("virtual_dataset") + def test_get_column_values_strs(self): + self.login(username="admin") + table = self.get_virtual_dataset() + rv = self.client.get(f"api/v1/datasource/table/{table.id}/column/col2/values/") + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + for val in ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"]: + assert val in response["result"] + + @pytest.mark.usefixtures("virtual_dataset") + def test_get_column_values_floats(self): + self.login(username="admin") + table = self.get_virtual_dataset() + rv = self.client.get(f"api/v1/datasource/table/{table.id}/column/col3/values/") + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + for val in [1.0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9]: + assert val in response["result"] + + @pytest.mark.usefixtures("virtual_dataset") + def test_get_column_values_nulls(self): + self.login(username="admin") + table = self.get_virtual_dataset() + rv = self.client.get(f"api/v1/datasource/table/{table.id}/column/col4/values/") + self.assertEqual(rv.status_code, 200) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(response["result"], [None]) + + @pytest.mark.usefixtures("virtual_dataset") + def test_get_column_values_invalid_datasource_type(self): + self.login(username="admin") + table = self.get_virtual_dataset() + rv = self.client.get( + f"api/v1/datasource/not_table/{table.id}/column/col1/values/" + ) + self.assertEqual(rv.status_code, 400) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(response["message"], "Invalid datasource type: not_table") + + @patch("superset.datasource.api.DatasourceDAO.get_datasource") + def test_get_column_values_datasource_type_not_supported(self, get_datasource_mock): + get_datasource_mock.side_effect = DatasourceTypeNotSupportedError + self.login(username="admin") + rv = self.client.get("api/v1/datasource/table/1/column/col1/values/") + self.assertEqual(rv.status_code, 400) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual( + response["message"], "DAO datasource query source type is not supported" + ) + + def test_get_column_values_datasource_not_found(self): + self.login(username="admin") + rv = self.client.get("api/v1/datasource/table/999/column/col1/values/") + self.assertEqual(rv.status_code, 404) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual(response["message"], "Datasource does not exist") + + @pytest.mark.usefixtures("virtual_dataset") + def test_get_column_values_no_datasource_access(self): + # Allow gamma user to use this endpoint, but does not have datasource access + perm = security_manager.find_permission_view_menu( + "can_get_column_values", "Datasource" + ) + gamma_role = security_manager.find_role("Gamma") + security_manager.add_permission_role(gamma_role, perm) + + self.login(username="gamma") + table = self.get_virtual_dataset() + rv = self.client.get(f"api/v1/datasource/table/{table.id}/column/col1/values/") + self.assertEqual(rv.status_code, 403) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual( + response["message"], + "This endpoint requires the datasource virtual_dataset, " + "database or `all_datasource_access` permission", + ) + + @patch("superset.datasource.api.DatasourceDAO.get_datasource") + def test_get_column_values_not_implemented_error(self, get_datasource_mock): + datasource = Mock() + datasource.values_for_column.side_effect = NotImplementedError + get_datasource_mock.return_value = datasource + + self.login(username="admin") + rv = self.client.get("api/v1/datasource/sl_table/1/column/col1/values/") + self.assertEqual(rv.status_code, 400) + response = json.loads(rv.data.decode("utf-8")) + self.assertEqual( + response["message"], + "Unable to get column values for datasource type: sl_table", + ) From 7e51ed31a6e6df87271c97a2ddf2c6a8808b5076 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 26 Jan 2023 20:25:28 -0800 Subject: [PATCH 3/7] Add openapi spec --- superset/datasource/api.py | 42 +++++++++++++++++++++++++++++++++- superset/datasource/schemas.py | 24 +++++++++++++++++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 superset/datasource/schemas.py diff --git a/superset/datasource/api.py b/superset/datasource/api.py index 6f16d8cd41cc9..d12c45da09bc6 100644 --- a/superset/datasource/api.py +++ b/superset/datasource/api.py @@ -22,6 +22,7 @@ from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP from superset.dao.exceptions import DatasourceNotFound, DatasourceTypeNotSupportedError from superset.datasource.dao import DatasourceDAO +from superset.datasource.schemas import GetColumnValuesResponseSchema from superset.exceptions import SupersetSecurityException from superset.superset_typing import FlaskResponse from superset.utils.core import apply_max_row_limit, DatasourceType @@ -36,6 +37,7 @@ class DatasourceRestApi(BaseApi): class_permission_name = "Datasource" resource_name = "datasource" openapi_spec_tag = "Datasources" + openapi_spec_component_schemas = (GetColumnValuesResponseSchema,) @expose( "///column//values/", @@ -51,6 +53,44 @@ class DatasourceRestApi(BaseApi): def get_column_values( self, datasource_type: str, datasource_id: int, column_name: str ) -> FlaskResponse: + """Get possible values for a datasource column + --- + get: + summary: Get possible values for a datasource column + parameters: + - in: path + schema: + type: string + name: datasource_type + description: The type of datasource + - in: path + schema: + type: integer + name: datasource_id + description: The id of the datasource + - in: path + schema: + type: string + name: column_name + description: The name of the column to get values for + responses: + 200: + description: A List of distinct values for the column + content: + application/json: + schema: + $ref: "#/components/schemas/GetColumnValuesResponseSchema" + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 403: + $ref: '#/components/responses/403' + 404: + $ref: '#/components/responses/404' + 500: + $ref: '#/components/responses/500' + """ try: datasource = DatasourceDAO.get_datasource( db.session, DatasourceType(datasource_type), datasource_id @@ -79,5 +119,5 @@ def get_column_values( message=( "Unable to get column values for " f"datasource type: {datasource_type}" - ) + ), ) diff --git a/superset/datasource/schemas.py b/superset/datasource/schemas.py new file mode 100644 index 0000000000000..c2bfe7610377f --- /dev/null +++ b/superset/datasource/schemas.py @@ -0,0 +1,24 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from marshmallow import fields, Schema + + +class GetColumnValuesResponseSchema(Schema): + result = fields.List( + fields.Raw(description="Can be any type, not just string"), + description="A list of distinct values for the column", + ) From 0abe72209653877ca66e512c87de825251ef769c Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 26 Jan 2023 20:35:16 -0800 Subject: [PATCH 4/7] Avoid app context issues in unrelated tests --- tests/integration_tests/conftest.py | 75 +++++++++---------- .../integration_tests/datasource/api_tests.py | 12 +-- 2 files changed, 42 insertions(+), 45 deletions(-) diff --git a/tests/integration_tests/conftest.py b/tests/integration_tests/conftest.py index 048a62ea25c64..5c132381b1930 100644 --- a/tests/integration_tests/conftest.py +++ b/tests/integration_tests/conftest.py @@ -293,48 +293,45 @@ def wrapper(*args, **kwargs): def virtual_dataset(): from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn - with app.app_context(): - dataset = SqlaTable( - table_name="virtual_dataset", - sql=( - "SELECT 0 as col1, 'a' as col2, 1.0 as col3, NULL as col4, '2000-01-01 00:00:00' as col5 " - "UNION ALL " - "SELECT 1, 'b', 1.1, NULL, '2000-01-02 00:00:00' " - "UNION ALL " - "SELECT 2 as col1, 'c' as col2, 1.2, NULL, '2000-01-03 00:00:00' " - "UNION ALL " - "SELECT 3 as col1, 'd' as col2, 1.3, NULL, '2000-01-04 00:00:00' " - "UNION ALL " - "SELECT 4 as col1, 'e' as col2, 1.4, NULL, '2000-01-05 00:00:00' " - "UNION ALL " - "SELECT 5 as col1, 'f' as col2, 1.5, NULL, '2000-01-06 00:00:00' " - "UNION ALL " - "SELECT 6 as col1, 'g' as col2, 1.6, NULL, '2000-01-07 00:00:00' " - "UNION ALL " - "SELECT 7 as col1, 'h' as col2, 1.7, NULL, '2000-01-08 00:00:00' " - "UNION ALL " - "SELECT 8 as col1, 'i' as col2, 1.8, NULL, '2000-01-09 00:00:00' " - "UNION ALL " - "SELECT 9 as col1, 'j' as col2, 1.9, NULL, '2000-01-10 00:00:00' " - ), - database=get_example_database(), - ) - TableColumn(column_name="col1", type="INTEGER", table=dataset) - TableColumn(column_name="col2", type="VARCHAR(255)", table=dataset) - TableColumn(column_name="col3", type="DECIMAL(4,2)", table=dataset) - TableColumn(column_name="col4", type="VARCHAR(255)", table=dataset) - # Different database dialect datetime type is not consistent, so temporarily use varchar - TableColumn(column_name="col5", type="VARCHAR(255)", table=dataset) - - SqlMetric(metric_name="count", expression="count(*)", table=dataset) - db.session.add(dataset) - db.session.commit() + dataset = SqlaTable( + table_name="virtual_dataset", + sql=( + "SELECT 0 as col1, 'a' as col2, 1.0 as col3, NULL as col4, '2000-01-01 00:00:00' as col5 " + "UNION ALL " + "SELECT 1, 'b', 1.1, NULL, '2000-01-02 00:00:00' " + "UNION ALL " + "SELECT 2 as col1, 'c' as col2, 1.2, NULL, '2000-01-03 00:00:00' " + "UNION ALL " + "SELECT 3 as col1, 'd' as col2, 1.3, NULL, '2000-01-04 00:00:00' " + "UNION ALL " + "SELECT 4 as col1, 'e' as col2, 1.4, NULL, '2000-01-05 00:00:00' " + "UNION ALL " + "SELECT 5 as col1, 'f' as col2, 1.5, NULL, '2000-01-06 00:00:00' " + "UNION ALL " + "SELECT 6 as col1, 'g' as col2, 1.6, NULL, '2000-01-07 00:00:00' " + "UNION ALL " + "SELECT 7 as col1, 'h' as col2, 1.7, NULL, '2000-01-08 00:00:00' " + "UNION ALL " + "SELECT 8 as col1, 'i' as col2, 1.8, NULL, '2000-01-09 00:00:00' " + "UNION ALL " + "SELECT 9 as col1, 'j' as col2, 1.9, NULL, '2000-01-10 00:00:00' " + ), + database=get_example_database(), + ) + TableColumn(column_name="col1", type="INTEGER", table=dataset) + TableColumn(column_name="col2", type="VARCHAR(255)", table=dataset) + TableColumn(column_name="col3", type="DECIMAL(4,2)", table=dataset) + TableColumn(column_name="col4", type="VARCHAR(255)", table=dataset) + # Different database dialect datetime type is not consistent, so temporarily use varchar + TableColumn(column_name="col5", type="VARCHAR(255)", table=dataset) + + SqlMetric(metric_name="count", expression="count(*)", table=dataset) + db.session.merge(dataset) yield dataset - with app.app_context(): - db.session.delete(dataset) - db.session.commit() + db.session.delete(dataset) + db.session.commit() @pytest.fixture diff --git a/tests/integration_tests/datasource/api_tests.py b/tests/integration_tests/datasource/api_tests.py index 0a89eaccf7db5..522aa33383e62 100644 --- a/tests/integration_tests/datasource/api_tests.py +++ b/tests/integration_tests/datasource/api_tests.py @@ -33,7 +33,7 @@ def get_virtual_dataset(self): .one() ) - @pytest.mark.usefixtures("virtual_dataset") + @pytest.mark.usefixtures("app_context", "virtual_dataset") def test_get_column_values_ints(self): self.login(username="admin") table = self.get_virtual_dataset() @@ -43,7 +43,7 @@ def test_get_column_values_ints(self): for val in range(10): assert val in response["result"] - @pytest.mark.usefixtures("virtual_dataset") + @pytest.mark.usefixtures("app_context", "virtual_dataset") def test_get_column_values_strs(self): self.login(username="admin") table = self.get_virtual_dataset() @@ -53,7 +53,7 @@ def test_get_column_values_strs(self): for val in ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"]: assert val in response["result"] - @pytest.mark.usefixtures("virtual_dataset") + @pytest.mark.usefixtures("app_context", "virtual_dataset") def test_get_column_values_floats(self): self.login(username="admin") table = self.get_virtual_dataset() @@ -63,7 +63,7 @@ def test_get_column_values_floats(self): for val in [1.0, 1.1, 1.2, 1.3, 1.4, 1.5, 1.6, 1.7, 1.8, 1.9]: assert val in response["result"] - @pytest.mark.usefixtures("virtual_dataset") + @pytest.mark.usefixtures("app_context", "virtual_dataset") def test_get_column_values_nulls(self): self.login(username="admin") table = self.get_virtual_dataset() @@ -72,7 +72,7 @@ def test_get_column_values_nulls(self): response = json.loads(rv.data.decode("utf-8")) self.assertEqual(response["result"], [None]) - @pytest.mark.usefixtures("virtual_dataset") + @pytest.mark.usefixtures("app_context", "virtual_dataset") def test_get_column_values_invalid_datasource_type(self): self.login(username="admin") table = self.get_virtual_dataset() @@ -101,7 +101,7 @@ def test_get_column_values_datasource_not_found(self): response = json.loads(rv.data.decode("utf-8")) self.assertEqual(response["message"], "Datasource does not exist") - @pytest.mark.usefixtures("virtual_dataset") + @pytest.mark.usefixtures("app_context", "virtual_dataset") def test_get_column_values_no_datasource_access(self): # Allow gamma user to use this endpoint, but does not have datasource access perm = security_manager.find_permission_view_menu( From 624cbab86cda4698b7dd40397f891c51ec47b463 Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Thu, 26 Jan 2023 20:46:05 -0800 Subject: [PATCH 5/7] Run pretty --- .../integration/explore/_skip.AdhocFilters.test.ts | 6 +++--- .../AdhocFilterEditPopoverSimpleTabContent/index.tsx | 12 +++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/superset-frontend/cypress-base/cypress/integration/explore/_skip.AdhocFilters.test.ts b/superset-frontend/cypress-base/cypress/integration/explore/_skip.AdhocFilters.test.ts index 5eb9e3a066daf..a4e9c8fe46888 100644 --- a/superset-frontend/cypress-base/cypress/integration/explore/_skip.AdhocFilters.test.ts +++ b/superset-frontend/cypress-base/cypress/integration/explore/_skip.AdhocFilters.test.ts @@ -18,9 +18,9 @@ */ describe.skip('AdhocFilters', () => { beforeEach(() => { - cy.intercept( - 'GET', '/api/v1/datasource/table/*/column/name/values' - ).as('filterValues'); + cy.intercept('GET', '/api/v1/datasource/table/*/column/name/values').as( + 'filterValues', + ); cy.intercept('POST', '/superset/explore_json/**').as('postJson'); cy.intercept('GET', '/superset/explore_json/**').as('getJson'); cy.visitChartByName('Boys'); // a table chart diff --git a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx index 32fcd1bdec9ea..f0b05d8f1f6fe 100644 --- a/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx +++ b/superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx @@ -408,16 +408,18 @@ const AdhocFilterEditPopoverSimpleTabContent: React.FC = props => { controller.abort(); } setLoadingComparatorSuggestions(true); - SupersetClient.get({ + SupersetClient.get({ signal, endpoint: `/api/v1/datasource/${datasource.type}/${datasource.id}/column/${col}/values/`, }) .then(({ json }) => { setSuggestions( - json.result.map((suggestion: null | number | boolean | string) => ({ - value: suggestion, - label: optionLabel(suggestion), - })), + json.result.map( + (suggestion: null | number | boolean | string) => ({ + value: suggestion, + label: optionLabel(suggestion), + }), + ), ); setLoadingComparatorSuggestions(false); }) From 1be29cea4013eeb181b170001514f6fda19eca0d Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Fri, 27 Jan 2023 07:50:10 -0800 Subject: [PATCH 6/7] Don't use marshmallow for OAS --- superset/datasource/api.py | 17 +++++++++++------ superset/datasource/schemas.py | 24 ------------------------ 2 files changed, 11 insertions(+), 30 deletions(-) delete mode 100644 superset/datasource/schemas.py diff --git a/superset/datasource/api.py b/superset/datasource/api.py index d12c45da09bc6..c31c787b3f4b9 100644 --- a/superset/datasource/api.py +++ b/superset/datasource/api.py @@ -19,10 +19,8 @@ from flask_appbuilder.api import BaseApi, expose, protect, safe from superset import app, db, event_logger -from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP from superset.dao.exceptions import DatasourceNotFound, DatasourceTypeNotSupportedError from superset.datasource.dao import DatasourceDAO -from superset.datasource.schemas import GetColumnValuesResponseSchema from superset.exceptions import SupersetSecurityException from superset.superset_typing import FlaskResponse from superset.utils.core import apply_max_row_limit, DatasourceType @@ -31,13 +29,10 @@ class DatasourceRestApi(BaseApi): - include_route_methods = {"get_column_values"} - method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP allow_browser_login = True class_permission_name = "Datasource" resource_name = "datasource" openapi_spec_tag = "Datasources" - openapi_spec_component_schemas = (GetColumnValuesResponseSchema,) @expose( "///column//values/", @@ -79,7 +74,17 @@ def get_column_values( content: application/json: schema: - $ref: "#/components/schemas/GetColumnValuesResponseSchema" + type: object + properties: + result: + type: array + items: + oneOf: + - type: string + - type: integer + - type: number + - type: boolean + - type: object 400: $ref: '#/components/responses/400' 401: diff --git a/superset/datasource/schemas.py b/superset/datasource/schemas.py deleted file mode 100644 index c2bfe7610377f..0000000000000 --- a/superset/datasource/schemas.py +++ /dev/null @@ -1,24 +0,0 @@ -# Licensed to the Apache Software Foundation (ASF) under one -# or more contributor license agreements. See the NOTICE file -# distributed with this work for additional information -# regarding copyright ownership. The ASF licenses this file -# to you under the Apache License, Version 2.0 (the -# "License"); you may not use this file except in compliance -# with the License. You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, -# software distributed under the License is distributed on an -# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -# KIND, either express or implied. See the License for the -# specific language governing permissions and limitations -# under the License. -from marshmallow import fields, Schema - - -class GetColumnValuesResponseSchema(Schema): - result = fields.List( - fields.Raw(description="Can be any type, not just string"), - description="A list of distinct values for the column", - ) From 5ac29ca34afe3edab2c7982b663038cf650da40c Mon Sep 17 00:00:00 2001 From: Jack Fragassi Date: Fri, 27 Jan 2023 10:09:52 -0800 Subject: [PATCH 7/7] BaseApi -> BaseSupersetApi, add statsd --- superset/datasource/api.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/superset/datasource/api.py b/superset/datasource/api.py index c31c787b3f4b9..be246c915d535 100644 --- a/superset/datasource/api.py +++ b/superset/datasource/api.py @@ -16,7 +16,7 @@ # under the License. import logging -from flask_appbuilder.api import BaseApi, expose, protect, safe +from flask_appbuilder.api import expose, protect, safe from superset import app, db, event_logger from superset.dao.exceptions import DatasourceNotFound, DatasourceTypeNotSupportedError @@ -24,11 +24,12 @@ from superset.exceptions import SupersetSecurityException from superset.superset_typing import FlaskResponse from superset.utils.core import apply_max_row_limit, DatasourceType +from superset.views.base_api import BaseSupersetApi, statsd_metrics logger = logging.getLogger(__name__) -class DatasourceRestApi(BaseApi): +class DatasourceRestApi(BaseSupersetApi): allow_browser_login = True class_permission_name = "Datasource" resource_name = "datasource" @@ -40,6 +41,7 @@ class DatasourceRestApi(BaseApi): ) @protect() @safe + @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}" f".get_column_values",