From 55014bf58b294c8c4d617635c3fe5c59e088e23c Mon Sep 17 00:00:00 2001 From: John Bodley <4567245+john-bodley@users.noreply.github.com> Date: Thu, 10 Aug 2023 07:41:14 -0700 Subject: [PATCH] chore: Add explicit ON DELETE CASCADE for embedded_dashboards (#24939) Co-authored-by: Michael S. Molina <70410625+michael-s-molina@users.noreply.github.com> --- UPDATING.md | 1 + superset/daos/dashboard.py | 5 -- ...lete_cascade_for_embedded_dashboards.py.py | 48 +++++++++++++++++++ superset/models/embedded_dashboard.py | 6 ++- 4 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 superset/migrations/versions/2023-08-09_15-39_4448fa6deeb1__dd_on_delete_cascade_for_embedded_dashboards.py.py diff --git a/UPDATING.md b/UPDATING.md index a6c826a38..3e9d79171 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,7 @@ assists people when migrating to a new version. ## Next +- [24939](https://github.com/apache/superset/pull/24939): Augments the foreign key constraints for the `embedded_dashboards` table to include an explicit CASCADE ON DELETE to ensure the relevant records are deleted when a dashboard is deleted. Scheduled downtime may be advised. - [24938](https://github.com/apache/superset/pull/24938): Augments the foreign key constraints for the `dashboard_slices` table to include an explicit CASCADE ON DELETE to ensure the relevant records are deleted when a dashboard or slice is deleted. Scheduled downtime may be advised. - [24657](https://github.com/apache/superset/pull/24657): Bumps the cryptography package to augment the OpenSSL security vulnerability. - [24628](https://github.com/apache/superset/pull/24628): Augments the foreign key constraints for the `dashboard_owner`, `report_schedule_owner`, and `slice_owner` tables to include an explicit CASCADE ON DELETE to ensure the relevant ownership records are deleted when a dataset is deleted. Scheduled downtime may be advised. diff --git a/superset/daos/dashboard.py b/superset/daos/dashboard.py index 425d640e7..f9544aa53 100644 --- a/superset/daos/dashboard.py +++ b/superset/daos/dashboard.py @@ -195,11 +195,6 @@ class DashboardDAO(BaseDAO[Dashboard]): @classmethod def delete(cls, items: Dashboard | list[Dashboard], commit: bool = True) -> None: item_ids = [item.id for item in get_iterable(items)] - # bulk delete, first delete related data - for item in get_iterable(items): - item.embedded = [] - db.session.merge(item) - # bulk delete itself try: db.session.query(Dashboard).filter(Dashboard.id.in_(item_ids)).delete( synchronize_session="fetch" diff --git a/superset/migrations/versions/2023-08-09_15-39_4448fa6deeb1__dd_on_delete_cascade_for_embedded_dashboards.py.py b/superset/migrations/versions/2023-08-09_15-39_4448fa6deeb1__dd_on_delete_cascade_for_embedded_dashboards.py.py new file mode 100644 index 000000000..b50f63751 --- /dev/null +++ b/superset/migrations/versions/2023-08-09_15-39_4448fa6deeb1__dd_on_delete_cascade_for_embedded_dashboards.py.py @@ -0,0 +1,48 @@ +# 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. +"""add on delete cascade for embedded dashboards + +Revision ID: 4448fa6deeb1 +Revises: 8ace289026f3 +Create Date: 2023-08-09 15:39:58.130228 + +""" + +# revision identifiers, used by Alembic. +revision = "4448fa6deeb1" +down_revision = "8ace289026f3" + +from superset.migrations.shared.constraints import ForeignKey, redefine + +foreign_keys = [ + ForeignKey( + table="embedded_dashboards", + referent_table="dashboards", + local_cols=["dashboard_id"], + remote_cols=["id"], + ), +] + + +def upgrade(): + for foreign_key in foreign_keys: + redefine(foreign_key, on_delete="CASCADE") + + +def downgrade(): + for foreign_key in foreign_keys: + redefine(foreign_key) diff --git a/superset/models/embedded_dashboard.py b/superset/models/embedded_dashboard.py index 32a8e4abc..3aed89534 100644 --- a/superset/models/embedded_dashboard.py +++ b/superset/models/embedded_dashboard.py @@ -40,7 +40,11 @@ class EmbeddedDashboard(Model, AuditMixinNullable): uuid = Column(UUIDType(binary=True), default=uuid.uuid4, primary_key=True) allow_domain_list = Column(Text) # reference the `allowed_domains` property instead - dashboard_id = Column(Integer, ForeignKey("dashboards.id"), nullable=False) + dashboard_id = Column( + Integer, + ForeignKey("dashboards.id", ondelete="CASCADE"), + nullable=False, + ) dashboard = relationship( "Dashboard", back_populates="embedded",