EnxDev commented on code in PR #43232:
URL: https://github.com/apache/superset/pull/43232#discussion_r4002971623
##########
superset/commands/chart/importers/v1/utils.py:
##########
@@ -271,3 +314,147 @@ def migrate_chart(config: dict[str, Any]) -> dict[str,
Any]:
output["query_context"] = json.dumps(query_context)
return output
+
+
+def topological_sort_charts(
+ chart_configs: list[dict[str, Any]],
+) -> list[dict[str, Any]]:
+ """Sort charts so that annotation dependencies are imported first.
+
+ Handles multi-level dependencies (A→B→C) by iteratively resolving
+ charts whose in-batch dependencies are already satisfied.
+
+ TODO: Add runtime circular annotation detection in
+ QueryContextProcessor.get_viz_annotation_data to prevent infinite
+ recursion when rendering charts with circular line annotations.
+ """
+ if len(chart_configs) <= 1:
+ return chart_configs
+
+ def _annotation_dependencies(chart_config: dict[str, Any]) -> set[str]:
+ refs = {
+ ann["value"]
+ for ann in chart_config.get("params", {}).get("annotation_layers",
[])
+ if ann.get("sourceType") in
ANNOTATION_SOURCE_TYPES_WITH_CHART_REFERENCE
+ and isinstance(ann.get("value"), str)
+ }
+ if query_context_raw := chart_config.get("query_context"):
+ try:
+ query_context = json.loads(query_context_raw)
+ except (json.JSONDecodeError, TypeError):
+ query_context = {}
+
+ for query in query_context.get("queries", []):
+ refs.update(
+ ann["value"]
+ for ann in query.get("annotation_layers", [])
+ if ann.get("sourceType")
+ in ANNOTATION_SOURCE_TYPES_WITH_CHART_REFERENCE
+ and isinstance(ann.get("value"), str)
+ )
+ refs.update(
+ ann["value"]
+ for ann in query_context.get("form_data", {}).get(
+ "annotation_layers", []
+ )
+ if ann.get("sourceType") in
ANNOTATION_SOURCE_TYPES_WITH_CHART_REFERENCE
+ and isinstance(ann.get("value"), str)
+ )
+ return refs
+
+ batch_uuids = {c["uuid"] for c in chart_configs}
+ sorted_refs: list[dict[str, Any]] = []
+ remaining = list(chart_configs)
+ resolved: set[str] = set()
+ while remaining:
+ next_remaining = []
+ for c in remaining:
+ unmet = _annotation_dependencies(c).intersection(batch_uuids -
resolved)
+ if not unmet:
+ sorted_refs.append(c)
+ resolved.add(c["uuid"])
+ else:
+ next_remaining.append(c)
+ if len(next_remaining) == len(remaining):
+ logger.warning(
+ "Circular annotation dependency detected for charts: %s — "
+ "these charts may have unresolved annotation references after
import.",
+ [c["uuid"] for c in next_remaining],
+ )
+ sorted_refs.extend(next_remaining)
Review Comment:
This fallback still loses one side of a cycle. With A→B and B→A, A is
imported first while `chart_ids` is empty, so `_resolve_annotation_list` drops
its reference to B; B can then resolve A. Since export includes both charts,
the round-trip becomes asymmetric and data-losing. Could we either reject
circular bundles before any writes, or create all charts first and resolve
references in a second pass? A two-chart cycle test would capture the behavior.
##########
superset/commands/chart/importers/v1/__init__.py:
##########
@@ -89,10 +133,21 @@ def _import(
dataset = import_dataset(config, overwrite=False)
datasets[str(dataset.uuid)] = dataset
+ # import annotation layers before charts so UUID→ID maps are ready
+ annotation_layer_ids: dict[str, int] = {}
+ for file_name, config in configs.items():
+ if file_name.startswith("annotation_layers/"):
+ layer = import_annotation_layer(config, overwrite=overwrite)
Review Comment:
Could we enforce the Annotation `can_write` permission before importing
bundled layers? This endpoint is authorized as `Chart`, yet `overwrite=true`
reaches a direct UUID lookup and can mutate an existing `AnnotationLayer`.
Under `SECURITY.md`’s **Write objects** row, a custom role granted chart
import/write but not annotation write should not gain annotation-layer write
access through a chart bundle. A focused permission test for that role would
make the boundary explicit.
--
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]
