QT/superset2
1
0
Fork 0
mirror of https://github.com/apache/superset.git synced 2026-07-16 11:46:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(versioning): change records + diff engine

Browse Source 
Adds a structured per-field change log alongside the foundational
shadow tables. Each save flush emits zero or more ``version_changes``
rows describing what changed relative to the previous version, with
shape ``[{kind, path, from_value, to_value, sequence}]`` keyed to
``version_transaction.id`` (FR-016 .. FR-021).

**Schema** — ``version_changes`` table, FK to ``version_transaction``
with ``ON DELETE CASCADE`` so retention drops dependent records
without explicit cleanup. Composite unique index on
``(transaction_id, entity_kind, entity_id, sequence)`` so the
listener can write monotonically and downstream readers see a
deterministic order.

**Diff engine** (``superset/versioning/diff.py``) — pure-function
diffing of pre-/post-state pairs:

- ``diff_scalar_fields`` for ordinary columns; emits one record per
  changed field with JSON-safe ``from_value`` / ``to_value``.
- ``diff_json_field`` for ``json_metadata`` and ``params``, walking
  the parsed structure and emitting per-sub-key records. Honours
  an ``exclude_keys`` set
  (``_DASHBOARD_JSON_METADATA_AUDIT_KEYS``: ``chart_configuration``,
  ``global_chart_configuration``, ``map_label_colors``,
  ``show_chart_timestamps``, ``color_namespace``;
  ``_CHART_PARAMS_AUDIT_KEYS``) so frontend-stamped sub-keys that
  mutate on every save don't dominate the change log (FR-022).
- ``diff_dashboard_layout`` walks ``position_json`` structurally
  and emits ``[verb, kind, id]`` records (verbs ``add``, ``remove``,
  ``move``, ``edit``; kinds from a ``CHART``/``ROW``/``COLUMN``/etc.
  → english map) so a UI can render "Added chart 'Foo'" without
  re-parsing JSON. ``HEADER_ID`` is suppressed because it duplicates
  the ``dashboard_title`` scalar record.
- ``fold_dashboard_layout_with_chart_changes`` deduplicates layout
  records against M2M / chart-membership records by UUID so an
  add-and-attach doesn't appear twice.
- ``_values_equivalent`` treats ``None`` and ``""`` as equal; this
  matches the save path's habit of normalising nullable strings to
  the empty string.

**Listener** — ``superset/versioning/changes.py`` registers a
``before_flush`` listener that captures pre-state for each dirty
entity and an ``after_flush`` listener that runs the diff engine
against the post-state and writes ``version_changes`` rows under
the resolved ``transaction_id``. Tracks processed transaction ids
on ``session.info`` so re-firings within a single transaction
(autoflush triggered by mid-commit queries) don't double-insert and
trip the unique constraint. Reads child rows via raw SELECT against
``table_columns`` / ``sql_metrics`` rather than ``dataset.columns``
because the live collection is stale during the restore path's raw
DELETE+INSERT cycle.

**Endpoint surface** — ``VersionDAO.list_change_records_batch``
batches the lookup across multiple transactions with a single
``WHERE transaction_id IN (...)`` query so the version-list
endpoint avoids N+1 round-trips. ``list_versions`` / ``get_version``
return entries with a populated ``changes`` array (empty for
``operation_type=0`` baseline rows).

**Tests** — ``test_diff.py`` covers the diff engine shape (39
unit cases across scalar, JSON, layout, child-collection, and
fold paths). ``change_records_tests.py`` exercises the listener
end-to-end with realistic save flows. ``perf_validation_tests.py``
is the T044 harness for SC-002/3/4 (list endpoint p95 < 1s,
restore < 3s, save overhead < 50ms).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mike Bridge
2026-05-07 15:31:03 -06:00
parent be01e4552c
commit f7d73e2e1b
6 changed files with 3616 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

130
superset/migrations/versions/2026-04-24_10-00_e1f3c5a7b9d0_add_version_changes_table.py Normal file
View File

