chore: Update DAOs to use singular deletion method instead of bulk (#24894)

superset/daos/chart.py

@@ -20,14 +20,12 @@ import logging
 from datetime import datetime
 from typing import TYPE_CHECKING
 
-from sqlalchemy.exc import SQLAlchemyError
-
 from superset.charts.filters import ChartFilter
 from superset.daos.base import BaseDAO
 from superset.extensions import db
 from superset.models.core import FavStar, FavStarClassName
 from superset.models.slice import Slice
-from superset.utils.core import get_iterable, get_user_id
+from superset.utils.core import get_user_id
 
 if TYPE_CHECKING:
     from superset.connectors.base.models import BaseDatasource
@@ -38,21 +36,6 @@ logger = logging.getLogger(__name__)
 class ChartDAO(BaseDAO[Slice]):
     base_filter = ChartFilter
 
-    @classmethod
-    def delete(cls, items: Slice | list[Slice], commit: bool = True) -> None:
-        item_ids = [item.id for item in get_iterable(items)]
-        # bulk delete, first delete related data
-        # bulk delete itself
-        try:
-            db.session.query(Slice).filter(Slice.id.in_(item_ids)).delete(
-                synchronize_session="fetch"
-            )
-            if commit:
-                db.session.commit()
-        except SQLAlchemyError as ex:
-            db.session.rollback()
-            raise ex
-
     @staticmethod
     def favorited_ids(charts: list[Slice]) -> list[FavStar]:
         ids = [chart.id for chart in charts]

superset/daos/dashboard.py

@@ -23,7 +23,6 @@ from typing import Any
 
 from flask import g
 from flask_appbuilder.models.sqla.interface import SQLAInterface
-from sqlalchemy.exc import SQLAlchemyError
 
 from superset import is_feature_enabled, security_manager
 from superset.daos.base import BaseDAO
@@ -48,7 +47,7 @@ from superset.models.dashboard import Dashboard, id_or_slug_filter
 from superset.models.embedded_dashboard import EmbeddedDashboard
 from superset.models.filter_set import FilterSet
 from superset.models.slice import Slice
-from superset.utils.core import get_iterable, get_user_id
+from superset.utils.core import get_user_id
 from superset.utils.dashboard_filter_scopes_converter import copy_filter_scopes
 
 logger = logging.getLogger(__name__)
@@ -190,19 +189,6 @@ class DashboardDAO(BaseDAO[Dashboard]):
             db.session.commit()
         return model
 
-    @classmethod
-    def delete(cls, items: Dashboard | list[Dashboard], commit: bool = True) -> None:
-        item_ids = [item.id for item in get_iterable(items)]
-        try:
-            db.session.query(Dashboard).filter(Dashboard.id.in_(item_ids)).delete(
-                synchronize_session="fetch"
-            )
-            if commit:
-                db.session.commit()
-        except SQLAlchemyError as ex:
-            db.session.rollback()
-            raise ex
-
     @staticmethod
     def set_dash_metadata(  # pylint: disable=too-many-locals
         dashboard: Dashboard,

superset/daos/dataset.py

@@ -21,14 +21,13 @@ from typing import Any
 
 from sqlalchemy.exc import SQLAlchemyError
 
-from superset import security_manager
 from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn
 from superset.daos.base import BaseDAO
 from superset.extensions import db
 from superset.models.core import Database
 from superset.models.dashboard import Dashboard
 from superset.models.slice import Slice
-from superset.utils.core import DatasourceType, get_iterable
+from superset.utils.core import DatasourceType
 from superset.views.base import DatasourceFilter
 
 logger = logging.getLogger(__name__)
@@ -309,42 +308,6 @@ class DatasetDAO(BaseDAO[SqlaTable]):
             return None
         return db.session.query(SqlMetric).get(metric_id)
 
-    @classmethod
-    def delete(
-        cls,
-        items: SqlaTable | list[SqlaTable],
-        commit: bool = True,
-    ) -> None:
-        """
-        Delete the specified items(s) including their associated relationships.
-
-        Note that bulk deletion via `delete` does not dispatch the `after_delete` event
-        and thus the ORM event listener callback needs to be invoked manually.
-
-        :param items: The item(s) 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
-        """
-
-        try:
-            db.session.query(SqlaTable).filter(
-                SqlaTable.id.in_(item.id for item in get_iterable(items))
-            ).delete(synchronize_session="fetch")
-
-            connection = db.session.connection()
-            mapper = next(iter(cls.model_cls.registry.mappers))  # type: ignore
-
-            for item in get_iterable(items):
-                security_manager.dataset_after_delete(mapper, connection, item)
-
-            if commit:
-                db.session.commit()
-        except SQLAlchemyError as ex:
-            if commit:
-                db.session.rollback()
-            raise ex
-
     @staticmethod
     def get_table_by_name(database_id: int, table_name: str) -> SqlaTable | None:
         return (

tests/integration_tests/datasets/api_tests.py

@@ -25,6 +25,7 @@ from zipfile import is_zipfile, ZipFile
 import prison
 import pytest
 import yaml
+from sqlalchemy import inspect
 from sqlalchemy.orm import joinedload
 from sqlalchemy.sql import func
 
@@ -153,7 +154,9 @@ class TestDatasetApi(SupersetTestCase):
 
             # rollback changes
             for dataset in datasets:
-                db.session.delete(dataset)
+                state = inspect(dataset)
+                if not state.was_deleted:
+                    db.session.delete(dataset)
             db.session.commit()
 
     @staticmethod