From dd3b634c5ca5f4b5acfe88fc06f70e1eb7bf36b9 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Thu, 16 Jun 2022 09:59:21 -0700 Subject: [PATCH] fix key error on permalink fetch for old permalinks --- superset/explore/permalink/commands/get.py | 9 +- superset/explore/permalink/types.py | 6 +- .../explore/permalink/commands_tests.py | 172 ++++++++++++++++++ 3 files changed, 184 insertions(+), 3 deletions(-) create mode 100644 tests/integration_tests/explore/permalink/commands_tests.py diff --git a/superset/explore/permalink/commands/get.py b/superset/explore/permalink/commands/get.py index f75df69d7a63e..ca4fe8c74fa2f 100644 --- a/superset/explore/permalink/commands/get.py +++ b/superset/explore/permalink/commands/get.py @@ -48,8 +48,13 @@ def run(self) -> Optional[ExplorePermalinkValue]: ).run() if value: chart_id: Optional[int] = value.get("chartId") - datasource_id: int = value["datasourceId"] - datasource_type = DatasourceType(value["datasourceType"]) + # keep this backward compatible for old permalinks + datasource_id: int = ( + value.get("datasourceId") or value.get("datasetId") or 0 + ) + datasource_type = DatasourceType( + value.get("datasourceType", DatasourceType.TABLE) + ) check_chart_access(datasource_id, chart_id, self.actor, datasource_type) return value return None diff --git a/superset/explore/permalink/types.py b/superset/explore/permalink/types.py index b90b4d760d4d0..393f0ed8d5890 100644 --- a/superset/explore/permalink/types.py +++ b/superset/explore/permalink/types.py @@ -24,7 +24,11 @@ class ExplorePermalinkState(TypedDict, total=False): class ExplorePermalinkValue(TypedDict): chartId: Optional[int] - datasourceId: int + # either datasetId or datasourceId is required + # TODO: deprecated - datasetId is deprecated + # and should be removed in next major release + datasetId: Optional[int] + datasourceId: Optional[int] datasourceType: str datasource: str state: ExplorePermalinkState diff --git a/tests/integration_tests/explore/permalink/commands_tests.py b/tests/integration_tests/explore/permalink/commands_tests.py new file mode 100644 index 0000000000000..2bb44bb068edd --- /dev/null +++ b/tests/integration_tests/explore/permalink/commands_tests.py @@ -0,0 +1,172 @@ +# 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 patch + +import pytest + +from superset import app, db, security, security_manager +from superset.commands.exceptions import DatasourceTypeInvalidError +from superset.connectors.sqla.models import SqlaTable +from superset.explore.form_data.commands.parameters import CommandParameters +from superset.explore.permalink.commands.create import CreateExplorePermalinkCommand +from superset.explore.permalink.commands.get import GetExplorePermalinkCommand +from superset.key_value.utils import decode_permalink_id +from superset.models.slice import Slice +from superset.models.sql_lab import Query +from superset.utils.core import DatasourceType, get_example_default_schema +from superset.utils.database import get_example_database +from tests.integration_tests.base_tests import SupersetTestCase + + +class TestCreatePermalinkDataCommand(SupersetTestCase): + @pytest.fixture() + def create_dataset(self): + with self.create_app().app_context(): + dataset = SqlaTable( + table_name="dummy_sql_table", + database=get_example_database(), + schema=get_example_default_schema(), + sql="select 123 as intcol, 'abc' as strcol", + ) + session = db.session + session.add(dataset) + session.commit() + + yield dataset + + # rollback + session.delete(dataset) + session.commit() + + @pytest.fixture() + def create_slice(self): + with self.create_app().app_context(): + session = db.session + dataset = ( + session.query(SqlaTable).filter_by(table_name="dummy_sql_table").first() + ) + slice = Slice( + datasource_id=dataset.id, + datasource_type=DatasourceType.TABLE, + datasource_name="tmp_perm_table", + slice_name="slice_name", + ) + + session.add(slice) + session.commit() + + yield slice + + # rollback + session.delete(slice) + session.commit() + + @pytest.fixture() + def create_query(self): + with self.create_app().app_context(): + session = db.session + + query = Query( + sql="select 1 as foo;", + client_id="sldkfjlk", + database=get_example_database(), + ) + + session.add(query) + session.commit() + + yield query + + # rollback + session.delete(query) + session.commit() + + @patch("superset.security.manager.g") + @pytest.mark.usefixtures("create_dataset", "create_slice") + def test_create_permalink_command(self, mock_g): + mock_g.user = security_manager.find_user("admin") + + dataset = ( + db.session.query(SqlaTable).filter_by(table_name="dummy_sql_table").first() + ) + slice = db.session.query(Slice).filter_by(slice_name="slice_name").first() + + datasource = f"{dataset.id}__{DatasourceType.TABLE}" + command = CreateExplorePermalinkCommand( + mock_g.user, {"formData": {"datasource": datasource, "slice_id": slice.id}} + ) + + assert isinstance(command.run(), str) + + @patch("superset.security.manager.g") + @pytest.mark.usefixtures("create_dataset", "create_slice") + def test_get_permalink_command(self, mock_g): + mock_g.user = security_manager.find_user("admin") + app.config["EXPLORE_FORM_DATA_CACHE_CONFIG"] = { + "REFRESH_TIMEOUT_ON_RETRIEVAL": True + } + + dataset = ( + db.session.query(SqlaTable).filter_by(table_name="dummy_sql_table").first() + ) + slice = db.session.query(Slice).filter_by(slice_name="slice_name").first() + + datasource = f"{dataset.id}__{DatasourceType.TABLE}" + + key = CreateExplorePermalinkCommand( + mock_g.user, {"formData": {"datasource": datasource, "slice_id": slice.id}} + ).run() + + get_command = GetExplorePermalinkCommand(mock_g.user, key) + cache_data = get_command.run() + + assert cache_data.get("datasource") == datasource + + @patch("superset.security.manager.g") + @patch("superset.key_value.commands.get.GetKeyValueCommand.run") + @patch("superset.explore.permalink.commands.get.decode_permalink_id") + @pytest.mark.usefixtures("create_dataset", "create_slice") + def test_get_permalink_command_with_old_dataset_key( + self, decode_id_mock, get_kv_command_mock, mock_g + ): + mock_g.user = security_manager.find_user("admin") + app.config["EXPLORE_FORM_DATA_CACHE_CONFIG"] = { + "REFRESH_TIMEOUT_ON_RETRIEVAL": True + } + + dataset = ( + db.session.query(SqlaTable).filter_by(table_name="dummy_sql_table").first() + ) + slice = db.session.query(Slice).filter_by(slice_name="slice_name").first() + + datasource_string = f"{dataset.id}__{DatasourceType.TABLE}" + + decode_id_mock.return_value = "123456" + get_kv_command_mock.return_value = { + "chartId": slice.id, + "datasetId": dataset.id, + "datasource": datasource_string, + "state": { + "formData": {"datasource": datasource_string, "slice_id": slice.id} + }, + } + get_command = GetExplorePermalinkCommand(mock_g.user, "thisisallmocked") + cache_data = get_command.run() + + assert cache_data.get("datasource") == datasource_string