diff --git a/superset/charts/api.py b/superset/charts/api.py index 602f082b40e..bb92b3da20e 100644 --- a/superset/charts/api.py +++ b/superset/charts/api.py @@ -63,7 +63,7 @@ from superset.charts.schemas import ( screenshot_query_schema, thumbnail_query_schema, ) -from superset.commands.exceptions import CommandInvalidError +from superset.commands.importers.exceptions import NoValidFilesFoundError from superset.commands.importers.v1.utils import get_contents_from_bundle from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.exceptions import QueryObjectValidationError @@ -977,7 +977,6 @@ class ChartRestApi(BaseSupersetModelRestApi): @expose("/import/", methods=["POST"]) @protect() - @safe @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_", @@ -1029,6 +1028,9 @@ class ChartRestApi(BaseSupersetModelRestApi): with ZipFile(upload) as bundle: contents = get_contents_from_bundle(bundle) + if not contents: + raise NoValidFilesFoundError() + passwords = ( json.loads(request.form["passwords"]) if "passwords" in request.form @@ -1039,12 +1041,5 @@ class ChartRestApi(BaseSupersetModelRestApi): command = ImportChartsCommand( contents, passwords=passwords, overwrite=overwrite ) - try: - command.run() - return self.response(200, message="OK") - except CommandInvalidError as exc: - logger.warning("Import chart failed") - return self.response_422(message=exc.normalized_messages()) - except Exception as exc: # pylint: disable=broad-except - logger.exception("Import chart failed") - return self.response_500(message=str(exc)) + command.run() + return self.response(200, message="OK") diff --git a/superset/commands/importers/exceptions.py b/superset/commands/importers/exceptions.py index e79cc1c5d24..c1beb8eb537 100644 --- a/superset/commands/importers/exceptions.py +++ b/superset/commands/importers/exceptions.py @@ -21,3 +21,8 @@ from superset.commands.exceptions import CommandException class IncorrectVersionError(CommandException): status = 422 message = "Import has incorrect version" + + +class NoValidFilesFoundError(CommandException): + status = 400 + message = "No valid import files were found" diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index 14e92ed2e8a..632f7f2c646 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -32,7 +32,7 @@ from werkzeug.wsgi import FileWrapper from superset import is_feature_enabled, thumbnail_cache from superset.charts.schemas import ChartEntityResponseSchema -from superset.commands.exceptions import CommandInvalidError +from superset.commands.importers.exceptions import NoValidFilesFoundError from superset.commands.importers.v1.utils import get_contents_from_bundle from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.dashboards.commands.bulk_delete import BulkDeleteDashboardCommand @@ -43,7 +43,6 @@ from superset.dashboards.commands.exceptions import ( DashboardCreateFailedError, DashboardDeleteFailedError, DashboardForbiddenError, - DashboardImportError, DashboardInvalidError, DashboardNotFoundError, DashboardUpdateFailedError, @@ -888,7 +887,6 @@ class DashboardRestApi(BaseSupersetModelRestApi): @expose("/import/", methods=["POST"]) @protect() - @safe @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_", @@ -944,6 +942,9 @@ class DashboardRestApi(BaseSupersetModelRestApi): upload.seek(0) contents = {upload.filename: upload.read()} + if not contents: + raise NoValidFilesFoundError() + passwords = ( json.loads(request.form["passwords"]) if "passwords" in request.form @@ -954,12 +955,5 @@ class DashboardRestApi(BaseSupersetModelRestApi): command = ImportDashboardsCommand( contents, passwords=passwords, overwrite=overwrite ) - try: - command.run() - return self.response(200, message="OK") - except CommandInvalidError as exc: - logger.warning("Import dashboard failed") - return self.response_422(message=exc.normalized_messages()) - except DashboardImportError as exc: - logger.exception("Import dashboard failed") - return self.response_500(message=str(exc)) + command.run() + return self.response(200, message="OK") diff --git a/superset/databases/api.py b/superset/databases/api.py index 75a218741b7..d64238baf4a 100644 --- a/superset/databases/api.py +++ b/superset/databases/api.py @@ -28,7 +28,7 @@ from marshmallow import ValidationError from sqlalchemy.exc import NoSuchTableError, OperationalError, SQLAlchemyError from superset import app, event_logger -from superset.commands.exceptions import CommandInvalidError +from superset.commands.importers.exceptions import NoValidFilesFoundError from superset.commands.importers.v1.utils import get_contents_from_bundle from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.databases.commands.create import CreateDatabaseCommand @@ -38,7 +38,6 @@ from superset.databases.commands.exceptions import ( DatabaseCreateFailedError, DatabaseDeleteDatasetsExistFailedError, DatabaseDeleteFailedError, - DatabaseImportError, DatabaseInvalidError, DatabaseNotFoundError, DatabaseUpdateFailedError, @@ -749,7 +748,6 @@ class DatabaseRestApi(BaseSupersetModelRestApi): @expose("/import/", methods=["POST"]) @protect() - @safe @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_", @@ -801,6 +799,9 @@ class DatabaseRestApi(BaseSupersetModelRestApi): with ZipFile(upload) as bundle: contents = get_contents_from_bundle(bundle) + if not contents: + raise NoValidFilesFoundError() + passwords = ( json.loads(request.form["passwords"]) if "passwords" in request.form @@ -811,15 +812,8 @@ class DatabaseRestApi(BaseSupersetModelRestApi): command = ImportDatabasesCommand( contents, passwords=passwords, overwrite=overwrite ) - try: - command.run() - return self.response(200, message="OK") - except CommandInvalidError as exc: - logger.warning("Import database failed") - return self.response_422(message=exc.normalized_messages()) - except DatabaseImportError as exc: - logger.error("Import database failed", exc_info=True) - return self.response_500(message=str(exc)) + command.run() + return self.response(200, message="OK") @expose("//function_names/", methods=["GET"]) @protect() diff --git a/superset/datasets/api.py b/superset/datasets/api.py index 278fc0b3ab4..312369c609e 100644 --- a/superset/datasets/api.py +++ b/superset/datasets/api.py @@ -29,7 +29,7 @@ from flask_babel import ngettext from marshmallow import ValidationError from superset import event_logger, is_feature_enabled -from superset.commands.exceptions import CommandInvalidError +from superset.commands.importers.exceptions import NoValidFilesFoundError from superset.commands.importers.v1.utils import get_contents_from_bundle from superset.connectors.sqla.models import SqlaTable from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod @@ -42,7 +42,6 @@ from superset.datasets.commands.exceptions import ( DatasetCreateFailedError, DatasetDeleteFailedError, DatasetForbiddenError, - DatasetImportError, DatasetInvalidError, DatasetNotFoundError, DatasetRefreshFailedError, @@ -655,7 +654,6 @@ class DatasetRestApi(BaseSupersetModelRestApi): @expose("/import/", methods=["POST"]) @protect() - @safe @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_", @@ -711,6 +709,9 @@ class DatasetRestApi(BaseSupersetModelRestApi): upload.seek(0) contents = {upload.filename: upload.read()} + if not contents: + raise NoValidFilesFoundError() + passwords = ( json.loads(request.form["passwords"]) if "passwords" in request.form @@ -721,12 +722,5 @@ class DatasetRestApi(BaseSupersetModelRestApi): command = ImportDatasetsCommand( contents, passwords=passwords, overwrite=overwrite ) - try: - command.run() - return self.response(200, message="OK") - except CommandInvalidError as exc: - logger.warning("Import dataset failed") - return self.response_422(message=exc.normalized_messages()) - except DatasetImportError as exc: - logger.error("Import dataset failed", exc_info=True) - return self.response_500(message=str(exc)) + command.run() + return self.response(200, message="OK") diff --git a/superset/queries/saved_queries/api.py b/superset/queries/saved_queries/api.py index ee39f0913cb..97b123e3444 100644 --- a/superset/queries/saved_queries/api.py +++ b/superset/queries/saved_queries/api.py @@ -26,7 +26,7 @@ from flask_appbuilder.api import expose, protect, rison, safe from flask_appbuilder.models.sqla.interface import SQLAInterface from flask_babel import ngettext -from superset.commands.exceptions import CommandInvalidError +from superset.commands.importers.exceptions import NoValidFilesFoundError from superset.commands.importers.v1.utils import get_contents_from_bundle from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.databases.filters import DatabaseFilter @@ -263,7 +263,6 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): @expose("/import/", methods=["POST"]) @protect() - @safe @statsd_metrics @event_logger.log_this_with_context( action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.import_", @@ -315,6 +314,9 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): with ZipFile(upload) as bundle: contents = get_contents_from_bundle(bundle) + if not contents: + raise NoValidFilesFoundError() + passwords = ( json.loads(request.form["passwords"]) if "passwords" in request.form @@ -325,12 +327,5 @@ class SavedQueryRestApi(BaseSupersetModelRestApi): command = ImportSavedQueriesCommand( contents, passwords=passwords, overwrite=overwrite ) - try: - command.run() - return self.response(200, message="OK") - except CommandInvalidError as exc: - logger.warning("Import Saved Query failed") - return self.response_422(message=exc.normalized_messages()) - except Exception as exc: # pylint: disable=broad-except - logger.exception("Import Saved Query failed") - return self.response_500(message=str(exc)) + command.run() + return self.response(200, message="OK") diff --git a/tests/charts/api_tests.py b/tests/charts/api_tests.py index 33927bbe1e7..b43abfb6641 100644 --- a/tests/charts/api_tests.py +++ b/tests/charts/api_tests.py @@ -1633,9 +1633,22 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin): assert rv.status_code == 422 assert response == { - "message": { - "charts/imported_chart.yaml": "Chart already exists and `overwrite=true` was not passed" - } + "errors": [ + { + "message": "Error importing chart", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "charts/imported_chart.yaml": "Chart already exists and `overwrite=true` was not passed", + "issue_codes": [ + { + "code": 1010, + "message": "Issue 1010 - Superset encountered an error while running a command.", + } + ], + }, + } + ] } # import with overwrite flag @@ -1691,7 +1704,25 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin): assert rv.status_code == 422 assert response == { - "message": {"metadata.yaml": {"type": ["Must be equal to Slice."]}} + "errors": [ + { + "message": "Error importing chart", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "metadata.yaml": {"type": ["Must be equal to Slice."]}, + "issue_codes": [ + { + "code": 1010, + "message": ( + "Issue 1010 - Superset encountered an " + "error while running a command." + ), + } + ], + }, + } + ] } @pytest.mark.usefixtures( diff --git a/tests/dashboards/api_tests.py b/tests/dashboards/api_tests.py index 02be01e32f0..7bec82bb93a 100644 --- a/tests/dashboards/api_tests.py +++ b/tests/dashboards/api_tests.py @@ -626,6 +626,14 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi buf.seek(0) return buf + def create_invalid_dashboard_import(self): + buf = BytesIO() + with ZipFile(buf, "w") as bundle: + with bundle.open("sql/dump.sql", "w") as fp: + fp.write("CREATE TABLE foo (bar INT)".encode()) + buf.seek(0) + return buf + def test_delete_dashboard(self): """ Dashboard API: Test delete @@ -1392,6 +1400,42 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi db.session.delete(database) db.session.commit() + def test_import_dashboard_invalid_file(self): + """ + Dashboard API: Test import invalid dashboard file + """ + self.login(username="admin") + uri = "api/v1/dashboard/import/" + + buf = self.create_invalid_dashboard_import() + form_data = { + "formData": (buf, "dashboard_export.zip"), + } + rv = self.client.post(uri, data=form_data, content_type="multipart/form-data") + response = json.loads(rv.data.decode("utf-8")) + + assert rv.status_code == 400 + assert response == { + "errors": [ + { + "message": "No valid import files were found", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "issue_codes": [ + { + "code": 1010, + "message": ( + "Issue 1010 - Superset encountered an " + "error while running a command." + ), + } + ] + }, + } + ] + } + def test_import_dashboard_v0_export(self): num_dashboards = db.session.query(Dashboard).count() @@ -1449,9 +1493,25 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi assert rv.status_code == 422 assert response == { - "message": { - "dashboards/imported_dashboard.yaml": "Dashboard already exists and `overwrite=true` was not passed" - } + "errors": [ + { + "message": "Error importing dashboard", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "dashboards/imported_dashboard.yaml": "Dashboard already exists and `overwrite=true` was not passed", + "issue_codes": [ + { + "code": 1010, + "message": ( + "Issue 1010 - Superset encountered an " + "error while running a command." + ), + } + ], + }, + } + ] } # import with overwrite flag @@ -1515,7 +1575,25 @@ class TestDashboardApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixi assert rv.status_code == 422 assert response == { - "message": {"metadata.yaml": {"type": ["Must be equal to Dashboard."]}} + "errors": [ + { + "message": "Error importing dashboard", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "metadata.yaml": {"type": ["Must be equal to Dashboard."]}, + "issue_codes": [ + { + "code": 1010, + "message": ( + "Issue 1010 - Superset encountered " + "an error while running a command." + ), + } + ], + }, + } + ] } def test_get_all_related_roles(self): diff --git a/tests/databases/api_tests.py b/tests/databases/api_tests.py index 8f17fdc2258..09711fd98a8 100644 --- a/tests/databases/api_tests.py +++ b/tests/databases/api_tests.py @@ -1208,9 +1208,25 @@ class TestDatabaseApi(SupersetTestCase): assert rv.status_code == 422 assert response == { - "message": { - "databases/imported_database.yaml": "Database already exists and `overwrite=true` was not passed" - } + "errors": [ + { + "message": "Error importing database", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "databases/imported_database.yaml": "Database already exists and `overwrite=true` was not passed", + "issue_codes": [ + { + "code": 1010, + "message": ( + "Issue 1010 - Superset encountered an " + "error while running a command." + ), + } + ], + }, + } + ] } # import with overwrite flag @@ -1263,7 +1279,25 @@ class TestDatabaseApi(SupersetTestCase): assert rv.status_code == 422 assert response == { - "message": {"metadata.yaml": {"type": ["Must be equal to Database."]}} + "errors": [ + { + "message": "Error importing database", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "metadata.yaml": {"type": ["Must be equal to Database."]}, + "issue_codes": [ + { + "code": 1010, + "message": ( + "Issue 1010 - Superset encountered an " + "error while running a command." + ), + } + ], + }, + } + ] } def test_import_database_masked_password(self): @@ -1300,11 +1334,27 @@ class TestDatabaseApi(SupersetTestCase): assert rv.status_code == 422 assert response == { - "message": { - "databases/imported_database.yaml": { - "_schema": ["Must provide a password for the database"] + "errors": [ + { + "message": "Error importing database", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "databases/imported_database.yaml": { + "_schema": ["Must provide a password for the database"] + }, + "issue_codes": [ + { + "code": 1010, + "message": ( + "Issue 1010 - Superset encountered an " + "error while running a command." + ), + } + ], + }, } - } + ] } def test_import_database_masked_password_provided(self): diff --git a/tests/datasets/api_tests.py b/tests/datasets/api_tests.py index b82ef75d414..ea0e17277a9 100644 --- a/tests/datasets/api_tests.py +++ b/tests/datasets/api_tests.py @@ -1543,9 +1543,22 @@ class TestDatasetApi(SupersetTestCase): assert rv.status_code == 422 assert response == { - "message": { - "datasets/imported_dataset.yaml": "Dataset already exists and `overwrite=true` was not passed" - } + "errors": [ + { + "message": "Error importing dataset", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "datasets/imported_dataset.yaml": "Dataset already exists and `overwrite=true` was not passed", + "issue_codes": [ + { + "code": 1010, + "message": "Issue 1010 - Superset encountered an error while running a command.", + } + ], + }, + } + ] } # import with overwrite flag @@ -1599,7 +1612,25 @@ class TestDatasetApi(SupersetTestCase): assert rv.status_code == 422 assert response == { - "message": {"metadata.yaml": {"type": ["Must be equal to SqlaTable."]}} + "errors": [ + { + "message": "Error importing dataset", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "metadata.yaml": {"type": ["Must be equal to SqlaTable."]}, + "issue_codes": [ + { + "code": 1010, + "message": ( + "Issue 1010 - Superset encountered " + "an error while running a command." + ), + } + ], + }, + } + ] } def test_import_dataset_invalid_v0_validation(self): @@ -1628,4 +1659,20 @@ class TestDatasetApi(SupersetTestCase): response = json.loads(rv.data.decode("utf-8")) assert rv.status_code == 422 - assert response == {"message": "Could not process entity"} + assert response == { + "errors": [ + { + "message": "Could not find a valid command to import file", + "error_type": "GENERIC_COMMAND_ERROR", + "level": "warning", + "extra": { + "issue_codes": [ + { + "code": 1010, + "message": "Issue 1010 - Superset encountered an error while running a command.", + } + ] + }, + } + ] + }