rusackas commented on code in PR #40588:
URL: https://github.com/apache/superset/pull/40588#discussion_r3425005505
##########
superset/commands/dashboard/export.py:
##########
@@ -105,6 +106,166 @@ def append_charts(position: dict[str, Any], charts:
set[Slice]) -> dict[str, Any
return position
+# Bound the derived id to a signed 31-bit positive integer so it stays well
+# within the range a database auto-increment primary key would occupy and
+# never collides with the sign bit. The concrete value is irrelevant — only
+# its stability across environments matters.
+_STABLE_CHART_ID_MODULO = 2_147_483_647
+
+
+def stable_chart_id(chart_uuid: str) -> int:
+ """Derive a deterministic, environment-independent integer from a chart
UUID.
+
+ The dashboard export format historically embedded ``meta.chartId`` — the
+ source environment's auto-increment primary key — inside every ``CHART-*``
+ position node (issue #32972). That integer differs between environments, so
+ re-exporting an imported dashboard produced a different bundle for the same
+ logical content. Deriving the id from the (stable) UUID instead makes the
+ export reproducible while still giving the importer an integer it can use
to
+ rewire the legacy, integer-keyed metadata references back to local IDs.
+ """
+ return (uuid_module.UUID(chart_uuid).int % _STABLE_CHART_ID_MODULO) + 1
+
+
+def _stabilize_chart_ids(payload: dict[str, Any]) -> None:
+ """Replace env-local integer chart IDs in ``payload`` with UUID-derived
ones.
+
+ Rewrites ``meta.chartId`` in every ``CHART`` position node to a value
derived
+ from ``meta.uuid`` and remaps the legacy, integer-keyed metadata references
+ (filter scopes, default filters, expanded slices, native filter scopes, and
+ cross-filter/chart configuration) accordingly so the bundle stays
internally
+ consistent and the import-side id remap resolves against the same IDs.
+ Mappings are applied defensively — a reference whose source id is unknown
is
+ dropped rather than raising, and a node with a malformed ``meta.uuid`` is
+ skipped — so a partially corrupt position never aborts the export. See
+ ``stable_chart_id`` and issue #32972 for the motivation.
+ """
+ position = payload.get("position")
+ if not isinstance(position, dict):
+ return
+
+ # Map each chart's env-local integer id -> stable UUID-derived id.
+ id_map: dict[int, int] = {}
+ for node in position.values():
+ if (
+ isinstance(node, dict)
+ and node.get("type") == "CHART"
+ and isinstance(node.get("meta"), dict)
+ ):
+ meta = node["meta"]
+ chart_uuid = meta.get("uuid")
+ old_id = meta.get("chartId")
+ if chart_uuid is None:
+ continue
+ try:
+ new_id = stable_chart_id(str(chart_uuid))
+ except ValueError:
+ # A malformed ``meta.uuid`` (corrupt position_json) must not
+ # abort the whole export — skip stabilizing this single node
+ # and leave its existing chartId untouched.
+ logger.warning(
+ "Skipping chart id stabilization for invalid uuid %r",
+ chart_uuid,
+ )
+ continue
+ if isinstance(old_id, int):
+ id_map[old_id] = new_id
+ meta["chartId"] = new_id
+
+ if not id_map:
+ return
+
+ metadata = payload.get("metadata")
+ if not isinstance(metadata, dict):
+ return
+
+ def remap_id(old_id: Any) -> Optional[int]:
+ try:
+ return id_map.get(int(old_id))
+ except (TypeError, ValueError):
+ return None
+
+ def remap_ids(old_ids: Any) -> list[int]:
+ return [
+ new_id for old_id in old_ids if (new_id := remap_id(old_id)) is
not None
+ ]
+
+ if isinstance(metadata.get("timed_refresh_immune_slices"), list):
+ metadata["timed_refresh_immune_slices"] = remap_ids(
+ metadata["timed_refresh_immune_slices"]
+ )
+
+ if isinstance(metadata.get("filter_scopes"), dict):
+ metadata["filter_scopes"] = {
+ str(new_key): columns
+ for old_key, columns in metadata["filter_scopes"].items()
+ if (new_key := remap_id(old_key)) is not None
+ }
+ for columns in metadata["filter_scopes"].values():
+ if not isinstance(columns, dict):
+ continue
+ for attributes in columns.values():
+ if isinstance(attributes, dict) and isinstance(
+ attributes.get("immune"), list
+ ):
+ attributes["immune"] = remap_ids(attributes["immune"])
+
+ if isinstance(metadata.get("expanded_slices"), dict):
+ metadata["expanded_slices"] = {
+ str(new_key): value
+ for old_key, value in metadata["expanded_slices"].items()
+ if (new_key := remap_id(old_key)) is not None
+ }
+
+ if metadata.get("default_filters"):
+ try:
+ default_filters = json.loads(metadata["default_filters"])
+ except (TypeError, json.JSONDecodeError):
+ default_filters = None
+ if isinstance(default_filters, dict):
+ metadata["default_filters"] = json.dumps(
+ {
+ str(new_key): value
+ for old_key, value in default_filters.items()
+ if (new_key := remap_id(old_key)) is not None
+ }
+ )
Review Comment:
`default_filters` remapping (the JSON round-trip included) is already
covered by `test_stabilize_chart_ids_remaps_default_filters`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
