From 4e4954822d1d4d451b5f71c4b5b4b3abed7aa3cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Schj=C3=B8lberg?= Date: Tue, 2 Dec 2025 15:43:25 +0100 Subject: [PATCH 1/4] Remove existingVersion from canvas download since this is needed for upload --- .../_cdf_tk/storageio/_applications.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cognite_toolkit/_cdf_tk/storageio/_applications.py b/cognite_toolkit/_cdf_tk/storageio/_applications.py index 7ee406422a..18ec1743a4 100644 --- a/cognite_toolkit/_cdf_tk/storageio/_applications.py +++ b/cognite_toolkit/_cdf_tk/storageio/_applications.py @@ -215,12 +215,27 @@ def _populate_id_cache(self, data_chunk: Sequence[IndustrialCanvas]) -> None: def _dump_resource(self, canvas: IndustrialCanvas) -> dict[str, JsonVal]: dumped = canvas.as_write().dump() + if isinstance(canvas_dict := dumped.get("canvas"), dict): + canvas_dict.pop("existingVersion", None) + if isinstance(annotations := dumped.get("annotations"), list): + for annotation in annotations: + if isinstance(annotation_dict := annotation, dict): + annotation_dict.pop("existingVersion", None) + if isinstance(fdm_instance_container_references := dumped.get("fdmInstanceContainerReferences"), list): + for fdm_instance_container_ref in fdm_instance_container_references: + if isinstance(fdm_instance_container_ref_dict := fdm_instance_container_ref, dict): + fdm_instance_container_ref_dict.pop("existingVersion", None) + if isinstance(solution_tags := dumped.get("solutionTags"), list): + for solution_tag in solution_tags: + if isinstance(solution_tag_dict := solution_tag, dict): + solution_tag_dict.pop("existingVersion", None) references = dumped.get("containerReferences", []) if not isinstance(references, list): return dumped for container_ref in references: if not isinstance(container_ref, dict): continue + container_ref.pop("existingVersion", None) sources = container_ref.get("sources", []) if not isinstance(sources, list) or len(sources) == 0: continue From da3f2dd70201c22df08c2f82f0c0a27fa4a0ac52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Schj=C3=B8lberg?= Date: Tue, 2 Dec 2025 15:43:51 +0100 Subject: [PATCH 2/4] Skip removing resourceId for charts and dataGrid containers --- cognite_toolkit/_cdf_tk/storageio/_applications.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cognite_toolkit/_cdf_tk/storageio/_applications.py b/cognite_toolkit/_cdf_tk/storageio/_applications.py index 18ec1743a4..b64b7afedb 100644 --- a/cognite_toolkit/_cdf_tk/storageio/_applications.py +++ b/cognite_toolkit/_cdf_tk/storageio/_applications.py @@ -245,10 +245,18 @@ def _dump_resource(self, canvas: IndustrialCanvas) -> dict[str, JsonVal]: properties = source["properties"] if not isinstance(properties, dict): continue + reference_type = properties.get("containerReferenceType") + if ( + reference_type + in { + "charts", + "dataGrid", + } + ): # These container reference types are special cases with a resourceId statically set to -1, which is why we skip them + continue resource_id = properties.pop("resourceId", None) if not isinstance(resource_id, int): continue - reference_type = properties.get("containerReferenceType") if reference_type == "asset": external_id = self.client.lookup.assets.external_id(resource_id) elif reference_type == "timeseries": From f66f2ae21c818dcd9b258917806c97c4affaf8ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Schj=C3=B8lberg?= Date: Tue, 2 Dec 2025 16:11:50 +0100 Subject: [PATCH 3/4] Simplify implementation and add testing covering new code paths --- .../_cdf_tk/storageio/_applications.py | 17 +----- tests/test_unit/conftest.py | 58 +++++++++++++++++++ .../test_storageio/test_applications.py | 11 +++- 3 files changed, 68 insertions(+), 18 deletions(-) diff --git a/cognite_toolkit/_cdf_tk/storageio/_applications.py b/cognite_toolkit/_cdf_tk/storageio/_applications.py index b64b7afedb..f5c31bf33b 100644 --- a/cognite_toolkit/_cdf_tk/storageio/_applications.py +++ b/cognite_toolkit/_cdf_tk/storageio/_applications.py @@ -214,28 +214,13 @@ def _populate_id_cache(self, data_chunk: Sequence[IndustrialCanvas]) -> None: self.client.lookup.files.external_id(list(file_ids)) def _dump_resource(self, canvas: IndustrialCanvas) -> dict[str, JsonVal]: - dumped = canvas.as_write().dump() - if isinstance(canvas_dict := dumped.get("canvas"), dict): - canvas_dict.pop("existingVersion", None) - if isinstance(annotations := dumped.get("annotations"), list): - for annotation in annotations: - if isinstance(annotation_dict := annotation, dict): - annotation_dict.pop("existingVersion", None) - if isinstance(fdm_instance_container_references := dumped.get("fdmInstanceContainerReferences"), list): - for fdm_instance_container_ref in fdm_instance_container_references: - if isinstance(fdm_instance_container_ref_dict := fdm_instance_container_ref, dict): - fdm_instance_container_ref_dict.pop("existingVersion", None) - if isinstance(solution_tags := dumped.get("solutionTags"), list): - for solution_tag in solution_tags: - if isinstance(solution_tag_dict := solution_tag, dict): - solution_tag_dict.pop("existingVersion", None) + dumped = canvas.as_write().dump(keep_existing_version=False) references = dumped.get("containerReferences", []) if not isinstance(references, list): return dumped for container_ref in references: if not isinstance(container_ref, dict): continue - container_ref.pop("existingVersion", None) sources = container_ref.get("sources", []) if not isinstance(sources, list) or len(sources) == 0: continue diff --git a/tests/test_unit/conftest.py b/tests/test_unit/conftest.py index a9b1499b5c..d56a0c0ed3 100644 --- a/tests/test_unit/conftest.py +++ b/tests/test_unit/conftest.py @@ -242,6 +242,7 @@ def asset_centric_canvas() -> tuple[IndustrialCanvas, NodeList[InstanceSource]]: }, "space": "IndustrialCanvasInstanceSpace", "version": 14, + "existingVersion": 14, } ], "containerReferences": [ @@ -270,6 +271,7 @@ def asset_centric_canvas() -> tuple[IndustrialCanvas, NodeList[InstanceSource]]: }, "space": "IndustrialCanvasInstanceSpace", "version": 1, + "existingVersion": 1, }, { "createdTime": 1751540275336, @@ -296,6 +298,7 @@ def asset_centric_canvas() -> tuple[IndustrialCanvas, NodeList[InstanceSource]]: }, "space": "IndustrialCanvasInstanceSpace", "version": 1, + "existingVersion": 1, }, { "createdTime": 1751540544349, @@ -322,6 +325,61 @@ def asset_centric_canvas() -> tuple[IndustrialCanvas, NodeList[InstanceSource]]: }, "space": "IndustrialCanvasInstanceSpace", "version": 4, + "existingVersion": 4, + }, + { + "createdTime": 1751540600000, + "externalId": "495af88f-fe1d-403d-91b1-76ef9f80f265_chart-ref-1", + "instanceType": "node", + "lastUpdatedTime": 1751540600000, + "properties": { + "cdf_industrial_canvas": { + "ContainerReference/v2": { + "chartsId": "my-chart-id", + "containerReferenceType": "charts", + "height": 400, + "id": "chart-ref-1", + "label": "My Chart", + "maxHeight": None, + "maxWidth": None, + "resourceId": -1, + "resourceSubId": None, + "width": 600, + "x": 100, + "y": 100, + } + } + }, + "space": "IndustrialCanvasInstanceSpace", + "version": 1, + "existingVersion": 1, + }, + { + "createdTime": 1751540700000, + "externalId": "495af88f-fe1d-403d-91b1-76ef9f80f265_datagrid-ref-1", + "instanceType": "node", + "lastUpdatedTime": 1751540700000, + "properties": { + "cdf_industrial_canvas": { + "ContainerReference/v2": { + "chartsId": None, + "containerReferenceType": "dataGrid", + "height": 300, + "id": "datagrid-ref-1", + "label": "My Data Grid", + "maxHeight": None, + "maxWidth": None, + "resourceId": -1, + "resourceSubId": None, + "width": 800, + "x": 200, + "y": 200, + } + } + }, + "space": "IndustrialCanvasInstanceSpace", + "version": 1, + "existingVersion": 1, }, ], } diff --git a/tests/test_unit/test_cdf_tk/test_storageio/test_applications.py b/tests/test_unit/test_cdf_tk/test_storageio/test_applications.py index a1441e8ae0..d1a79473de 100644 --- a/tests/test_unit/test_cdf_tk/test_storageio/test_applications.py +++ b/tests/test_unit/test_cdf_tk/test_storageio/test_applications.py @@ -146,7 +146,12 @@ def test_download_iterable( class TestCanvasIO: def test_download_iterable(self, asset_centric_canvas: tuple[IndustrialCanvas, NodeList[InstanceSource]]) -> None: canvas, _ = asset_centric_canvas - ids = [container_ref.resource_id for container_ref in canvas.container_references or []] + # Exclude charts/dataGrid types which have resourceId=-1 by design + ids = [ + container_ref.resource_id + for container_ref in canvas.container_references or [] + if container_ref.container_reference_type not in {"charts", "dataGrid"} + ] assert len(ids) > 0, "Test canvas must have container references for this test to be valid." mapping = {id_: f"external_id_{no}" for no, id_ in enumerate(ids)} reverse = {v: k for k, v in mapping.items()} @@ -186,8 +191,10 @@ def lookup(external_id: str | list[str]) -> int | list[int]: json_str = json.dumps(json_format) internal_id_in_json = [id_ for id_ in ids if str(id_) in json_str] assert len(internal_id_in_json) == 0, "Internal IDs should not be present in the JSON export." + assert "existingVersion" not in json_str, "existingVersion should not be present in the JSON export." restored_canvases = io.json_chunk_to_data([("line 1", item) for item in json_format]) assert len(restored_canvases) == 1 restored_canvas = restored_canvases[0] - assert restored_canvas.item.dump() == canvas.as_write().dump() + # The restored canvas should match the original without existingVersion fields + assert restored_canvas.item.dump() == canvas.as_write().dump(keep_existing_version=False) From 40f91af25ddcf78148551a2a0707d14b7374c5d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Magnus=20Schj=C3=B8lberg?= Date: Tue, 2 Dec 2025 16:50:35 +0100 Subject: [PATCH 4/4] Fix canvas migration tests --- .../test_commands/test_migration_cmd/test_migrate_canvas.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_unit/test_cdf_tk/test_commands/test_migration_cmd/test_migrate_canvas.py b/tests/test_unit/test_cdf_tk/test_commands/test_migration_cmd/test_migrate_canvas.py index 509e3b3418..e7b25849a5 100644 --- a/tests/test_unit/test_cdf_tk/test_commands/test_migration_cmd/test_migrate_canvas.py +++ b/tests/test_unit/test_cdf_tk/test_commands/test_migration_cmd/test_migrate_canvas.py @@ -47,7 +47,11 @@ def test_migrate_canvas_happy_path( backup = client.canvas.industrial.create.call_args[0][0] assert isinstance(backup, IndustrialCanvasApply) - assert len(update.fdm_instance_container_references) == len(canvas.container_references) + # Only asset-centric container references should be migrated (not charts/dataGrid) + migratable_refs = [ + ref for ref in canvas.container_references if ref.container_reference_type not in {"charts", "dataGrid"} + ] + assert len(update.fdm_instance_container_references) == len(migratable_refs) assert len(backup.fdm_instance_container_references) == 0 def test_migrate_canvas_missing(self) -> None: