From 843c7ab58a7a4ccc45c1fbdede6a1766d36b1c84 Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Wed, 22 Nov 2023 03:52:30 -0800 Subject: [PATCH] chore: Allow only iterables for BaseDAO.delete() (#25844) --- superset/daos/base.py | 11 +++++------ superset/dashboards/api.py | 3 +-- superset/dashboards/filter_sets/commands/delete.py | 2 +- superset/databases/commands/delete.py | 2 +- superset/databases/ssh_tunnel/commands/delete.py | 2 +- superset/datasets/columns/commands/delete.py | 2 +- superset/datasets/metrics/commands/delete.py | 2 +- .../dashboards/security/security_dataset_tests.py | 2 +- 8 files changed, 12 insertions(+), 14 deletions(-) diff --git a/superset/daos/base.py b/superset/daos/base.py index d2c1842c17..1133a76a1e 100644 --- a/superset/daos/base.py +++ b/superset/daos/base.py @@ -16,7 +16,7 @@ # under the License. from __future__ import annotations -from typing import Any, cast, Generic, get_args, TypeVar +from typing import Any, Generic, get_args, TypeVar from flask_appbuilder.models.filters import BaseFilter from flask_appbuilder.models.sqla import Model @@ -30,7 +30,6 @@ from superset.daos.exceptions import ( DAOUpdateFailedError, ) from superset.extensions import db -from superset.utils.core import as_list T = TypeVar("T", bound=Model) @@ -197,9 +196,9 @@ class BaseDAO(Generic[T]): return item # type: ignore @classmethod - def delete(cls, item_or_items: T | list[T], commit: bool = True) -> None: + def delete(cls, items: list[T], commit: bool = True) -> None: """ - Delete the specified item(s) including their associated relationships. + Delete the specified items including their associated relationships. Note that bulk deletion via `delete` is not invoked in the base class as this does not dispatch the ORM `after_delete` event which may be required to augment @@ -209,12 +208,12 @@ class BaseDAO(Generic[T]): Subclasses may invoke bulk deletion but are responsible for instrumenting any post-deletion logic. - :param items: The item(s) to delete + :param items: The items to delete :param commit: Whether to commit the transaction :raises DAODeleteFailedError: If the deletion failed :see: https://docs.sqlalchemy.org/en/latest/orm/queryguide/dml.html """ - items = cast(list[T], as_list(item_or_items)) + try: for item in items: db.session.delete(item) diff --git a/superset/dashboards/api.py b/superset/dashboards/api.py index b2aa43b0ee..b6ba689d83 100644 --- a/superset/dashboards/api.py +++ b/superset/dashboards/api.py @@ -1349,8 +1349,7 @@ class DashboardRestApi(BaseSupersetModelRestApi): 500: $ref: '#/components/responses/500' """ - for embedded in dashboard.embedded: - EmbeddedDashboardDAO.delete(embedded) + EmbeddedDashboardDAO.delete(dashboard.embedded) return self.response(200, message="OK") @expose("//copy/", methods=("POST",)) diff --git a/superset/dashboards/filter_sets/commands/delete.py b/superset/dashboards/filter_sets/commands/delete.py index edde4b9b45..8ac2107ca5 100644 --- a/superset/dashboards/filter_sets/commands/delete.py +++ b/superset/dashboards/filter_sets/commands/delete.py @@ -38,7 +38,7 @@ class DeleteFilterSetCommand(BaseFilterSetCommand): assert self._filter_set try: - FilterSetDAO.delete(self._filter_set) + FilterSetDAO.delete([self._filter_set]) except DAODeleteFailedError as err: raise FilterSetDeleteFailedError(str(self._filter_set_id), "") from err diff --git a/superset/databases/commands/delete.py b/superset/databases/commands/delete.py index 254380a906..59a247a506 100644 --- a/superset/databases/commands/delete.py +++ b/superset/databases/commands/delete.py @@ -44,7 +44,7 @@ class DeleteDatabaseCommand(BaseCommand): assert self._model try: - DatabaseDAO.delete(self._model) + DatabaseDAO.delete([self._model]) except DAODeleteFailedError as ex: logger.exception(ex.exception) raise DatabaseDeleteFailedError() from ex diff --git a/superset/databases/ssh_tunnel/commands/delete.py b/superset/databases/ssh_tunnel/commands/delete.py index 04d6e68338..70be55ce41 100644 --- a/superset/databases/ssh_tunnel/commands/delete.py +++ b/superset/databases/ssh_tunnel/commands/delete.py @@ -43,7 +43,7 @@ class DeleteSSHTunnelCommand(BaseCommand): assert self._model try: - SSHTunnelDAO.delete(self._model) + SSHTunnelDAO.delete([self._model]) except DAODeleteFailedError as ex: raise SSHTunnelDeleteFailedError() from ex diff --git a/superset/datasets/columns/commands/delete.py b/superset/datasets/columns/commands/delete.py index 23b0d93b6a..0eaa78b0d5 100644 --- a/superset/datasets/columns/commands/delete.py +++ b/superset/datasets/columns/commands/delete.py @@ -43,7 +43,7 @@ class DeleteDatasetColumnCommand(BaseCommand): assert self._model try: - DatasetColumnDAO.delete(self._model) + DatasetColumnDAO.delete([self._model]) except DAODeleteFailedError as ex: logger.exception(ex.exception) raise DatasetColumnDeleteFailedError() from ex diff --git a/superset/datasets/metrics/commands/delete.py b/superset/datasets/metrics/commands/delete.py index 8f27e98a3d..c19aff7aa5 100644 --- a/superset/datasets/metrics/commands/delete.py +++ b/superset/datasets/metrics/commands/delete.py @@ -43,7 +43,7 @@ class DeleteDatasetMetricCommand(BaseCommand): assert self._model try: - DatasetMetricDAO.delete(self._model) + DatasetMetricDAO.delete([self._model]) except DAODeleteFailedError as ex: logger.exception(ex.exception) raise DatasetMetricDeleteFailedError() from ex diff --git a/tests/integration_tests/dashboards/security/security_dataset_tests.py b/tests/integration_tests/dashboards/security/security_dataset_tests.py index 550d25ab59..4ccfa981b1 100644 --- a/tests/integration_tests/dashboards/security/security_dataset_tests.py +++ b/tests/integration_tests/dashboards/security/security_dataset_tests.py @@ -192,4 +192,4 @@ class TestDashboardDatasetSecurity(DashboardTestCase): self.assert200(rv) data = json.loads(rv.data.decode("utf-8")) self.assertEqual(0, data["count"]) - DashboardDAO.delete(dashboard) + DashboardDAO.delete([dashboard])