mirror of
https://github.com/apache/superset.git
synced 2026-07-21 06:05:46 +00:00
d0520f67669c0bebc1fec23ed356cfb07fe03e35
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
a62d85d798 |
feat(versioning): force-parent-dirty on versioned-child change
SQLAlchemy doesn't mark a parent as dirty when only its children (``TableColumn`` / ``SqlMetric`` on ``SqlaTable``) are modified. Continuum's UnitOfWork only creates operations for entities in ``session.dirty``, so a column-only edit produces shadow rows in ``table_columns_version`` but no parent shadow row in ``tables_version``. ``VersionDAO.list_versions`` queries the parent shadow, so the version dropdown is empty for child-only saves — exactly the failure mode reported when "I edited a column description but no version appeared." Extends ``register_baseline_listener`` with a new before-flush hook ``_force_parent_dirty_on_child_change`` that walks the existing ``_child_to_parent_registry`` and ``attributes.flag_modified(parent, <first non-excluded versioned column>)`` whenever a versioned child is dirty / new / deleted but the parent's own scalars haven't been touched. The flag puts the parent in ``session.dirty`` so Continuum's UoW creates a parent UPDATE operation; the resulting shadow row's scalar columns mirror the previous version (only the children actually changed), and the row exists to anchor the transaction in the parent's version chain. ``SkipUnmodifiedPlugin._is_no_op_update`` is updated in this commit's predecessor to recognize the "scalars match but children dirty" case via ``_has_dirty_versioned_children`` so the forced parent UPDATE isn't skipped. Integration coverage: ``test_dataset_column_edit_creates_parent_version``. |
||
|
|
8a46573018 |
feat(versioning): SkipUnmodifiedPlugin audit-key normalize for Dashboard.json_metadata
Continuum's no-op suppression compared post-flush column values byte-for-byte against the previous live shadow row. For ``Dashboard.json_metadata`` that produced false-positive version rows on saves where the user authored nothing — the frontend re-stamps ``map_label_colors`` (regenerated from the ``LabelsColorMap`` singleton) on every save, plus ``chart_configuration`` / ``global_chart_configuration`` / ``show_chart_timestamps`` / ``color_namespace`` (derived from the current chart set), so two consecutive identical saves produce different bytes for the column. The diff engine already excluded those keys via ``DASHBOARD_JSON_METADATA_AUDIT_KEYS`` when computing change records; the skip-plugin diverged. Adds a ``_COLUMN_NORMALIZERS`` registry keyed on ``(class_name, column_name)`` that maps to a per-column normalizer applied to both pre- and post-image before equating. The first entry parses ``Dashboard.json_metadata`` as JSON and drops the audit-key set before comparing. The same registry is the extension point for analogous transient fields on charts and datasets. Promotes ``_DASHBOARD_JSON_METADATA_AUDIT_KEYS`` to a public name (``DASHBOARD_JSON_METADATA_AUDIT_KEYS``) so the skip-plugin can import it from ``superset.versioning.diff`` without reaching across a leading-underscore boundary. Integration coverage: ``test_map_label_colors_only_change_does_not_create_version``. |
||
|
|
7ce5f1d0e7 |
test(versioning): integration tests for SkipUnmodifiedPlugin (FR-026)
Locks in the no-op-suppression behavior implemented by ``SkipUnmodifiedPlugin`` (which lives in ``superset/versioning/factory.py`` shipping with the foundation commit). Five integration tests: 1. Owners-only edit doesn't mint a version row — exercises the case where every dirty column is an excluded relationship. 2. Re-save with identical scalar values doesn't mint a row — exercises the json_metadata re-serialise path where ``set_dash_metadata`` rewrites the column to a different byte sequence with identical parsed content; the plugin must compare post-flush values against the prior shadow row to detect this. 3. Real scalar change DOES mint a row — guards against the plugin over-suppressing. 4. Same assertion on a Slice (covers the ``String`` column path on a different entity type). 5. ``json_metadata`` sub-key edit DOES mint a row — covers the ``MediumText`` column path past the plugin's content-equality check. Tests are designed so a column-type change in the parent entities (e.g. flipping ``json_metadata`` from ``MediumText`` to ``JSON``) will fail one of these if the plugin's Python ``!=`` comparison breaks for the new type. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
9bc95ef819 |
feat(versioning): ETag helper module + integration tests (T055)
Helper module that derives the strong-validator ``ETag`` value from an entity's current live ``version_uuid`` and attaches it to a Flask response. Two functions: - ``set_version_etag(response, version_uuid)`` — direct path used by PUT handlers that already compute ``new_version_uuid`` (see the REST API commit two prior). Cheap; no extra query. - ``set_version_etag_by_uuid(response, model_cls, entity_uuid)`` — used by version endpoints that operate on ``entity_uuid``; looks up ``entity_id`` then derives ``version_uuid`` via ``VersionDAO``. Costs one extra ``SELECT id WHERE uuid = ?``; documented in the docstring so callers prefer the cheap variant when they have the id already. Integration tests cover all three entity types and four endpoint shapes (entity GET, save PUT, version-list GET, single-version GET) plus the entity-with-no-versions edge case (header is correctly absent). The ETag is wired into the API endpoints in the REST-API commit (group 3) and the CORS ``expose_headers: ["ETag"]`` ships with the retention commit (group 4) since both touch ``superset/config.py``. Locking enforcement (``If-Match`` → 412) is explicitly NOT in this change — deferred to the follow-up UI SIP per Open Question §7. ``ETag`` is informational in v1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
|
f7d73e2e1b |
feat(versioning): change records + diff engine
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>
|
||
|
|
be01e4552c |
feat(versioning): foundation — Continuum capture + parent/child shadow tables + VersionDAO
Adds SQLAlchemy-Continuum as a dependency and wires it as the canonical capture mechanism for chart, dashboard, and dataset edits. **Schema** — three Alembic migrations, leaving the chain at one foundation revision plus one child-shadow revision: - ``version_transaction`` (renamed from Continuum's default ``transaction``; SQL-reserved-word workaround) carries the per-save ``user_id`` / ``issued_at`` and is the join target for all shadow rows. Auto-incrementing PK; user_id has no FK so import / Celery / CLI saves can write rows without an active Flask user. - Parent shadow tables for the three entity types: ``dashboards_version``, ``slices_version``, ``tables_version``. - Child shadow tables for dataset children + dashboard M2M: ``table_columns_version``, ``sql_metrics_version``, ``dashboard_slices_version`` (composite PK on the M2M shadow, matching the live ``dashboard_slices`` reshape from sc-105349-composite-association-pks). **Models** — ``Dashboard``, ``Slice``, ``SqlaTable`` (and dataset children ``TableColumn`` / ``SqlMetric``) gain ``__versioned__`` class attributes. The exclude lists carry both M2M relationships (``owners``, ``roles``, ``dashboards``) and the ``AuditMixin`` columns (``changed_on`` / ``created_on`` / ``changed_by_fk`` / ``created_by_fk`` plus ``last_saved_at`` / ``last_saved_by_fk`` on ``Slice``) so auto-bumped audit fields cannot trigger a version row on their own (FR-025). **Plugins** — ``superset/versioning/factory.py`` ships three Continuum plugins: - ``VersionTransactionFactory`` renames the transaction table and appends the unconditional ``user_id`` column. - ``VersioningFlaskPlugin`` sources the acting user from Superset's ``g.user`` rather than ``flask_login.current_user`` (Superset's JWT auth populates ``g.user`` but leaves ``current_user`` anonymous on API routes). - ``SkipUnmodifiedPlugin`` filters Continuum's UPDATE operations, marking content-equivalent re-saves as ``processed=True`` so they don't mint no-op shadow rows (FR-026; see follow-up commits for the test). Lives in this commit because it shares the file with the other plugins. **Save-path glue** — a ``before_flush`` baseline listener (``superset/versioning/baseline.py``) inserts an ``operation_type=0`` shadow row the first time a pre-existing entity is saved, including the slice-baseline-under-dashboard pattern that gives the dashboard M2M shadow a row to join against. ``UpdateDashboardCommand`` wraps its body in ``no_autoflush`` so ``process_tab_diff`` / ``process_native_filter_diff`` don't fire intermediate flushes that would mint extra version rows. ``DatasetDAO.update_columns`` is rewritten as a natural-key upsert keyed on ``column_name`` so child edits flow through ORM events Continuum sees. **DAO** — ``superset/daos/version.py`` exposes the read API used by the version endpoints in the next commits: ``current_version_number`` (0-based index, unstable under retention pruning), ``current_live_transaction_id`` (stable across pruning), ``current_live_version_uuid`` (deterministic UUIDv5), plus ``list_versions`` / ``get_version`` / ``restore_version`` and a batch ``list_change_records_batch`` for N+1 avoidance. **Initialization** — ``superset/initialization/__init__.py`` wires ``init_versioning()`` after ``make_versioned()`` runs and the versioned mappers are configured. Registers the baseline listener plus the change-record listener (the latter's body lives in the next commit but the registration site is here because it shares the init function). **Tests** — version-capture and version-list integration tests for each entity type, plus a ``VersionDAO`` unit test suite. Retention test uses a backdated ``issued_at`` so it can drive ``_prune_old_versions_impl`` synchronously. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |