fix: Handle errors during content_bytes

capella2polarion/connectors/polarion_worker.py

@@ -14,7 +14,7 @@
 from capellambse import helpers as chelpers
 from lxml import etree
 
-from capella2polarion import data_model
+from capella2polarion import data_model, errors
 from capella2polarion.connectors import polarion_repo
 from capella2polarion.elements import data_session
 
@@ -35,18 +35,6 @@
     "rel1.FK_URI_MODULE = doc.C_URI AND rel1.FK_URI_WORKITEM = item.C_URI))"
 )
 """An SQL query to get work items which are inserted in a given document."""
-RENDER_ERROR_CHECKSUM = "__RENDER_ERROR__"
-"""Marker used as checksum when diagram rendering fails."""
-ERROR_IMAGE = b"""<svg xmlns="http://www.w3.org/2000/svg" width="400" height="200">
-  <rect width="400" height="200" fill="#d32f2f"/>
-  <text x="200" y="90" text-anchor="middle" fill="white" font-size="24" font-weight="bold">
-    Capella2Polarion: Diagram Failed to Render
-  </text>
-  <text x="200" y="130" text-anchor="middle" fill="white" font-size="18">
-    Please contact support for assistance
-  </text>
-</svg>"""
-"""Static SVG image to use when diagram rendering fails."""
 
 
 class PolarionWorkerParams:
@@ -374,11 +362,15 @@ def set_attachment_id(node: etree._Element) -> None:
     def _prepare_attachment(
         self,
         attachment: polarion_api.WorkItemAttachment,
-        work_item_id: str,
         new_checksums: dict[str, str],
         old_checksum: str | None = None,
         is_update: bool = False,
     ) -> list[polarion_api.WorkItemAttachment]:
+        # PNG attachments are already in the attachment list and should pass through
+        # without modification to avoid triggering re-render of failed diagrams
+        if isinstance(attachment, data_model.PngConvertedSvgAttachment):
+            return [attachment]
+
         if not isinstance(attachment, data_model.Capella2PolarionAttachment):
             return [attachment]
 
@@ -388,24 +380,17 @@ def _prepare_attachment(
             else ""
         )
 
-        try:
-            _ = attachment.content_bytes
+        if attachment.content_checksum != errors.RENDER_ERROR_CHECKSUM:
             return [attachment]
-        except Exception:
-            new_checksums[base_file_name] = RENDER_ERROR_CHECKSUM
 
-            logger.exception(
-                "Failed to render diagram %s for WorkItem %s",
-                attachment.file_name,
-                work_item_id,
-            )
+        new_checksums[base_file_name] = errors.RENDER_ERROR_CHECKSUM
+        if is_update and old_checksum == errors.RENDER_ERROR_CHECKSUM:
+            return []
 
