From 221968357ac4f0170149314b05e0d6eb0ee8b6ec Mon Sep 17 00:00:00 2001 From: "Michael S. Molina" <70410625+michael-s-molina@users.noreply.github.com> Date: Tue, 25 Jan 2022 11:14:50 -0300 Subject: [PATCH] refactor: Moves the Explore form_data endpoint (#18151) * refactor: Moves the Explore form_data endpoint * Removes unused imports * Fixes openapi schema error * Fixes typo * Renames and UPDATING.md --- UPDATING.md | 1 + superset/charts/form_data/api.py | 259 --------------- superset/charts/form_data/commands/create.py | 35 -- superset/charts/form_data/commands/delete.py | 37 --- superset/charts/form_data/commands/get.py | 44 --- superset/charts/form_data/commands/update.py | 40 --- superset/config.py | 2 +- .../{charts/form_data => explore}/__init__.py | 0 .../form_data}/__init__.py | 0 superset/explore/form_data/api.py | 305 ++++++++++++++++++ .../explore/form_data/commands}/__init__.py | 0 superset/explore/form_data/commands/create.py | 58 ++++ superset/explore/form_data/commands/delete.py | 57 ++++ superset/explore/form_data/commands/get.py | 58 ++++ .../explore/form_data/commands/parameters.py | 29 ++ superset/explore/form_data/commands/state.py | 26 ++ superset/explore/form_data/commands/update.py | 69 ++++ superset/explore/form_data/schemas.py | 37 +++ .../{charts => explore}/form_data/utils.py | 27 +- superset/initialization/__init__.py | 4 +- superset/utils/cache_manager.py | 10 +- tests/integration_tests/explore/__init__.py | 16 + .../explore/form_data/__init__.py | 16 + .../form_data/api_tests.py | 146 ++++----- tests/unit_tests/explore/__init__.py | 16 + .../unit_tests/explore/form_data/__init__.py | 16 + .../form_data/utils_test.py | 73 ++--- 27 files changed, 803 insertions(+), 578 deletions(-) delete mode 100644 superset/charts/form_data/api.py delete mode 100644 superset/charts/form_data/commands/create.py delete mode 100644 superset/charts/form_data/commands/delete.py delete mode 100644 superset/charts/form_data/commands/get.py delete mode 100644 superset/charts/form_data/commands/update.py rename superset/{charts/form_data => explore}/__init__.py (100%) rename superset/{charts/form_data/commands => explore/form_data}/__init__.py (100%) create mode 100644 superset/explore/form_data/api.py rename {tests/integration_tests/charts/form_data => superset/explore/form_data/commands}/__init__.py (100%) create mode 100644 superset/explore/form_data/commands/create.py create mode 100644 superset/explore/form_data/commands/delete.py create mode 100644 superset/explore/form_data/commands/get.py create mode 100644 superset/explore/form_data/commands/parameters.py create mode 100644 superset/explore/form_data/commands/state.py create mode 100644 superset/explore/form_data/commands/update.py create mode 100644 superset/explore/form_data/schemas.py rename superset/{charts => explore}/form_data/utils.py (72%) create mode 100644 tests/integration_tests/explore/__init__.py create mode 100644 tests/integration_tests/explore/form_data/__init__.py rename tests/integration_tests/{charts => explore}/form_data/api_tests.py (56%) create mode 100644 tests/unit_tests/explore/__init__.py create mode 100644 tests/unit_tests/explore/form_data/__init__.py rename tests/unit_tests/{charts => explore}/form_data/utils_test.py (68%) diff --git a/UPDATING.md b/UPDATING.md index 26ab65ff53892..4ded7b25422a3 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -46,6 +46,7 @@ assists people when migrating to a new version. - [17589](https://github.com/apache/incubator-superset/pull/17589): It is now possible to limit access to users' recent activity data by setting the `ENABLE_BROAD_ACTIVITY_ACCESS` config flag to false, or customizing the `raise_for_user_activity_access` method in the security manager. - [17536](https://github.com/apache/superset/pull/17536): introduced a key-value endpoint to store dashboard filter state. This endpoint is backed by Flask-Caching and the default configuration assumes that the values will be stored in the file system. If you are already using another cache backend like Redis or Memchached, you'll probably want to change this setting in `superset_config.py`. The key is `FILTER_STATE_CACHE_CONFIG` and the available settings can be found in Flask-Caching [docs](https://flask-caching.readthedocs.io/en/latest/). +- [17882](https://github.com/apache/superset/pull/17882): introduced a key-value endpoint to store Explore form data. This endpoint is backed by Flask-Caching and the default configuration assumes that the values will be stored in the file system. If you are already using another cache backend like Redis or Memchached, you'll probably want to change this setting in `superset_config.py`. The key is `EXPLORE_FORM_DATA_CACHE_CONFIG` and the available settings can be found in Flask-Caching [docs](https://flask-caching.readthedocs.io/en/latest/). ## 1.4.0 diff --git a/superset/charts/form_data/api.py b/superset/charts/form_data/api.py deleted file mode 100644 index bc908b25d6b81..0000000000000 --- a/superset/charts/form_data/api.py +++ /dev/null @@ -1,259 +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. -import logging -from typing import Type - -from flask import Response -from flask_appbuilder.api import expose, protect, safe - -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 ChartFormDataRestApi(KeyValueRestApi): - class_permission_name = "ChartFormDataRestApi" - resource_name = "chart" - openapi_spec_tag = "Chart Form Data" - - def get_create_command(self) -> Type[CreateFormDataCommand]: - return CreateFormDataCommand - - def get_update_command(self) -> Type[UpdateFormDataCommand]: - return UpdateFormDataCommand - - def get_get_command(self) -> Type[GetFormDataCommand]: - return GetFormDataCommand - - def get_delete_command(self) -> Type[DeleteFormDataCommand]: - return DeleteFormDataCommand - - @expose("//form_data", methods=["POST"]) - @protect() - @safe - @event_logger.log_this_with_context( - action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post", - log_to_statsd=False, - ) - def post(self, pk: int) -> Response: - """Stores a new value. - --- - post: - description: >- - Stores a new value. - parameters: - - in: path - schema: - type: integer - name: pk - - in: query - schema: - type: integer - name: dataset_id - required: true - requestBody: - required: true - content: - application/json: - schema: - $ref: '#/components/schemas/KeyValuePostSchema' - responses: - 201: - description: The value was stored successfully. - content: - application/json: - schema: - type: object - properties: - key: - type: string - description: The key to retrieve the value. - 400: - $ref: '#/components/responses/400' - 401: - $ref: '#/components/responses/401' - 422: - $ref: '#/components/responses/422' - 500: - $ref: '#/components/responses/500' - """ - return super().post(pk) - - @expose("//form_data/", methods=["PUT"]) - @protect() - @safe - @event_logger.log_this_with_context( - action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.put", - log_to_statsd=False, - ) - def put(self, pk: int, key: str) -> Response: - """Updates an existing value. - --- - put: - description: >- - Updates an existing value. - parameters: - - in: path - schema: - type: integer - name: pk - - in: path - schema: - type: string - name: key - - in: query - schema: - type: integer - name: dataset_id - required: true - requestBody: - required: true - content: - application/json: - schema: - $ref: '#/components/schemas/KeyValuePutSchema' - responses: - 200: - description: The value was stored successfully. - content: - application/json: - schema: - type: object - properties: - message: - type: string - description: The result of the operation - 400: - $ref: '#/components/responses/400' - 401: - $ref: '#/components/responses/401' - 404: - $ref: '#/components/responses/404' - 422: - $ref: '#/components/responses/422' - 500: - $ref: '#/components/responses/500' - """ - return super().put(pk, key) - - @expose("//form_data/", methods=["GET"]) - @protect() - @safe - @event_logger.log_this_with_context( - action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get", - log_to_statsd=False, - ) - def get(self, pk: int, key: str) -> Response: - """Retrives a value. - --- - get: - description: >- - Retrives a value. - parameters: - - in: path - schema: - type: integer - name: pk - - in: path - schema: - type: string - name: key - - in: query - schema: - type: integer - name: dataset_id - required: true - responses: - 200: - description: Returns the stored value. - content: - application/json: - schema: - type: object - properties: - value: - type: string - description: The stored value - 400: - $ref: '#/components/responses/400' - 401: - $ref: '#/components/responses/401' - 404: - $ref: '#/components/responses/404' - 422: - $ref: '#/components/responses/422' - 500: - $ref: '#/components/responses/500' - """ - return super().get(pk, key) - - @expose("//form_data/", methods=["DELETE"]) - @protect() - @safe - @event_logger.log_this_with_context( - action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.delete", - log_to_statsd=False, - ) - def delete(self, pk: int, key: str) -> Response: - """Deletes a value. - --- - delete: - description: >- - Deletes a value. - parameters: - - in: path - schema: - type: integer - name: pk - - in: path - schema: - 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. - content: - application/json: - schema: - type: object - properties: - message: - type: string - description: The result of the operation - 400: - $ref: '#/components/responses/400' - 401: - $ref: '#/components/responses/401' - 404: - $ref: '#/components/responses/404' - 422: - $ref: '#/components/responses/422' - 500: - $ref: '#/components/responses/500' - """ - return super().delete(pk, key) diff --git a/superset/charts/form_data/commands/create.py b/superset/charts/form_data/commands/create.py deleted file mode 100644 index d7c2ad413a4ff..0000000000000 --- a/superset/charts/form_data/commands/create.py +++ /dev/null @@ -1,35 +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 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 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.chart_form_data_cache.set(key, entry) - return False diff --git a/superset/charts/form_data/commands/delete.py b/superset/charts/form_data/commands/delete.py deleted file mode 100644 index be6c33a00508f..0000000000000 --- a/superset/charts/form_data/commands/delete.py +++ /dev/null @@ -1,37 +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 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 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 deleted file mode 100644 index f23acfc2d186e..0000000000000 --- a/superset/charts/form_data/commands/get.py +++ /dev/null @@ -1,44 +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 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/charts/form_data/commands/update.py b/superset/charts/form_data/commands/update.py deleted file mode 100644 index cab988da87138..0000000000000 --- a/superset/charts/form_data/commands/update.py +++ /dev/null @@ -1,40 +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 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 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/config.py b/superset/config.py index 612bcbfc034d8..56841853a1900 100644 --- a/superset/config.py +++ b/superset/config.py @@ -590,7 +590,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: } # Cache for chart form data -CHART_FORM_DATA_CACHE_CONFIG: CacheConfig = { +EXPLORE_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()), diff --git a/superset/charts/form_data/__init__.py b/superset/explore/__init__.py similarity index 100% rename from superset/charts/form_data/__init__.py rename to superset/explore/__init__.py diff --git a/superset/charts/form_data/commands/__init__.py b/superset/explore/form_data/__init__.py similarity index 100% rename from superset/charts/form_data/commands/__init__.py rename to superset/explore/form_data/__init__.py diff --git a/superset/explore/form_data/api.py b/superset/explore/form_data/api.py new file mode 100644 index 0000000000000..2ed7096768932 --- /dev/null +++ b/superset/explore/form_data/api.py @@ -0,0 +1,305 @@ +# 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 abc import ABC + +from flask import g, request, Response +from flask_appbuilder.api import BaseApi, expose, protect, safe +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.datasets.commands.exceptions import ( + DatasetAccessDeniedError, + DatasetNotFoundError, +) +from superset.exceptions import InvalidPayloadFormatError +from superset.explore.form_data.commands.create import CreateFormDataCommand +from superset.explore.form_data.commands.delete import DeleteFormDataCommand +from superset.explore.form_data.commands.get import GetFormDataCommand +from superset.explore.form_data.commands.parameters import CommandParameters +from superset.explore.form_data.commands.update import UpdateFormDataCommand +from superset.explore.form_data.schemas import FormDataPostSchema, FormDataPutSchema +from superset.extensions import event_logger +from superset.key_value.commands.exceptions import KeyValueAccessDeniedError + +logger = logging.getLogger(__name__) + + +class ExploreFormDataRestApi(BaseApi, ABC): + add_model_schema = FormDataPostSchema() + edit_model_schema = FormDataPutSchema() + method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP + include_route_methods = { + RouteMethod.POST, + RouteMethod.PUT, + RouteMethod.GET, + RouteMethod.DELETE, + } + allow_browser_login = True + class_permission_name = "ExploreFormDataRestApi" + resource_name = "explore" + openapi_spec_tag = "Explore Form Data" + openapi_spec_component_schemas = (FormDataPostSchema, FormDataPutSchema) + + @expose("/form_data", methods=["POST"]) + @protect() + @safe + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post", + log_to_statsd=False, + ) + def post(self) -> Response: + """Stores a new form_data. + --- + post: + description: >- + Stores a new form_data. + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/FormDataPostSchema' + responses: + 201: + description: The form_data was stored successfully. + content: + application/json: + schema: + type: object + properties: + key: + type: string + description: The key to retrieve the form_data. + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + if not request.is_json: + raise InvalidPayloadFormatError("Request is not JSON") + try: + item = self.add_model_schema.load(request.json) + args = CommandParameters( + actor=g.user, + dataset_id=item["dataset_id"], + chart_id=item.get("chart_id"), + form_data=item["form_data"], + ) + key = CreateFormDataCommand(args).run() + return self.response(201, key=key) + except ValidationError as ex: + return self.response(400, message=ex.messages) + except ( + ChartAccessDeniedError, + DatasetAccessDeniedError, + KeyValueAccessDeniedError, + ) as ex: + return self.response(403, message=str(ex)) + except (ChartNotFoundError, DatasetNotFoundError) as ex: + return self.response(404, message=str(ex)) + + @expose("/form_data/", methods=["PUT"]) + @protect() + @safe + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.put", + log_to_statsd=False, + ) + def put(self, key: str) -> Response: + """Updates an existing form_data. + --- + put: + description: >- + Updates an existing form_data. + parameters: + - in: path + schema: + type: string + name: key + requestBody: + required: true + content: + application/json: + schema: + $ref: '#/components/schemas/FormDataPutSchema' + responses: + 200: + description: The form_data was stored successfully. + content: + application/json: + schema: + type: object + properties: + message: + type: string + description: The result of the operation + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + if not request.is_json: + raise InvalidPayloadFormatError("Request is not JSON") + try: + item = self.edit_model_schema.load(request.json) + args = CommandParameters( + actor=g.user, + dataset_id=item["dataset_id"], + chart_id=item.get("chart_id"), + key=key, + form_data=item["form_data"], + ) + result = UpdateFormDataCommand(args).run() + if not result: + return self.response_404() + return self.response(200, message="Value updated successfully.") + except ValidationError as ex: + return self.response(400, message=ex.messages) + except ( + ChartAccessDeniedError, + DatasetAccessDeniedError, + KeyValueAccessDeniedError, + ) as ex: + return self.response(403, message=str(ex)) + except (ChartNotFoundError, DatasetNotFoundError) as ex: + return self.response(404, message=str(ex)) + + @expose("/form_data/", methods=["GET"]) + @protect() + @safe + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.get", + log_to_statsd=False, + ) + def get(self, key: str) -> Response: + """Retrives a form_data. + --- + get: + description: >- + Retrives a form_data. + parameters: + - in: path + schema: + type: string + name: key + responses: + 200: + description: Returns the stored form_data. + content: + application/json: + schema: + type: object + properties: + form_data: + type: string + description: The stored form_data + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + try: + args = CommandParameters(actor=g.user, key=key) + form_data = GetFormDataCommand(args).run() + if not form_data: + return self.response_404() + return self.response(200, form_data=form_data) + except ( + ChartAccessDeniedError, + DatasetAccessDeniedError, + KeyValueAccessDeniedError, + ) as ex: + return self.response(403, message=str(ex)) + except (ChartNotFoundError, DatasetNotFoundError) as ex: + return self.response(404, message=str(ex)) + + @expose("/form_data/", methods=["DELETE"]) + @protect() + @safe + @event_logger.log_this_with_context( + action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.delete", + log_to_statsd=False, + ) + def delete(self, key: str) -> Response: + """Deletes a form_data. + --- + delete: + description: >- + Deletes a form_data. + parameters: + - in: path + schema: + type: string + name: key + description: The form_data key. + responses: + 200: + description: Deleted the stored form_data. + content: + application/json: + schema: + type: object + properties: + message: + type: string + description: The result of the operation + 400: + $ref: '#/components/responses/400' + 401: + $ref: '#/components/responses/401' + 404: + $ref: '#/components/responses/404' + 422: + $ref: '#/components/responses/422' + 500: + $ref: '#/components/responses/500' + """ + try: + args = CommandParameters(actor=g.user, key=key) + result = DeleteFormDataCommand(args).run() + if not result: + return self.response_404() + return self.response(200, message="Deleted successfully") + except ( + ChartAccessDeniedError, + DatasetAccessDeniedError, + KeyValueAccessDeniedError, + ) as ex: + return self.response(403, message=str(ex)) + except (ChartNotFoundError, DatasetNotFoundError) as ex: + return self.response(404, message=str(ex)) diff --git a/tests/integration_tests/charts/form_data/__init__.py b/superset/explore/form_data/commands/__init__.py similarity index 100% rename from tests/integration_tests/charts/form_data/__init__.py rename to superset/explore/form_data/commands/__init__.py diff --git a/superset/explore/form_data/commands/create.py b/superset/explore/form_data/commands/create.py new file mode 100644 index 0000000000000..a2325f0be924e --- /dev/null +++ b/superset/explore/form_data/commands/create.py @@ -0,0 +1,58 @@ +# 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 secrets import token_urlsafe + +from sqlalchemy.exc import SQLAlchemyError + +from superset.commands.base import BaseCommand +from superset.explore.form_data.commands.parameters import CommandParameters +from superset.explore.form_data.commands.state import TemporaryExploreState +from superset.explore.form_data.utils import check_access +from superset.extensions import cache_manager +from superset.key_value.commands.exceptions import KeyValueCreateFailedError + +logger = logging.getLogger(__name__) + + +class CreateFormDataCommand(BaseCommand): + def __init__(self, cmd_params: CommandParameters): + self._cmd_params = cmd_params + + def run(self) -> str: + try: + dataset_id = self._cmd_params.dataset_id + chart_id = self._cmd_params.chart_id + actor = self._cmd_params.actor + form_data = self._cmd_params.form_data + check_access(dataset_id, chart_id, actor) + key = token_urlsafe(48) + if form_data: + state: TemporaryExploreState = { + "owner": actor.get_user_id(), + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": form_data, + } + cache_manager.explore_form_data_cache.set(key, state) + return key + except SQLAlchemyError as ex: + logger.exception("Error running create command") + raise KeyValueCreateFailedError() from ex + + def validate(self) -> None: + pass diff --git a/superset/explore/form_data/commands/delete.py b/superset/explore/form_data/commands/delete.py new file mode 100644 index 0000000000000..8dc414ee14cd2 --- /dev/null +++ b/superset/explore/form_data/commands/delete.py @@ -0,0 +1,57 @@ +# 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 abc import ABC + +from sqlalchemy.exc import SQLAlchemyError + +from superset.commands.base import BaseCommand +from superset.explore.form_data.commands.parameters import CommandParameters +from superset.explore.form_data.commands.state import TemporaryExploreState +from superset.explore.form_data.utils import check_access +from superset.extensions import cache_manager +from superset.key_value.commands.exceptions import ( + KeyValueAccessDeniedError, + KeyValueDeleteFailedError, +) + +logger = logging.getLogger(__name__) + + +class DeleteFormDataCommand(BaseCommand, ABC): + def __init__(self, cmd_params: CommandParameters): + self._cmd_params = cmd_params + + def run(self) -> bool: + try: + actor = self._cmd_params.actor + key = self._cmd_params.key + state: TemporaryExploreState = cache_manager.explore_form_data_cache.get( + key + ) + if state: + check_access(state["dataset_id"], state["chart_id"], actor) + if state["owner"] != actor.get_user_id(): + raise KeyValueAccessDeniedError() + return cache_manager.explore_form_data_cache.delete(key) + return False + except SQLAlchemyError as ex: + logger.exception("Error running delete command") + raise KeyValueDeleteFailedError() from ex + + def validate(self) -> None: + pass diff --git a/superset/explore/form_data/commands/get.py b/superset/explore/form_data/commands/get.py new file mode 100644 index 0000000000000..2252387b34084 --- /dev/null +++ b/superset/explore/form_data/commands/get.py @@ -0,0 +1,58 @@ +# 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 abc import ABC +from typing import Optional + +from flask import current_app as app +from sqlalchemy.exc import SQLAlchemyError + +from superset.commands.base import BaseCommand +from superset.explore.form_data.commands.parameters import CommandParameters +from superset.explore.form_data.commands.state import TemporaryExploreState +from superset.explore.form_data.utils import check_access +from superset.extensions import cache_manager +from superset.key_value.commands.exceptions import KeyValueGetFailedError + +logger = logging.getLogger(__name__) + + +class GetFormDataCommand(BaseCommand, ABC): + def __init__(self, cmd_params: CommandParameters) -> None: + self._cmd_params = cmd_params + config = app.config["EXPLORE_FORM_DATA_CACHE_CONFIG"] + self._refresh_timeout = config.get("REFRESH_TIMEOUT_ON_RETRIEVAL") + + def run(self) -> Optional[str]: + try: + actor = self._cmd_params.actor + key = self._cmd_params.key + state: TemporaryExploreState = cache_manager.explore_form_data_cache.get( + key + ) + if state: + check_access(state["dataset_id"], state["chart_id"], actor) + if self._refresh_timeout: + cache_manager.explore_form_data_cache.set(key, state) + return state["form_data"] + return None + except SQLAlchemyError as ex: + logger.exception("Error running get command") + raise KeyValueGetFailedError() from ex + + def validate(self) -> None: + pass diff --git a/superset/explore/form_data/commands/parameters.py b/superset/explore/form_data/commands/parameters.py new file mode 100644 index 0000000000000..c4d41a184a781 --- /dev/null +++ b/superset/explore/form_data/commands/parameters.py @@ -0,0 +1,29 @@ +# 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 dataclasses import dataclass +from typing import Optional + +from flask_appbuilder.security.sqla.models import User + + +@dataclass +class CommandParameters: + actor: User + dataset_id: int = 0 + chart_id: int = 0 + key: Optional[str] = None + form_data: Optional[str] = None diff --git a/superset/explore/form_data/commands/state.py b/superset/explore/form_data/commands/state.py new file mode 100644 index 0000000000000..2aba14a8cb28f --- /dev/null +++ b/superset/explore/form_data/commands/state.py @@ -0,0 +1,26 @@ +# 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 typing_extensions import TypedDict + + +class TemporaryExploreState(TypedDict): + owner: int + dataset_id: int + chart_id: Optional[int] + form_data: str diff --git a/superset/explore/form_data/commands/update.py b/superset/explore/form_data/commands/update.py new file mode 100644 index 0000000000000..8b69ae885f714 --- /dev/null +++ b/superset/explore/form_data/commands/update.py @@ -0,0 +1,69 @@ +# 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 abc import ABC + +from sqlalchemy.exc import SQLAlchemyError + +from superset.commands.base import BaseCommand +from superset.explore.form_data.commands.parameters import CommandParameters +from superset.explore.form_data.commands.state import TemporaryExploreState +from superset.explore.form_data.utils import check_access +from superset.extensions import cache_manager +from superset.key_value.commands.exceptions import ( + KeyValueAccessDeniedError, + KeyValueUpdateFailedError, +) + +logger = logging.getLogger(__name__) + + +class UpdateFormDataCommand(BaseCommand, ABC): + def __init__( + self, cmd_params: CommandParameters, + ): + self._cmd_params = cmd_params + + def run(self) -> bool: + try: + dataset_id = self._cmd_params.dataset_id + chart_id = self._cmd_params.chart_id + actor = self._cmd_params.actor + key = self._cmd_params.key + form_data = self._cmd_params.form_data + check_access(dataset_id, chart_id, actor) + state: TemporaryExploreState = cache_manager.explore_form_data_cache.get( + key + ) + if state and form_data: + user_id = actor.get_user_id() + if state["owner"] != user_id: + raise KeyValueAccessDeniedError() + new_state: TemporaryExploreState = { + "owner": actor.get_user_id(), + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": form_data, + } + return cache_manager.explore_form_data_cache.set(key, new_state) + return False + except SQLAlchemyError as ex: + logger.exception("Error running update command") + raise KeyValueUpdateFailedError() from ex + + def validate(self) -> None: + pass diff --git a/superset/explore/form_data/schemas.py b/superset/explore/form_data/schemas.py new file mode 100644 index 0000000000000..6d5509d777a3c --- /dev/null +++ b/superset/explore/form_data/schemas.py @@ -0,0 +1,37 @@ +# 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 FormDataPostSchema(Schema): + dataset_id = fields.Integer( + required=True, allow_none=False, description="The dataset ID" + ) + chart_id = fields.Integer(required=False, description="The chart ID") + form_data = fields.String( + required=True, allow_none=False, description="Any type of JSON supported text." + ) + + +class FormDataPutSchema(Schema): + dataset_id = fields.Integer( + required=True, allow_none=False, description="The dataset ID" + ) + chart_id = fields.Integer(required=False, description="The chart ID") + form_data = fields.String( + required=True, allow_none=False, description="Any type of JSON supported text." + ) diff --git a/superset/charts/form_data/utils.py b/superset/explore/form_data/utils.py similarity index 72% rename from superset/charts/form_data/utils.py rename to superset/explore/form_data/utils.py index c1f98a2388407..3eeb1bab9964a 100644 --- a/superset/charts/form_data/utils.py +++ b/superset/explore/form_data/utils.py @@ -16,6 +16,8 @@ # under the License. from typing import Optional +from flask_appbuilder.security.sqla.models import User + from superset import security_manager from superset.charts.commands.exceptions import ( ChartAccessDeniedError, @@ -27,22 +29,13 @@ 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) +def check_dataset_access(dataset_id: int) -> Optional[bool]: if dataset_id: - dataset = DatasetDAO.find_by_id(int(dataset_id)) + dataset = DatasetDAO.find_by_id(dataset_id) if dataset: can_access_datasource = security_manager.can_access_datasource(dataset) if can_access_datasource: @@ -51,13 +44,13 @@ def check_dataset_access(cmd_params: CommandParameters) -> Optional[bool]: 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: +def check_access( + dataset_id: int, chart_id: Optional[int], actor: User +) -> Optional[bool]: + check_dataset_access(dataset_id) + if not chart_id: return True - chart = ChartDAO.find_by_id(resource_id) + chart = ChartDAO.find_by_id(chart_id) if chart: can_access_chart = ( is_user_admin() diff --git a/superset/initialization/__init__.py b/superset/initialization/__init__.py index 98cd9af264cf6..f2bce87afbc5b 100644 --- a/superset/initialization/__init__.py +++ b/superset/initialization/__init__.py @@ -119,7 +119,6 @@ def init_views(self) -> None: 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, @@ -141,6 +140,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.explore.form_data.api import ExploreFormDataRestApi from superset.queries.api import QueryRestApi from superset.queries.saved_queries.api import SavedQueryRestApi from superset.reports.api import ReportScheduleRestApi @@ -204,7 +204,6 @@ def init_views(self) -> None: 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) @@ -212,6 +211,7 @@ def init_views(self) -> None: appbuilder.add_api(DatasetRestApi) appbuilder.add_api(DatasetColumnsRestApi) appbuilder.add_api(DatasetMetricRestApi) + appbuilder.add_api(ExploreFormDataRestApi) appbuilder.add_api(FilterSetRestApi) appbuilder.add_api(QueryRestApi) appbuilder.add_api(ReportScheduleRestApi) diff --git a/superset/utils/cache_manager.py b/superset/utils/cache_manager.py index 3624d918f9c75..e92d930d2ef8f 100644 --- a/superset/utils/cache_manager.py +++ b/superset/utils/cache_manager.py @@ -26,7 +26,7 @@ def __init__(self) -> None: self._data_cache = Cache() self._thumbnail_cache = Cache() self._filter_state_cache = Cache() - self._chart_form_data_cache = Cache() + self._explore_form_data_cache = Cache() def init_app(self, app: Flask) -> None: self._cache.init_app( @@ -57,11 +57,11 @@ def init_app(self, app: Flask) -> None: **app.config["FILTER_STATE_CACHE_CONFIG"], }, ) - self._chart_form_data_cache.init_app( + self._explore_form_data_cache.init_app( app, { "CACHE_DEFAULT_TIMEOUT": app.config["CACHE_DEFAULT_TIMEOUT"], - **app.config["CHART_FORM_DATA_CACHE_CONFIG"], + **app.config["EXPLORE_FORM_DATA_CACHE_CONFIG"], }, ) @@ -82,5 +82,5 @@ 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 + def explore_form_data_cache(self) -> Cache: + return self._explore_form_data_cache diff --git a/tests/integration_tests/explore/__init__.py b/tests/integration_tests/explore/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/integration_tests/explore/__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/explore/form_data/__init__.py b/tests/integration_tests/explore/form_data/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/integration_tests/explore/form_data/__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/charts/form_data/api_tests.py b/tests/integration_tests/explore/form_data/api_tests.py similarity index 56% rename from tests/integration_tests/charts/form_data/api_tests.py rename to tests/integration_tests/explore/form_data/api_tests.py index f411c1a6edc99..bf0fc6cab4bec 100644 --- a/tests/integration_tests/charts/form_data/api_tests.py +++ b/tests/integration_tests/explore/form_data/api_tests.py @@ -23,9 +23,8 @@ from superset.connectors.sqla.models import SqlaTable from superset.datasets.commands.exceptions import DatasetAccessDeniedError +from superset.explore.form_data.commands.state import TemporaryExploreState 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 ( @@ -35,7 +34,7 @@ from tests.integration_tests.test_app import app key = "test-key" -value = "test" +form_data = "test" @pytest.fixture @@ -75,171 +74,144 @@ def dataset_id() -> int: @pytest.fixture(autouse=True) def cache(chart_id, admin_id, dataset_id): - app.config["CHART_FORM_DATA_CACHE_CONFIG"] = {"CACHE_TYPE": "SimpleCache"} + app.config["EXPLORE_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) + entry: TemporaryExploreState = { + "owner": admin_id, + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": form_data, + } + cache_manager.explore_form_data_cache.set(key, entry) def test_post(client, chart_id: int, dataset_id: int): login(client, "admin") payload = { - "value": value, + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": form_data, } - resp = client.post( - f"api/v1/chart/{chart_id}/form_data?dataset_id={dataset_id}", json=payload - ) + resp = client.post("api/v1/explore/form_data", 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, + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": 1234, } - resp = client.post( - f"api/v1/chart/{chart_id}/form_data?dataset_id={dataset_id}", json=payload - ) + resp = client.post("api/v1/explore/form_data", 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, + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": form_data, } - resp = client.post( - f"api/v1/chart/{chart_id}/form_data?dataset_id={dataset_id}", json=payload - ) + resp = client.post("api/v1/explore/form_data", json=payload) assert resp.status_code == 404 def test_put(client, chart_id: int, dataset_id: int): login(client, "admin") payload = { - "value": "new value", + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": "new form_data", } - resp = client.put( - f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}", json=payload - ) + resp = client.put(f"api/v1/explore/form_data/{key}", 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, + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": 1234, } - resp = client.put( - f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}", json=payload - ) + resp = client.put(f"api/v1/explore/form_data/{key}", 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", + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": "new form_data", } - resp = client.put( - f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}", json=payload - ) + resp = client.put(f"api/v1/explore/form_data/{key}", 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", + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": "new form_data", } - 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}" - ) + resp = client.put(f"api/v1/explore/form_data/{key}", json=payload) assert resp.status_code == 404 -def test_get_chart_not_found(client, dataset_id: int): +def test_get_key_not_found(client): login(client, "admin") - resp = client.get(f"api/v1/chart/-1/form_data/{key}?dataset_id={dataset_id}") + resp = client.get(f"api/v1/explore/form_data/unknown-key") assert resp.status_code == 404 -def test_get(client, chart_id: int, dataset_id: int): +def test_get(client): login(client, "admin") - resp = client.get( - f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}" - ) + resp = client.get(f"api/v1/explore/form_data/{key}") assert resp.status_code == 200 data = json.loads(resp.data.decode("utf-8")) - assert value == data.get("value") + assert form_data == data.get("form_data") -def test_get_access_denied(client, chart_id: int, dataset_id: int): +def test_get_access_denied(client): 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") + resp = client.get(f"api/v1/explore/form_data/{key}") 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): +def test_get_dataset_access_denied(mock_can_access_datasource, client): 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}") + resp = client.get(f"api/v1/explore/form_data/{key}") assert resp.status_code == 403 -def test_delete(client, chart_id: int, dataset_id: int): +def test_delete(client): login(client, "admin") - resp = client.delete( - f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}" - ) + resp = client.delete(f"api/v1/explore/form_data/{key}") assert resp.status_code == 200 -def test_delete_access_denied(client, chart_id: int, dataset_id: int): +def test_delete_access_denied(client): login(client, "gamma") - resp = client.delete( - f"api/v1/chart/{chart_id}/form_data/{key}?dataset_id={dataset_id}" - ) + resp = client.delete(f"api/v1/explore/form_data/{key}") 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) + entry: TemporaryExploreState = { + "owner": another_owner, + "dataset_id": dataset_id, + "chart_id": chart_id, + "form_data": form_data, + } + cache_manager.explore_form_data_cache.set(another_key, entry) login(client, "admin") - resp = client.delete( - f"api/v1/chart/{chart_id}/form_data/{another_key}?dataset_id={dataset_id}" - ) + resp = client.delete(f"api/v1/explore/form_data/{another_key}") assert resp.status_code == 403 diff --git a/tests/unit_tests/explore/__init__.py b/tests/unit_tests/explore/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/unit_tests/explore/__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/unit_tests/explore/form_data/__init__.py b/tests/unit_tests/explore/form_data/__init__.py new file mode 100644 index 0000000000000..13a83393a9124 --- /dev/null +++ b/tests/unit_tests/explore/form_data/__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/unit_tests/charts/form_data/utils_test.py b/tests/unit_tests/explore/form_data/utils_test.py similarity index 68% rename from tests/unit_tests/charts/form_data/utils_test.py rename to tests/unit_tests/explore/form_data/utils_test.py index 1180b10d36ed9..4025dc951e11c 100644 --- a/tests/unit_tests/charts/form_data/utils_test.py +++ b/tests/unit_tests/explore/form_data/utils_test.py @@ -27,12 +27,11 @@ 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" +is_user_admin = "superset.explore.form_data.utils.is_user_admin" +is_owner = "superset.explore.form_data.utils.is_owner" can_access_datasource = ( "superset.security.SupersetSecurityManager.can_access_datasource" ) @@ -40,104 +39,85 @@ def test_unsaved_chart_no_dataset_id(app_context: AppContext) -> None: - from superset.charts.form_data.utils import check_access + from superset.explore.form_data.utils import check_access with raises(DatasetNotFoundError): - cmd_params = CommandParameters(resource_id=0, actor=User(), query_params={}) - check_access(cmd_params) + check_access(dataset_id=0, chart_id=0, actor=User()) def test_unsaved_chart_unknown_dataset_id( mocker: MockFixture, app_context: AppContext ) -> None: - from superset.charts.form_data.utils import check_access + from superset.explore.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) + check_access(dataset_id=1, chart_id=0, actor=User()) 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 + from superset.explore.form_data import utils 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) + utils.check_access(dataset_id=1, chart_id=0, actor=User()) 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 + from superset.explore.form_data.utils import check_access 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 + assert check_access(dataset_id=1, chart_id=0, actor=User()) == 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 + from superset.explore.form_data.utils import check_access 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) + check_access(dataset_id=1, chart_id=1, actor=User()) 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 + from superset.explore.form_data import utils 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) + utils.check_access(dataset_id=1, chart_id=1, actor=User()) 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.explore.form_data.utils import check_access 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 + assert check_access(dataset_id=1, chart_id=1, actor=User()) == 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.explore.form_data.utils import check_access from superset.models.slice import Slice mocker.patch(dataset_find_by_id, return_value=SqlaTable()) @@ -145,15 +125,12 @@ def test_saved_chart_is_owner(mocker: MockFixture, app_context: AppContext) -> N 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 + assert check_access(dataset_id=1, chart_id=1, actor=User()) == 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.explore.form_data.utils import check_access from superset.models.slice import Slice mocker.patch(dataset_find_by_id, return_value=SqlaTable()) @@ -162,15 +139,12 @@ def test_saved_chart_has_access(mocker: MockFixture, app_context: AppContext) -> 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 + assert check_access(dataset_id=1, chart_id=1, actor=User()) == 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.explore.form_data.utils import check_access from superset.models.slice import Slice with raises(ChartAccessDeniedError): @@ -180,7 +154,4 @@ def test_saved_chart_no_access(mocker: MockFixture, app_context: AppContext) -> 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) + check_access(dataset_id=1, chart_id=1, actor=User())