diff --git a/capella2polarion/connectors/polarion_worker.py b/capella2polarion/connectors/polarion_worker.py index 8f7df92..5fce5c8 100644 --- a/capella2polarion/connectors/polarion_worker.py +++ b/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""" - - - Capella2Polarion: Diagram Failed to Render - - - Please contact support for assistance - -""" -"""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: diff --git a/capella2polarion/data_model/work_item_attachments.py b/capella2polarion/data_model/work_item_attachments.py index 85294fd..e2c7b6f 100644 --- a/capella2polarion/data_model/work_item_attachments.py +++ b/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 diff --git a/capella2polarion/data_model/work_items.py b/capella2polarion/data_model/work_items.py index 0ebdc86..028eef8 100644 --- a/capella2polarion/data_model/work_items.py +++ b/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())) diff --git a/capella2polarion/errors.py b/capella2polarion/errors.py new file mode 100644 index 0000000..20cec8f --- /dev/null +++ b/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""" + + + Capella2Polarion: Diagram Failed to Render + + + Please contact support for assistance + +""" +"""Static SVG image to use when diagram rendering fails.""" diff --git a/tests/test_workitem_attachments.py b/tests/test_workitem_attachments.py index 9a0e960..2f01be8 100644 --- a/tests/test_workitem_attachments.py +++ b/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,9 +925,8 @@ 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, ) @@ -918,3 +934,63 @@ def test_prepare_attachment_succeeds_after_error( 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 + )