You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2022/01/20 19:29:06 UTC
[superset] branch master updated: feat: Adds a key-value endpoint to store charts form data (#17882)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 959b15e feat: Adds a key-value endpoint to store charts form data (#17882)
959b15e is described below
commit 959b15eeca0a1e2e93c1a8688f8629d0f6ee5776
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Thu Jan 20 16:27:57 2022 -0300
feat: Adds a key-value endpoint to store charts form data (#17882)
* feat: Adds a key-value endpoint to store charts form data
* Fixes linting problems
* Removes the query_params from the endpoints
* Refactors the commands
* Removes unused imports
* Changes the parameters to use dataclass
* Adds more access tests
* Gets the first dataset while testing
* Adds unit tests for the check_access function
* Changes the can_access check
* Always check for dataset access
---
superset/charts/commands/exceptions.py | 4 +
.../entry.py => charts/form_data/__init__.py} | 6 -
.../filter_state => charts/form_data}/api.py | 60 +++--
.../form_data/commands/__init__.py} | 6 -
.../form_data}/commands/create.py | 27 +--
.../form_data}/commands/delete.py | 35 ++-
superset/charts/form_data/commands/get.py | 44 ++++
.../form_data}/commands/update.py | 40 ++--
superset/charts/form_data/utils.py | 70 ++++++
superset/config.py | 11 +-
superset/dashboards/filter_state/api.py | 4 +-
.../dashboards/filter_state/commands/create.py | 21 +-
.../dashboards/filter_state/commands/delete.py | 20 +-
superset/dashboards/filter_state/commands/get.py | 16 +-
.../dashboards/filter_state/commands/update.py | 25 +--
superset/datasets/commands/exceptions.py | 4 +
superset/initialization/__init__.py | 8 +-
superset/key_value/api.py | 112 +++++++---
superset/key_value/commands/create.py | 23 +-
superset/key_value/commands/delete.py | 16 +-
.../filter_state => key_value}/commands/entry.py | 0
superset/key_value/commands/get.py | 18 +-
.../entry.py => key_value/commands/parameters.py} | 15 +-
superset/key_value/commands/update.py | 19 +-
superset/utils/cache_manager.py | 12 +
.../integration_tests/charts/form_data/__init__.py | 6 -
.../charts/form_data/api_tests.py | 245 +++++++++++++++++++++
.../dashboards/filter_state/api_tests.py | 2 +-
tests/unit_tests/charts/form_data/utils_test.py | 186 ++++++++++++++++
29 files changed, 826 insertions(+), 229 deletions(-)
diff --git a/superset/charts/commands/exceptions.py b/superset/charts/commands/exceptions.py
index 60a62e1..6d5c078 100644
--- a/superset/charts/commands/exceptions.py
+++ b/superset/charts/commands/exceptions.py
@@ -127,6 +127,10 @@ class ChartDeleteFailedReportsExistError(ChartDeleteFailedError):
message = _("There are associated alerts or reports")
+class ChartAccessDeniedError(ForbiddenError):
+ message = _("You don't have access to this chart.")
+
+
class ChartForbiddenError(ForbiddenError):
message = _("Changing this chart is forbidden")
diff --git a/superset/dashboards/filter_state/commands/entry.py b/superset/charts/form_data/__init__.py
similarity index 89%
copy from superset/dashboards/filter_state/commands/entry.py
copy to superset/charts/form_data/__init__.py
index 0e9ad0a..13a8339 100644
--- a/superset/dashboards/filter_state/commands/entry.py
+++ b/superset/charts/form_data/__init__.py
@@ -14,9 +14,3 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing_extensions import TypedDict
-
-
-class Entry(TypedDict):
- owner: int
- value: str
diff --git a/superset/dashboards/filter_state/api.py b/superset/charts/form_data/api.py
similarity index 79%
copy from superset/dashboards/filter_state/api.py
copy to superset/charts/form_data/api.py
index 2a3e405..bc908b2 100644
--- a/superset/dashboards/filter_state/api.py
+++ b/superset/charts/form_data/api.py
@@ -20,34 +20,34 @@ from typing import Type
from flask import Response
from flask_appbuilder.api import expose, protect, safe
-from superset.dashboards.filter_state.commands.create import CreateFilterStateCommand
-from superset.dashboards.filter_state.commands.delete import DeleteFilterStateCommand
-from superset.dashboards.filter_state.commands.get import GetFilterStateCommand
-from superset.dashboards.filter_state.commands.update import UpdateFilterStateCommand
+from superset.charts.form_data.commands.create import CreateFormDataCommand
+from superset.charts.form_data.commands.delete import DeleteFormDataCommand
+from superset.charts.form_data.commands.get import GetFormDataCommand
+from superset.charts.form_data.commands.update import UpdateFormDataCommand
from superset.extensions import event_logger
from superset.key_value.api import KeyValueRestApi
logger = logging.getLogger(__name__)
-class DashboardFilterStateRestApi(KeyValueRestApi):
- class_permission_name = "FilterStateRestApi"
- resource_name = "dashboard"
- openapi_spec_tag = "Filter State"
+class ChartFormDataRestApi(KeyValueRestApi):
+ class_permission_name = "ChartFormDataRestApi"
+ resource_name = "chart"
+ openapi_spec_tag = "Chart Form Data"
- def get_create_command(self) -> Type[CreateFilterStateCommand]:
- return CreateFilterStateCommand
+ def get_create_command(self) -> Type[CreateFormDataCommand]:
+ return CreateFormDataCommand
- def get_update_command(self) -> Type[UpdateFilterStateCommand]:
- return UpdateFilterStateCommand
+ def get_update_command(self) -> Type[UpdateFormDataCommand]:
+ return UpdateFormDataCommand
- def get_get_command(self) -> Type[GetFilterStateCommand]:
- return GetFilterStateCommand
+ def get_get_command(self) -> Type[GetFormDataCommand]:
+ return GetFormDataCommand
- def get_delete_command(self) -> Type[DeleteFilterStateCommand]:
- return DeleteFilterStateCommand
+ def get_delete_command(self) -> Type[DeleteFormDataCommand]:
+ return DeleteFormDataCommand
- @expose("/<int:pk>/filter_state", methods=["POST"])
+ @expose("/<int:pk>/form_data", methods=["POST"])
@protect()
@safe
@event_logger.log_this_with_context(
@@ -65,6 +65,11 @@ class DashboardFilterStateRestApi(KeyValueRestApi):
schema:
type: integer
name: pk
+ - in: query
+ schema:
+ type: integer
+ name: dataset_id
+ required: true
requestBody:
required: true
content:
@@ -93,7 +98,7 @@ class DashboardFilterStateRestApi(KeyValueRestApi):
"""
return super().post(pk)
- @expose("/<int:pk>/filter_state/<string:key>/", methods=["PUT"])
+ @expose("/<int:pk>/form_data/<string:key>", methods=["PUT"])
@protect()
@safe
@event_logger.log_this_with_context(
@@ -115,6 +120,11 @@ class DashboardFilterStateRestApi(KeyValueRestApi):
schema:
type: string
name: key
+ - in: query
+ schema:
+ type: integer
+ name: dataset_id
+ required: true
requestBody:
required: true
content:
@@ -145,7 +155,7 @@ class DashboardFilterStateRestApi(KeyValueRestApi):
"""
return super().put(pk, key)
- @expose("/<int:pk>/filter_state/<string:key>/", methods=["GET"])
+ @expose("/<int(signed=True):pk>/form_data/<string:key>", methods=["GET"])
@protect()
@safe
@event_logger.log_this_with_context(
@@ -167,6 +177,11 @@ class DashboardFilterStateRestApi(KeyValueRestApi):
schema:
type: string
name: key
+ - in: query
+ schema:
+ type: integer
+ name: dataset_id
+ required: true
responses:
200:
description: Returns the stored value.
@@ -191,7 +206,7 @@ class DashboardFilterStateRestApi(KeyValueRestApi):
"""
return super().get(pk, key)
- @expose("/<int:pk>/filter_state/<string:key>/", methods=["DELETE"])
+ @expose("/<int:pk>/form_data/<string:key>", methods=["DELETE"])
@protect()
@safe
@event_logger.log_this_with_context(
@@ -214,6 +229,11 @@ class DashboardFilterStateRestApi(KeyValueRestApi):
type: string
name: key
description: The value key.
+ - in: query
+ schema:
+ type: integer
+ name: dataset_id
+ required: true
responses:
200:
description: Deleted the stored value.
diff --git a/superset/dashboards/filter_state/commands/entry.py b/superset/charts/form_data/commands/__init__.py
similarity index 89%
copy from superset/dashboards/filter_state/commands/entry.py
copy to superset/charts/form_data/commands/__init__.py
index 0e9ad0a..13a8339 100644
--- a/superset/dashboards/filter_state/commands/entry.py
+++ b/superset/charts/form_data/commands/__init__.py
@@ -14,9 +14,3 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing_extensions import TypedDict
-
-
-class Entry(TypedDict):
- owner: int
- value: str
diff --git a/superset/dashboards/filter_state/commands/create.py b/superset/charts/form_data/commands/create.py
similarity index 62%
copy from superset/dashboards/filter_state/commands/create.py
copy to superset/charts/form_data/commands/create.py
index 045b2fa..d7c2ad4 100644
--- a/superset/dashboards/filter_state/commands/create.py
+++ b/superset/charts/form_data/commands/create.py
@@ -14,25 +14,22 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing import Optional
-
-from flask_appbuilder.security.sqla.models import User
-
-from superset.dashboards.dao import DashboardDAO
-from superset.dashboards.filter_state.commands.entry import Entry
+from superset.charts.form_data.utils import check_access, get_dataset_id
from superset.extensions import cache_manager
from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.entry import Entry
+from superset.key_value.commands.parameters import CommandParameters
from superset.key_value.utils import cache_key
-class CreateFilterStateCommand(CreateKeyValueCommand):
- def create(
- self, actor: User, resource_id: int, key: str, value: str
- ) -> Optional[bool]:
- dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
- if dashboard:
+class CreateFormDataCommand(CreateKeyValueCommand):
+ def create(self, cmd_params: CommandParameters) -> bool:
+ check_access(cmd_params)
+ resource_id = cmd_params.resource_id
+ actor = cmd_params.actor
+ key = cache_key(resource_id or get_dataset_id(cmd_params), cmd_params.key)
+ value = cmd_params.value
+ if value:
entry: Entry = {"owner": actor.get_user_id(), "value": value}
- return cache_manager.filter_state_cache.set(
- cache_key(resource_id, key), entry
- )
+ return cache_manager.chart_form_data_cache.set(key, entry)
return False
diff --git a/superset/dashboards/filter_state/commands/delete.py b/superset/charts/form_data/commands/delete.py
similarity index 55%
copy from superset/dashboards/filter_state/commands/delete.py
copy to superset/charts/form_data/commands/delete.py
index 0f27911..be6c33a 100644
--- a/superset/dashboards/filter_state/commands/delete.py
+++ b/superset/charts/form_data/commands/delete.py
@@ -14,29 +14,24 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing import Optional
-
-from flask_appbuilder.security.sqla.models import User
-
-from superset.dashboards.dao import DashboardDAO
-from superset.dashboards.filter_state.commands.entry import Entry
+from superset.charts.form_data.utils import check_access, get_dataset_id
from superset.extensions import cache_manager
from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.entry import Entry
from superset.key_value.commands.exceptions import KeyValueAccessDeniedError
+from superset.key_value.commands.parameters import CommandParameters
from superset.key_value.utils import cache_key
-class DeleteFilterStateCommand(DeleteKeyValueCommand):
- def delete(self, actor: User, resource_id: int, key: str) -> Optional[bool]:
- dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
- if dashboard:
- entry: Entry = cache_manager.filter_state_cache.get(
- cache_key(resource_id, key)
- )
- if entry:
- if entry["owner"] != actor.get_user_id():
- raise KeyValueAccessDeniedError()
- return cache_manager.filter_state_cache.delete(
- cache_key(resource_id, key)
- )
- return False
+class DeleteFormDataCommand(DeleteKeyValueCommand):
+ def delete(self, cmd_params: CommandParameters) -> bool:
+ check_access(cmd_params)
+ resource_id = cmd_params.resource_id
+ actor = cmd_params.actor
+ key = cache_key(resource_id or get_dataset_id(cmd_params), cmd_params.key)
+ entry: Entry = cache_manager.chart_form_data_cache.get(key)
+ if entry:
+ if entry["owner"] != actor.get_user_id():
+ raise KeyValueAccessDeniedError()
+ return cache_manager.chart_form_data_cache.delete(key)
+ return True
diff --git a/superset/charts/form_data/commands/get.py b/superset/charts/form_data/commands/get.py
new file mode 100644
index 0000000..f23acfc
--- /dev/null
+++ b/superset/charts/form_data/commands/get.py
@@ -0,0 +1,44 @@
+# 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 typing import Optional
+
+from flask import current_app as app
+
+from superset.charts.form_data.utils import check_access, get_dataset_id
+from superset.extensions import cache_manager
+from superset.key_value.commands.entry import Entry
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.parameters import CommandParameters
+from superset.key_value.utils import cache_key
+
+
+class GetFormDataCommand(GetKeyValueCommand):
+ def __init__(self, cmd_params: CommandParameters) -> None:
+ super().__init__(cmd_params)
+ config = app.config["CHART_FORM_DATA_CACHE_CONFIG"]
+ self._refresh_timeout = config.get("REFRESH_TIMEOUT_ON_RETRIEVAL")
+
+ def get(self, cmd_params: CommandParameters) -> Optional[str]:
+ check_access(cmd_params)
+ resource_id = cmd_params.resource_id
+ key = cache_key(resource_id or get_dataset_id(cmd_params), cmd_params.key)
+ entry: Entry = cache_manager.chart_form_data_cache.get(key)
+ if entry:
+ if self._refresh_timeout:
+ cache_manager.chart_form_data_cache.set(key, entry)
+ return entry["value"]
+ return None
diff --git a/superset/dashboards/filter_state/commands/update.py b/superset/charts/form_data/commands/update.py
similarity index 52%
copy from superset/dashboards/filter_state/commands/update.py
copy to superset/charts/form_data/commands/update.py
index 1412348..cab988d 100644
--- a/superset/dashboards/filter_state/commands/update.py
+++ b/superset/charts/form_data/commands/update.py
@@ -14,33 +14,27 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing import Optional
-
-from flask_appbuilder.security.sqla.models import User
-
-from superset.dashboards.dao import DashboardDAO
-from superset.dashboards.filter_state.commands.entry import Entry
+from superset.charts.form_data.utils import check_access, get_dataset_id
from superset.extensions import cache_manager
+from superset.key_value.commands.entry import Entry
from superset.key_value.commands.exceptions import KeyValueAccessDeniedError
+from superset.key_value.commands.parameters import CommandParameters
from superset.key_value.commands.update import UpdateKeyValueCommand
from superset.key_value.utils import cache_key
-class UpdateFilterStateCommand(UpdateKeyValueCommand):
- def update(
- self, actor: User, resource_id: int, key: str, value: str
- ) -> Optional[bool]:
- dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
- if dashboard:
- entry: Entry = cache_manager.filter_state_cache.get(
- cache_key(resource_id, key)
- )
- if entry:
- user_id = actor.get_user_id()
- if entry["owner"] != user_id:
- raise KeyValueAccessDeniedError()
- new_entry: Entry = {"owner": actor.get_user_id(), "value": value}
- return cache_manager.filter_state_cache.set(
- cache_key(resource_id, key), new_entry
- )
+class UpdateFormDataCommand(UpdateKeyValueCommand):
+ def update(self, cmd_params: CommandParameters) -> bool:
+ check_access(cmd_params)
+ resource_id = cmd_params.resource_id
+ actor = cmd_params.actor
+ key = cache_key(resource_id or get_dataset_id(cmd_params), cmd_params.key)
+ value = cmd_params.value
+ entry: Entry = cache_manager.chart_form_data_cache.get(key)
+ if entry and value:
+ user_id = actor.get_user_id()
+ if entry["owner"] != user_id:
+ raise KeyValueAccessDeniedError()
+ new_entry: Entry = {"owner": actor.get_user_id(), "value": value}
+ return cache_manager.chart_form_data_cache.set(key, new_entry)
return False
diff --git a/superset/charts/form_data/utils.py b/superset/charts/form_data/utils.py
new file mode 100644
index 0000000..c1f98a2
--- /dev/null
+++ b/superset/charts/form_data/utils.py
@@ -0,0 +1,70 @@
+# 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 typing import Optional
+
+from superset import security_manager
+from superset.charts.commands.exceptions import (
+ ChartAccessDeniedError,
+ ChartNotFoundError,
+)
+from superset.charts.dao import ChartDAO
+from superset.datasets.commands.exceptions import (
+ DatasetAccessDeniedError,
+ DatasetNotFoundError,
+)
+from superset.datasets.dao import DatasetDAO
+from superset.key_value.commands.parameters import CommandParameters
+from superset.views.base import is_user_admin
+from superset.views.utils import is_owner
+
+
+def get_dataset_id(cmd_params: CommandParameters) -> Optional[str]:
+ query_params = cmd_params.query_params
+ if query_params:
+ return query_params.get("dataset_id")
+ return None
+
+
+def check_dataset_access(cmd_params: CommandParameters) -> Optional[bool]:
+ dataset_id = get_dataset_id(cmd_params)
+ if dataset_id:
+ dataset = DatasetDAO.find_by_id(int(dataset_id))
+ if dataset:
+ can_access_datasource = security_manager.can_access_datasource(dataset)
+ if can_access_datasource:
+ return True
+ raise DatasetAccessDeniedError()
+ raise DatasetNotFoundError()
+
+
+def check_access(cmd_params: CommandParameters) -> Optional[bool]:
+ resource_id = cmd_params.resource_id
+ actor = cmd_params.actor
+ check_dataset_access(cmd_params)
+ if resource_id == 0:
+ return True
+ chart = ChartDAO.find_by_id(resource_id)
+ if chart:
+ can_access_chart = (
+ is_user_admin()
+ or is_owner(chart, actor)
+ or security_manager.can_access("can_read", "Chart")
+ )
+ if can_access_chart:
+ return True
+ raise ChartAccessDeniedError()
+ raise ChartNotFoundError()
diff --git a/superset/config.py b/superset/config.py
index f555296..b9a98c4 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -577,13 +577,22 @@ DATA_CACHE_CONFIG: CacheConfig = {"CACHE_TYPE": "null"}
# Cache for filters state
FILTER_STATE_CACHE_CONFIG: CacheConfig = {
- "CACHE_TYPE": "filesystem",
+ "CACHE_TYPE": "FileSystemCache",
"CACHE_DIR": os.path.join(DATA_DIR, "cache"),
"CACHE_DEFAULT_TIMEOUT": int(timedelta(days=90).total_seconds()),
"CACHE_THRESHOLD": 0,
"REFRESH_TIMEOUT_ON_RETRIEVAL": True,
}
+# Cache for chart form data
+CHART_FORM_DATA_CACHE_CONFIG: CacheConfig = {
+ "CACHE_TYPE": "FileSystemCache",
+ "CACHE_DIR": os.path.join(DATA_DIR, "cache"),
+ "CACHE_DEFAULT_TIMEOUT": int(timedelta(days=7).total_seconds()),
+ "CACHE_THRESHOLD": 0,
+ "REFRESH_TIMEOUT_ON_RETRIEVAL": True,
+}
+
# store cache keys by datasource UID (via CacheKey) for custom processing/invalidation
STORE_CACHE_KEYS_IN_METADATA_DB = False
diff --git a/superset/dashboards/filter_state/api.py b/superset/dashboards/filter_state/api.py
index 2a3e405..68209f3 100644
--- a/superset/dashboards/filter_state/api.py
+++ b/superset/dashboards/filter_state/api.py
@@ -31,9 +31,9 @@ logger = logging.getLogger(__name__)
class DashboardFilterStateRestApi(KeyValueRestApi):
- class_permission_name = "FilterStateRestApi"
+ class_permission_name = "DashboardFilterStateRestApi"
resource_name = "dashboard"
- openapi_spec_tag = "Filter State"
+ openapi_spec_tag = "Dashboard Filter State"
def get_create_command(self) -> Type[CreateFilterStateCommand]:
return CreateFilterStateCommand
diff --git a/superset/dashboards/filter_state/commands/create.py b/superset/dashboards/filter_state/commands/create.py
index 045b2fa..40a70ba 100644
--- a/superset/dashboards/filter_state/commands/create.py
+++ b/superset/dashboards/filter_state/commands/create.py
@@ -14,25 +14,22 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing import Optional
-
-from flask_appbuilder.security.sqla.models import User
-
from superset.dashboards.dao import DashboardDAO
-from superset.dashboards.filter_state.commands.entry import Entry
from superset.extensions import cache_manager
from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.entry import Entry
+from superset.key_value.commands.parameters import CommandParameters
from superset.key_value.utils import cache_key
class CreateFilterStateCommand(CreateKeyValueCommand):
- def create(
- self, actor: User, resource_id: int, key: str, value: str
- ) -> Optional[bool]:
+ def create(self, cmd_params: CommandParameters) -> bool:
+ resource_id = cmd_params.resource_id
+ actor = cmd_params.actor
+ key = cache_key(resource_id, cmd_params.key)
+ value = cmd_params.value
dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
- if dashboard:
+ if dashboard and value:
entry: Entry = {"owner": actor.get_user_id(), "value": value}
- return cache_manager.filter_state_cache.set(
- cache_key(resource_id, key), entry
- )
+ return cache_manager.filter_state_cache.set(key, entry)
return False
diff --git a/superset/dashboards/filter_state/commands/delete.py b/superset/dashboards/filter_state/commands/delete.py
index 0f27911..225dd8a 100644
--- a/superset/dashboards/filter_state/commands/delete.py
+++ b/superset/dashboards/filter_state/commands/delete.py
@@ -14,29 +14,25 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing import Optional
-
-from flask_appbuilder.security.sqla.models import User
-
from superset.dashboards.dao import DashboardDAO
-from superset.dashboards.filter_state.commands.entry import Entry
from superset.extensions import cache_manager
from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.entry import Entry
from superset.key_value.commands.exceptions import KeyValueAccessDeniedError
+from superset.key_value.commands.parameters import CommandParameters
from superset.key_value.utils import cache_key
class DeleteFilterStateCommand(DeleteKeyValueCommand):
- def delete(self, actor: User, resource_id: int, key: str) -> Optional[bool]:
+ def delete(self, cmd_params: CommandParameters) -> bool:
+ resource_id = cmd_params.resource_id
+ actor = cmd_params.actor
+ key = cache_key(resource_id, cmd_params.key)
dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
if dashboard:
- entry: Entry = cache_manager.filter_state_cache.get(
- cache_key(resource_id, key)
- )
+ entry: Entry = cache_manager.filter_state_cache.get(key)
if entry:
if entry["owner"] != actor.get_user_id():
raise KeyValueAccessDeniedError()
- return cache_manager.filter_state_cache.delete(
- cache_key(resource_id, key)
- )
+ return cache_manager.filter_state_cache.delete(key)
return False
diff --git a/superset/dashboards/filter_state/commands/get.py b/superset/dashboards/filter_state/commands/get.py
index bb3e95a..509960f 100644
--- a/superset/dashboards/filter_state/commands/get.py
+++ b/superset/dashboards/filter_state/commands/get.py
@@ -16,16 +16,26 @@
# under the License.
from typing import Optional
+from flask import current_app as app
+
from superset.dashboards.dao import DashboardDAO
from superset.extensions import cache_manager
from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.parameters import CommandParameters
from superset.key_value.utils import cache_key
class GetFilterStateCommand(GetKeyValueCommand):
- def get(self, resource_id: int, key: str, refresh_timeout: bool) -> Optional[str]:
+ def __init__(self, cmd_params: CommandParameters) -> None:
+ super().__init__(cmd_params)
+ config = app.config["FILTER_STATE_CACHE_CONFIG"]
+ self._refresh_timeout = config.get("REFRESH_TIMEOUT_ON_RETRIEVAL")
+
+ def get(self, cmd_params: CommandParameters) -> Optional[str]:
+ resource_id = cmd_params.resource_id
+ key = cache_key(resource_id, cmd_params.key)
DashboardDAO.get_by_id_or_slug(str(resource_id))
- entry = cache_manager.filter_state_cache.get(cache_key(resource_id, key)) or {}
- if entry and refresh_timeout:
+ entry = cache_manager.filter_state_cache.get(key) or {}
+ if entry and self._refresh_timeout:
cache_manager.filter_state_cache.set(key, entry)
return entry.get("value")
diff --git a/superset/dashboards/filter_state/commands/update.py b/superset/dashboards/filter_state/commands/update.py
index 1412348..e8d0606 100644
--- a/superset/dashboards/filter_state/commands/update.py
+++ b/superset/dashboards/filter_state/commands/update.py
@@ -14,33 +14,28 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing import Optional
-
-from flask_appbuilder.security.sqla.models import User
-
from superset.dashboards.dao import DashboardDAO
-from superset.dashboards.filter_state.commands.entry import Entry
from superset.extensions import cache_manager
+from superset.key_value.commands.entry import Entry
from superset.key_value.commands.exceptions import KeyValueAccessDeniedError
+from superset.key_value.commands.parameters import CommandParameters
from superset.key_value.commands.update import UpdateKeyValueCommand
from superset.key_value.utils import cache_key
class UpdateFilterStateCommand(UpdateKeyValueCommand):
- def update(
- self, actor: User, resource_id: int, key: str, value: str
- ) -> Optional[bool]:
+ def update(self, cmd_params: CommandParameters) -> bool:
+ resource_id = cmd_params.resource_id
+ actor = cmd_params.actor
+ key = cache_key(resource_id, cmd_params.key)
+ value = cmd_params.value
dashboard = DashboardDAO.get_by_id_or_slug(str(resource_id))
- if dashboard:
- entry: Entry = cache_manager.filter_state_cache.get(
- cache_key(resource_id, key)
- )
+ if dashboard and value:
+ entry: Entry = cache_manager.filter_state_cache.get(key)
if entry:
user_id = actor.get_user_id()
if entry["owner"] != user_id:
raise KeyValueAccessDeniedError()
new_entry: Entry = {"owner": actor.get_user_id(), "value": value}
- return cache_manager.filter_state_cache.set(
- cache_key(resource_id, key), new_entry
- )
+ return cache_manager.filter_state_cache.set(key, new_entry)
return False
diff --git a/superset/datasets/commands/exceptions.py b/superset/datasets/commands/exceptions.py
index 44064f0..34c1572 100644
--- a/superset/datasets/commands/exceptions.py
+++ b/superset/datasets/commands/exceptions.py
@@ -179,3 +179,7 @@ class DatasetForbiddenError(ForbiddenError):
class DatasetImportError(ImportFailedError):
message = _("Import dataset failed for an unknown reason")
+
+
+class DatasetAccessDeniedError(ForbiddenError):
+ message = _("You don't have access to this dataset.")
diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py
index 95e260d..98cd9af 100644
--- a/superset/initialization/__init__.py
+++ b/superset/initialization/__init__.py
@@ -119,6 +119,7 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods
from superset.cachekeys.api import CacheRestApi
from superset.charts.api import ChartRestApi
from superset.charts.data.api import ChartDataRestApi
+ from superset.charts.form_data.api import ChartFormDataRestApi
from superset.connectors.druid.views import (
Druid,
DruidClusterModelView,
@@ -203,18 +204,19 @@ class SupersetAppInitializer: # pylint: disable=too-many-public-methods
appbuilder.add_api(CacheRestApi)
appbuilder.add_api(ChartRestApi)
appbuilder.add_api(ChartDataRestApi)
+ appbuilder.add_api(ChartFormDataRestApi)
appbuilder.add_api(CssTemplateRestApi)
+ appbuilder.add_api(DashboardFilterStateRestApi)
appbuilder.add_api(DashboardRestApi)
appbuilder.add_api(DatabaseRestApi)
appbuilder.add_api(DatasetRestApi)
appbuilder.add_api(DatasetColumnsRestApi)
appbuilder.add_api(DatasetMetricRestApi)
+ appbuilder.add_api(FilterSetRestApi)
appbuilder.add_api(QueryRestApi)
- appbuilder.add_api(SavedQueryRestApi)
appbuilder.add_api(ReportScheduleRestApi)
appbuilder.add_api(ReportExecutionLogRestApi)
- appbuilder.add_api(FilterSetRestApi)
- appbuilder.add_api(DashboardFilterStateRestApi)
+ appbuilder.add_api(SavedQueryRestApi)
#
# Setup regular views
#
diff --git a/superset/key_value/api.py b/superset/key_value/api.py
index 2dc3400..85aa6e3 100644
--- a/superset/key_value/api.py
+++ b/superset/key_value/api.py
@@ -19,17 +19,27 @@ from abc import ABC, abstractmethod
from typing import Any
from apispec import APISpec
+from apispec.exceptions import DuplicateComponentNameError
from flask import g, request, Response
from flask_appbuilder.api import BaseApi
from marshmallow import ValidationError
+from superset.charts.commands.exceptions import (
+ ChartAccessDeniedError,
+ ChartNotFoundError,
+)
from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.dashboards.commands.exceptions import (
DashboardAccessDeniedError,
DashboardNotFoundError,
)
+from superset.datasets.commands.exceptions import (
+ DatasetAccessDeniedError,
+ DatasetNotFoundError,
+)
from superset.exceptions import InvalidPayloadFormatError
from superset.key_value.commands.exceptions import KeyValueAccessDeniedError
+from superset.key_value.commands.parameters import CommandParameters
from superset.key_value.schemas import KeyValuePostSchema, KeyValuePutSchema
logger = logging.getLogger(__name__)
@@ -48,12 +58,15 @@ class KeyValueRestApi(BaseApi, ABC):
allow_browser_login = True
def add_apispec_components(self, api_spec: APISpec) -> None:
- api_spec.components.schema(
- KeyValuePostSchema.__name__, schema=KeyValuePostSchema,
- )
- api_spec.components.schema(
- KeyValuePutSchema.__name__, schema=KeyValuePutSchema,
- )
+ try:
+ api_spec.components.schema(
+ KeyValuePostSchema.__name__, schema=KeyValuePostSchema,
+ )
+ api_spec.components.schema(
+ KeyValuePutSchema.__name__, schema=KeyValuePutSchema,
+ )
+ except DuplicateComponentNameError:
+ pass
super().add_apispec_components(api_spec)
def post(self, pk: int) -> Response:
@@ -61,52 +74,91 @@ class KeyValueRestApi(BaseApi, ABC):
raise InvalidPayloadFormatError("Request is not JSON")
try:
item = self.add_model_schema.load(request.json)
- key = self.get_create_command()(g.user, pk, item["value"]).run()
+ args = CommandParameters(
+ actor=g.user,
+ resource_id=pk,
+ value=item["value"],
+ query_params=request.args,
+ )
+ key = self.get_create_command()(args).run()
return self.response(201, key=key)
- except ValidationError as error:
- return self.response_400(message=error.messages)
- except (DashboardAccessDeniedError, KeyValueAccessDeniedError):
- return self.response_403()
- except DashboardNotFoundError:
- return self.response_404()
+ except ValidationError as ex:
+ return self.response(400, message=ex.messages)
+ except (
+ ChartAccessDeniedError,
+ DashboardAccessDeniedError,
+ DatasetAccessDeniedError,
+ KeyValueAccessDeniedError,
+ ) as ex:
+ return self.response(403, message=str(ex))
+ except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex:
+ return self.response(404, message=str(ex))
def put(self, pk: int, key: str) -> Response:
if not request.is_json:
raise InvalidPayloadFormatError("Request is not JSON")
try:
item = self.edit_model_schema.load(request.json)
- result = self.get_update_command()(g.user, pk, key, item["value"]).run()
+ args = CommandParameters(
+ actor=g.user,
+ resource_id=pk,
+ key=key,
+ value=item["value"],
+ query_params=request.args,
+ )
+ result = self.get_update_command()(args).run()
if not result:
return self.response_404()
return self.response(200, message="Value updated successfully.")
- except ValidationError as error:
- return self.response_400(message=error.messages)
- except (DashboardAccessDeniedError, KeyValueAccessDeniedError):
- return self.response_403()
- except DashboardNotFoundError:
- return self.response_404()
+ except ValidationError as ex:
+ return self.response(400, message=ex.messages)
+ except (
+ ChartAccessDeniedError,
+ DashboardAccessDeniedError,
+ DatasetAccessDeniedError,
+ KeyValueAccessDeniedError,
+ ) as ex:
+ return self.response(403, message=str(ex))
+ except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex:
+ return self.response(404, message=str(ex))
def get(self, pk: int, key: str) -> Response:
try:
- value = self.get_get_command()(g.user, pk, key).run()
+ args = CommandParameters(
+ actor=g.user, resource_id=pk, key=key, query_params=request.args
+ )
+ value = self.get_get_command()(args).run()
if not value:
return self.response_404()
return self.response(200, value=value)
- except (DashboardAccessDeniedError, KeyValueAccessDeniedError):
- return self.response_403()
- except DashboardNotFoundError:
- return self.response_404()
+ except (
+ ChartAccessDeniedError,
+ DashboardAccessDeniedError,
+ DatasetAccessDeniedError,
+ KeyValueAccessDeniedError,
+ ) as ex:
+ return self.response(403, message=str(ex))
+ except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex:
+ return self.response(404, message=str(ex))
def delete(self, pk: int, key: str) -> Response:
try:
- result = self.get_delete_command()(g.user, pk, key).run()
+ args = CommandParameters(
+ actor=g.user, resource_id=pk, key=key, query_params=request.args
+ )
+ result = self.get_delete_command()(args).run()
if not result:
return self.response_404()
return self.response(200, message="Deleted successfully")
- except (DashboardAccessDeniedError, KeyValueAccessDeniedError):
- return self.response_403()
- except DashboardNotFoundError:
- return self.response_404()
+ except (
+ ChartAccessDeniedError,
+ DashboardAccessDeniedError,
+ DatasetAccessDeniedError,
+ KeyValueAccessDeniedError,
+ ) as ex:
+ return self.response(403, message=str(ex))
+ except (ChartNotFoundError, DashboardNotFoundError, DatasetNotFoundError) as ex:
+ return self.response(404, message=str(ex))
@abstractmethod
def get_create_command(self) -> Any:
diff --git a/superset/key_value/commands/create.py b/superset/key_value/commands/create.py
index 3c56f73..8a7eec9 100644
--- a/superset/key_value/commands/create.py
+++ b/superset/key_value/commands/create.py
@@ -17,30 +17,25 @@
import logging
from abc import ABC, abstractmethod
from secrets import token_urlsafe
-from typing import Optional
-from flask_appbuilder.models.sqla import Model
-from flask_appbuilder.security.sqla.models import User
from sqlalchemy.exc import SQLAlchemyError
from superset.commands.base import BaseCommand
from superset.key_value.commands.exceptions import KeyValueCreateFailedError
+from superset.key_value.commands.parameters import CommandParameters
logger = logging.getLogger(__name__)
class CreateKeyValueCommand(BaseCommand, ABC):
- def __init__(
- self, actor: User, resource_id: int, value: str,
- ):
- self._actor = actor
- self._resource_id = resource_id
- self._value = value
-
- def run(self) -> Model:
+ def __init__(self, cmd_params: CommandParameters):
+ self._cmd_params = cmd_params
+
+ def run(self) -> str:
try:
key = token_urlsafe(48)
- self.create(self._actor, self._resource_id, key, self._value)
+ self._cmd_params.key = key
+ self.create(self._cmd_params)
return key
except SQLAlchemyError as ex:
logger.exception("Error running create command")
@@ -50,7 +45,5 @@ class CreateKeyValueCommand(BaseCommand, ABC):
pass
@abstractmethod
- def create(
- self, actor: User, resource_id: int, key: str, value: str
- ) -> Optional[bool]:
+ def create(self, cmd_params: CommandParameters) -> bool:
...
diff --git a/superset/key_value/commands/delete.py b/superset/key_value/commands/delete.py
index 9c885d0..6d2983c 100644
--- a/superset/key_value/commands/delete.py
+++ b/superset/key_value/commands/delete.py
@@ -16,27 +16,23 @@
# under the License.
import logging
from abc import ABC, abstractmethod
-from typing import Optional
-from flask_appbuilder.models.sqla import Model
-from flask_appbuilder.security.sqla.models import User
from sqlalchemy.exc import SQLAlchemyError
from superset.commands.base import BaseCommand
from superset.key_value.commands.exceptions import KeyValueDeleteFailedError
+from superset.key_value.commands.parameters import CommandParameters
logger = logging.getLogger(__name__)
class DeleteKeyValueCommand(BaseCommand, ABC):
- def __init__(self, actor: User, resource_id: int, key: str):
- self._actor = actor
- self._resource_id = resource_id
- self._key = key
+ def __init__(self, cmd_params: CommandParameters):
+ self._cmd_params = cmd_params
- def run(self) -> Model:
+ def run(self) -> bool:
try:
- return self.delete(self._actor, self._resource_id, self._key)
+ return self.delete(self._cmd_params)
except SQLAlchemyError as ex:
logger.exception("Error running delete command")
raise KeyValueDeleteFailedError() from ex
@@ -45,5 +41,5 @@ class DeleteKeyValueCommand(BaseCommand, ABC):
pass
@abstractmethod
- def delete(self, actor: User, resource_id: int, key: str) -> Optional[bool]:
+ def delete(self, cmd_params: CommandParameters) -> bool:
...
diff --git a/superset/dashboards/filter_state/commands/entry.py b/superset/key_value/commands/entry.py
similarity index 100%
copy from superset/dashboards/filter_state/commands/entry.py
copy to superset/key_value/commands/entry.py
diff --git a/superset/key_value/commands/get.py b/superset/key_value/commands/get.py
index 43a5806..a697c80 100644
--- a/superset/key_value/commands/get.py
+++ b/superset/key_value/commands/get.py
@@ -18,28 +18,22 @@ import logging
from abc import ABC, abstractmethod
from typing import Optional
-from flask import current_app as app
-from flask_appbuilder.models.sqla import Model
-from flask_appbuilder.security.sqla.models import User
from sqlalchemy.exc import SQLAlchemyError
from superset.commands.base import BaseCommand
from superset.key_value.commands.exceptions import KeyValueGetFailedError
+from superset.key_value.commands.parameters import CommandParameters
logger = logging.getLogger(__name__)
class GetKeyValueCommand(BaseCommand, ABC):
- def __init__(self, actor: User, resource_id: int, key: str):
- self._actor = actor
- self._resource_id = resource_id
- self._key = key
+ def __init__(self, cmd_params: CommandParameters):
+ self._cmd_params = cmd_params
- def run(self) -> Model:
+ def run(self) -> Optional[str]:
try:
- config = app.config["FILTER_STATE_CACHE_CONFIG"]
- refresh_timeout = config.get("REFRESH_TIMEOUT_ON_RETRIEVAL")
- return self.get(self._resource_id, self._key, refresh_timeout)
+ return self.get(self._cmd_params)
except SQLAlchemyError as ex:
logger.exception("Error running get command")
raise KeyValueGetFailedError() from ex
@@ -48,5 +42,5 @@ class GetKeyValueCommand(BaseCommand, ABC):
pass
@abstractmethod
- def get(self, resource_id: int, key: str, refresh_timeout: bool) -> Optional[str]:
+ def get(self, cmd_params: CommandParameters) -> Optional[str]:
...
diff --git a/superset/dashboards/filter_state/commands/entry.py b/superset/key_value/commands/parameters.py
similarity index 72%
copy from superset/dashboards/filter_state/commands/entry.py
copy to superset/key_value/commands/parameters.py
index 0e9ad0a..00a933c 100644
--- a/superset/dashboards/filter_state/commands/entry.py
+++ b/superset/key_value/commands/parameters.py
@@ -14,9 +14,16 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing_extensions import TypedDict
+from dataclasses import dataclass
+from typing import Dict, Optional
+from flask_appbuilder.security.sqla.models import User
-class Entry(TypedDict):
- owner: int
- value: str
+
+@dataclass
+class CommandParameters:
+ actor: User
+ resource_id: int
+ query_params: Dict[str, str]
+ key: Optional[str] = None
+ value: Optional[str] = None
diff --git a/superset/key_value/commands/update.py b/superset/key_value/commands/update.py
index 3f322c7..7990d21 100644
--- a/superset/key_value/commands/update.py
+++ b/superset/key_value/commands/update.py
@@ -16,30 +16,25 @@
# under the License.
import logging
from abc import ABC, abstractmethod
-from typing import Optional
-from flask_appbuilder.models.sqla import Model
-from flask_appbuilder.security.sqla.models import User
from sqlalchemy.exc import SQLAlchemyError
from superset.commands.base import BaseCommand
from superset.key_value.commands.exceptions import KeyValueUpdateFailedError
+from superset.key_value.commands.parameters import CommandParameters
logger = logging.getLogger(__name__)
class UpdateKeyValueCommand(BaseCommand, ABC):
def __init__(
- self, actor: User, resource_id: int, key: str, value: str,
+ self, cmd_params: CommandParameters,
):
- self._actor = actor
- self._resource_id = resource_id
- self._key = key
- self._value = value
+ self._parameters = cmd_params
- def run(self) -> Model:
+ def run(self) -> bool:
try:
- return self.update(self._actor, self._resource_id, self._key, self._value)
+ return self.update(self._parameters)
except SQLAlchemyError as ex:
logger.exception("Error running update command")
raise KeyValueUpdateFailedError() from ex
@@ -48,7 +43,5 @@ class UpdateKeyValueCommand(BaseCommand, ABC):
pass
@abstractmethod
- def update(
- self, actor: User, resource_id: int, key: str, value: str
- ) -> Optional[bool]:
+ def update(self, cmd_params: CommandParameters) -> bool:
...
diff --git a/superset/utils/cache_manager.py b/superset/utils/cache_manager.py
index 5e6e96e..3624d91 100644
--- a/superset/utils/cache_manager.py
+++ b/superset/utils/cache_manager.py
@@ -26,6 +26,7 @@ class CacheManager:
self._data_cache = Cache()
self._thumbnail_cache = Cache()
self._filter_state_cache = Cache()
+ self._chart_form_data_cache = Cache()
def init_app(self, app: Flask) -> None:
self._cache.init_app(
@@ -56,6 +57,13 @@ class CacheManager:
**app.config["FILTER_STATE_CACHE_CONFIG"],
},
)
+ self._chart_form_data_cache.init_app(
+ app,
+ {
+ "CACHE_DEFAULT_TIMEOUT": app.config["CACHE_DEFAULT_TIMEOUT"],
+ **app.config["CHART_FORM_DATA_CACHE_CONFIG"],
+ },
+ )
@property
def data_cache(self) -> Cache:
@@ -72,3 +80,7 @@ class CacheManager:
@property
def filter_state_cache(self) -> Cache:
return self._filter_state_cache
+
+ @property
+ def chart_form_data_cache(self) -> Cache:
+ return self._chart_form_data_cache
diff --git a/superset/dashboards/filter_state/commands/entry.py b/tests/integration_tests/charts/form_data/__init__.py
similarity index 89%
rename from superset/dashboards/filter_state/commands/entry.py
rename to tests/integration_tests/charts/form_data/__init__.py
index 0e9ad0a..13a8339 100644
--- a/superset/dashboards/filter_state/commands/entry.py
+++ b/tests/integration_tests/charts/form_data/__init__.py
@@ -14,9 +14,3 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-from typing_extensions import TypedDict
-
-
-class Entry(TypedDict):
- owner: int
- value: str
diff --git a/tests/integration_tests/charts/form_data/api_tests.py b/tests/integration_tests/charts/form_data/api_tests.py
new file mode 100644
index 0000000..f411c1a
--- /dev/null
+++ b/tests/integration_tests/charts/form_data/api_tests.py
@@ -0,0 +1,245 @@
+# 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 flask_appbuilder.security.sqla.models import User
+from sqlalchemy.orm import Session
+
+from superset.connectors.sqla.models import SqlaTable
+from superset.datasets.commands.exceptions import DatasetAccessDeniedError
+from superset.extensions import cache_manager
+from superset.key_value.commands.entry import Entry
+from superset.key_value.utils import cache_key
+from superset.models.slice import Slice
+from tests.integration_tests.base_tests import login
+from tests.integration_tests.fixtures.world_bank_dashboard import (
+ load_world_bank_dashboard_with_slices,
+ load_world_bank_data,
+)
+from tests.integration_tests.test_app import app
+
+key = "test-key"
+value = "test"
+
+
+@pytest.fixture
+def client():
+ with app.test_client() as client:
+ with app.app_context():
+ yield client
+
+
+@pytest.fixture
+def chart_id(load_world_bank_dashboard_with_slices) -> int:
+ with app.app_context() as ctx:
+ session: Session = ctx.app.appbuilder.get_session
+ chart = session.query(Slice).filter_by(slice_name="World's Population").one()
+ return chart.id
+
+
+@pytest.fixture
+def admin_id() -> int:
+ with app.app_context() as ctx:
+ session: Session = ctx.app.appbuilder.get_session
+ admin = session.query(User).filter_by(username="admin").one()
+ return admin.id
+
+
+@pytest.fixture
+def dataset_id() -> int:
+ with app.app_context() as ctx:
+ session: Session = ctx.app.appbuilder.get_session
+ dataset = (
+ session.query(SqlaTable)
+ .filter_by(table_name="wb_health_population")
+ .first()
+ )
+ return dataset.id
+
+
+@pytest.fixture(autouse=True)
+def cache(chart_id, admin_id, dataset_id):
+ app.config["CHART_FORM_DATA_CACHE_CONFIG"] = {"CACHE_TYPE": "SimpleCache"}
+ cache_manager.init_app(app)
+ entry: Entry = {"owner": admin_id, "value": value}
+ cache_manager.chart_form_data_cache.set(cache_key(chart_id, key), entry)
+ cache_manager.chart_form_data_cache.set(cache_key(dataset_id, key), entry)
+
+
+def test_post(client, chart_id: int, dataset_id: int):
+ login(client, "admin")
+ payload = {
+ "value": value,
+ }
+ resp = client.post(
+ f"api/v1/chart/{chart_id}/form_data?dataset_id={dataset_id}", json=payload
+ )
+ assert resp.status_code == 201
+
+
+def test_post_bad_request(client, chart_id: int, dataset_id: int):
+ login(client, "admin")
+ payload = {
+ "value": 1234,
+ }
+ resp = client.post(
+ f"api/v1/chart/{chart_id}/form_data?dataset_id={dataset_id}", json=payload
+ )
+ assert resp.status_code == 400
+
+
+def test_post_access_denied(client, chart_id: int, dataset_id: int):
+ login(client, "gamma")
+ payload = {
+ "value": value,
+ }
+ resp = client.post(
+ f"api/v1/chart/{chart_id}/form_data?dataset_id={dataset_id}", json=payload
+ )
+ assert resp.status_code == 404
+
+
+def test_put(client, chart_id: int, dataset_id: int):
+ login(client, "admin")
+ payload = {
+ "value": "new value",
+ }
+ resp = client.put(
+ f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}", json=payload
+ )
+ assert resp.status_code == 200
+
+
+def test_put_bad_request(client, chart_id: int, dataset_id: int):
+ login(client, "admin")
+ payload = {
+ "value": 1234,
+ }
+ resp = client.put(
+ f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}", json=payload
+ )
+ assert resp.status_code == 400
+
+
+def test_put_access_denied(client, chart_id: int, dataset_id: int):
+ login(client, "gamma")
+ payload = {
+ "value": "new value",
+ }
+ resp = client.put(
+ f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}", json=payload
+ )
+ assert resp.status_code == 404
+
+
+def test_put_not_owner(client, chart_id: int, dataset_id: int):
+ login(client, "gamma")
+ payload = {
+ "value": "new value",
+ }
+ resp = client.put(
+ f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}", json=payload
+ )
+ assert resp.status_code == 404
+
+
+def test_get_key_not_found(client, chart_id: int, dataset_id: int):
+ login(client, "admin")
+ resp = client.get(
+ f"api/v1/chart/{chart_id}/form_data/unknown-key?dataset_id={dataset_id}"
+ )
+ assert resp.status_code == 404
+
+
+def test_get_chart_not_found(client, dataset_id: int):
+ login(client, "admin")
+ resp = client.get(f"api/v1/chart/-1/form_data/{key}?dataset_id={dataset_id}")
+ assert resp.status_code == 404
+
+
+def test_get(client, chart_id: int, dataset_id: int):
+ login(client, "admin")
+ resp = client.get(
+ f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}"
+ )
+ assert resp.status_code == 200
+ data = json.loads(resp.data.decode("utf-8"))
+ assert value == data.get("value")
+
+
+def test_get_access_denied(client, chart_id: int, dataset_id: int):
+ login(client, "gamma")
+ resp = client.get(
+ f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}"
+ )
+ assert resp.status_code == 404
+
+
+def test_get_no_dataset(client):
+ login(client, "admin")
+ resp = client.get(f"api/v1/chart/0/form_data/{key}")
+ assert resp.status_code == 404
+
+
+def test_get_dataset(client, dataset_id: int):
+ login(client, "admin")
+ resp = client.get(f"api/v1/chart/0/form_data/{key}?dataset_id={dataset_id}")
+ assert resp.status_code == 200
+
+
+def test_get_dataset_not_found(client):
+ login(client, "admin")
+ resp = client.get(f"api/v1/chart/0/form_data/{key}?dataset_id=-1")
+ assert resp.status_code == 404
+
+
+@patch("superset.security.SupersetSecurityManager.can_access_datasource")
+def test_get_dataset_access_denied(mock_can_access_datasource, client, dataset_id):
+ mock_can_access_datasource.side_effect = DatasetAccessDeniedError()
+ login(client, "admin")
+ resp = client.get(f"api/v1/chart/0/form_data/{key}?dataset_id={dataset_id}")
+ assert resp.status_code == 403
+
+
+def test_delete(client, chart_id: int, dataset_id: int):
+ login(client, "admin")
+ resp = client.delete(
+ f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}"
+ )
+ assert resp.status_code == 200
+
+
+def test_delete_access_denied(client, chart_id: int, dataset_id: int):
+ login(client, "gamma")
+ resp = client.delete(
+ f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}"
+ )
+ assert resp.status_code == 404
+
+
+def test_delete_not_owner(client, chart_id: int, dataset_id: int, admin_id: int):
+ another_key = "another_key"
+ another_owner = admin_id + 1
+ entry: Entry = {"owner": another_owner, "value": value}
+ cache_manager.chart_form_data_cache.set(cache_key(chart_id, another_key), entry)
+ login(client, "admin")
+ resp = client.delete(
+ f"api/v1/chart/{chart_id}/form_data/{another_key}?dataset_id={dataset_id}"
+ )
+ assert resp.status_code == 403
diff --git a/tests/integration_tests/dashboards/filter_state/api_tests.py b/tests/integration_tests/dashboards/filter_state/api_tests.py
index ae667ec..bec6dfc 100644
--- a/tests/integration_tests/dashboards/filter_state/api_tests.py
+++ b/tests/integration_tests/dashboards/filter_state/api_tests.py
@@ -22,8 +22,8 @@ from flask_appbuilder.security.sqla.models import User
from sqlalchemy.orm import Session
from superset.dashboards.commands.exceptions import DashboardAccessDeniedError
-from superset.dashboards.filter_state.commands.entry import Entry
from superset.extensions import cache_manager
+from superset.key_value.commands.entry import Entry
from superset.key_value.utils import cache_key
from superset.models.dashboard import Dashboard
from tests.integration_tests.base_tests import login
diff --git a/tests/unit_tests/charts/form_data/utils_test.py b/tests/unit_tests/charts/form_data/utils_test.py
new file mode 100644
index 0000000..1180b10
--- /dev/null
+++ b/tests/unit_tests/charts/form_data/utils_test.py
@@ -0,0 +1,186 @@
+# 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 flask.ctx import AppContext
+from flask_appbuilder.security.sqla.models import User
+from pytest import raises
+from pytest_mock import MockFixture
+
+from superset.charts.commands.exceptions import (
+ ChartAccessDeniedError,
+ ChartNotFoundError,
+)
+from superset.datasets.commands.exceptions import (
+ DatasetAccessDeniedError,
+ DatasetNotFoundError,
+)
+from superset.key_value.commands.parameters import CommandParameters
+
+dataset_find_by_id = "superset.datasets.dao.DatasetDAO.find_by_id"
+chart_find_by_id = "superset.charts.dao.ChartDAO.find_by_id"
+is_user_admin = "superset.charts.form_data.utils.is_user_admin"
+is_owner = "superset.charts.form_data.utils.is_owner"
+can_access_datasource = (
+ "superset.security.SupersetSecurityManager.can_access_datasource"
+)
+can_access = "superset.security.SupersetSecurityManager.can_access"
+
+
+def test_unsaved_chart_no_dataset_id(app_context: AppContext) -> None:
+ from superset.charts.form_data.utils import check_access
+
+ with raises(DatasetNotFoundError):
+ cmd_params = CommandParameters(resource_id=0, actor=User(), query_params={})
+ check_access(cmd_params)
+
+
+def test_unsaved_chart_unknown_dataset_id(
+ mocker: MockFixture, app_context: AppContext
+) -> None:
+ from superset.charts.form_data.utils import check_access
+
+ with raises(DatasetNotFoundError):
+ mocker.patch(dataset_find_by_id, return_value=None)
+ cmd_params = CommandParameters(
+ resource_id=0, actor=User(), query_params={"dataset_id": "1"}
+ )
+ check_access(cmd_params)
+
+
+def test_unsaved_chart_unauthorized_dataset(
+ mocker: MockFixture, app_context: AppContext
+) -> None:
+ from superset.charts.form_data import utils
+ from superset.connectors.sqla.models import SqlaTable
+
+ with raises(DatasetAccessDeniedError):
+ mocker.patch(dataset_find_by_id, return_value=SqlaTable())
+ mocker.patch(can_access_datasource, return_value=False)
+ cmd_params = CommandParameters(
+ resource_id=0, actor=User(), query_params={"dataset_id": "1"}
+ )
+ utils.check_access(cmd_params)
+
+
+def test_unsaved_chart_authorized_dataset(
+ mocker: MockFixture, app_context: AppContext
+) -> None:
+ from superset.charts.form_data.utils import check_access
+ from superset.connectors.sqla.models import SqlaTable
+
+ mocker.patch(dataset_find_by_id, return_value=SqlaTable())
+ mocker.patch(can_access_datasource, return_value=True)
+ cmd_params = CommandParameters(
+ resource_id=0, actor=User(), query_params={"dataset_id": "1"}
+ )
+ assert check_access(cmd_params) == True
+
+
+def test_saved_chart_unknown_chart_id(
+ mocker: MockFixture, app_context: AppContext
+) -> None:
+ from superset.charts.form_data.utils import check_access
+ from superset.connectors.sqla.models import SqlaTable
+
+ with raises(ChartNotFoundError):
+ mocker.patch(dataset_find_by_id, return_value=SqlaTable())
+ mocker.patch(can_access_datasource, return_value=True)
+ mocker.patch(chart_find_by_id, return_value=None)
+ cmd_params = CommandParameters(
+ resource_id=1, actor=User(), query_params={"dataset_id": "1"}
+ )
+ check_access(cmd_params)
+
+
+def test_saved_chart_unauthorized_dataset(
+ mocker: MockFixture, app_context: AppContext
+) -> None:
+ from superset.charts.form_data import utils
+ from superset.connectors.sqla.models import SqlaTable
+
+ with raises(DatasetAccessDeniedError):
+ mocker.patch(dataset_find_by_id, return_value=SqlaTable())
+ mocker.patch(can_access_datasource, return_value=False)
+ cmd_params = CommandParameters(
+ resource_id=1, actor=User(), query_params={"dataset_id": "1"}
+ )
+ utils.check_access(cmd_params)
+
+
+def test_saved_chart_is_admin(mocker: MockFixture, app_context: AppContext) -> None:
+ from superset.charts.form_data.utils import check_access
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.slice import Slice
+
+ mocker.patch(dataset_find_by_id, return_value=SqlaTable())
+ mocker.patch(can_access_datasource, return_value=True)
+ mocker.patch(is_user_admin, return_value=True)
+ mocker.patch(chart_find_by_id, return_value=Slice())
+ cmd_params = CommandParameters(
+ resource_id=1, actor=User(), query_params={"dataset_id": "1"}
+ )
+ assert check_access(cmd_params) == True
+
+
+def test_saved_chart_is_owner(mocker: MockFixture, app_context: AppContext) -> None:
+ from superset.charts.form_data.utils import check_access
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.slice import Slice
+
+ mocker.patch(dataset_find_by_id, return_value=SqlaTable())
+ mocker.patch(can_access_datasource, return_value=True)
+ mocker.patch(is_user_admin, return_value=False)
+ mocker.patch(is_owner, return_value=True)
+ mocker.patch(chart_find_by_id, return_value=Slice())
+ cmd_params = CommandParameters(
+ resource_id=1, actor=User(), query_params={"dataset_id": "1"}
+ )
+ assert check_access(cmd_params) == True
+
+
+def test_saved_chart_has_access(mocker: MockFixture, app_context: AppContext) -> None:
+ from superset.charts.form_data.utils import check_access
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.slice import Slice
+
+ mocker.patch(dataset_find_by_id, return_value=SqlaTable())
+ mocker.patch(can_access_datasource, return_value=True)
+ mocker.patch(is_user_admin, return_value=False)
+ mocker.patch(is_owner, return_value=False)
+ mocker.patch(can_access, return_value=True)
+ mocker.patch(chart_find_by_id, return_value=Slice())
+ cmd_params = CommandParameters(
+ resource_id=1, actor=User(), query_params={"dataset_id": "1"}
+ )
+ assert check_access(cmd_params) == True
+
+
+def test_saved_chart_no_access(mocker: MockFixture, app_context: AppContext) -> None:
+ from superset.charts.form_data.utils import check_access
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.slice import Slice
+
+ with raises(ChartAccessDeniedError):
+ mocker.patch(dataset_find_by_id, return_value=SqlaTable())
+ mocker.patch(can_access_datasource, return_value=True)
+ mocker.patch(is_user_admin, return_value=False)
+ mocker.patch(is_owner, return_value=False)
+ mocker.patch(can_access, return_value=False)
+ mocker.patch(chart_find_by_id, return_value=Slice())
+ cmd_params = CommandParameters(
+ resource_id=1, actor=User(), query_params={"dataset_id": "1"}
+ )
+ check_access(cmd_params)