@@ -0,0 +1,130 @@
# 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_version_changes_table
Creates ``version_changes``, a field-level diff log keyed to a
(transaction, entity) pair. Each row describes one atomic change
(one field or one child-collection element) that occurred to one
entity during a save. Phase-2 UI will render these rows into
human-readable summaries via the frontend translator.
Shape:
(id, transaction_id, entity_kind, entity_id,
sequence, kind, path, from_value, to_value)
- ``transaction_id`` joins to ``version_transaction`` with ON DELETE
CASCADE so retention pruning of a version row drops its change
records automatically.
- ``entity_kind`` identifies which model type the record is about
(``"chart"`` / ``"dashboard"`` / ``"dataset"``). Required because
a single Continuum transaction can touch more than one versioned
entity (import pipelines, bulk operations, fixture loads), and the
API needs to filter a given entity's records precisely.
- ``entity_id`` is the entity's primary key — joins to ``slices.id``
/ ``dashboards.id`` / ``tables.id`` depending on ``entity_kind``.
- ``sequence`` orders records within one ``(transaction, entity)``
triple — deterministic replay is ``set(state, path, to_value)`` in
ascending sequence.
- ``kind`` is indexed for the Phase-2 "filter history by change type"
query (``WHERE kind = 'filter'``).
- ``path``, ``from_value``, ``to_value`` are JSON because they are
inherently structured (arrays of segments, scalar or object values).
See spec FR-016..FR-021 and data-model.md §``version_changes``.
Revision ID: e1f3c5a7b9d0
Revises: c9d7e21a4b3f
Create Date: 2026-04-24 10:00:00.000000
"""
from __future__ import annotations
import sqlalchemy as sa
from alembic import op
revision = "e1f3c5a7b9d0"
down_revision = "56cd24c07170"
def upgrade() -> None:
op.create_table(
"version_changes",
sa.Column(
"id",
sa.BigInteger(),
primary_key=True,
autoincrement=True,
nullable=False,
),
sa.Column(
"transaction_id",
sa.BigInteger(),
sa.ForeignKey("version_transaction.id", ondelete="CASCADE"),
nullable=False,
),
sa.Column(
"entity_kind",
sa.String(length=32),
nullable=False,
),
sa.Column(
"entity_id",
sa.Integer(),
nullable=False,
),
sa.Column(
"sequence",
sa.SmallInteger(),
nullable=False,
),
sa.Column(
"kind",
sa.String(length=32),
nullable=False,
),
sa.Column("path", sa.JSON(), nullable=False),
sa.Column("from_value", sa.JSON(), nullable=True),
sa.Column("to_value", sa.JSON(), nullable=True),
sa.UniqueConstraint(
"transaction_id",
"entity_kind",
"entity_id",
"sequence",
name="uq_version_changes_tx_entity_sequence",
),
)
op.create_index(
"ix_version_changes_kind",
"version_changes",
["kind"],
)
op.create_index(
"ix_version_changes_transaction_id",
"version_changes",
["transaction_id"],
)
op.create_index(
"ix_version_changes_entity",
"version_changes",
["entity_kind", "entity_id"],
)
def downgrade() -> None:
op.drop_table("version_changes")

806
superset/versioning/changes.py Normal file
View File

@@ -0,0 +1,806 @@
# 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.
"""Capture listener for ``version_changes`` (T048).
Two session events cooperate:
- ``before_flush``: for each versioned entity in ``session.dirty``,
reads the pre-save scalar state from the DB via raw SQL inside
``session.no_autoflush`` (same idiom as the baseline listener, not
Continuum's internal ``units_of_work`` which is a private API), reads
the post-save state from the in-memory ORM object, calls the diff
engine, and buffers the resulting :class:`ChangeRecord` list on
``session.info``. This must run before the flush because after the
flush the DB already reflects the post-state; we can't recover the
pre-state from it.
- ``after_flush``: drains the buffer, resolves the current Continuum
transaction id via ``versioning_manager.units_of_work``, and bulk-
inserts one ``version_changes`` row per record with a monotonic
``sequence`` number. Records accumulated across multiple before_flush
calls within one transaction share the same ``transaction_id`` and
contiguous sequence numbers.
Scope in this iteration:
- Slice, Dashboard, SqlaTable **scalar fields** (via
:func:`scalar_fields_for` — new columns are picked up automatically
without editing this module).
- ``Slice.params`` kind-classification (filter / metric / time_range /
color_palette / dimension, plus generic ``field`` fallback).
Deferred to T048b:
- Dataset children (TableColumn / SqlMetric) — requires reading the
prior ``dataset_snapshots`` row for pre-state and the just-written
snapshot for post-state, which depends on listener ordering with
:func:`superset.versioning.dataset_snapshots.register_dataset_snapshot_listener`.
- Dashboard chart membership (``dashboard_slices``) — same pattern
against ``dashboard_snapshots``.
``session.new`` entities are not processed in this listener:
operation_type=0 transactions (baseline capture and first-save INSERTs)
produce zero change records per spec §Clarifications 2026-04-24.
"""
from __future__ import annotations
import logging
from datetime import date, datetime
from typing import Any, Optional
from uuid import UUID
import sqlalchemy as sa
from flask_appbuilder import Model
from sqlalchemy import event
from sqlalchemy.exc import OperationalError
from sqlalchemy.orm import Session
from superset.versioning.diff import (
ChangeRecord,
diff_dashboard,
diff_dashboard_slices,
diff_dataset,
diff_dataset_columns,
diff_dataset_metrics,
diff_slice,
fold_dashboard_layout_with_chart_changes,
scalar_fields_for,
)
logger = logging.getLogger(__name__)
# Declared against the shared Model.metadata so integration tests that
# build schema via ``metadata.create_all()`` pick it up without the
# Alembic migration running. Mirrors the shape of the T046 migration
# (``e1f3c5a7b9d0_add_version_changes_table``) byte-for-byte. Typed
# columns (``sa.JSON`` for path / values) are required so the
# connection's bulk-insert path marshals Python lists/dicts into JSON
# — a lightweight ``sa.table(...)`` would not carry the type info and
# SQLite's driver would reject the ``list`` as an unsupported bind.
_metadata = Model.metadata # pylint: disable=no-member
version_changes_table = sa.Table(
"version_changes",
_metadata,
sa.Column("id", sa.BigInteger, primary_key=True, autoincrement=True),
# ``transaction_id`` references ``version_transaction.id`` at the DB
# level only — the FK + ON DELETE CASCADE live in the Alembic
# migration. Declaring the FK here would fail to resolve at Table
# creation time because ``version_transaction`` is built
# dynamically by SQLAlchemy-Continuum at mapper-configuration time;
# integration tests that materialise schema via ``metadata.create_all``
# before Continuum runs would hit ``NoReferencedTableError``. Same
# pattern as the other versioning tables.
sa.Column("transaction_id", sa.BigInteger, nullable=False),
sa.Column("entity_kind", sa.String(32), nullable=False),
sa.Column("entity_id", sa.Integer, nullable=False),
sa.Column("sequence", sa.SmallInteger, nullable=False),
sa.Column("kind", sa.String(32), nullable=False),
sa.Column("path", sa.JSON, nullable=False),
sa.Column("from_value", sa.JSON, nullable=True),
sa.Column("to_value", sa.JSON, nullable=True),
sa.UniqueConstraint(
"transaction_id",
"entity_kind",
"entity_id",
"sequence",
name="uq_version_changes_tx_entity_sequence",
),
sa.Index("ix_version_changes_kind", "kind"),
sa.Index("ix_version_changes_transaction_id", "transaction_id"),
sa.Index("ix_version_changes_entity", "entity_kind", "entity_id"),
extend_existing=True,
)
# Mapping from Python class name to the ``entity_kind`` value written
# to ``version_changes.entity_kind``. The API filters change records
# by this value (``WHERE entity_kind = 'chart'`` for the chart history
# endpoint, etc.) — kept short and user-facing-ish so downstream tools
# consuming the raw table read sensibly.
_ENTITY_KIND_BY_CLASS_NAME: dict[str, str] = {
"Slice": "chart",
"Dashboard": "dashboard",
"SqlaTable": "dataset",
}
# Key under which the pending-records buffer is stored on ``session.info``.
# Using ``session.info`` (SQLAlchemy's user-data dict) avoids the need
# for a module-level WeakKeyDictionary and keeps buffers naturally scoped
# to the session's lifetime.
_BUFFER_KEY = "_version_changes_pending"
# Key for the set of Continuum transaction ids whose change records
# have already been written in this session. ``after_flush`` can fire
# more than once for a single transaction (e.g. autoflush triggered by
# a mid-commit query), and our child-diff path reads snapshot tables
# that don't care about the buffer state — without this marker we'd
# re-insert the same child records on the second flush and hit the
# UNIQUE(transaction_id, entity_kind, entity_id, sequence) constraint.
_PROCESSED_TXS_KEY = "_version_changes_processed_txs"
# Per-model-class cache of the scalar-field set. Populated lazily on
# first save of a model. Reading from ``__table__.columns`` is cheap
# but not free; memoising keeps the save-path overhead budget (FR-021)
# from slowly growing with the set of distinct model classes seen.
_SCALAR_FIELDS_CACHE: dict[type, frozenset[str]] = {}
def _cached_scalar_fields(model_cls: type) -> frozenset[str]:
"""Cached wrapper around :func:`scalar_fields_for`."""
if model_cls not in _SCALAR_FIELDS_CACHE:
# ``Slice.params`` is walked by ``diff_slice_params`` for kind
# promotion; emitting it as one opaque ``field`` change would
# defeat that and flood the log with meaningless records.
# ``last_saved_at`` / ``last_saved_by_fk`` are stamped by
# ``UpdateChartCommand`` on every chart save; they're audit
# noise (same shape as ``changed_on`` / ``changed_by_fk``) and
# don't carry user-authored signal.
# ``Dashboard.json_metadata`` and ``position_json`` are JSON
# blobs walked structurally by ``diff_json_field`` (one record
# per changed top-level key); the raw scalar diff would emit
# one giant multi-KB record per save and swamp the response.
special: frozenset[str] = frozenset()
audit: frozenset[str] = frozenset()
if model_cls.__name__ == "Slice":
special = frozenset({"params"})
audit = frozenset({"last_saved_at", "last_saved_by_fk"})
elif model_cls.__name__ == "Dashboard":
special = frozenset({"json_metadata", "position_json"})
_SCALAR_FIELDS_CACHE[model_cls] = scalar_fields_for(
model_cls, special=special, audit=audit
)
return _SCALAR_FIELDS_CACHE[model_cls]
def _jsonable(value: Any) -> Any:
"""Convert a column value into a JSON-serialisable form.
Mirrors the helper in :mod:`superset.versioning.dataset_snapshots`:
Slice has ``last_saved_at`` (datetime), datasets have datetime
columns, and any of these fields can land in ``from_value`` /
``to_value`` of a ``version_changes`` row, which is a JSON column.
Python's default JSON encoder rejects ``datetime`` / ``UUID`` /
``bytes``, so the whole bulk insert fails if a single record
carries one. Convert to ISO / hex / str at record-construction time.
"""
if isinstance(value, (datetime, date)):
return value.isoformat()
if isinstance(value, UUID):
return str(value)
if isinstance(value, bytes):
return value.hex()
return value
def _orm_to_post_state(obj: Any) -> dict[str, Any]:
"""Serialise an ORM object's column attributes to a plain dict.
We only read declared column attributes — not relationships or
hybrid properties — because the diff engine operates on scalar
values per its documented API. Values are passed through
:func:`_jsonable` so the dict is JSON-safe end-to-end.
"""
state = sa.inspect(obj)
return {
col.key: _jsonable(getattr(obj, col.key)) for col in state.mapper.column_attrs
}
def _read_pre_state(
session: Session, model_cls: type, entity_id: int
) -> dict[str, Any] | None:
"""Read the entity's pre-flush row directly from the DB.
Uses ``session.no_autoflush`` + a raw connection execute — the same
pattern as ``register_baseline_listener`` — to avoid a re-entrant
flush that would apply the pending edit before we've captured the
pre-state.
Returns ``None`` if the row is missing (shouldn't happen for a
dirty existing object, but defensive against race conditions).
"""
table = model_cls.__table__ # type: ignore[attr-defined]
with session.no_autoflush:
result = (
session.connection()
.execute(sa.select(table).where(table.c.id == entity_id))
.mappings()
.one_or_none()
)
if result is None:
return None
# Convert non-JSON-safe types (datetime, UUID, bytes) to strings so
# both sides of the diff compare on the same form and any value
# that ends up in ``from_value`` / ``to_value`` is acceptable to
# the JSON column on insert.
return {key: _jsonable(value) for key, value in dict(result).items()}
def _compute_records_for_entity(session: Session, obj: Any) -> list[ChangeRecord]:
"""Diff the pre-state (from DB) against the post-state (in memory).
Dispatches to :func:`diff_slice` / :func:`diff_dashboard` /
:func:`diff_dataset` based on the model class name — string-based
dispatch is used to keep this module free of hard imports on the
three entity classes, which in turn avoids import-order coupling
at app-init time.
"""
model_cls = type(obj)
entity_id = getattr(obj, "id", None)
if entity_id is None:
return []
try:
pre_state = _read_pre_state(session, model_cls, entity_id)
except Exception: # pylint: disable=broad-except
logger.exception(
"version_changes: pre-state read failed for %s id=%s",
model_cls.__name__,
entity_id,
)
return []
if pre_state is None:
return []
post_state = _orm_to_post_state(obj)
fields = _cached_scalar_fields(model_cls)
name = model_cls.__name__
if name == "Slice":
return diff_slice(pre_state, post_state, fields=fields)
if name == "Dashboard":
return diff_dashboard(pre_state, post_state, fields=fields)
if name == "SqlaTable":
return diff_dataset(pre_state, post_state, fields=fields)
return []
def _bulk_insert_records(
session: Session,
transaction_id: int,
buffered: dict[tuple[str, int], list[ChangeRecord]],
) -> None:
"""Insert ``version_changes`` rows for one transaction via raw SQL.
Uses the module-level :data:`version_changes_table` Table object
(which carries JSON column types, unlike ``sa.table(...)``) so the
connection marshals ``path`` / ``from_value`` / ``to_value`` Python
structures into JSON on insert. Skips the ORM flush round that
``session.bulk_insert_mappings`` would cost inside an already-
active flush.
``buffered`` is a dict keyed on ``(entity_kind, entity_id)`` so
records for one entity — scalars from ``before_flush`` plus
children collected in ``after_flush`` — merge naturally under the
same key. ``sequence`` resets per entity so each entity's records
form a self-contained replay sequence.
"""
if not buffered:
return
rows = []
for (entity_kind, entity_id), records in buffered.items():
for seq, r in enumerate(records):
rows.append(
{
"transaction_id": transaction_id,
"entity_kind": entity_kind,
"entity_id": entity_id,
"sequence": seq,
"kind": r.kind,
"path": r.path,
"from_value": r.from_value,
"to_value": r.to_value,
}
)
if rows:
session.connection().execute(version_changes_table.insert(), rows)
def _shadow_rows_valid_at(
session: Session,
shadow_table: sa.Table,
fk_col_name: str,
fk_value: int,
tx: int,
) -> list[dict[str, Any]]:
"""Return the live state of *shadow_table* rows whose FK column
(``fk_col_name``) equals *fk_value*, as of transaction *tx*.
Uses Continuum's validity-strategy semantics: a row is "valid at tx"
when ``transaction_id <= tx`` AND (``end_transaction_id`` IS NULL OR
``end_transaction_id`` > tx) AND it isn't a DELETE shadow.
The returned dicts mirror the live row's column set (no Continuum
bookkeeping columns), so they can be passed straight to the
natural-key diff helpers (``diff_dataset_columns`` etc.).
"""
fk_col = getattr(shadow_table.c, fk_col_name)
rows = (
session.connection()
.execute(
sa.select(shadow_table).where(
fk_col == fk_value,
shadow_table.c.transaction_id <= tx,
sa.or_(
shadow_table.c.end_transaction_id.is_(None),
shadow_table.c.end_transaction_id > tx,
),
shadow_table.c.operation_type != 2,
)
)
.mappings()
.all()
)
# Coerce values to JSON-safe forms — raw shadow rows can carry
# ``UUID``, ``datetime``, ``bytes`` etc. that don't survive the
# ``version_changes.from_value/to_value`` JSON column write.
meta_cols = {"transaction_id", "end_transaction_id", "operation_type"}
return [
{k: _jsonable(v) for k, v in dict(row).items() if k not in meta_cols}
for row in rows
]
def _affected_dataset_ids_at_tx(session: Session, tx: int) -> set[int]:
"""Datasets touched at *tx* — directly (parent shadow at tx) or
indirectly (column / metric shadow at tx)."""
# pylint: disable=import-outside-toplevel
from sqlalchemy_continuum import version_class
from superset.connectors.sqla.models import SqlaTable, SqlMetric, TableColumn
dataset_ids: set[int] = set()
parent_tbl = version_class(SqlaTable).__table__
for row in session.connection().execute(
sa.select(parent_tbl.c.id).where(parent_tbl.c.transaction_id == tx)
):
dataset_ids.add(row[0])
for child_cls in (TableColumn, SqlMetric):
child_tbl = version_class(child_cls).__table__
for row in session.connection().execute(
sa.select(child_tbl.c.table_id).where(child_tbl.c.transaction_id == tx)
):
if row[0] is not None:
dataset_ids.add(row[0])
return dataset_ids
def _dataset_child_records_for_tx_from_shadows(
session: Session, transaction_id: int
) -> dict[int, list[ChangeRecord]]:
"""Compute column + metric diff records for each dataset touched at
*transaction_id*, reading from Continuum shadow tables instead of
``dataset_snapshots``.
For each dataset:
* Post-state = rows valid at ``transaction_id`` in
``table_columns_version`` / ``sql_metrics_version``.
* Pre-state = rows valid at ``transaction_id - 1`` in the same
shadow tables.
With Continuum's validity-strategy semantics, "valid at tx N - 1"
is the state immediately before this transaction's effects (the
row that gets superseded at tx=N has ``end_transaction_id=N``, so
it satisfies ``end > N - 1``). Unrelated transactions between this
dataset's edits are transparent — they don't change validity for
this dataset's children.
First-edit case: when there is no prior tx (the dataset's earliest
shadow IS at *transaction_id*), pre-state is empty. We skip rather
than emit "Added X" for every column — same "baseline = zero
records" semantics as the snapshot path.
"""
# pylint: disable=import-outside-toplevel
from sqlalchemy_continuum import version_class
from superset.connectors.sqla.models import SqlMetric, TableColumn
cols_tbl = version_class(TableColumn).__table__
metrics_tbl = version_class(SqlMetric).__table__
result: dict[int, list[ChangeRecord]] = {}
for dataset_id in _affected_dataset_ids_at_tx(session, transaction_id):
# Skip the very first transaction for this dataset (no pre-state).
prior_tx = (
session.connection()
.execute(
sa.select(sa.func.max(cols_tbl.c.transaction_id)).where(
cols_tbl.c.table_id == dataset_id,
cols_tbl.c.transaction_id < transaction_id,
)
)
.scalar()
)
if prior_tx is None:
# No prior column shadow — could still be a metric-only edit;
# check metrics shadow too.
prior_tx = (
session.connection()
.execute(
sa.select(sa.func.max(metrics_tbl.c.transaction_id)).where(
metrics_tbl.c.table_id == dataset_id,
metrics_tbl.c.transaction_id < transaction_id,
)
)
.scalar()
)
if prior_tx is None:
continue
post_cols = _shadow_rows_valid_at(
session, cols_tbl, "table_id", dataset_id, transaction_id
)
pre_cols = _shadow_rows_valid_at(
session, cols_tbl, "table_id", dataset_id, prior_tx
)
post_metrics = _shadow_rows_valid_at(
session, metrics_tbl, "table_id", dataset_id, transaction_id
)
pre_metrics = _shadow_rows_valid_at(
session, metrics_tbl, "table_id", dataset_id, prior_tx
)
records: list[ChangeRecord] = []
records.extend(diff_dataset_columns(pre_cols, post_cols))
records.extend(diff_dataset_metrics(pre_metrics, post_metrics))
if records:
result[dataset_id] = records
return result
def _affected_dashboard_ids_at_tx(session: Session, tx: int) -> set[int]:
"""Dashboards touched at *tx* — directly (parent shadow at tx) or
indirectly (slice-membership shadow at tx)."""
# pylint: disable=import-outside-toplevel
from sqlalchemy_continuum import version_class
from superset.models.dashboard import Dashboard
dashboard_ids: set[int] = set()
parent_tbl = version_class(Dashboard).__table__
for row in session.connection().execute(
sa.select(parent_tbl.c.id).where(parent_tbl.c.transaction_id == tx)
):
dashboard_ids.add(row[0])
# M2M shadow: ``dashboard_slices_version`` is auto-generated by
# Continuum and lives in metadata — not a model class. Look it up
# from the metadata bag rather than via ``version_class``.
metadata = parent_tbl.metadata
if (m2m_tbl := metadata.tables.get("dashboard_slices_version")) is not None:
for row in session.connection().execute(
sa.select(m2m_tbl.c.dashboard_id).where(m2m_tbl.c.transaction_id == tx)
):
if row[0] is not None:
dashboard_ids.add(row[0])
return dashboard_ids
def _dashboard_slice_uuids_at_tx(
session: Session, dashboard_id: int, tx: int
) -> list[str]:
"""Slice UUIDs attached to *dashboard_id* as of *tx*, read by joining
``dashboard_slices_version`` (M2M membership) against
``slices_version`` (slice content).
Joining through both is necessary — and matches the same query
Continuum's M2M ``Reverter`` uses — because a slice that's
referenced by the M2M but has no slice-version row at this tx is
treated as "not yet versioned" and excluded.
Returns UUIDs (strings) so the result can be diffed by the existing
:func:`diff_dashboard_slices` helper, which keys on uuid.
"""
# pylint: disable=import-outside-toplevel
from sqlalchemy_continuum import version_class
from superset.models.slice import Slice
metadata = version_class(Slice).__table__.metadata
m2m_tbl = metadata.tables.get("dashboard_slices_version")
slices_tbl = version_class(Slice).__table__
if m2m_tbl is None:
return []
rows = (
session.connection()
.execute(
sa.select(slices_tbl.c.uuid).where(
slices_tbl.c.id == m2m_tbl.c.slice_id,
m2m_tbl.c.dashboard_id == dashboard_id,
m2m_tbl.c.transaction_id <= tx,
sa.or_(
m2m_tbl.c.end_transaction_id.is_(None),
m2m_tbl.c.end_transaction_id > tx,
),
m2m_tbl.c.operation_type != 2,
slices_tbl.c.transaction_id <= tx,
sa.or_(
slices_tbl.c.end_transaction_id.is_(None),
slices_tbl.c.end_transaction_id > tx,
),
slices_tbl.c.operation_type != 2,
)
)
.all()
)
return [str(r[0]) for r in rows if r[0] is not None]
def _dashboard_child_records_for_tx_from_shadows(
session: Session, transaction_id: int
) -> dict[int, list[ChangeRecord]]:
"""Compute slice-membership diff records for each dashboard touched
at *transaction_id*, reading from Continuum shadow tables instead
of ``dashboard_snapshots``.
Same pre/post logic as
:func:`_dataset_child_records_for_tx_from_shadows`.
"""
# pylint: disable=import-outside-toplevel
from sqlalchemy_continuum import version_class
from superset.models.dashboard import Dashboard
metadata = version_class(Dashboard).__table__.metadata
m2m_tbl = metadata.tables.get("dashboard_slices_version")
result: dict[int, list[ChangeRecord]] = {}
for dashboard_id in _affected_dashboard_ids_at_tx(session, transaction_id):
prior_tx = None
if m2m_tbl is not None:
prior_tx = (
session.connection()
.execute(
sa.select(sa.func.max(m2m_tbl.c.transaction_id)).where(
m2m_tbl.c.dashboard_id == dashboard_id,
m2m_tbl.c.transaction_id < transaction_id,
)
)
.scalar()
)
if prior_tx is None:
continue
post_uuids = _dashboard_slice_uuids_at_tx(session, dashboard_id, transaction_id)
pre_uuids = _dashboard_slice_uuids_at_tx(session, dashboard_id, prior_tx)
records = diff_dashboard_slices(pre_uuids, post_uuids)
if records:
result[dashboard_id] = records
return result
# Sentinel attribute set on the session target after first successful
# registration. Subsequent calls become no-ops. Storing the flag on the
# target itself (rather than module-level state) keeps the guard
# naturally scoped — a fresh session proxy gets a fresh registration —
# and avoids the TOCTOU race between ``event.contains`` and
# ``event.listen`` that a module-level ref would have under concurrent
# init. In test fixtures that instantiate multiple Superset apps per
# process, the shared ``db.session`` carries the sentinel and re-entry
# is correctly deduped.
_REGISTERED_SENTINEL = "_versioning_change_listener_registered"
def _process_dirty_entity_into_buffer(
session: Session,
obj: Any,
buffer: dict[tuple[str, int], list[ChangeRecord]],
) -> None:
"""Compute scalar change records for one dirty entity + append to buffer."""
entity_kind = _ENTITY_KIND_BY_CLASS_NAME.get(type(obj).__name__)
if entity_kind is None:
return
entity_id = getattr(obj, "id", None)
if entity_id is None:
return
try:
records = _compute_records_for_entity(session, obj)
except Exception: # pylint: disable=broad-except
logger.exception(
"version_changes: diff failed for %s id=%s",
type(obj).__name__,
entity_id,
)
return
if records:
buffer.setdefault((entity_kind, entity_id), []).extend(records)
def _append_child_records_to_buffer(
session: Session,
tx_id: int,
buffer: dict[tuple[str, int], list[ChangeRecord]],
) -> None:
"""Compute dataset + dashboard child-collection records + append to buffer.
Runs in ``after_flush`` so the shadow tables already have the
current-tx rows. Reads from Continuum shadow tables
(``table_columns_version`` / ``sql_metrics_version`` /
``dashboard_slices_version`` / ``slices_version``) — the
``dataset_snapshots`` and ``dashboard_snapshots`` JSON-blob path is
still populated by its listeners but no longer driving the diff.
"""
try:
for dataset_id, records in _dataset_child_records_for_tx_from_shadows(
session, tx_id
).items():
buffer.setdefault(("dataset", dataset_id), []).extend(records)
for dashboard_id, records in (
_dashboard_child_records_for_tx_from_shadows(session, tx_id)
).items():
buffer.setdefault(("dashboard", dashboard_id), []).extend(records)
# Post-merge fold: when a dashboard save adds/removes charts,
# drop the redundant ``position_json.*`` records that mirror
# the membership change. See
# ``diff.fold_dashboard_layout_with_chart_changes``.
for key in list(buffer.keys()):
if key[0] == "dashboard":
buffer[key] = fold_dashboard_layout_with_chart_changes(buffer[key])
if not buffer[key]:
del buffer[key]
except Exception: # pylint: disable=broad-except
logger.exception("version_changes: child-diff failed for tx %s", tx_id)
def _current_transaction_id(session: Session) -> Optional[int]:
"""Return the Continuum transaction id for *session*'s current unit of
work, or ``None`` when Continuum has no active transaction (e.g. raw
SQL execution outside the ORM's flush flow).
"""
# pylint: disable=import-outside-toplevel
from sqlalchemy_continuum import versioning_manager
uow = versioning_manager.units_of_work.get(session.connection())
if uow is None or uow.current_transaction is None:
return None
return uow.current_transaction.id
def _persist_buffered_records(
session: Session,
tx_id: int,
buffer: dict[tuple[str, int], list[ChangeRecord]],
) -> None:
"""Bulk-insert *buffer*'s records under *tx_id* and reset the buffer.
Catches ``OperationalError`` to handle the pre-migration startup race
(version_changes table missing), and ``Exception`` as the listener-
boundary safety net so a malformed record can't crash the user's save.
"""
try:
_bulk_insert_records(session, tx_id, buffer)
except OperationalError:
# version_changes table missing (migration not yet applied).
pass
except Exception: # pylint: disable=broad-except
logger.exception(
"version_changes: bulk insert failed for tx %s (%d entities)",
tx_id,
len(buffer),
)
def register_change_record_listener() -> None:
"""Attach the before_flush + after_flush listeners.
Registered from :class:`superset.initialization.SupersetAppInitializer`
(``init_versioning``) alongside the baseline, dataset-snapshot,
and dashboard-snapshot listeners. Must run after Continuum's
``make_versioned()`` so the ``versioning_manager`` is available
and has installed its own before_flush hook.
"""
# pylint: disable=import-outside-toplevel
from superset.connectors.sqla.models import SqlaTable
from superset.extensions import db
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
if getattr(db.session, _REGISTERED_SENTINEL, False):
return
versioned_classes: tuple[type, ...] = (Dashboard, Slice, SqlaTable)
def compute_change_records(
session: Session, _flush_context: Any, _instances: Any
) -> None:
# session.info persists across before_flush/after_flush within
# a single transaction. The buffer is keyed on
# ``(entity_kind, entity_id)`` so scalar records captured here
# and child records captured in after_flush (T048b) merge
# under the same entity without duplication.
buffer: dict[tuple[str, int], list[ChangeRecord]] = session.info.setdefault(
_BUFFER_KEY, {}
)
for obj in list(session.dirty):
if isinstance(obj, versioned_classes):
_process_dirty_entity_into_buffer(session, obj, buffer)
def flush_change_records(session: Session, _flush_context: Any) -> None:
buffer: dict[tuple[str, int], list[ChangeRecord]] = session.info.setdefault(
_BUFFER_KEY, {}
)
tx_id = _current_transaction_id(session)
if tx_id is None:
session.info[_BUFFER_KEY] = {}
return
# Skip if we've already written records for this tx (after_flush
# can fire more than once per commit — e.g. autoflush from a
# mid-commit query). Without this guard the child-diff path would
# re-read the same shadow rows and re-emit the same records,
# tripping the UNIQUE(transaction_id, entity_kind, entity_id,
# sequence) constraint on insert.
processed: set[int] = session.info.setdefault(_PROCESSED_TXS_KEY, set())
if tx_id in processed:
return
_append_child_records_to_buffer(session, tx_id, buffer)
if not buffer:
# Don't mark tx as processed when nothing was inserted. A
# later after_flush firing for the same tx may carry the
# records — e.g. when an entity's edit lands across two
# flushes (a child-only flush followed by a parent-dirty
# flush): the parent shadow only lands in the parent-dirty
# flush, so the child-diff path can't find a prior tx to
# compare against until then.
session.info[_BUFFER_KEY] = {}
return
try:
_persist_buffered_records(session, tx_id, buffer)
finally:
session.info[_BUFFER_KEY] = {}
processed.add(tx_id)
def reset_processed_after_commit(session: Session) -> None:
# ``_PROCESSED_TXS_KEY`` accumulates Continuum tx ids whose change
# records have already been written, to dedup against multiple
# ``after_flush`` firings within one transaction. After commit
# the tx is closed and its id will never recur on this session
# — drop the set so a long-lived session (Celery worker, CLI)
# doesn't grow it without bound.
session.info.pop(_PROCESSED_TXS_KEY, None)
event.listen(db.session, "before_flush", compute_change_records)
event.listen(db.session, "after_flush", flush_change_records)
event.listen(db.session, "after_commit", reset_processed_after_commit)
setattr(db.session, _REGISTERED_SENTINEL, True)

882
superset/versioning/diff.py Normal file
View File

@@ -0,0 +1,882 @@
# 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.
"""Diff engine for the ``version_changes`` table (FR-016..FR-019).
Hand-rolled because:
- The on-disk ``path`` shape (array of segments) is a direct
representation of our chosen format; external diff libraries
return string paths or JSON-Pointer forms that would need
translation.
- Kind classification (``filter`` vs ``metric`` vs ``field`` etc.)
is co-located with diff walking, avoiding a second classification
pass over the generic diff output.
- Child-collection identity uses natural keys (``column_name``,
``metric_name``, slice ``uuid``) — the same identity model ADR-004
settled on for ``dataset_snapshots``. External libraries default
to list-index matching, which is wrong for our data.
See ADR (plan.md §"Key Design Decision: Hand-rolled diff engine") for
the full rationale.
All functions in this module are pure: they take dicts (or lists of
dicts) and return a list of :class:`ChangeRecord`. The ORM->dict
conversion and Continuum transaction lookup happen in the capture
listener (T048), not here. This keeps the engine unit-testable without
an app context or DB.
"""
from __future__ import annotations
from collections.abc import Iterable
from dataclasses import dataclass
from typing import Any, Callable, Optional
from superset.utils import json as _json
# Columns that are always excluded from change records, regardless of
# what ``__versioned__`` says. ``id`` / ``uuid`` are stable identifiers
# (not edited in normal flows). The four audit fields change on every
# save — emitting records for them would double every history entry
# with meaningless "timestamp changed, user stamped" rows that the UI
# would have to filter out anyway.
_AUDIT_FIELDS: frozenset[str] = frozenset(
{
"id",
"uuid",
"created_on",
"changed_on",
"created_by_fk",
"changed_by_fk",
}
)
# Fields stripped from child-collection dict items (TableColumn,
# SqlMetric) before comparison and emission. ``changed_on`` /
# ``created_on`` / ``*_by_fk`` are audit fields that update on every
# save of the parent — without this filter, saving a dataset to add
# one column produces a record per existing column too (because their
# ``changed_on`` timestamps all refreshed). ``id`` and ``table_id``
# are implementation details — ``id`` can change under the
# ``override_columns`` delete-and-reinsert pattern (ADR-004) even
# when the column is semantically unchanged; ``table_id`` is the
# parent FK and never meaningfully differs within one dataset's
# history. ``uuid`` stays stable across normal saves and is kept so
# the renderer can use it for identity if it needs to.
_CHILD_ITEM_OPAQUE_FIELDS: frozenset[str] = frozenset(
{
"id",
"table_id",
"changed_on",
"created_on",
"changed_by_fk",
"created_by_fk",
}
)
def _strip_opaque_fields(item: Any) -> Any:
"""Return *item* with child-item audit/implementation fields removed.
Pass-through for non-dict values (scalars, strings) — the strip
only applies where it matters (dataset column / metric dicts).
"""
if not isinstance(item, dict):
return item
return {k: v for k, v in item.items() if k not in _CHILD_ITEM_OPAQUE_FIELDS}
# Chart ``params`` sub-keys that are promoted to first-class kinds.
# Every other params sub-key falls through to ``kind="field"``.
_CHART_PARAMS_KIND_BY_KEY: dict[str, str] = {
"adhoc_filters": "filter",
"time_range": "time_range",
"color_scheme": "color_palette",
"metrics": "metric",
"groupby": "dimension",
"columns": "dimension",
}
# Chart ``params`` sub-keys that are machine-stamped on save and don't
# carry user-authored signal — same category as ``last_saved_at`` on
# the scalar side. ``slice_id`` is a self-reference to the chart's
# own primary id; Superset's save paths add or refresh it on every
# save, producing a spurious "field" record on the first save after
# a chart's params were stored without it.
_CHART_PARAMS_AUDIT_KEYS: frozenset[str] = frozenset({"slice_id"})
def scalar_fields_for(
model_cls: Any,
*,
special: frozenset[str] = frozenset(),
audit: frozenset[str] = frozenset(),
) -> frozenset[str]:
"""Scalar columns on ``model_cls`` that should produce change records.
Derived from the model itself at call time so contributors (and
downstream derivatives) don't have to maintain a parallel whitelist
in this module. Adding a new column to ``Dashboard``, ``Slice``, or
``SqlaTable`` — whether upstream or in a fork — automatically flows
through to ``version_changes`` on the next save.
Excludes, in order:
1. The model's own ``__versioned__.exclude`` list, so change records
stay consistent with Continuum's shadow tables. If Continuum
isn't tracking a column, the change log shouldn't either.
2. :data:`_AUDIT_FIELDS` — ``id``, ``uuid``, and the audit
timestamps / user-id columns shared across the three entity types.
3. The caller's ``audit`` set — model-specific save-side-effect
columns that aren't user-authored content. ``Slice.last_saved_at``
/ ``last_saved_by_fk`` are stamped on every chart save by
``UpdateChartCommand``, similar to how ``changed_on`` is stamped
by the ORM event listener; emitting "field" records for them
would noise up the change log with one entry per save that
carries no user-meaningful signal.
4. The caller's ``special`` set — columns handled by a dedicated
differ elsewhere. ``Slice.params``, for example, is walked by
:func:`diff_slice_params` to produce first-class ``filter`` /
``time_range`` / ``metric`` / ``dimension`` records; emitting
it as a single opaque ``field`` would defeat that.
"""
try:
table = model_cls.__table__
except AttributeError:
return frozenset()
columns = frozenset(c.name for c in table.columns)
continuum_exclude = frozenset(
getattr(model_cls, "__versioned__", {}).get("exclude", []) or []
)
return columns - continuum_exclude - _AUDIT_FIELDS - audit - special
@dataclass(frozen=True)
class ChangeRecord:
"""One atomic change, as stored in ``version_changes``.
Fields match the ``version_changes`` columns one-to-one so the
capture listener can serialise a list of these to
``session.bulk_insert_mappings`` without translation.
"""
kind: str
path: list[Any]
from_value: Any
to_value: Any
Key = str | int
def _values_equivalent(from_value: Any, to_value: Any) -> bool:
"""True if a transition from ``from_value`` to ``to_value`` should
NOT produce a record.
Beyond plain ``==`` equality, treats ``None`` and ``""`` as equivalent:
Superset's save paths normalize nullable strings to ``""`` on first
write (e.g. ``Dashboard.css``, ``certified_by``,
``certification_details``), so a first-save transition between
null and empty string carries no user-authored signal.
"""
if from_value == to_value:
return True
if from_value in (None, "") and to_value in (None, ""):
return True
return False
def _diff_scalar(
field_name: str,
from_value: Any,
to_value: Any,
) -> ChangeRecord | None:
"""Emit a generic ``kind="field"`` record when a scalar differs."""
if _values_equivalent(from_value, to_value):
return None
return ChangeRecord(
kind="field",
path=[field_name],
from_value=from_value,
to_value=to_value,
)
def _diff_list_by_natural_key(
kind: str,
path_prefix: list[Any],
from_list: list[Any] | None,
to_list: list[Any] | None,
key_fn: Callable[[Any], Key | None],
) -> list[ChangeRecord]:
"""Diff two lists, matching elements by natural key.
Emits one record per add / remove / modify. When ``key_fn`` returns
``None`` for an item (natural key missing or empty), the item falls
back to its position as a synthetic key — so insertions in the
middle of a keyless list still produce sensible records, at the
cost of position-dependent identity.
"""
from_list = from_list or []
to_list = to_list or []
def _effective_key(raw: Key | None, idx: int) -> Key:
if raw is None or raw == "":
return idx
return raw
from_by_key: dict[Key, Any] = {}
for idx, item in enumerate(from_list):
from_by_key[_effective_key(key_fn(item), idx)] = item
to_by_key: dict[Key, Any] = {}
for idx, item in enumerate(to_list):
to_by_key[_effective_key(key_fn(item), idx)] = item
records: list[ChangeRecord] = []
# Preserve `from` order then append `to`-only keys, so sequence is
# deterministic across runs. For dict items (dataset columns /
# metrics) we strip audit/implementation fields before comparing
# AND before emitting — otherwise a save that only adds a new
# column would also emit "changed" records for every existing
# column, because their ``changed_on`` timestamps all refreshed.
# The stripped from/to are what the renderer sees; the per-column
# audit trail is already aggregated at the transaction level in
# ``version_transaction`` (``user_id`` + ``issued_at``).
for k, from_item in from_by_key.items():
to_item = to_by_key.get(k)
stripped_from = _strip_opaque_fields(from_item)
if to_item is None:
records.append(
ChangeRecord(
kind=kind,
path=[*path_prefix, k],
from_value=stripped_from,
to_value=None,
)
)
continue
stripped_to = _strip_opaque_fields(to_item)
if stripped_from != stripped_to:
records.append(
ChangeRecord(
kind=kind,
path=[*path_prefix, k],
from_value=stripped_from,
to_value=stripped_to,
)
)
for k, to_item in to_by_key.items():
if k not in from_by_key:
records.append(
ChangeRecord(
kind=kind,
path=[*path_prefix, k],
from_value=None,
to_value=_strip_opaque_fields(to_item),
)
)
return records
def _filter_key(f: Any) -> Key | None:
"""Natural key for an adhoc filter — its subject (column name).
Users rarely have two filters on the same column; when they do the
secondary dimensions (operator, comparator) appear in the record's
from/to values so the renderer can disambiguate.
"""
return f.get("subject") if isinstance(f, dict) else None
def _metric_key(m: Any) -> Key | None:
"""Natural key for a metric: prefer ``label``, fall back to column+aggregate."""
if not isinstance(m, dict):
return None
if label := m.get("label"):
return label
column = m.get("column")
col_name = column.get("column_name") if isinstance(column, dict) else None
agg = m.get("aggregate")
if col_name and agg:
return f"{agg}({col_name})"
return None
def _dimension_key(d: Any) -> Key | None:
"""Natural key for a groupby/columns element — usually a bare string."""
if isinstance(d, str):
return d
if isinstance(d, dict):
return d.get("label") or d.get("column_name")
return None
def _coerce_params(p: Any) -> dict[str, Any]:
"""Decode ``Slice.params`` which is stored as a JSON string."""
if p is None:
return {}
if isinstance(p, str):
try:
decoded = _json.loads(p)
except _json.JSONDecodeError:
return {}
return decoded if isinstance(decoded, dict) else {}
if isinstance(p, dict):
return p
return {}
def diff_slice_params(
from_params: Any,
to_params: Any,
) -> list[ChangeRecord]:
"""Diff the ``Slice.params`` JSON blob, promoting known keys to kinds."""
from_p = _coerce_params(from_params)
to_p = _coerce_params(to_params)
records: list[ChangeRecord] = []
all_keys = (set(from_p) | set(to_p)) - _CHART_PARAMS_AUDIT_KEYS
for key in sorted(all_keys):
from_v = from_p.get(key)
to_v = to_p.get(key)
if _values_equivalent(from_v, to_v):
continue
kind = _CHART_PARAMS_KIND_BY_KEY.get(key)
if kind == "filter" and isinstance(from_v, list) and isinstance(to_v, list):
records.extend(
_diff_list_by_natural_key(
"filter",
["params", "adhoc_filters"],
from_v,
to_v,
_filter_key,
)
)
elif kind == "metric" and isinstance(from_v, list) and isinstance(to_v, list):
records.extend(
_diff_list_by_natural_key(
"metric",
["params", "metrics"],
from_v,
to_v,
_metric_key,
)
)
elif (
kind == "dimension" and isinstance(from_v, list) and isinstance(to_v, list)
):
records.extend(
_diff_list_by_natural_key(
"dimension",
["params", key],
from_v,
to_v,
_dimension_key,
)
)
elif kind:
# scalar first-class kind (time_range, color_palette) —
# single record carrying the whole value
records.append(
ChangeRecord(
kind=kind,
path=["params", key],
from_value=from_v,
to_value=to_v,
)
)
else:
# unknown params sub-key: generic field change
records.append(
ChangeRecord(
kind="field",
path=["params", key],
from_value=from_v,
to_value=to_v,
)
)
return records
def diff_scalar_fields(
pre: dict[str, Any],
post: dict[str, Any],
*,
fields: Iterable[str],
) -> list[ChangeRecord]:
"""Emit one ``kind="field"`` record per differing field in ``fields``.
The ``fields`` iterable is supplied by the caller — typically
:func:`scalar_fields_for` at listener wiring time. Keeping the
field list outside this function means adding a new column to a
model does not require a matching edit here.
"""
records: list[ChangeRecord] = []
for field in sorted(fields):
record = _diff_scalar(field, pre.get(field), post.get(field))
if record is not None:
records.append(record)
return records
def diff_slice(
pre: dict[str, Any],
post: dict[str, Any],
*,
fields: Iterable[str],
) -> list[ChangeRecord]:
"""Full Slice (chart) diff — scalars plus params classification.
Pass ``fields=scalar_fields_for(Slice, special=frozenset({"params"}))``
to get the ``params``-excluded scalar set; ``Slice.params`` is diffed
separately by :func:`diff_slice_params` for kind promotion.
"""
records = diff_scalar_fields(pre, post, fields=fields)
records.extend(diff_slice_params(pre.get("params"), post.get("params")))
return records
def diff_json_field(
field_name: str,
from_value: Any,
to_value: Any,
*,
exclude_keys: frozenset[str] = frozenset(),
) -> list[ChangeRecord]:
"""Diff a TEXT column that stores a JSON dict, emitting one record
per top-level key whose value changed.
Used for ``Dashboard.json_metadata`` (``position_json`` has its
own structural diff via :func:`diff_dashboard_layout`). Saving the
blob verbatim into ``from_value`` / ``to_value`` would swamp the
change log with multi-KB strings on every save; walking the parsed
dict at the top level reduces noise to "what changed".
*exclude_keys* names sub-keys that are frontend-derived /
auto-stamped on save and don't carry user-authored signal. Same
rationale as the ``audit`` parameter on
:func:`scalar_fields_for` for the parent-column level.
Path is ``[field_name, key]``, mirroring ``diff_slice_params``'s
``["params", key]`` shape so renderers can use a single addressing
scheme across the chart and dashboard sides.
"""
from_p = _coerce_params(from_value)
to_p = _coerce_params(to_value)
records: list[ChangeRecord] = []
for key in sorted(set(from_p) | set(to_p)):
if key in exclude_keys:
continue
from_v = from_p.get(key)
to_v = to_p.get(key)
if _values_equivalent(from_v, to_v):
continue
records.append(
ChangeRecord(
kind="field",
path=[field_name, key],
from_value=from_v,
to_value=to_v,
)
)
return records
# json_metadata sub-keys that the frontend auto-stamps / auto-derives
# on save. They mirror dashboard membership and chart inventory, not
# user-authored content, so they noise up the change log without
# carrying intent. The records produced for these keys can be ~50KB
# (full label-colour dict) for a one-chart save.
#
# chart_configuration: per-chart cross-filter scope state,
# re-derived when charts are added/removed.
# global_chart_configuration: dashboard-wide filter scope; the
# ``chartsInScope`` list mirrors live
# dashboard membership.
# map_label_colors: label → colour map, re-stamped on save
# from currently-visible filter values.
# show_chart_timestamps: frontend toggle, defaults applied on
# save when missing.
# color_namespace: scoped colour-scheme namespace, frontend-
# derived from the chart set.
_DASHBOARD_JSON_METADATA_AUDIT_KEYS: frozenset[str] = frozenset(
{
"chart_configuration",
"global_chart_configuration",
"map_label_colors",
"show_chart_timestamps",
"color_namespace",
}
)
# Layout component types and how they map to record ``kind`` strings.
# ``HEADER_ID`` is excluded — that's the dashboard's title bar, mirrored
# from ``dashboard_title``. ``ROOT_ID`` and ``GRID_ID`` are structural
# singletons whose only deltas are children lists, which we infer from
# the moves of the children themselves.
_LAYOUT_TYPE_TO_KIND: dict[str, str] = {
"CHART": "chart",
"ROW": "row",
"COLUMN": "column",
"TAB": "tab",
"TABS": "tabs",
"HEADER": "header",
"MARKDOWN": "markdown",
"DIVIDER": "divider",
}
# Layout components we never emit records for: ROOT_ID is the layout
# root (always present, never moves); GRID_ID is the singleton vertical
# stack inside ROOT_ID; HEADER_ID is the dashboard's title bar (already
# covered by the ``dashboard_title`` scalar field).
_LAYOUT_SUPPRESSED_IDS: frozenset[str] = frozenset({"ROOT_ID", "GRID_ID", "HEADER_ID"})
def _layout_component_label(node: dict[str, Any]) -> str | None:
"""Extract a human-readable label from a layout node, when one
exists. Used to build the ``from_value`` / ``to_value`` payload so
the UI can render messages like "Added chart 'Foo'" without
needing to fetch related entities.
"""
meta = node.get("meta") or {}
if not isinstance(meta, dict):
return None
for key in ("sliceName", "label", "text"):
value = meta.get(key)
if isinstance(value, str) and value.strip():
return value
return None
def _layout_node_payload(node: dict[str, Any]) -> dict[str, Any]:
"""Minimal payload describing a layout node — enough for the UI
to render the change without dragging the full layout snippet
(which can be ~1KB per row when CHART nodes carry colour configs).
"""
meta = node.get("meta") or {}
if not isinstance(meta, dict):
meta = {}
payload: dict[str, Any] = {"id": node.get("id"), "type": node.get("type")}
if (label := _layout_component_label(node)) is not None:
payload["name"] = label
if (chart_id := meta.get("chartId")) is not None:
payload["chartId"] = chart_id
# ``uuid`` (slice uuid for CHART nodes) lets the M2M-vs-layout
# dedupe in :func:`fold_dashboard_layout_with_chart_changes`
# match on the same key — :func:`diff_dashboard_slices` keys its
# records by uuid, not chartId.
if (slice_uuid := meta.get("uuid")) is not None:
payload["uuid"] = slice_uuid
return payload
def _layout_parent_id(node: dict[str, Any]) -> Any:
"""The immediate-parent node id for a layout component — the last
entry in ``parents``. Used to detect moves: same id, different
parent."""
parents = node.get("parents") or []
if not isinstance(parents, list) or not parents:
return None
return parents[-1]
def _meta_excluding_position(node: dict[str, Any]) -> dict[str, Any]:
"""Meta dict with ``parents``-equivalent positional bits removed
so two nodes that differ ONLY in where they sit compare equal at
the meta level. Move detection uses ``parents`` directly; this is
for "edit" (meta change) detection."""
meta = node.get("meta") or {}
return dict(meta) if isinstance(meta, dict) else {}
def _diff_layout_node(
node_id: str,
pre_node: Optional[dict[str, Any]],
post_node: Optional[dict[str, Any]],
) -> Optional[ChangeRecord]:
"""Diff one component slot in the layout dict and return a record for
the logical action — add, remove, move, edit — or ``None`` when the
slot is unchanged or holds an unknown component type.
"""
node_for_kind = post_node or pre_node or {}
kind = _LAYOUT_TYPE_TO_KIND.get(node_for_kind.get("type") or "")
if kind is None:
return None # unknown component type — skip rather than emit garbage
if pre_node is None and post_node is not None:
return ChangeRecord(
kind=kind,
path=["add", kind, node_id],
from_value=None,
to_value=_layout_node_payload(post_node),
)
if post_node is None and pre_node is not None:
return ChangeRecord(
kind=kind,
path=["remove", kind, node_id],
from_value=_layout_node_payload(pre_node),
to_value=None,
)
# Both present — check move first, then edit.
assert pre_node is not None
assert post_node is not None
pre_parent = _layout_parent_id(pre_node)
if pre_parent != (post_parent := _layout_parent_id(post_node)):
return ChangeRecord(
kind=kind,
path=["move", kind, node_id],
from_value={**_layout_node_payload(pre_node), "parent": pre_parent},
to_value={**_layout_node_payload(post_node), "parent": post_parent},
)
pre_meta = _meta_excluding_position(pre_node)
if pre_meta != (post_meta := _meta_excluding_position(post_node)):
return ChangeRecord(
kind=kind,
path=["edit", kind, node_id],
from_value={**_layout_node_payload(pre_node), "meta": pre_meta},
to_value={**_layout_node_payload(post_node), "meta": post_meta},
)
return None
def diff_dashboard_layout(
pre: Any,
post: Any,
) -> list[ChangeRecord]:
"""Structural diff of a dashboard's ``position_json``, emitting one
record per logical layout action.
Walks both sides keyed on the component ``id`` (e.g.
``"CHART-mkPZLOnWCElgL0Udp1gVK"``):
* id present only in *post* → ``op=add``, ``from_value=None``,
``to_value=<minimal payload>``
* id present only in *pre* → ``op=remove``, payload swapped
* id in both, ``parents`` differs → ``op=move``, payloads carry
old + new parent
* id in both, parents equal, ``meta`` differs → ``op=edit``,
payloads carry old + new meta
* id in both, equal → no record
The ``operation_type``-style verb is encoded in
``path[0]`` as ``["add"|"remove"|"move"|"edit", <component-kind>,
<component-id>]`` so the UI's path-based renderer can read it
without inspecting from/to.
``ROOT_ID`` / ``GRID_ID`` / ``HEADER_ID`` are suppressed (see
:data:`_LAYOUT_SUPPRESSED_IDS`).
"""
pre_nodes = _layout_nodes(pre)
post_nodes = _layout_nodes(post)
records: list[ChangeRecord] = []
for node_id in sorted(set(pre_nodes) | set(post_nodes)):
record = _diff_layout_node(
node_id, pre_nodes.get(node_id), post_nodes.get(node_id)
)
if record is not None:
records.append(record)
return records
def _layout_nodes(raw: Any) -> dict[str, dict[str, Any]]:
"""Coerce *raw* (a ``position_json`` blob or already-parsed dict) into
the ``{node_id: node_dict}`` shape used by the layout diff, filtering
out non-dict values and the always-present root/grid/header singletons.
"""
parsed = _coerce_params(raw)
return {
k: v
for k, v in parsed.items()
if isinstance(v, dict) and k not in _LAYOUT_SUPPRESSED_IDS
}
def diff_dashboard(
pre: dict[str, Any],
post: dict[str, Any],
*,
fields: Iterable[str],
) -> list[ChangeRecord]:
"""Dashboard diff: scalar fields plus structural diff of
``json_metadata`` and ``position_json``.
Promoting ``position_json`` to ``kind="layout"`` or
``json_metadata.native_filter_configuration`` to ``kind="filter"``
is deferred to Phase 2 alongside the UI that would render them
(spec Clarifications §Session 2026-04-24); until then, both fields
fall through to ``kind="field"`` records keyed by sub-key.
"""
records = diff_scalar_fields(pre, post, fields=fields)
records.extend(
diff_json_field(
"json_metadata",
pre.get("json_metadata"),
post.get("json_metadata"),
exclude_keys=_DASHBOARD_JSON_METADATA_AUDIT_KEYS,
)
)
records.extend(
diff_dashboard_layout(pre.get("position_json"), post.get("position_json"))
)
return records
def _layout_chart_uuids_by_verb(
records: list[ChangeRecord],
) -> tuple[set[Any], set[Any]]:
"""Scan *records* for layout ``add``/``remove`` records on charts and
return ``(added_uuids, removed_uuids)`` sets.
"""
added: set[Any] = set()
removed: set[Any] = set()
for r in records:
if r.kind != "chart" or len(r.path) < 3:
continue
verb = r.path[0]
if verb == "add" and isinstance(r.to_value, dict):
uuid_ = r.to_value.get("uuid")
if uuid_ is not None:
added.add(uuid_)
elif verb == "remove" and isinstance(r.from_value, dict):
uuid_ = r.from_value.get("uuid")
if uuid_ is not None:
removed.add(uuid_)
return added, removed
def _is_redundant_m2m_chart_record(
r: ChangeRecord, added_uuids: set[Any], removed_uuids: set[Any]
) -> bool:
"""Return ``True`` when *r* is an M2M-style slice record that
duplicates an already-captured layout add/remove for the same uuid.
M2M slice records have path ``["slices", uuid]`` (length 2); their
info is strictly less than the corresponding layout record's
(no name, no parent), so the layout side wins on dedup.
"""
if r.kind != "chart" or len(r.path) != 2 or r.path[0] != "slices":
return False
slice_uuid = r.path[1]
if r.from_value is None and r.to_value is not None:
return slice_uuid in added_uuids
if r.to_value is None and r.from_value is not None:
return slice_uuid in removed_uuids
return False
def fold_dashboard_layout_with_chart_changes(
records: list[ChangeRecord],
) -> list[ChangeRecord]:
"""When a dashboard save adds/removes charts, the ``slices`` M2M
diff and the layout diff each emit a record for the same logical
action. Drop the M2M ``kind="chart"`` records — the layout-side
record carries more information (chart name, parent container).
The matching is by slice uuid: ``diff_dashboard_slices`` produces
records with path ``["slices", <slice-uuid>]``; the layout
payloads carry the same uuid (sourced from
``position_json.CHART-x.meta.uuid``). We dedupe on that key.
Called from the change-records listener after the M2M and layout
diffs are both merged into the per-entity buffer.
"""
added_uuids, removed_uuids = _layout_chart_uuids_by_verb(records)
return [
r
for r in records
if not _is_redundant_m2m_chart_record(r, added_uuids, removed_uuids)
]
def diff_dataset(
pre: dict[str, Any],
post: dict[str, Any],
*,
fields: Iterable[str],
) -> list[ChangeRecord]:
"""SqlaTable scalar-field diff. All paths emit ``kind="field"``.
Children (columns, metrics) are diffed separately via
:func:`diff_dataset_columns` / :func:`diff_dataset_metrics` because
the listener reads them via raw SQL (same pattern as
``dataset_snapshots``) rather than walking the ORM collection.
"""
return diff_scalar_fields(pre, post, fields=fields)
def diff_dataset_columns(
from_columns: list[dict[str, Any]] | None,
to_columns: list[dict[str, Any]] | None,
) -> list[ChangeRecord]:
"""Child-collection diff on TableColumn rows, keyed by column_name."""
return _diff_list_by_natural_key(
kind="column",
path_prefix=["columns"],
from_list=from_columns,
to_list=to_columns,
key_fn=lambda c: c.get("column_name") if isinstance(c, dict) else None,
)
def diff_dataset_metrics(
from_metrics: list[dict[str, Any]] | None,
to_metrics: list[dict[str, Any]] | None,
) -> list[ChangeRecord]:
"""Child-collection diff on SqlMetric rows, keyed by metric_name."""
return _diff_list_by_natural_key(
kind="metric",
path_prefix=["metrics"],
from_list=from_metrics,
to_list=to_metrics,
key_fn=lambda m: m.get("metric_name") if isinstance(m, dict) else None,
)
def diff_dashboard_slices(
from_slice_uuids: list[str] | None,
to_slice_uuids: list[str] | None,
) -> list[ChangeRecord]:
"""Diff a dashboard's chart membership, keyed by slice uuid.
Pure set-diff: added uuids get ``from_value=None, to_value=uuid``;
removed uuids get the inverse. No "changed" case because chart
associations are identity-only (the list element IS the uuid).
"""
from_set = set(from_slice_uuids or [])
to_set = set(to_slice_uuids or [])
records: list[ChangeRecord] = []
for uuid_ in sorted(from_set - to_set):
records.append(
ChangeRecord(
kind="chart",
path=["slices", uuid_],
from_value=uuid_,
to_value=None,
)
)
for uuid_ in sorted(to_set - from_set):
records.append(
ChangeRecord(
kind="chart",
path=["slices", uuid_],
from_value=None,
to_value=uuid_,
)
)
return records

442
tests/integration_tests/versioning/change_records_tests.py Normal file
View File

@@ -0,0 +1,442 @@
# 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.
"""Integration tests for ``version_changes`` capture (T052, partial).
Covers in this file:
(a) saving a chart with three field changes produces three rows
(f) baseline / INSERT transactions produce zero records *for that entity*
+ unchanged-save / dashboard / params-classification cases
Deferred:
(b) ``GET /versions/`` response includes ``changes`` array — lands with
T050 (API integration).
(c) FK cascade — exercisable in principle (the migration declares
``ON DELETE CASCADE``) but can't be isolated in a unit-style test
because ``version_transaction`` is referenced by non-cascading FKs
from slices_version / dashboards_version / etc. Covered instead
by (d) below once it lands, and by the structural declaration in
T046's migration.
(d) retention prune drops change records alongside the pruned
version — will land when T049 extends ``VersionDAO.prune_versions``
to include ``version_changes`` alongside the shadow-row delete.
(e) ``kind`` index query plan on Postgres — deferred to T053 perf
validation.
"""
from __future__ import annotations
from datetime import datetime, timedelta
from typing import Any
import pytest
import sqlalchemy as sa
from sqlalchemy_continuum import version_class
from superset.extensions import db
from superset.models.dashboard import Dashboard
from superset.models.slice import Slice
from superset.utils import json as _json
from tests.integration_tests.base_tests import SupersetTestCase
from tests.integration_tests.fixtures.birth_names_dashboard import ( # noqa: F401
load_birth_names_dashboard_with_slices,
load_birth_names_data,
)
_VERSION_CHANGES = sa.table(
"version_changes",
sa.column("id"),
sa.column("transaction_id"),
sa.column("entity_kind"),
sa.column("entity_id"),
sa.column("sequence"),
sa.column("kind"),
sa.column("path"),
sa.column("from_value"),
sa.column("to_value"),
)
def _change_rows_for(
tx_id: int,
*,
entity_kind: str | None = None,
entity_id: int | None = None,
) -> list[dict[str, Any]]:
"""Raw fetch of ``version_changes`` rows for a tx + optional entity filter."""
query = sa.select(_VERSION_CHANGES).where(
_VERSION_CHANGES.c.transaction_id == tx_id
)
if entity_kind is not None:
query = query.where(_VERSION_CHANGES.c.entity_kind == entity_kind)
if entity_id is not None:
query = query.where(_VERSION_CHANGES.c.entity_id == entity_id)
query = query.order_by(_VERSION_CHANGES.c.sequence.asc())
result = db.session.connection().execute(query)
return [dict(row._mapping) for row in result]
def _persist_fixture_state() -> None:
"""Commit fixture INSERTs so the baseline row exists before the test edits.
Without this, the test's first commit batches the fixture's pending
INSERTs with the test's UPDATE into a single Continuum transaction
and no diff records are emitted (no pre-state).
"""
db.session.commit()
class TestChartChangeRecords(SupersetTestCase):
"""Change-record capture for chart (Slice) saves."""
@pytest.fixture(autouse=True)
def _load_data(self, load_birth_names_dashboard_with_slices): # noqa: F811, PT004
pass
def test_single_scalar_edit_produces_one_change_record(self) -> None:
"""(a) — one field changed, one ``version_changes`` row."""
_persist_fixture_state()
chart = db.session.query(Slice).first()
assert chart is not None
chart.slice_name = f"{chart.slice_name[:64]}_renamed"
db.session.commit()
# The save produces one new version row (the UPDATE). Fetch its tx_id.
ver_cls = version_class(Slice)
update_tx_id = (
db.session.query(ver_cls.transaction_id)
.filter(ver_cls.id == chart.id)
.filter(ver_cls.operation_type == 1)
.order_by(ver_cls.transaction_id.desc())
.first()
.transaction_id
)
rows = _change_rows_for(update_tx_id, entity_kind="chart", entity_id=chart.id)
assert len(rows) == 1
assert rows[0]["kind"] == "field"
path = (
_json.loads(rows[0]["path"])
if isinstance(rows[0]["path"], str)
else rows[0]["path"]
)
assert path == ["slice_name"]
assert rows[0]["sequence"] == 0
def test_last_saved_at_is_excluded_as_audit_noise(self) -> None:
"""``last_saved_at`` / ``last_saved_by_fk`` are save-side-effect
fields stamped by ``UpdateChartCommand`` and must not produce
change records — same category as ``changed_on``.
Saving a chart with ONLY a ``last_saved_at`` bump must produce
zero ``version_changes`` rows for that transaction. (Continuum
still records the shadow row; we just don't want to noise up
the per-edit diff log.)
"""
_persist_fixture_state()
chart = db.session.query(Slice).first()
assert chart is not None
chart.last_saved_at = datetime.now() + timedelta(seconds=1)
db.session.commit()
ver_cls = version_class(Slice)
latest_tx = (
db.session.query(ver_cls.transaction_id)
.filter(ver_cls.id == chart.id)
.filter(ver_cls.operation_type == 1)
.order_by(ver_cls.transaction_id.desc())
.first()
)
# If the save produced no version row at all (no actual model
# change beyond the audit field), nothing to assert. If it did,
# there must be no ``last_saved_at`` row in version_changes.
if latest_tx is None:
return
rows = _change_rows_for(
latest_tx.transaction_id, entity_kind="chart", entity_id=chart.id
)
paths = [
_json.loads(r["path"]) if isinstance(r["path"], str) else r["path"]
for r in rows
]
assert ["last_saved_at"] not in paths
assert ["last_saved_by_fk"] not in paths
def test_three_scalar_edits_produce_three_records_in_sequence(self) -> None:
"""(a) — three fields changed, three rows, ``sequence`` 0..2."""
_persist_fixture_state()
chart = db.session.query(Slice).first()
assert chart is not None
# Derive from CURRENT values so every run guarantees a real
# change even against a persistent test DB where prior runs
# have already mutated the chart.
chart.slice_name = f"{chart.slice_name[:60]}_x"
chart.description = f"{chart.description or ''}_x"
chart.cache_timeout = (chart.cache_timeout or 0) + 1
db.session.commit()
ver_cls = version_class(Slice)
update_tx_id = (
db.session.query(ver_cls.transaction_id)
.filter(ver_cls.id == chart.id)
.filter(ver_cls.operation_type == 1)
.order_by(ver_cls.transaction_id.desc())
.first()
.transaction_id
)
rows = _change_rows_for(update_tx_id, entity_kind="chart", entity_id=chart.id)
assert len(rows) == 3
assert [r["sequence"] for r in rows] == [0, 1, 2]
# Sorted by field name (diff engine emits in sorted field order)
paths = [
_json.loads(r["path"]) if isinstance(r["path"], str) else r["path"]
for r in rows
]
assert paths == [["cache_timeout"], ["description"], ["slice_name"]]
def test_params_filter_add_produces_filter_kind_record(self) -> None:
"""(a) — params classification still flows through the listener.
Adds an adhoc_filter with a natural key (``subject``) derived
from the chart id so it's unique across test runs on a
persistent DB. Whatever was in ``adhoc_filters`` before stays;
we only want to confirm at least one ``kind='filter'`` record
is emitted.
"""
_persist_fixture_state()
chart = db.session.query(Slice).first()
assert chart is not None
unique_subject = (
f"col_{chart.id}_{db.session.connection().engine.url.database[-8:]}"
)
params = _json.loads(chart.params or "{}")
existing = params.get("adhoc_filters", []) or []
params["adhoc_filters"] = [
*existing,
{
"subject": unique_subject,
"operator": "==",
"comparator": "x",
"expressionType": "SIMPLE",
},
]
chart.params = _json.dumps(params)
db.session.commit()
ver_cls = version_class(Slice)
update_tx_id = (
db.session.query(ver_cls.transaction_id)
.filter(ver_cls.id == chart.id)
.filter(ver_cls.operation_type == 1)
.order_by(ver_cls.transaction_id.desc())
.first()
.transaction_id
)
rows = _change_rows_for(update_tx_id, entity_kind="chart", entity_id=chart.id)
filter_rows = [r for r in rows if r["kind"] == "filter"]
assert len(filter_rows) >= 1, (
f"expected at least one filter record, got rows: {rows}"
)
def test_unchanged_save_produces_zero_change_records(self) -> None:
"""An edit that sets fields to identical values emits nothing."""
_persist_fixture_state()
chart = db.session.query(Slice).first()
ver_cls = version_class(Slice)
# Capture the latest tx_id BEFORE this test's save so we can
# distinguish "the no-op save produced nothing new" (the intent)
# from "prior tests left tx rows with records on them" (noise).
pre_save_tx_row = (
db.session.query(ver_cls.transaction_id)
.filter(ver_cls.id == chart.id)
.filter(ver_cls.operation_type == 1)
.order_by(ver_cls.transaction_id.desc())
.first()
)
pre_save_tx_id = pre_save_tx_row.transaction_id if pre_save_tx_row else 0
# Touch the object (mark dirty) but assign the same value.
current_name = chart.slice_name
chart.slice_name = current_name
db.session.commit()
post_save_tx_row = (
db.session.query(ver_cls.transaction_id)
.filter(ver_cls.id == chart.id)
.filter(ver_cls.operation_type == 1)
.filter(ver_cls.transaction_id > pre_save_tx_id)
.order_by(ver_cls.transaction_id.desc())
.first()
)
# Either no new tx at all (nothing dirty, best case), or a new
# tx with zero change records for this chart.
if post_save_tx_row is not None:
assert (
_change_rows_for(
post_save_tx_row.transaction_id,
entity_kind="chart",
entity_id=chart.id,
)
== []
)
class TestDashboardChangeRecords(SupersetTestCase):
"""Same flow for dashboards — all scalar fields land in ``kind='field'``."""
@pytest.fixture(autouse=True)
def _load_data(self, load_birth_names_dashboard_with_slices): # noqa: F811, PT004
pass
def test_dashboard_title_edit_produces_field_record(self) -> None:
_persist_fixture_state()
dashboard = db.session.query(Dashboard).first()
assert dashboard is not None
dashboard.dashboard_title = f"{dashboard.dashboard_title}_rev"
db.session.commit()
ver_cls = version_class(Dashboard)
update_tx_id = (
db.session.query(ver_cls.transaction_id)
.filter(ver_cls.id == dashboard.id)
.filter(ver_cls.operation_type == 1)
.order_by(ver_cls.transaction_id.desc())
.first()
.transaction_id
)
rows = _change_rows_for(
update_tx_id, entity_kind="dashboard", entity_id=dashboard.id
)
assert len(rows) >= 1
field_rows = [r for r in rows if r["kind"] == "field"]
paths = [
_json.loads(r["path"]) if isinstance(r["path"], str) else r["path"]
for r in field_rows
]
assert ["dashboard_title"] in paths
class TestDatasetChildChangeRecords(SupersetTestCase):
"""T048b — column and metric diff records for dataset saves.
Two snapshots must exist for any child diff to emit: the prior
save's and the current one. The fixture ``load_birth_names_data``
has already created the dataset before these tests run; their
first commit produces snapshot #1. The test's edit produces
snapshot #2, and the listener diffs the two.
"""
@pytest.fixture(autouse=True)
def _load_data(self, load_birth_names_dashboard_with_slices): # noqa: F811, PT004
pass
def test_column_description_change_produces_column_record(self) -> None:
# pylint: disable=import-outside-toplevel
from sqlalchemy_continuum import version_class
from superset.connectors.sqla.models import SqlaTable
_persist_fixture_state()
dataset = (
db.session.query(SqlaTable)
.filter(SqlaTable.table_name == "birth_names")
.first()
)
assert dataset is not None
assert dataset.columns, "birth_names fixture should produce columns"
# First save establishes snapshot #1 (the pre-edit state).
# Scalar + child diffs won't emit anything yet because there's
# no prior snapshot to diff against.
dataset.description = f"{dataset.description or ''}_v1"
db.session.commit()
# Second save: edit a column AND touch a dataset scalar so
# the parent SqlaTable ends up in session.dirty. In real
# flows DatasetDAO.update_columns() marks the parent via its
# individual session.add / session.delete calls (T011); the
# direct-ORM test here needs an explicit parent touch.
column = dataset.columns[0]
column.description = f"{column.description or ''}_edited"
dataset.description = f"{dataset.description}_v2"
db.session.commit()
ver_cls = version_class(SqlaTable)
latest_tx_id = (
db.session.query(ver_cls.transaction_id)
.filter(ver_cls.id == dataset.id)
.filter(ver_cls.operation_type == 1)
.order_by(ver_cls.transaction_id.desc())
.first()
.transaction_id
)
rows = _change_rows_for(
latest_tx_id, entity_kind="dataset", entity_id=dataset.id
)
column_rows = [r for r in rows if r["kind"] == "column"]
assert len(column_rows) >= 1, (
f"expected at least one kind='column' record, got {rows}"
)
class TestBaselineProducesZeroChangeRecords(SupersetTestCase):
"""(f) — operation_type=0 (baseline / INSERT) transactions emit no records."""
@pytest.fixture(autouse=True)
def _load_data(self, load_birth_names_dashboard_with_slices): # noqa: F811, PT004
pass
def test_baseline_transaction_has_no_change_records_for_this_entity(
self,
) -> None:
"""(f) — baseline tx produces zero records *for that entity*.
A single transaction can touch multiple entities (fixture loads,
import pipelines). A tx that's a baseline for this chart might
still legitimately carry update records for some *other* entity
that shared the flush. The spec's M4 clarification means:
records filtered to this entity's (tx, entity_kind, entity_id)
are empty for its baseline tx.
"""
_persist_fixture_state()
chart = db.session.query(Slice).first()
chart.slice_name = f"{chart.slice_name[:64]}_force_baseline"
db.session.commit()
ver_cls = version_class(Slice)
rows_by_tx = (
db.session.query(ver_cls.transaction_id, ver_cls.operation_type)
.filter(ver_cls.id == chart.id)
.order_by(ver_cls.transaction_id.asc())
.all()
)
baseline_tx_ids = [tx for tx, op in rows_by_tx if op == 0]
assert baseline_tx_ids, "expected at least one baseline version row"
for tx_id in baseline_tx_ids:
records_for_this_chart = _change_rows_for(
tx_id, entity_kind="chart", entity_id=chart.id
)
assert records_for_this_chart == [], (
f"baseline tx {tx_id} unexpectedly has change records for "
f"chart id={chart.id}: {records_for_this_chart}"
)

272
tests/integration_tests/versioning/perf_validation_tests.py Normal file
View File

@@ -0,0 +1,272 @@
# 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.
"""T044 — Performance validation for entity version history.
Skipped by default. Run on demand:
SUPERSET_PERF_VALIDATION=1 pytest \
tests/integration_tests/versioning/perf_validation_tests.py -v -s
Measures the three success criteria defined in the spec:
* SC-002: version list endpoint responds in under 1 second
* SC-003: restore endpoint completes in under 3 seconds
* SC-004: save path p95 overhead under 50 ms with Continuum tracking
on vs. off (FR-014)
The test prints a summary table suitable for pasting into the PR
description. It also asserts each target so regressions fail loudly
when the harness is re-run.
"""
from __future__ import annotations
import os
import statistics
import time
from typing import Any
import pytest
import sqlalchemy as sa
from sqlalchemy_continuum import version_class, versioning_manager
from superset.extensions import db
from superset.models.slice import Slice
from tests.integration_tests.base_tests import SupersetTestCase
from tests.integration_tests.constants import ADMIN_USERNAME
from tests.integration_tests.fixtures.birth_names_dashboard import ( # noqa: F401
load_birth_names_dashboard_with_slices,
load_birth_names_data,
)
SKIP_REASON = "Performance validation is manual. Set SUPERSET_PERF_VALIDATION=1 to run."
# Thresholds from spec.md §Success Criteria.
LIST_ENDPOINT_MAX_MS = 1000 # SC-002
RESTORE_ENDPOINT_MAX_MS = 3000 # SC-003
SAVE_OVERHEAD_P95_MAX_MS = 50 # SC-004
def _save_chart_once(chart: Slice, suffix: str) -> None:
"""One ORM-level save path, mimicking what ChartDAO.update does."""
chart.slice_name = f"{chart.slice_name[:64]}_{suffix}"
db.session.commit()
def _timings_ms(seconds: list[float]) -> dict[str, float]:
ms = sorted(s * 1000.0 for s in seconds)
return {
"p50": statistics.median(ms),
"p95": ms[int(len(ms) * 0.95) - 1] if len(ms) >= 20 else max(ms),
"max": max(ms),
"n": len(ms),
}
@pytest.mark.skipif(
not os.environ.get("SUPERSET_PERF_VALIDATION"),
reason=SKIP_REASON,
)
class PerfValidationTests(SupersetTestCase):
"""Runs only when SUPERSET_PERF_VALIDATION=1 is set."""
@pytest.fixture(autouse=True)
def _load_data(self, load_birth_names_dashboard_with_slices: Any) -> None: # noqa: F811, PT004
pass
def _seed_chart_with_n_versions(self, n: int) -> Slice:
"""Save a chart N times to produce N version rows."""
chart = db.session.query(Slice).first()
assert chart is not None, "birth_names fixture should provide charts"
for i in range(n):
_save_chart_once(chart, f"v{i}")
db.session.commit()
return chart
def test_sc002_list_endpoint_under_1s(self) -> None:
"""SC-002: list endpoint responds in under 1 second."""
self.login(ADMIN_USERNAME)
# Generate enough versions to exercise the retention-capped state.
chart = self._seed_chart_with_n_versions(24)
chart_uuid = str(chart.uuid)
url = f"/api/v1/chart/{chart_uuid}/versions/"
# Warm up the endpoint once (JIT caching, mapper configuration, etc.)
self.client.get(url)
timings: list[float] = []
for _ in range(10):
t0 = time.perf_counter()
response = self.client.get(url)
timings.append(time.perf_counter() - t0)
assert response.status_code == 200
stats = _timings_ms(timings)
print(
f"\n[SC-002] GET /versions/ (24 versions) "
f"p50={stats['p50']:.1f}ms p95={stats['p95']:.1f}ms "
f"max={stats['max']:.1f}ms n={stats['n']}"
)
assert stats["p95"] < LIST_ENDPOINT_MAX_MS, (
f"SC-002 failed: list endpoint p95 {stats['p95']:.1f}ms "
f">= {LIST_ENDPOINT_MAX_MS}ms"
)
def test_sc003_restore_endpoint_under_3s(self) -> None:
"""SC-003: restore endpoint completes in under 3 seconds."""
self.login(ADMIN_USERNAME)
chart = self._seed_chart_with_n_versions(5)
chart_uuid = str(chart.uuid)
list_response = self.client.get(f"/api/v1/chart/{chart_uuid}/versions/")
assert list_response.status_code == 200
versions = list_response.get_json()["result"]
assert len(versions) >= 2, "need at least two versions to restore"
target_version_uuid = versions[-1]["version_uuid"]
restore_url = (
f"/api/v1/chart/{chart_uuid}/versions/{target_version_uuid}/restore"
)
# Warm up once
self.client.post(restore_url)
timings: list[float] = []
for _ in range(5):
t0 = time.perf_counter()
response = self.client.post(restore_url)
timings.append(time.perf_counter() - t0)
assert response.status_code == 200
stats = _timings_ms(timings)
print(
f"\n[SC-003] POST /restore chart "
f"p50={stats['p50']:.1f}ms max={stats['max']:.1f}ms n={stats['n']}"
)
assert stats["max"] < RESTORE_ENDPOINT_MAX_MS, (
f"SC-003 failed: restore max {stats['max']:.1f}ms "
f">= {RESTORE_ENDPOINT_MAX_MS}ms"
)
def test_sc004_save_overhead_under_50ms(self) -> None:
"""SC-004: save path p95 overhead under 50ms (FR-014).
Toggling Continuum on and off mid-process corrupts its internal
``units_of_work`` state and is not a reliable measurement. Instead
this test directly measures the wall-clock time spent inside the
four session-level listeners Continuum attaches to
``sa.orm.session.Session`` — ``before_flush``, ``after_flush``,
``after_commit``, ``after_rollback`` — plus Superset's own
baseline / snapshot / retention-prune listeners (attached to
``db.session``). The cumulative listener time per save is the
marginal overhead version capture adds over a save with
versioning removed entirely, because without these listeners
the ORM would not execute any of that code.
The approach:
1. Wrap each known listener with a timing proxy that adds its
wall-clock time to a per-save accumulator.
2. Save the same chart N times, recording each save's
accumulator value.
3. Compute p50 / p95 of the per-save overhead.
This matches the measurement intent of SC-004 (how much does
versioning cost per save) without the fragility of toggling
Continuum mid-test.
"""
self.login(ADMIN_USERNAME)
chart = db.session.query(Slice).first()
assert chart is not None
# Per-save accumulator incremented by the wrapped listeners.
acc = [0.0]
def wrap_listener(original: Any) -> Any:
def wrapper(*args: Any, **kwargs: Any) -> Any:
t0 = time.perf_counter()
try:
return original(*args, **kwargs)
finally:
acc[0] += time.perf_counter() - t0
wrapper.__wrapped__ = original # type: ignore[attr-defined]
return wrapper
# Instrument Continuum's four session listeners by detaching the
# bound method, wrapping, and re-attaching under a single-use
# listener handle we can cleanly remove on teardown.
session_target = sa.orm.session.Session
attached: list[tuple[str, Any]] = []
for event_name, listener in list(versioning_manager.session_listeners.items()):
sa.event.remove(session_target, event_name, listener)
wrapped = wrap_listener(listener)
sa.event.listen(session_target, event_name, wrapped)
attached.append((event_name, wrapped))
iterations = 100
warmup = 5
try:
# Warmup (first baseline INSERT, JIT, cache warming).
for i in range(warmup):
_save_chart_once(chart, f"warm_{i}")
acc[0] = 0.0
total_timings: list[float] = []
overhead_timings: list[float] = []
for i in range(iterations):
acc[0] = 0.0
t0 = time.perf_counter()
_save_chart_once(chart, f"run_{i}")
total_timings.append(time.perf_counter() - t0)
overhead_timings.append(acc[0])
finally:
for event_name, wrapped in attached:
sa.event.remove(session_target, event_name, wrapped)
sa.event.listen(
session_target,
event_name,
wrapped.__wrapped__,
)
total = _timings_ms(total_timings)
overhead = _timings_ms(overhead_timings)
ver_cls = version_class(Slice)
produced = db.session.query(ver_cls).filter(ver_cls.id == chart.id).count()
print(
f"\n[SC-004] save iterations={iterations} chart_id={chart.id} "
f"version_rows_produced={produced}"
)
print(
f"[SC-004] full save: "
f"p50={total['p50']:.2f}ms p95={total['p95']:.2f}ms "
f"max={total['max']:.2f}ms"
)
print(
f"[SC-004] version-cap overhead: "
f"p50={overhead['p50']:.2f}ms p95={overhead['p95']:.2f}ms "
f"max={overhead['max']:.2f}ms"
)
assert overhead["p95"] < SAVE_OVERHEAD_P95_MAX_MS, (
f"SC-004 failed: version-capture p95 overhead "
f"{overhead['p95']:.2f}ms >= {SAVE_OVERHEAD_P95_MAX_MS}ms"
)

1084
tests/unit_tests/versioning/test_diff.py Normal file
View File

File diff suppressed because it is too large Load Diff