-            if is_update and old_checksum == RENDER_ERROR_CHECKSUM:
-                return []
-
-            attachment.content_bytes = ERROR_IMAGE
-            png_attachment = data_model.PngConvertedSvgAttachment(attachment)
-            return [attachment, png_attachment]
+        # Set error image on SVG - the PNG will convert this cached error image
+        # when its content_bytes is accessed, avoiding re-render of the failed diagram
+        attachment.content_bytes = errors.ERROR_IMAGE
+        return [attachment]
 
     def update_attachments(
         self,
@@ -463,9 +448,7 @@ def update_attachments(
 
         validated_new_attachments = []
         for attachment in filter(None, new_attachments):
-            prepared = self._prepare_attachment(
-                attachment, new.id or "", new_checksums
-            )
+            prepared = self._prepare_attachment(attachment, new_checksums)
             validated_new_attachments.extend(prepared)
 
         if validated_new_attachments:
@@ -498,11 +481,7 @@ def update_attachments(
                 continue
 
             prepared = self._prepare_attachment(
-                attachment,
-                new.id or "",
-                new_checksums,
-                old_checksum,
-                is_update=True,
+                attachment, new_checksums, old_checksum, is_update=True
             )
             for att in prepared:
                 if att.file_name:

capella2polarion/data_model/work_item_attachments.py

@@ -16,6 +16,8 @@
 from capellambse import model
 from capellambse_context_diagrams import context
 
+from capella2polarion import errors
+
 SVG_MIME_TYPE = "image/svg+xml"
 PNG_MIME_TYPE = "image/png"
 logger = logging.getLogger(__name__)
@@ -34,7 +36,16 @@ def calculate_content_checksum(
     attachment: polarion_api.WorkItemAttachment,
 ) -> str:
     """Calculate content checksum for an attachment."""
-    return base64.b64encode(attachment.content_bytes or b"").decode("utf8")
+    try:
+        return base64.b64encode(attachment.content_bytes or b"").decode("utf8")
+    except Exception as e:
+        logger.error(
+            "Failed to read content bytes for attachment %s of WorkItem %s.",
+            attachment.file_name,
+            attachment.work_item_id,
+            exc_info=e,
+        )
+        return errors.RENDER_ERROR_CHECKSUM
 
 
 @dataclasses.dataclass
@@ -255,7 +266,6 @@ def content_bytes(self) -> bytes | None:
             self._content_bytes = cairosvg.svg2png(
                 self._svg_attachment.content_bytes
             )
-
         return self._content_bytes
 
     @content_bytes.setter

capella2polarion/data_model/work_items.py

@@ -11,6 +11,7 @@
 
 import polarion_rest_api_client as polarion_api
 
+from capella2polarion import errors
 from capella2polarion.data_model import work_item_attachments as wi_att
 
 WORK_ITEM_CHECKSUM_KEY = "__C2P__WORK_ITEM"
@@ -121,17 +122,22 @@ def _calculate_attachment_checksums(self) -> dict[str, str]:
                     attachment
                 )
 
-            attachment_checksums[base_file_name] = hashlib.sha256(
-                json.dumps(
-                    {
-                        "work_item_id": attachment.work_item_id,
-                        "title": attachment.title,
-                        "content_bytes": content_checksum,
-                        "mime_type": attachment.mime_type,
-                        "file_name": attachment.file_name,
-                    }
-                ).encode("utf8")
-            ).hexdigest()
+            if content_checksum != errors.RENDER_ERROR_CHECKSUM:
+                attachment_checksums[base_file_name] = hashlib.sha256(
+                    json.dumps(
+                        {
+                            "work_item_id": attachment.work_item_id,
+                            "title": attachment.title,
+                            "content_bytes": content_checksum,
+                            "mime_type": attachment.mime_type,
+                            "file_name": attachment.file_name,
+                        }
+                    ).encode("utf8")
+                ).hexdigest()
+            else:
+                attachment_checksums[base_file_name] = (
+                    errors.RENDER_ERROR_CHECKSUM
+                )
 
         return dict(sorted(attachment_checksums.items()))
 

capella2polarion/errors.py

@@ -0,0 +1,16 @@
+# Copyright DB InfraGO AG and contributors
+# SPDX-License-Identifier: Apache-2.0
+"""Module for Capella-Polarion errors and handling."""
+
+RENDER_ERROR_CHECKSUM = "__RENDER_ERROR__"
+"""Marker used as checksum when diagram rendering fails."""
+ERROR_IMAGE = b"""<svg xmlns="http://www.w3.org/2000/svg" width="800" height="200">
+  <rect width="800" height="200" fill="#d32f2f"/>
+  <text x="400" y="90" text-anchor="middle" fill="white" font-size="24" font-weight="bold">
+    Capella2Polarion: Diagram Failed to Render
+  </text>
+  <text x="400" y="130" text-anchor="middle" fill="white" font-size="18">
+    Please contact support for assistance
+  </text>
+</svg>"""
+"""Static SVG image to use when diagram rendering fails."""

tests/test_workitem_attachments.py

@@ -10,8 +10,9 @@
 import polarion_rest_api_client as polarion_api
 import pytest
 from capellambse_context_diagrams import context
+from capellambse_context_diagrams import errors as context_errors
 
-from capella2polarion import data_model
+from capella2polarion import data_model, errors
 from capella2polarion.connectors import polarion_repo, polarion_worker
 from capella2polarion.elements import (
     converter_config,
@@ -799,6 +800,37 @@ def test_context_diagram_checksum_does_not_trigger_rendering(
     assert isinstance(checksum, str)
 
 
+def test_context_diagram_checksum_handles_elk_input_and_render_failure(
+    model: capellambse.MelodyModel,
+):
+    obj = model.by_uuid("d4a22478-5717-4ca7-bfc9-9a193e6218a8")
+    attachment = data_model.CapellaContextDiagramAttachment(
+        obj.context_diagram,
+        "__C2P__context_diagram.svg",
+        {},
+        "Diagram",
+    )
+    attachment.work_item_id = "TEST-WI"
+
+    with (
+        mock.patch.object(
+            attachment.diagram,
+            "elk_input_data",
+            side_effect=ValueError("Malformed link: '__Derived-CP_INOUT:-1'"),
+        ),
+        mock.patch.object(
+            attachment.diagram,
+            "render",
+            side_effect=context_errors.CycleError(
+                "The interface is a cycle, connecting the same source and target."
+            ),
+        ),
+    ):
+        checksum = attachment.content_checksum
+
+    assert checksum == errors.RENDER_ERROR_CHECKSUM
+
+
 def test_prepare_attachment_uploads_static_error_on_creation_failure(
     model: capellambse.MelodyModel,
     worker: polarion_worker.CapellaPolarionWorker,
@@ -809,26 +841,20 @@ def test_prepare_attachment_uploads_static_error_on_creation_failure(
     )
 
     with mock.patch.object(
-        diag,
-        "render",
-        side_effect=Exception("Rendering failed"),
+        diag, "render", side_effect=Exception("Rendering failed")
     ):
         new_checksums: dict[str, str] = {}
         result = worker._prepare_attachment(
-            attachment, "TEST-WI", new_checksums, is_update=False
+            attachment, new_checksums, is_update=False
         )
 
     assert result is not None
-    assert len(result) == 2
+    assert len(result) == 1
     assert result[0].mime_type == "image/svg+xml"
-    assert result[1].mime_type == "image/png"
     error_svg = result[0].content_bytes
     assert b"Diagram Failed to Render" in error_svg
     assert b"contact support" in error_svg.lower()
-    assert (
-        new_checksums["__C2P__diagram"]
-        == polarion_worker.RENDER_ERROR_CHECKSUM
-    )
+    assert new_checksums["__C2P__diagram"] == errors.RENDER_ERROR_CHECKSUM
 
 
 def test_prepare_attachment_skips_update_when_error_persists(
@@ -847,17 +873,13 @@ def test_prepare_attachment_skips_update_when_error_persists(
         new_checksums: dict[str, str] = {}
         result = worker._prepare_attachment(
             attachment,
-            "TEST-WI",
             new_checksums,
-            old_checksum=polarion_worker.RENDER_ERROR_CHECKSUM,
+            old_checksum=errors.RENDER_ERROR_CHECKSUM,
             is_update=True,
         )
 
     assert result == []
-    assert (
-        new_checksums["__C2P__diagram"]
-        == polarion_worker.RENDER_ERROR_CHECKSUM
-    )
+    assert new_checksums["__C2P__diagram"] == errors.RENDER_ERROR_CHECKSUM
 
 
 def test_prepare_attachment_uploads_error_when_success_becomes_error(
@@ -877,23 +899,18 @@ def test_prepare_attachment_uploads_error_when_success_becomes_error(
         new_checksums: dict[str, str] = {}
         result = worker._prepare_attachment(
             attachment,
-            "TEST-WI",
             new_checksums,
             old_checksum="some_valid_checksum_hash",  # Was successful before
             is_update=True,
         )
 
     assert result is not None
-    assert len(result) == 2
+    assert len(result) == 1
     assert result[0].mime_type == "image/svg+xml"
-    assert result[1].mime_type == "image/png"
     error_svg = result[0].content_bytes
     assert b"Diagram Failed to Render" in error_svg
     assert b"contact support" in error_svg.lower()
-    assert (
-        new_checksums["__C2P__diagram"]
-        == polarion_worker.RENDER_ERROR_CHECKSUM
-    )
+    assert new_checksums["__C2P__diagram"] == errors.RENDER_ERROR_CHECKSUM
 
 
 def test_prepare_attachment_succeeds_after_error(
@@ -908,13 +925,72 @@ def test_prepare_attachment_succeeds_after_error(
 
     result = worker._prepare_attachment(
         attachment,
-        "TEST-WI",
         new_checksums,
-        old_checksum=polarion_worker.RENDER_ERROR_CHECKSUM,  # Was error before
+        old_checksum=errors.RENDER_ERROR_CHECKSUM,  # Was error before
         is_update=True,
     )
 
     assert result is not None
     assert len(result) == 1
     assert result[0] is attachment
     assert "__C2P__diagram" in new_checksums or len(new_checksums) == 0
+
+
+def test_cycle_error_during_compare_and_update_work_item(
+    model: capellambse.MelodyModel,
+    worker: polarion_worker.CapellaPolarionWorker,
+):
+    func = model.by_uuid(TEST_PHYS_FNC)
+    attachment = data_model.CapellaContextDiagramAttachment(
+        func.context_diagram,
+        "__C2P__context_diagram.svg",
+        {},
+        "Context Diagram",
+    )
+    attachment.work_item_id = "TEST-WI"
+    work_item = data_model.CapellaWorkItem(
+        "TEST-WI",
+        type="function",
+        uuid_capella=TEST_PHYS_FNC,
+        attachments=[attachment],
+    )
+    converter_data = data_session.ConverterData(
+        "Physical Architecture",
+        converter_config.CapellaTypeConfig("function", {}, []),
+        func,
+        work_item,
+    )
+    old_wi = data_model.CapellaWorkItem(
+        "TEST-WI", type="function", uuid_capella=TEST_PHYS_FNC
+    )
+    old_wi.calculate_checksum()
+    worker.polarion_data_repo = polarion_repo.PolarionDataRepository([old_wi])
+    worker.project_client.work_items.get.return_value = old_wi
+    worker.project_client.work_items.attachments.get_all.return_value = []
+
+    with (
+        mock.patch.object(
+            attachment.diagram,
+            "elk_input_data",
+            side_effect=ValueError("Malformed link: '__Derived-CP_INOUT:-1'"),
+        ),
+        mock.patch.object(
+            attachment.diagram,
+            "render",
+            side_effect=context_errors.CycleError(
+                "The interface is a cycle, connecting the same source and target."
+            ),
+        ),
+    ):
+        worker.compare_and_update_work_item(converter_data)
+
+    assert worker.project_client.work_items.update.call_count == 1
+    updated_work_item: data_model.CapellaWorkItem = (
+        worker.project_client.work_items.update.call_args.args[0]
+    )
+    assert updated_work_item.checksum is not None
+    checksum_dict = json.loads(updated_work_item.checksum)
+    assert "__C2P__context_diagram" in checksum_dict
+    assert (
+        checksum_dict["__C2P__context_diagram"] == errors.RENDER_ERROR_CHECKSUM
+    )