mikebridge commented on code in PR #41549: URL: https://github.com/apache/superset/pull/41549#discussion_r3658924016
########## superset/cli/deletion_retention.py: ########## @@ -0,0 +1,98 @@ +# 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. +"""Operator CLI for deletion retention. + +``force-purge`` and ``set-window`` are **operator-gated** — they are +protected by deployment/shell access (the ``SECURITY.md`` operator trust +boundary), not Flask-AppBuilder RBAC: a CLI invocation has no ``g.user``, so +there is no ``403`` to enforce. A future REST route would carry real +Admin/workspace-admin RBAC. +""" + +import logging + +import click +from flask.cli import with_appcontext + +logger = logging.getLogger(__name__) + + [email protected]() +def deletion_retention() -> None: + """Manage purge of soft-deleted entities (operator-gated).""" + + +@deletion_retention.command() +@with_appcontext [email protected]( + "--days", + "-d", + required=True, + type=int, + help="Retention window in days; 0 disables.", +) +def set_window(days: int) -> None: + """Set the per-workspace retention window (SharedKey, upsert).""" + from superset.key_value.shared_entries import upsert_shared_value + from superset.key_value.types import SharedKey + + if days < 0: + raise click.BadParameter("--days must be >= 0") + upsert_shared_value(SharedKey.SOFT_DELETE_RETENTION_DAYS, days) Review Comment: The premise here was our own doc wording, corrected in 91e9a6930a: the override is deployment-scoped by design. Each Superset deployment has its own metadata DB, so one SharedKey per deployment is exactly one override per deployment — there is no narrower tenancy unit to scope to. ########## superset/commands/deletion_retention/window.py: ########## @@ -0,0 +1,81 @@ +# 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. +"""Resolve the soft-delete retention window.""" + +from __future__ import annotations + +import logging + +from flask import current_app + +from superset.key_value.shared_entries import get_shared_value +from superset.key_value.types import SharedKey + +logger: logging.Logger = logging.getLogger(__name__) + +_DEFAULT_RETENTION_DAYS: int = 30 + + +def _config_retention_days() -> int: + """Return a validated config fallback without breaking scheduled runs.""" + configured = current_app.config.get( + "SUPERSET_SOFT_DELETE_RETENTION_DAYS", _DEFAULT_RETENTION_DAYS + ) + try: + if isinstance(configured, bool): + raise ValueError + days = int(configured) + if days < 0: + raise ValueError + return days + except (TypeError, ValueError): + logger.warning( + "deletion_retention: ignoring malformed config retention value %r; " + "falling back to %d days", + configured, + _DEFAULT_RETENTION_DAYS, + ) + return _DEFAULT_RETENTION_DAYS + + +def resolve_retention_window() -> int: + """Return the retention window in days, read live on each call. + + Resolution order: + + 1. The per-workspace value persisted under + ``SharedKey.SOFT_DELETE_RETENTION_DAYS`` (read live; takes + precedence when present). + 2. Otherwise the ``SUPERSET_SOFT_DELETE_RETENTION_DAYS`` config / + environment seed default (itself defaulting to 30). + + ``0`` from either source is a meaningful "disable", so the shared + value is selected with an explicit ``is None`` check — never ``or``, + which would treat ``0`` as unset. A malformed shared value is + rejected (logged) and the fallback is used rather than crashing the + scheduled task. + """ + if (shared := get_shared_value(SharedKey.SOFT_DELETE_RETENTION_DAYS)) is not None: Review Comment: Same as the CLI thread: deployment-scoped by design (one metadata DB per deployment); the per-workspace wording that suggested a narrower scope was fixed in 91e9a6930a. ########## superset/tasks/deletion_retention.py: ########## @@ -0,0 +1,260 @@ +# 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. +"""Celery beat task: purge soft-deleted entities past the retention window. + +The deletion-domain analog of ``version_history.prune_old_versions``: where +that ages out version rows while keeping the live entity, this removes +entities that are already soft-deleted. For each +``SoftDeleteMixin`` model it selects rows whose ``deleted_at`` is older than +the per-workspace window and runs the shared cascade per entity, in bounded +id-ordered batches. Convergent, not strictly idempotent: a re-run with the +same clock and data removes nothing, but rows that have since crossed the +cutoff are purged on a later run. +""" + +from __future__ import annotations + +import logging +from collections.abc import Iterator +from datetime import datetime, timedelta +from typing import Any, cast + +import sqlalchemy as sa +from flask import current_app + +from superset import db +from superset.commands.deletion_retention import audit +from superset.commands.deletion_retention.purge_cascade import ( + cascade_hard_delete, + CascadeResult, + dashboard_slice_count, + entity_uuid, + suppress_purge_association_versions, +) +from superset.commands.deletion_retention.window import resolve_retention_window +from superset.extensions import celery_app, feature_flag_manager, stats_logger_manager +from superset.models.helpers import ( + skip_visibility_filter, + SoftDeleteMixin, +) + +logger: logging.Logger = logging.getLogger(__name__) + +_METRIC_PREFIX: str = "deletion_retention" +# Below SQLite's historical 999 bind-variable limit; well under PostgreSQL and +# MySQL limits. There is no existing SQLITE_MAX_VARIABLE_NUMBER symbol to reuse. +_PURGE_DELETE_CHUNK: int = 500 +_BATCH: int = _PURGE_DELETE_CHUNK + + +def _soft_delete_models() -> list[type[SoftDeleteMixin]]: + """The registered ``SoftDeleteMixin`` subclasses (dashboards, charts, + datasets), in a stable order.""" + return list(SoftDeleteMixin._registered_subclasses) # noqa: SLF001 + + +def _model_table(model: type[SoftDeleteMixin]) -> sa.Table: + """Return SQLAlchemy table metadata for a registered soft-delete model.""" + return cast(sa.Table, cast(Any, model).__table__) + + +def _model_table_name(model: type[SoftDeleteMixin]) -> str: + """Return the table name for a registered soft-delete model.""" + return str(cast(Any, model).__tablename__) + + +def _iter_eligible_ids( + model: type[SoftDeleteMixin], cutoff: datetime, batch: int +) -> Iterator[list[int]]: + """Yield id-ordered batches of eligible row ids — ``deleted_at IS NOT NULL + AND deleted_at < cutoff`` — querying with the visibility-filter bypass so + soft-deleted rows are visible. Windowed by an ``id`` watermark so memory + and lock-hold stay bounded on a large first run.""" + table = _model_table(model) + after_id = 0 + while True: + with skip_visibility_filter(db.session, model): + ids = [ + row[0] + for row in db.session.execute( + sa.select(table.c.id) + .where(table.c.deleted_at.is_not(None)) + .where(table.c.deleted_at < cutoff) + .where(table.c.id > after_id) + .order_by(table.c.id) + .limit(batch) + ) + ] + if not ids: + return + yield ids + if len(ids) < batch: + return + after_id = ids[-1] + + +def _purge_impl(window_days: int, dry_run: bool) -> dict[str, Any]: + """Run one purge pass across all soft-delete models.""" + if window_days <= 0: + logger.info("deletion_retention: window is 0 (disabled); skipping") + stats_logger_manager.instance.incr(f"{_METRIC_PREFIX}.skipped") + return {"skipped": 1} + + cutoff = datetime.now() - timedelta(days=window_days) + audit.reconcile_pending() + purged: dict[str, int] = {} + would_purge: dict[str, int] = {} + failures = 0 + blocked = 0 + + for model in _soft_delete_models(): + entity_type = _model_table_name(model) + purged_n, would_n, failed_n, blocked_n = _purge_model(model, cutoff, dry_run) + if would_n: + would_purge[entity_type] = would_n + if purged_n: + purged[entity_type] = purged_n + failures += failed_n + blocked += blocked_n + + if dry_run: + for entity_type, count in would_purge.items(): + stats_logger_manager.instance.gauge( + f"{_METRIC_PREFIX}.would_purge.{entity_type}", count + ) + logger.info("deletion_retention: DRY RUN would_purge=%s", would_purge) + return {"dry_run": 1, "would_purge": would_purge} + + for entity_type, count in purged.items(): + stats_logger_manager.instance.gauge( + f"{_METRIC_PREFIX}.purged.{entity_type}", count + ) + if failures: + stats_logger_manager.instance.incr(f"{_METRIC_PREFIX}.cascade_failures") + if blocked: + stats_logger_manager.instance.gauge( + f"{_METRIC_PREFIX}.blocked_by_reference", blocked + ) + stats = { + "purged": purged, + "cascade_failures": failures, + "blocked_by_reference": blocked, + } + logger.info("deletion_retention: %s", stats) + return stats + + +def _purge_model( + model: type[SoftDeleteMixin], cutoff: datetime, dry_run: bool +) -> tuple[int, int, int, int]: + """Process one model's eligible rows. Returns ``(purged, would_purge, + failures, blocked)``. A single entity's blocked/failed cascade never aborts + the batch.""" + entity_type = _model_table_name(model) + purged = would = failures = blocked = 0 + for id_batch in _iter_eligible_ids(model, cutoff, _BATCH): + if dry_run: + would += len(id_batch) + continue + for entity_id in id_batch: + try: + result = _purge_one(model, entity_id, cutoff) + if result is not None and result.purged: + purged += 1 + elif result is not None and result.blocked_reason is not None: + blocked += 1 + except Exception: # pylint: disable=broad-except + db.session.rollback() # pylint: disable=consider-using-transaction + failures += 1 + logger.exception( + "deletion_retention: cascade failed for %s id=%s", + entity_type, + entity_id, + ) + return purged, would, failures, blocked + + +def _purge_one( + model: type[SoftDeleteMixin], entity_id: int, cutoff: datetime +) -> CascadeResult | None: + """Purge a single entity in its own transaction with a write-ahead audit.""" + with skip_visibility_filter(db.session, model): + entity = db.session.get(model, entity_id) + if entity is None: + return None + entity_uuid_value = entity_uuid(entity) + removed_dashboard_slices = dashboard_slice_count(db.session, entity) + # The audit row commits on a separate connection. End the read transaction + # before that write so SQLite can later promote this session to a writer. + # Re-resolving below also ensures the cascade acts on post-audit state. + db.session.rollback() # pylint: disable=consider-using-transaction + record_id = audit.write_ahead( + trigger=audit.TRIGGER_RETENTION, + actor=audit.ACTOR_SYSTEM, + entity_type=_model_table_name(model), + entity_uuid=entity_uuid_value, + removed_dashboard_slices=removed_dashboard_slices, + ) + with skip_visibility_filter(db.session, model): + entity = db.session.get(model, entity_id) + if entity is None: + audit.fail(record_id) + return None + try: + with suppress_purge_association_versions(db.session): + result = cascade_hard_delete( + db.session, entity, enforce_window=True, cutoff=cutoff + ) + # Commit/rollback are managed manually so audit.fail() can + # record the outcome after the purge transaction resolves. + db.session.commit() # pylint: disable=consider-using-transaction + except Exception: + db.session.rollback() # pylint: disable=consider-using-transaction + audit.fail(record_id) + raise + if result.purged: + audit.confirm( + record_id, + affected_referrers=result.dangling_chart_uuids, + removed_dashboard_slices=result.removed_dashboard_slices, + ) + elif result.blocked_reason is not None: + audit.block(record_id) + else: + audit.fail(record_id) + return result + + +@celery_app.task(name="deletion_retention.purge_soft_deleted") +def purge_soft_deleted() -> dict[str, Any]: + """Beat entry point. Resolves the window live, honors the SOFT_DELETE + rollout gate and dry-run flag, and isolates failures so one bad run does + not poison the schedule.""" + # While the temporary SOFT_DELETE rollout gate is off the delete path + # writes no ``deleted_at`` rows, so the task already no-ops; check the gate + # explicitly for clarity (the check is removed when the gate is). + if not feature_flag_manager.is_feature_enabled("SOFT_DELETE"): + logger.info("deletion_retention: SOFT_DELETE gate off; skipping") + return {"skipped": 1} + window_days = resolve_retention_window() + dry_run = bool(current_app.config.get("SUPERSET_SOFT_DELETE_PURGE_DRY_RUN", True)) Review Comment: Same as the resolver thread: one retention window per deployment is the intended behavior; the misleading per-workspace wording was fixed in 91e9a6930a. ########## superset/cli/deletion_retention.py: ########## @@ -0,0 +1,98 @@ +# 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. +"""Operator CLI for deletion retention. + +``force-purge`` and ``set-window`` are **operator-gated** — they are +protected by deployment/shell access (the ``SECURITY.md`` operator trust +boundary), not Flask-AppBuilder RBAC: a CLI invocation has no ``g.user``, so +there is no ``403`` to enforce. A future REST route would carry real +Admin/workspace-admin RBAC. +""" + +import logging + +import click +from flask.cli import with_appcontext + +logger = logging.getLogger(__name__) + + [email protected]() +def deletion_retention() -> None: + """Manage purge of soft-deleted entities (operator-gated).""" Review Comment: The group is registered automatically: superset/cli/main.py walks every module under superset.cli via pkgutil.walk_packages and add_command()s any click Command/Group it finds — same mechanism as every other CLI module in that package. ########## superset/commands/deletion_retention/audit.py: ########## @@ -0,0 +1,225 @@ +# 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. +"""Write-ahead purge audit record. + +Every purge — time-based or force — writes an immutable record that +**survives** the entity it names, on a **dedicated session** outside the +purge transaction so it neither entangles with the ``DBEventLogger`` +(which shares ``db.session`` and commits mid-request) nor vanishes if the +purge rolls back. The record is written ``pending`` *before* the purge and +flipped to ``confirmed`` *after* it commits, so a crash leaves at most a +``pending`` row, never a missing one. ``pending`` rows are reconciled on the +next run (the purge is convergent). + +The dedicated ``purge_audit_log`` table is content-free (no name or PII; only +action, actor, UTC time, entity type, UUID, and affected referrers) and is never +removed by the purge cascade. +""" + +from __future__ import annotations + +import logging +from datetime import datetime, timedelta +from typing import Any, cast +from uuid import UUID, uuid4 + +import sqlalchemy as sa +from sqlalchemy import Column, DateTime, Integer, String, Text +from sqlalchemy.orm import Session, sessionmaker +from sqlalchemy_utils import UUIDType + +from superset import db + +logger: logging.Logger = logging.getLogger(__name__) + + +def _dedicated_session() -> Session: + """A fresh session on its own connection, independent of the request / + task ``db.session``. The audit write must commit on its own so it survives + a rolled-back or crashed purge.""" + return sessionmaker(bind=db.engine)() + + +STATUS_PENDING = "pending" +STATUS_CONFIRMED = "confirmed" +STATUS_FAILED = "failed" +STATUS_BLOCKED = "blocked" + +_PENDING_STALE_AFTER = timedelta(hours=1) + +TRIGGER_RETENTION = "retention" +TRIGGER_FORCE = "force" + +ACTOR_SYSTEM = "system" + + +class PurgeAuditLog(db.Model): + """Immutable, content-free record of a purge.""" + + __tablename__ = "purge_audit_log" + + id = Column(UUIDType(binary=True), primary_key=True, default=uuid4) + status = Column(String(16), nullable=False, default=STATUS_PENDING) + trigger = Column(String(16), nullable=False) + actor = Column(String(256), nullable=False) + entity_type = Column(String(64), nullable=False) + entity_uuid = Column(String(36), nullable=True, index=True) + # Comma-joined UUIDs of charts left dangling / dashboards that lost a join + # row (force-purge visibility). Free text, content-free. + affected_referrers = Column(Text, nullable=True) + removed_dashboard_slices = Column(Integer, nullable=False, default=0) + created_on = Column(DateTime, nullable=False) + confirmed_on = Column(DateTime, nullable=True) + + +def write_ahead( + *, + trigger: str, + actor: str, + entity_type: str, + entity_uuid: str | None, + removed_dashboard_slices: int = 0, +) -> UUID | None: + """Insert a ``pending`` audit row on a dedicated session, before the + purge runs. Returns the row id to confirm later, or ``None`` if the audit + write itself fails (which must not block the purge).""" + session = _dedicated_session() + try: + record = PurgeAuditLog( + status=STATUS_PENDING, + trigger=trigger, + actor=actor, + entity_type=entity_type, + entity_uuid=entity_uuid, + removed_dashboard_slices=removed_dashboard_slices, + created_on=datetime.utcnow(), + ) + session.add(record) + session.commit() + return cast(UUID, record.id) + except Exception: # pylint: disable=broad-except + session.rollback() + logger.warning( + "deletion_retention: failed to write pending audit row", exc_info=True + ) + return None + finally: + session.close() + + +def finalize(record_id: UUID | None, status: str, **details: Any) -> None: + """Finalize a pending attempt on the dedicated audit session.""" + if record_id is None: + return + session = _dedicated_session() + try: + record = session.get(PurgeAuditLog, record_id) + if record is None: + return + record.status = status + if status == STATUS_CONFIRMED: + record.confirmed_on = datetime.utcnow() + referrers = details.get("affected_referrers") + if referrers: + record.affected_referrers = ",".join(referrers) + removed_dashboard_slices = details.get("removed_dashboard_slices") + if removed_dashboard_slices is not None: + record.removed_dashboard_slices = removed_dashboard_slices + session.commit() + except Exception: # pylint: disable=broad-except + session.rollback() + logger.warning( + "deletion_retention: failed to finalize audit row %s as %s", + record_id, + status, + exc_info=True, + ) + finally: + session.close() + + +def confirm(record_id: UUID | None, **details: Any) -> None: + """Mark an attempt confirmed after the entity transaction commits.""" + finalize(record_id, STATUS_CONFIRMED, **details) + + +def fail(record_id: UUID | None) -> None: + """Mark a known failed/no-op attempt so it does not remain pending.""" + finalize(record_id, STATUS_FAILED) + + +def block(record_id: UUID | None) -> None: + """Mark an attempt blocked by ordinary deletion policy.""" + finalize(record_id, STATUS_BLOCKED) + + +def _entity_exists(session: Session, record: PurgeAuditLog) -> bool | None: + """Return whether the audit target exists, or None if it cannot resolve.""" + # pylint: disable=import-outside-toplevel + from superset.models.helpers import SoftDeleteMixin + + if record.entity_uuid is None: + return None + for model in SoftDeleteMixin._registered_subclasses: # noqa: SLF001 + table = cast(Any, model).__table__ + if table.name != record.entity_type or "uuid" not in table.c: + continue Review Comment: The premise doesn't hold: audit rows are written with the physical table name in both paths (force_purge uses type(entity).__tablename__, the task uses _model_table_name(model)), and reconcile_pending matches table.name against that same value — the user-facing chart/dashboard/dataset labels never reach entity_type. ########## superset/commands/deletion_retention/purge_cascade.py: ########## @@ -0,0 +1,534 @@ +# 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. +"""Shared hard-delete cascade for the purge task and force-purge command. + +A single code path keeps the two surfaces from drifting. Every dependent row +is removed by an explicit ``sa.delete``; +the DB ``ON DELETE CASCADE`` constraints are a backstop only — SQLite does +not enforce FKs unless ``PRAGMA foreign_keys=ON`` and Core bulk DML fires +only DB-level cascades, so relying on cascade silently leaks rows. + +Cascade tiers: + +* **M:N join rows** — hard-deleted for the purged entity, including join + rows owned by *surviving* entities (e.g. a live dashboard's + ``dashboard_slices`` row to a purged chart). The entity on the other side + is never touched except to lose that one relationship row. +* **Owned children** (``delete-orphan``, no independent existence) — a + dataset's columns and metrics, hard-deleted with it. +* **Independently-owned entities** (a dashboard's charts, a chart's dataset) + are **preserved**. A live chart's loose ``datasource_id`` to a purged + dataset is left dangling — legacy hard-delete semantics, no guard. +* **Version history** — the entity's own ``*_version`` shadows and + the ``version_changes`` scoped to them, plus an orphan-sweep of any + ``version_transaction`` left owning zero surviving shadows. Runs + behind a ``has_table`` check so it no-ops when versioning is absent. +""" + +from __future__ import annotations + +import logging +from collections.abc import Iterator +from contextlib import contextmanager +from dataclasses import dataclass, field +from datetime import datetime +from typing import Any + +import sqlalchemy as sa +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import Session + +logger: logging.Logger = logging.getLogger(__name__) + + +@contextmanager +def suppress_purge_association_versions(session: Session) -> Iterator[None]: + """Discard only association versions generated by this purge session. + + ``dashboard_slices`` is a Continuum-tracked association table. Its + engine-level listener expects a unit-of-work even for Core deletes and + queues a shadow-table insert for each removed relationship. A purge + removes history rather than creating it, so preserve any pending work + that predates this block and discard only statements queued by the purge. + + This is deliberately session-scoped. Mutating Continuum's process-global + ``options["versioning"]`` would let concurrent requests silently lose + unrelated history while a purge is running. + """ + try: + from sqlalchemy_continuum import versioning_manager + except ImportError: + yield + return + + options = versioning_manager.options + if not (options.get("versioning") or options.get("native_versioning")): + yield + return + + unit_of_work = versioning_manager.unit_of_work(session) + pending_before = len(unit_of_work.pending_statements) + try: + yield + finally: + del unit_of_work.pending_statements[pending_before:] + + +def entity_uuid(entity: Any) -> str | None: + """Return the entity's UUID as a string, or ``None`` if it has none.""" + value = getattr(entity, "uuid", None) + return str(value) if value is not None else None + + +def _version_tables_present(bind: Any) -> bool: + """Whether the version-history tables exist (a testable seam, so the + version cascade no-ops cleanly before versioning is installed).""" + return sa.inspect(bind).has_table("version_transaction") + + +@dataclass +class CascadeResult: + """Outcome of one entity's cascade. + + ``purged`` is False when the conditional entity-row delete matched no + row — the entity was restored between selection and delete or was already + gone. In that case + no dependents are touched. + """ + + purged: bool + entity_type: str + entity_uuid: str | None + dangling_chart_uuids: list[str] = field(default_factory=list) + removed_dashboard_slices: int = 0 + version_rows_removed: int = 0 + blocked_reason: str | None = None + + +class PurgeBlockedError(Exception): + """Raised when ordinary deletion policy forbids purging an entity.""" + + +class PurgeRaceLostError(Exception): + """Raised to roll back dependent cleanup when the entity delete loses.""" + + +def cascade_hard_delete( + session: Session, + entity: Any, + *, + enforce_window: bool, + cutoff: datetime | None = None, +) -> CascadeResult: + """Remove *entity* and everything that depends on it in one transaction. + + The entity row is locked and its eligibility is re-checked before any + dependent state is touched. Ordinary deletion blockers remain + authoritative. All cleanup and the conditional parent delete run inside a + savepoint so a lost race or restrictive foreign key leaves no side effects. + """ + # pylint: disable=import-outside-toplevel + from superset.connectors.sqla.models import SqlaTable + from superset.models.slice import Slice + + if enforce_window and cutoff is None: + raise ValueError("cutoff is required when enforce_window=True") + + model = type(entity) + table = model.__table__ + entity_id = entity.id + uuid = entity_uuid(entity) + entity_type = _USER_FACING_TYPE.get(table.name, table.name) + + dangling_chart_uuids: list[str] = [] + removed_dashboard_slices = 0 + version_rows = 0 + permission_name = _dataset_permission_name(entity) if model is SqlaTable else None + + try: + with session.begin_nested(): + claim = sa.select(table.c.id).where(table.c.id == entity_id) + if enforce_window: + claim = claim.where(table.c.deleted_at.is_not(None)).where( + table.c.deleted_at < cutoff + ) + if session.execute(claim.with_for_update()).scalar_one_or_none() is None: + raise PurgeRaceLostError + + _validate_deletion_allowed(session, model, entity_id) + removed_dashboard_slices = _count_dashboard_slices( + session, model, entity_id + ) + if model is SqlaTable: + dangling_chart_uuids = [ + str(chart_uuid) + for (chart_uuid,) in session.execute( + sa.select(Slice.uuid) + .where(Slice.datasource_id == entity_id) + .where(Slice.datasource_type == "table") + ) + ] + + _delete_m2m_joins(session, model, entity_id) + _delete_owned_children(session, model, entity_id) + version_rows = _delete_version_history(session, entity, entity_id) + + delete_entity = sa.delete(table).where(table.c.id == entity_id) + if enforce_window: + delete_entity = delete_entity.where( + table.c.deleted_at.is_not(None) + ).where(table.c.deleted_at < cutoff) + if session.execute(delete_entity).rowcount == 0: + raise PurgeRaceLostError + + if permission_name is not None: + _cleanup_dataset_permission(session, permission_name, entity_id) + except PurgeRaceLostError: + logger.info( + "deletion_retention: %s id=%s not purged (restored or already gone)", + entity_type, + entity_id, + ) + return CascadeResult(purged=False, entity_type=entity_type, entity_uuid=uuid) + except (PurgeBlockedError, IntegrityError) as ex: + logger.info( + "deletion_retention: %s id=%s blocked by existing deletion rules", + entity_type, + entity_id, + ) + return CascadeResult( + purged=False, + entity_type=entity_type, + entity_uuid=uuid, + blocked_reason=str(ex), + ) + + return CascadeResult( + purged=True, + entity_type=entity_type, + entity_uuid=uuid, + dangling_chart_uuids=dangling_chart_uuids, + removed_dashboard_slices=removed_dashboard_slices, + version_rows_removed=version_rows, + ) + + +_USER_FACING_TYPE: dict[str, str] = { + "slices": "chart", + "dashboards": "dashboard", + "tables": "dataset", +} + + +def _validate_deletion_allowed( + session: Session, model: type[Any], entity_id: int +) -> None: + """Apply the dependency guards used by ordinary delete commands.""" + # pylint: disable=import-outside-toplevel + from superset.models.dashboard import Dashboard + from superset.models.slice import Slice + from superset.reports.models import ReportSchedule + + column: Any | None = None + if model is Slice: + column = ReportSchedule.chart_id + elif model is Dashboard: + column = ReportSchedule.dashboard_id + if ( + column is not None + and session.execute( + sa.select(ReportSchedule.id).where(column == entity_id).limit(1) + ).first() + ): + raise PurgeBlockedError("associated alerts or reports exist") + + +def _count_dashboard_slices(session: Session, model: type[Any], entity_id: int) -> int: + """Snapshot relationship counts before DB cascades can remove rows.""" + # pylint: disable=import-outside-toplevel + from superset.models.dashboard import Dashboard, dashboard_slices + from superset.models.slice import Slice + + predicate: Any | None = None + if model is Dashboard: + predicate = dashboard_slices.c.dashboard_id == entity_id + elif model is Slice: + predicate = dashboard_slices.c.slice_id == entity_id + if predicate is None: + return 0 + return int( + session.execute( + sa.select(sa.func.count()).select_from(dashboard_slices).where(predicate) + ).scalar_one() + ) + + +def dashboard_slice_count(session: Session, entity: Any) -> int: + """Return the current dashboard relationship count for audit write-ahead.""" + return _count_dashboard_slices(session, type(entity), entity.id) + + +def _delete_m2m_joins(session: Session, model: type[Any], entity_id: int) -> None: + """Hard-delete every M:N join / association row the entity owns. + + Relationship counts are captured before this function runs so database + cascades cannot make the reported values dialect-dependent. + """ + # pylint: disable=import-outside-toplevel + from superset.connectors.sqla.models import SqlaTable + from superset.models.dashboard import Dashboard, dashboard_slices + from superset.models.slice import Slice + from superset.subjects.models import ( + chart_editors, + chart_viewers, + dashboard_editors, + dashboard_viewers, + sqlatable_editors, + ) + from superset.tags.models import ObjectType, TaggedObject + + if model is Dashboard: + session.execute( + sa.delete(dashboard_slices).where( + dashboard_slices.c.dashboard_id == entity_id + ) + ) Review Comment: Both call sites already wrap cascade_hard_delete in suppress_purge_association_versions (force_purge.py and tasks/deletion_retention.py), so the association delete here executes inside the session-scoped suppression. ########## superset/commands/deletion_retention/purge_cascade.py: ########## @@ -0,0 +1,534 @@ +# 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. +"""Shared hard-delete cascade for the purge task and force-purge command. + +A single code path keeps the two surfaces from drifting. Every dependent row +is removed by an explicit ``sa.delete``; +the DB ``ON DELETE CASCADE`` constraints are a backstop only — SQLite does +not enforce FKs unless ``PRAGMA foreign_keys=ON`` and Core bulk DML fires +only DB-level cascades, so relying on cascade silently leaks rows. + +Cascade tiers: + +* **M:N join rows** — hard-deleted for the purged entity, including join + rows owned by *surviving* entities (e.g. a live dashboard's + ``dashboard_slices`` row to a purged chart). The entity on the other side + is never touched except to lose that one relationship row. +* **Owned children** (``delete-orphan``, no independent existence) — a + dataset's columns and metrics, hard-deleted with it. +* **Independently-owned entities** (a dashboard's charts, a chart's dataset) + are **preserved**. A live chart's loose ``datasource_id`` to a purged + dataset is left dangling — legacy hard-delete semantics, no guard. +* **Version history** — the entity's own ``*_version`` shadows and + the ``version_changes`` scoped to them, plus an orphan-sweep of any + ``version_transaction`` left owning zero surviving shadows. Runs + behind a ``has_table`` check so it no-ops when versioning is absent. +""" + +from __future__ import annotations + +import logging +from collections.abc import Iterator +from contextlib import contextmanager +from dataclasses import dataclass, field +from datetime import datetime +from typing import Any + +import sqlalchemy as sa +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import Session + +logger: logging.Logger = logging.getLogger(__name__) + + +@contextmanager +def suppress_purge_association_versions(session: Session) -> Iterator[None]: + """Discard only association versions generated by this purge session. + + ``dashboard_slices`` is a Continuum-tracked association table. Its + engine-level listener expects a unit-of-work even for Core deletes and + queues a shadow-table insert for each removed relationship. A purge + removes history rather than creating it, so preserve any pending work + that predates this block and discard only statements queued by the purge. + + This is deliberately session-scoped. Mutating Continuum's process-global + ``options["versioning"]`` would let concurrent requests silently lose + unrelated history while a purge is running. + """ + try: + from sqlalchemy_continuum import versioning_manager + except ImportError: + yield + return + + options = versioning_manager.options + if not (options.get("versioning") or options.get("native_versioning")): + yield + return + + unit_of_work = versioning_manager.unit_of_work(session) + pending_before = len(unit_of_work.pending_statements) + try: + yield + finally: + del unit_of_work.pending_statements[pending_before:] + + +def entity_uuid(entity: Any) -> str | None: + """Return the entity's UUID as a string, or ``None`` if it has none.""" + value = getattr(entity, "uuid", None) + return str(value) if value is not None else None + + +def _version_tables_present(bind: Any) -> bool: + """Whether the version-history tables exist (a testable seam, so the + version cascade no-ops cleanly before versioning is installed).""" + return sa.inspect(bind).has_table("version_transaction") + + +@dataclass +class CascadeResult: + """Outcome of one entity's cascade. + + ``purged`` is False when the conditional entity-row delete matched no + row — the entity was restored between selection and delete or was already + gone. In that case + no dependents are touched. + """ + + purged: bool + entity_type: str + entity_uuid: str | None + dangling_chart_uuids: list[str] = field(default_factory=list) + removed_dashboard_slices: int = 0 + version_rows_removed: int = 0 + blocked_reason: str | None = None + + +class PurgeBlockedError(Exception): + """Raised when ordinary deletion policy forbids purging an entity.""" + + +class PurgeRaceLostError(Exception): + """Raised to roll back dependent cleanup when the entity delete loses.""" + + +def cascade_hard_delete( + session: Session, + entity: Any, + *, + enforce_window: bool, + cutoff: datetime | None = None, +) -> CascadeResult: + """Remove *entity* and everything that depends on it in one transaction. + + The entity row is locked and its eligibility is re-checked before any + dependent state is touched. Ordinary deletion blockers remain + authoritative. All cleanup and the conditional parent delete run inside a + savepoint so a lost race or restrictive foreign key leaves no side effects. + """ + # pylint: disable=import-outside-toplevel + from superset.connectors.sqla.models import SqlaTable + from superset.models.slice import Slice + + if enforce_window and cutoff is None: + raise ValueError("cutoff is required when enforce_window=True") + + model = type(entity) + table = model.__table__ + entity_id = entity.id + uuid = entity_uuid(entity) + entity_type = _USER_FACING_TYPE.get(table.name, table.name) + + dangling_chart_uuids: list[str] = [] + removed_dashboard_slices = 0 + version_rows = 0 + permission_name = _dataset_permission_name(entity) if model is SqlaTable else None + + try: + with session.begin_nested(): + claim = sa.select(table.c.id).where(table.c.id == entity_id) + if enforce_window: + claim = claim.where(table.c.deleted_at.is_not(None)).where( + table.c.deleted_at < cutoff + ) + if session.execute(claim.with_for_update()).scalar_one_or_none() is None: + raise PurgeRaceLostError + + _validate_deletion_allowed(session, model, entity_id) + removed_dashboard_slices = _count_dashboard_slices( + session, model, entity_id + ) + if model is SqlaTable: + dangling_chart_uuids = [ + str(chart_uuid) + for (chart_uuid,) in session.execute( + sa.select(Slice.uuid) + .where(Slice.datasource_id == entity_id) + .where(Slice.datasource_type == "table") + ) + ] + + _delete_m2m_joins(session, model, entity_id) + _delete_owned_children(session, model, entity_id) + version_rows = _delete_version_history(session, entity, entity_id) + + delete_entity = sa.delete(table).where(table.c.id == entity_id) + if enforce_window: + delete_entity = delete_entity.where( + table.c.deleted_at.is_not(None) + ).where(table.c.deleted_at < cutoff) + if session.execute(delete_entity).rowcount == 0: + raise PurgeRaceLostError + + if permission_name is not None: + _cleanup_dataset_permission(session, permission_name, entity_id) + except PurgeRaceLostError: + logger.info( + "deletion_retention: %s id=%s not purged (restored or already gone)", + entity_type, + entity_id, + ) + return CascadeResult(purged=False, entity_type=entity_type, entity_uuid=uuid) + except (PurgeBlockedError, IntegrityError) as ex: + logger.info( + "deletion_retention: %s id=%s blocked by existing deletion rules", + entity_type, + entity_id, + ) + return CascadeResult( + purged=False, + entity_type=entity_type, + entity_uuid=uuid, + blocked_reason=str(ex), + ) Review Comment: Deliberate: RESTRICT-style FK violations — including from plugin or external tables referencing these entities — are precisely what "blocked by existing deletion rules" means, and there is no portable way to distinguish constraint subtypes across PostgreSQL/MySQL/SQLite. blocked_reason carries the original error text for diagnosis, and non-integrity operational failures still propagate normally. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
