From 5eee7fbf985d308102f73de682f8130c3518d604 Mon Sep 17 00:00:00 2001 From: Daniel Gaspar Date: Tue, 14 Apr 2020 15:15:40 +0100 Subject: [PATCH] Merge remote-tracking branch 'upstream/master' into feature/thumbnail-backend # Conflicts: # superset/views/base_api.py --- superset/app.py | 2 + superset/commands/exceptions.py | 2 +- superset/dashboards/api.py | 77 +++++----------------- superset/dashboards/commands/exceptions.py | 1 + superset/errors.py | 47 +++++++++++++ tests/dashboards/api_tests.py | 4 +- 6 files changed, 69 insertions(+), 64 deletions(-) create mode 100644 superset/errors.py diff --git a/superset/app.py b/superset/app.py index b1ce22c369f5b..2b91ac9f291ab 100644 --- a/superset/app.py +++ b/superset/app.py @@ -26,6 +26,7 @@ from flask_wtf import CSRFProtect from superset.connectors.connector_registry import ConnectorRegistry +from superset.errors import init_global_error_handlers from superset.extensions import ( _event_logger, APP_DIR, @@ -454,6 +455,7 @@ def init_app_in_ctx(self) -> None: if flask_app_mutator: flask_app_mutator(self.flask_app) + init_global_error_handlers(self.flask_app) self.init_views() def init_app(self) -> None: diff --git a/superset/commands/exceptions.py b/superset/commands/exceptions.py index cf67ea9227159..692cc2d038021 100644 --- a/superset/commands/exceptions.py +++ b/superset/commands/exceptions.py @@ -69,7 +69,7 @@ class DeleteFailedError(CommandException): class ForbiddenError(CommandException): - status = 500 + status = 403 message = "Action is forbidden" diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index eb4f796cd0d36..7eccdaac271c8 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -18,7 +18,7 @@ from typing import Any from flask import g, make_response, request, Response -from flask_appbuilder.api import expose, protect, rison, safe +from flask_appbuilder.api import expose, protect, rison from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import ngettext @@ -26,15 +26,6 @@ from superset.dashboards.commands.bulk_delete import BulkDeleteDashboardCommand from superset.dashboards.commands.create import CreateDashboardCommand from superset.dashboards.commands.delete import DeleteDashboardCommand -from superset.dashboards.commands.exceptions import ( - DashboardBulkDeleteFailedError, - DashboardCreateFailedError, - DashboardDeleteFailedError, - DashboardForbiddenError, - DashboardInvalidError, - DashboardNotFoundError, - DashboardUpdateFailedError, -) from superset.dashboards.commands.update import UpdateDashboardCommand from superset.dashboards.filters import DashboardFilter, DashboardTitleOrSlugFilter from superset.dashboards.schemas import ( @@ -125,7 +116,6 @@ class DashboardRestApi(BaseSupersetModelRestApi): @expose("/", methods=["POST"]) @protect() - @safe def post(self) -> Response: """Creates a new Dashboard --- @@ -166,18 +156,11 @@ def post(self) -> Response: # This validates custom Schema with custom validations if item.errors: return self.response_400(message=item.errors) - try: - new_model = CreateDashboardCommand(g.user, item.data).run() - return self.response(201, id=new_model.id, result=item.data) - except DashboardInvalidError as ex: - return self.response_422(message=ex.normalized_messages()) - except DashboardCreateFailedError as ex: - logger.error(f"Error creating model {self.__class__.__name__}: {ex}") - return self.response_422(message=str(ex)) + new_model = CreateDashboardCommand(g.user, item.data).run() + return self.response(201, id=new_model.id, result=item.data) @expose("/", methods=["PUT"]) @protect() - @safe def put( # pylint: disable=too-many-return-statements, arguments-differ self, pk: int ) -> Response: @@ -229,22 +212,11 @@ def put( # pylint: disable=too-many-return-statements, arguments-differ # This validates custom Schema with custom validations if item.errors: return self.response_400(message=item.errors) - try: - changed_model = UpdateDashboardCommand(g.user, pk, item.data).run() - return self.response(200, id=changed_model.id, result=item.data) - except DashboardNotFoundError: - return self.response_404() - except DashboardForbiddenError: - return self.response_403() - except DashboardInvalidError as ex: - return self.response_422(message=ex.normalized_messages()) - except DashboardUpdateFailedError as ex: - logger.error(f"Error updating model {self.__class__.__name__}: {ex}") - return self.response_422(message=str(ex)) + changed_model = UpdateDashboardCommand(g.user, pk, item.data).run() + return self.response(200, id=changed_model.id, result=item.data) @expose("/", methods=["DELETE"]) @protect() - @safe def delete(self, pk: int) -> Response: # pylint: disable=arguments-differ """Deletes a Dashboard --- @@ -277,20 +249,11 @@ def delete(self, pk: int) -> Response: # pylint: disable=arguments-differ 500: $ref: '#/components/responses/500' """ - try: - DeleteDashboardCommand(g.user, pk).run() - return self.response(200, message="OK") - except DashboardNotFoundError: - return self.response_404() - except DashboardForbiddenError: - return self.response_403() - except DashboardDeleteFailedError as ex: - logger.error(f"Error deleting model {self.__class__.__name__}: {ex}") - return self.response_422(message=str(ex)) + DeleteDashboardCommand(g.user, pk).run() + return self.response(200, message="OK") @expose("/", methods=["DELETE"]) @protect() - @safe @rison(get_delete_ids_schema) def bulk_delete( self, **kwargs: Any @@ -331,26 +294,18 @@ def bulk_delete( $ref: '#/components/responses/500' """ item_ids = kwargs["rison"] - try: - BulkDeleteDashboardCommand(g.user, item_ids).run() - return self.response( - 200, - message=ngettext( - f"Deleted %(num)d dashboard", - f"Deleted %(num)d dashboards", - num=len(item_ids), - ), - ) - except DashboardNotFoundError: - return self.response_404() - except DashboardForbiddenError: - return self.response_403() - except DashboardBulkDeleteFailedError as ex: - return self.response_422(message=str(ex)) + BulkDeleteDashboardCommand(g.user, item_ids).run() + return self.response( + 200, + message=ngettext( + f"Deleted %(num)d dashboard", + f"Deleted %(num)d dashboards", + num=len(item_ids), + ), + ) @expose("/export/", methods=["GET"]) @protect() - @safe @rison(get_export_ids_schema) def export(self, **kwargs: Any) -> Response: """Export dashboards diff --git a/superset/dashboards/commands/exceptions.py b/superset/dashboards/commands/exceptions.py index 13b0d5b74bc91..fa466b0f7155c 100644 --- a/superset/dashboards/commands/exceptions.py +++ b/superset/dashboards/commands/exceptions.py @@ -41,6 +41,7 @@ class DashboardInvalidError(CommandInvalidError): class DashboardNotFoundError(CommandException): + status = 404 message = _("Dashboard not found.") diff --git a/superset/errors.py b/superset/errors.py new file mode 100644 index 0000000000000..7a568135faf42 --- /dev/null +++ b/superset/errors.py @@ -0,0 +1,47 @@ +# 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 import Flask, jsonify, make_response, Response + +from superset.commands.exceptions import CommandInvalidError +from superset.exceptions import SupersetException + + +def init_global_error_handlers(app: Flask) -> None: + def error_response(code, **kwargs) -> Response: + """ + Generic HTTP JSON response method + + :param code: HTTP code (int) + :param kwargs: Data structure for response (dict) + :return: HTTP Json response + """ + _ret_json = jsonify(kwargs) + resp = make_response(_ret_json, code) + resp.headers["Content-Type"] = "application/json; charset=utf-8" + return resp + + @app.errorhandler(CommandInvalidError) + def command_invalid_error(ex): # pylint: disable=unused-variable + return error_response(ex.status, message=ex.normalized_messages()) + + @app.errorhandler(SupersetException) + def superset_generic_error(ex): # pylint: disable=unused-variable + return error_response(ex.status, message=str(ex)) + + @app.errorhandler(Exception) + def generic_error(ex): # pylint: disable=unused-variable + return error_response(500, message=str(ex)) diff --git a/tests/dashboards/api_tests.py b/tests/dashboards/api_tests.py index 1d2241e089197..0464843fa63ef 100644 --- a/tests/dashboards/api_tests.py +++ b/tests/dashboards/api_tests.py @@ -420,7 +420,7 @@ def test_delete_bulk_dashboard_not_owned(self): rv = self.client.delete(uri) self.assertEqual(rv.status_code, 403) response = json.loads(rv.data.decode("utf-8")) - expected_response = {"message": "Forbidden"} + expected_response = {"message": "Changing this Dashboard is forbidden"} self.assertEqual(response, expected_response) # nothing is deleted in bulk with a list of owned and not owned dashboards @@ -429,7 +429,7 @@ def test_delete_bulk_dashboard_not_owned(self): rv = self.client.delete(uri) self.assertEqual(rv.status_code, 403) response = json.loads(rv.data.decode("utf-8")) - expected_response = {"message": "Forbidden"} + expected_response = {"message": "Changing this Dashboard is forbidden"} self.assertEqual(response, expected_response) for dashboard in dashboards: