From a5755056528e9f8f8f85c2c69da666d17951b93f Mon Sep 17 00:00:00 2001 From: ewuerger Date: Wed, 17 Jul 2024 12:19:13 +0200 Subject: [PATCH] fix(context-diagrams)!: Handle nested functions Possibly with no owners faulty state. --- .../collectors/default.py | 4 +- .../collectors/exchanges.py | 88 ++- .../collectors/generic.py | 15 +- capellambse_context_diagrams/context.py | 32 +- docs/gen_images.py | 65 +- tests/data/ContextDiagram.aird | 745 +++++++++++------- tests/data/ContextDiagram.capella | 35 +- tests/test_interface_diagrams.py | 34 +- 8 files changed, 558 insertions(+), 460 deletions(-) diff --git a/capellambse_context_diagrams/collectors/default.py b/capellambse_context_diagrams/collectors/default.py index 57602a88..36b1c5e5 100644 --- a/capellambse_context_diagrams/collectors/default.py +++ b/capellambse_context_diagrams/collectors/default.py @@ -51,8 +51,8 @@ def __init__( self.boxes_to_delete = {self.centerbox.id} self.edges: list[fa.AbstractExchange] = [] if self.diagram._display_parent_relation: - self.diagram_target_owners = generic.get_all_owners( - self.diagram.target + self.diagram_target_owners = list( + generic.get_all_owners(self.diagram.target) ) self.common_owners: set[str] = set() diff --git a/capellambse_context_diagrams/collectors/exchanges.py b/capellambse_context_diagrams/collectors/exchanges.py index 1e4acf47..15870ebf 100644 --- a/capellambse_context_diagrams/collectors/exchanges.py +++ b/capellambse_context_diagrams/collectors/exchanges.py @@ -144,31 +144,6 @@ def __init__( super().__init__(diagram, data, params) - self._functional_exchanges: common.ElementList[ - common.GenericElement - ] = self.get_alloc_fex(diagram.target) - self._derived_functional_exchanges: dict[ - str, common.GenericElement - ] = {} - self._ex_validity: dict[ - str, dict[str, common.GenericElement | None] - ] = { - fex.uuid: {"source": None, "target": None} - for fex in self._functional_exchanges - } - - self.get_left_and_right() - if diagram.hide_functions: - assert self.left is not None - self.left.children = [] # type: ignore[unreachable] - assert self.right is not None - self.right.children = [] - self.incoming_edges = {} - self.outgoing_edges = {} - - if diagram.include_interface or diagram.hide_functions: - self.add_interface() - def get_left_and_right(self) -> None: try: self.collect_context() @@ -197,13 +172,36 @@ def collect_context(self): self.right.id: self.right, } for fex in self.get_alloc_fex(self.obj): - srid = self.make_all_owners(fex.source, boxes) - trid = self.make_all_owners(fex.target, boxes) - if [srid, trid] == [self.right.id, self.left.id]: + try: + src_id = self.make_all_owners(fex.source, boxes) + except ValueError as error: + logger.warning(error.args[0]) + continue + + try: + trg_id = self.make_all_owners(fex.target, boxes) + except ValueError as error: + src_port = boxes[fex.source.owner.uuid].ports.pop() + assert src_port.id == fex.source.uuid + logger.warning(error.args[0]) + continue + + if [src_id, trg_id] == [self.right.id, self.left.id]: self.incoming_edges[fex.uuid] = fex - elif [srid, trid] == [self.left.id, self.right.id]: + elif [src_id, trg_id] == [self.left.id, self.right.id]: self.outgoing_edges[fex.uuid] = fex + for uuid, box in boxes.items(): + element = self.obj._model.by_uuid(uuid) + if isinstance(element, fa.AbstractFunction) and ( + parent_box := boxes.get(element.parent.uuid) + ): + owner_box = boxes[element.owner.uuid] + owner_box.children.remove(box) + parent_box.children.append(box) + for label in parent_box.labels: + label.layoutOptions = makers.DEFAULT_LABEL_LAYOUT_OPTIONS + if self.left.children: for label in self.left.labels: label.layoutOptions = makers.DEFAULT_LABEL_LAYOUT_OPTIONS @@ -212,7 +210,9 @@ def collect_context(self): label.layoutOptions = makers.DEFAULT_LABEL_LAYOUT_OPTIONS def make_all_owners( - self, obj: fa.AbstractFunction, boxes: dict[str, _elkjs.ELKInputChild] + self, + obj: fa.AbstractFunction | fa.FunctionPort, + boxes: dict[str, _elkjs.ELKInputChild], ) -> str: owners: list[fa.AbstractFunction | cs.Component] = [] assert self.right is not None and self.left is not None @@ -225,10 +225,15 @@ def make_all_owners( owners.append(element) - assert root is not None, f"No root found for {obj._short_repr_()}" + if root is None: + raise ValueError(f"No root found for {obj._short_repr_()}") + owner_box: common.GenericElement = root - for owner in owners[::-1]: + for owner in reversed(owners): if isinstance(owner, fa.FunctionPort): + if owner.uuid in (p.id for p in owner_box.ports): + continue + owner_box.ports.append(makers.make_port(owner.uuid)) else: if owner.uuid in (b.id for b in owner_box.children): @@ -242,6 +247,7 @@ def make_all_owners( for label in owner_box.labels: label.layoutOptions = makers.DEFAULT_LABEL_LAYOUT_OPTIONS owner_box = box + return root.id def add_interface(self) -> None: @@ -260,6 +266,18 @@ def add_interface(self) -> None: def collect(self) -> None: """Collect all allocated `FunctionalExchange`s in the context.""" + self.get_left_and_right() + if self.diagram._hide_functions: + assert self.left is not None + self.left.children = [] + assert self.right is not None + self.right.children = [] + self.incoming_edges = {} + self.outgoing_edges = {} + + if self.diagram._include_interface or self.diagram._hide_functions: + self.add_interface() + try: for ex in (self.incoming_edges | self.outgoing_edges).values(): ex_data = generic.ExchangeData( @@ -270,12 +288,6 @@ def collect(self) -> None: is_hierarchical=False, ) src, tgt = generic.exchange_data_collector(ex_data) - if ex.uuid in self._derived_functional_exchanges: - class_ = type(ex).__name__ - self.data.edges[-1].id = ( - f"{makers.STYLECLASS_PREFIX}-{class_}:{ex.uuid}" - ) - if ex in self.incoming_edges.values(): self.data.edges[-1].sources = [tgt.uuid] self.data.edges[-1].targets = [src.uuid] diff --git a/capellambse_context_diagrams/collectors/generic.py b/capellambse_context_diagrams/collectors/generic.py index dc98581e..474a3709 100644 --- a/capellambse_context_diagrams/collectors/generic.py +++ b/capellambse_context_diagrams/collectors/generic.py @@ -228,8 +228,8 @@ def move_edges( """Move edges to boxes.""" edges_to_remove: list[str] = [] for c in connections: - source_owner_uuids = get_all_owners(c.source) - target_owner_uuids = get_all_owners(c.target) + source_owner_uuids = list(get_all_owners(c.source)) + target_owner_uuids = list(get_all_owners(c.target)) if c.source == c.target: source_owner_uuids.remove(c.source.uuid) target_owner_uuids.remove(c.source.uuid) @@ -252,14 +252,9 @@ def move_edges( data.edges = [e for e in data.edges if e.id not in edges_to_remove] -def get_all_owners(obj: common.GenericElement) -> list[str]: +def get_all_owners(obj: common.GenericElement) -> cabc.Iterator[str]: """Return the UUIDs from all owners of ``obj``.""" - owners: list[str] = [] current = obj while current is not None: - owners.append(current.uuid) - try: - current = current.owner - except AttributeError: - break - return owners + yield current.uuid + current = getattr(current, "owner", None) diff --git a/capellambse_context_diagrams/context.py b/capellambse_context_diagrams/context.py index 0d64b218..473c1957 100644 --- a/capellambse_context_diagrams/context.py +++ b/capellambse_context_diagrams/context.py @@ -76,30 +76,12 @@ def _get( diagram_class: type[ContextDiagram], diagram_id: str = "{}_context", ) -> common.Accessor | ContextDiagram: - try: - cache = getattr( - obj._model, ".".join((__name__, diagram_class.__qualname__)) - ) - except AttributeError: - cache = {} - setattr( - obj._model, - ".".join((__name__, diagram_class.__qualname__)), - cache, - ) - diagram_id = diagram_id.format(obj.uuid) - try: - return cache[diagram_id] - except KeyError: - pass - new_diagram = diagram_class( self._dgcls, obj, default_render_parameters=self._default_render_params, ) new_diagram.filters.add(filters.NO_UUID) - cache[diagram_id] = new_diagram return new_diagram @@ -348,7 +330,7 @@ def _create_diagram(self, params: dict[str, t.Any]) -> cdiagram.Diagram: if not isinstance( self, (ClassTreeDiagram, InterfaceContextDiagram) ) and has_single_child(data): - self.display_derived_interfaces = True + self._display_derived_interfaces = True data = get_elkdata(self, params) layout = try_to_layout(data) @@ -387,9 +369,6 @@ class InterfaceContextDiagram(ContextDiagram): context diagram target: The interface ComponentExchange. * hide_functions — Boolean flag to enable white box view: Only displaying Components or Entities. - * display_derived_exchanges — Boolean flag to enable inclusion of - functional exchanges that are not allocated to the interface but - connect allocated functions of collected components. In addition to all other render parameters of [`ContextDiagram`][capellambse_context_diagrams.context.ContextDiagram]. @@ -397,7 +376,6 @@ class InterfaceContextDiagram(ContextDiagram): _include_interface: bool _hide_functions: bool - _display_derived_exchanges: bool def __init__( self, @@ -410,7 +388,6 @@ def __init__( default_render_parameters = { "include_interface": False, "hide_functions": False, - "display_derived_exchanges": False, "display_symbols_as_boxes": True, } | default_render_parameters super().__init__( @@ -420,21 +397,20 @@ def __init__( default_render_parameters=default_render_parameters, ) - self.dangling_functional_exchanges: list[fa.AbstractExchange] = [] - @property def name(self) -> str: # type: ignore return f"Interface Context of {self.target.name}" def _create_diagram(self, params: dict[str, t.Any]) -> cdiagram.Diagram: + super_params = params.copy() params = self._default_render_parameters | params for param_name in self._default_render_parameters: setattr(self, f"_{param_name}", params.pop(param_name)) - params["elkdata"] = exchanges.get_elkdata_for_exchanges( + super_params["elkdata"] = exchanges.get_elkdata_for_exchanges( self, exchanges.InterfaceContextCollector, params ) - return super()._create_diagram(params) + return super()._create_diagram(super_params) class FunctionalContextDiagram(ContextDiagram): diff --git a/docs/gen_images.py b/docs/gen_images.py index f9338b76..4e6e3206 100644 --- a/docs/gen_images.py +++ b/docs/gen_images.py @@ -17,26 +17,26 @@ dest = pathlib.Path("assets") / "images" model_path = pathlib.Path(__file__).parent.parent / "tests" / "data" model = MelodyModel(path=model_path, entrypoint="ContextDiagram.aird") -general_context_diagram_uuids: dict[str, tuple[str, dict[str, t.Any]]] = { - "Environment": ("e37510b9-3166-4f80-a919-dfaac9b696c7", {}), - "Eat": ("8bcb11e6-443b-4b92-bec2-ff1d87a224e7", {}), - "Middle": ("da08ddb6-92ba-4c3b-956a-017424dbfe85", {}), - "Capability": ("9390b7d5-598a-42db-bef8-23677e45ba06", {}), - "Lost": ("a5642060-c9cc-4d49-af09-defaa3024bae", {}), - "Left": ("f632888e-51bc-4c9f-8e81-73e9404de784", {}), - "educate Wizards": ("957c5799-1d4a-4ac0-b5de-33a65bf1519c", {}), - "Weird guy": ("098810d9-0325-4ae8-a111-82202c0d2016", {}), - "Top secret": ("5bf3f1e3-0f5e-4fec-81d5-c113d3a1b3a6", {}), - "Physical Node": ("fdb34c92-7c49-491d-bf11-dd139930786e", {}), - "Physical Behavior": ("313f48f4-fb7e-47a8-b28a-76440932fcb9", {}), - "Maintain": ("ee745644-07d7-40b9-ad7a-910dc8cbb805", {}), +general_context_diagram_uuids: dict[str, str] = { + "Environment": "e37510b9-3166-4f80-a919-dfaac9b696c7", + "Eat": "8bcb11e6-443b-4b92-bec2-ff1d87a224e7", + "Middle": "da08ddb6-92ba-4c3b-956a-017424dbfe85", + "Capability": "9390b7d5-598a-42db-bef8-23677e45ba06", + "Lost": "a5642060-c9cc-4d49-af09-defaa3024bae", + "Left": "f632888e-51bc-4c9f-8e81-73e9404de784", + "educate Wizards": "957c5799-1d4a-4ac0-b5de-33a65bf1519c", + "Weird guy": "098810d9-0325-4ae8-a111-82202c0d2016", + "Top secret": "5bf3f1e3-0f5e-4fec-81d5-c113d3a1b3a6", + "Physical Node": "fdb34c92-7c49-491d-bf11-dd139930786e", + "Physical Behavior": "313f48f4-fb7e-47a8-b28a-76440932fcb9", + "Maintain": "ee745644-07d7-40b9-ad7a-910dc8cbb805", } -interface_context_diagram_uuids: dict[str, tuple[str, dict[str, t.Any]]] = { - "Left to right": ("3ef23099-ce9a-4f7d-812f-935f47e7938d", {}), - "Interface": ("2f8ed849-fbda-4902-82ec-cbf8104ae686", {}), +interface_context_diagram_uuids: dict[str, str] = { + "Left to right": "3ef23099-ce9a-4f7d-812f-935f47e7938d", + "Interface": "2f8ed849-fbda-4902-82ec-cbf8104ae686", } hierarchy_context = "16b4fcc5-548d-4721-b62a-d3d5b1c1d2eb" -diagram_uuids = general_context_diagram_uuids | interface_context_diagram_uuids +diagram_uuids = interface_context_diagram_uuids class_tree_uuid = "b7c7f442-377f-492c-90bf-331e66988bda" realization_fnc_uuid = "beaf5ba4-8fa9-4342-911f-0266bb29be45" realization_comp_uuid = "b9f9a83c-fb02-44f7-9123-9d86326de5f1" @@ -45,16 +45,15 @@ def generate_index_images() -> None: - for uuid, render_params in diagram_uuids.values(): + for uuid in diagram_uuids.values(): diag: context.ContextDiagram = model.by_uuid(uuid).context_diagram with mkdocs_gen_files.open(f"{str(dest / diag.name)}.svg", "w") as fd: - render_params["transparent_background"] = False # type: ignore[index] - print(diag.render("svg", **render_params), file=fd) # type: ignore[arg-type] + print(diag.render("svg", transparent_background=False), file=fd) # type: ignore[arg-type] def generate_symbol_images() -> None: for name in ("Capability", "Middle"): - uuid, _ = general_context_diagram_uuids[name] + uuid = general_context_diagram_uuids[name] diag: context.ContextDiagram = model.by_uuid(uuid).context_diagram diag._display_symbols_as_boxes = True diag.invalidate_cache() @@ -175,17 +174,16 @@ def generate_derived_image() -> None: def generate_interface_with_hide_functions_image(): - uuid = interface_context_diagram_uuids["Interface"][0] + uuid = interface_context_diagram_uuids["Interface"] diag: context.ContextDiagram = model.by_uuid(uuid).context_diagram - params = {"hide_functions": True} with mkdocs_gen_files.open( f"{str(dest / diag.name)}-hide-functions.svg", "w" ) as fd: - print(diag.render("svg", **params), file=fd) + print(diag.render("svg", hide_functions=True), file=fd) def generate_interface_with_hide_interface_image(): - uuid = interface_context_diagram_uuids["Interface"][0] + uuid = interface_context_diagram_uuids["Interface"] diag: context.ContextDiagram = model.by_uuid(uuid).context_diagram params = {"include_interface": False} with mkdocs_gen_files.open( @@ -194,24 +192,14 @@ def generate_interface_with_hide_interface_image(): print(diag.render("svg", **params), file=fd) -def generate_interface_with_display_derived_exchanges_image(): - uuid = "86a1afc2-b7fd-4023-bbd5-ab44f5dc2c28" - diag: context.ContextDiagram = model.by_uuid(uuid).context_diagram - params = {"display_derived_exchanges": True} - with mkdocs_gen_files.open( - f"{str(dest / diag.name)}-derived-exchanges.svg", "w" - ) as fd: - print(diag.render("svg", **params), file=fd) - - generate_index_images() generate_hierarchy_image() generate_symbol_images() -wizard_uuid = general_context_diagram_uuids["educate Wizards"][0] +wizard_uuid = general_context_diagram_uuids["educate Wizards"] generate_no_edgelabel_image(wizard_uuid) -lost_uuid = general_context_diagram_uuids["Lost"][0] +lost_uuid = general_context_diagram_uuids["Lost"] generate_filter_image(lost_uuid, filters.EX_ITEMS, "ex") generate_filter_image(lost_uuid, filters.SHOW_EX_ITEMS, "fex and ex") generate_filter_image(lost_uuid, filters.EXCH_OR_EX_ITEMS, "fex or ex") @@ -229,6 +217,5 @@ def generate_interface_with_display_derived_exchanges_image(): generate_realization_view_images() generate_data_flow_image() generate_derived_image() -# generate_interface_with_hide_functions_image() +generate_interface_with_hide_functions_image() generate_interface_with_hide_interface_image() -generate_interface_with_display_derived_exchanges_image() diff --git a/tests/data/ContextDiagram.aird b/tests/data/ContextDiagram.aird index 83077280..e0515cdb 100644 --- a/tests/data/ContextDiagram.aird +++ b/tests/data/ContextDiagram.aird @@ -134,7 +134,7 @@ - +
@@ -150,7 +150,7 @@ - + @@ -12018,10 +12018,10 @@ - + - + @@ -12038,7 +12038,7 @@ - + @@ -12116,49 +12116,93 @@ - - - + + + - - - + + + - - - + + + - - + + - - - + + + + + + + - - - + + + - - + + - - - + + + - - - + + + - - + + + + + + + + + - - - + + + + + + + + + + + + + + - - + + + + + + + + + + + + + + + + + + + + + + + + @@ -12175,70 +12219,70 @@ - + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -12258,22 +12302,6 @@ - - - - - - - - - - - - - - - - @@ -12354,117 +12382,165 @@ - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - + + - - + + - - - - - + + + + + - - - + + + - - + + - - + + - - - - - + + + + + - - - + + + - - + + - - + + - - - - - + + + + + - - - + + + - - + + - - + + - - - - - + + + + + - - - + + + - - + + - - + + - - - - - + + + + + - - - + + + - - + + - - + + - - - - - + + + + + - - - + + + - - + + - - + + - - - - - + + + + + + + + + + + + + + + + + + + + + @@ -12475,7 +12551,7 @@ - + @@ -12544,7 +12620,7 @@ - + @@ -12553,10 +12629,13 @@ - + + KEEP_LOCATION + KEEP_SIZE + KEEP_RATIO @@ -12572,7 +12651,7 @@ - + @@ -12581,6 +12660,9 @@ + KEEP_LOCATION + KEEP_SIZE + KEEP_RATIO @@ -12658,32 +12740,32 @@ - + - + - - + + - - - - - + + + + + - - - - - + + + + + @@ -12691,12 +12773,52 @@ KEEP_LOCATION KEEP_SIZE KEEP_RATIO - + italic + + + + + + + + + + + + + KEEP_LOCATION + KEEP_SIZE + KEEP_RATIO + + + + + + + + + + + + + + + + + + KEEP_LOCATION + KEEP_SIZE + KEEP_RATIO + + + + + @@ -12749,15 +12871,6 @@ - - - - - - - - - @@ -12803,72 +12916,103 @@ - - - - - - - - - - + - + + + + + + + + + + + + - - - + + + - - - - + + + + - - - + + + - - - - + + + + - - - + + + - - - - + + + + + + + + + + + + + + + + + + + + + + + + - + + + + + + + + + + - - - - + + + + - + - - - - + + + + - + @@ -14958,80 +15102,80 @@ - - - - - - - - - - - - - + + + + + + + + + + + + + - - - + + + - - - + + + - - + + - - - + + + - - - + + + - - + + - - - - + + + + - - + + - - + + - - - - - + + + + + - - - + + + - - + + - - + + - - - - - + + + + + - - - + + + bold @@ -15039,52 +15183,49 @@ - + - KEEP_LOCATION - KEEP_SIZE - KEEP_RATIO - + bold - - - - + + + + - - - - + + + + - - - + + + - + - - - + + + - + - + diff --git a/tests/data/ContextDiagram.capella b/tests/data/ContextDiagram.capella index 94637d48..fe7ed768 100644 --- a/tests/data/ContextDiagram.capella +++ b/tests/data/ContextDiagram.capella @@ -3214,14 +3214,14 @@ The predator is far away + id="f713ba11-b18c-48f8-aabf-5ee57d5c87b7" name="RFNC Part 1"> + id="be863470-e4a8-4930-bb9f-4a6574907abe" name="FIP 1"/> + id="7cd5ae5b-6de7-42f6-8a35-9375dd5bbde8" name="RFNC Part 2"> + id="5267b95c-9f78-40d6-88b3-6e390bd31526" name="FIP 1"/> id="350a1be3-aae7-44ef-a9f3-82eb8a0a0075" name="Not allocated FEX1" target="#d49c3e57-eb23-4bb6-b920-562b08834b1d" source="#61327b3c-ed26-40d2-b51c-73c718c6cb1d"/> @@ -3673,9 +3673,6 @@ The predator is far away - @@ -3683,10 +3680,10 @@ The predator is far away id="c92eb72c-69ad-4bb0-b5ef-6b00f8716222" targetElement="#5f67436c-9742-4b84-b99e-7ee719ff02a0" sourceElement="#2f8ed849-fbda-4902-82ec-cbf8104ae686"/> - @@ -4261,7 +4255,7 @@ The predator is far away id="f5a12be6-8219-4262-8afc-6a69ebf8a828" targetElement="#f6447eb1-a4d3-4af6-84ad-22fdd29c05c8" sourceElement="#50d8bc7b-c029-45eb-a6d5-dfa4ef2abcf2"/> @@ -4287,9 +4281,6 @@ The predator is far away - @@ -4297,10 +4288,10 @@ The predator is far away id="c5d2e490-d9f2-4d1c-a5ba-3a6f3af90ce8" targetElement="#3d03fde7-3c12-409f-9a4a-5ccbc5cb9ee1" sourceElement="#10c28db9-efbb-4486-90ee-042cf867db1a"/> None: obj = model.by_uuid(TEST_INTERFACE_UUID) - obj.context_diagram.render("svgdiagram", hide_functions=True).save( - pretty=True - ) - obj.context_diagram.render("svgdiagram", include_interface=False).save( - pretty=True - ) diag = obj.context_diagram.render(None, include_interface=False) with pytest.raises(KeyError): @@ -68,16 +61,19 @@ def test_interface_diagram_with_hide_functions( diag[uuid] # pylint: disable=pointless-statement -def test_interface_diagram_with_derived_exchanges( +def test_interface_diagram_with_nested_functions( model: capellambse.MelodyModel, ) -> None: - obj = model.by_uuid(TEST_SA_INTERFACE_UUID) - expected_derived_exchanges = ( - "d69dcf31-a7d4-40c5-8dd4-b4747aa3ece7", - "7a61fcb7-aae5-4698-86de-8b0d70d8c09b", - ) - - diag = obj.context_diagram.render(None, display_derived_exchanges=True) - - for uuid in expected_derived_exchanges: - assert diag[f"__Derived-FunctionalExchange:{uuid}"] + obj = model.by_uuid(TEST_INTERFACE_UUID) + fex = model.by_uuid("2b30434f-a087-40f1-917b-c9d0af15be23") + fnc = fex.target.owner + obj.target.owner.allocated_functions.append(fnc) + obj.allocated_functional_exchanges.append(fex) + expected_uuids = { + "f713ba11-b18c-48f8-aabf-5ee57d5c87b7", + "7cd5ae5b-6de7-42f6-8a35-9375dd5bbde8", + } + + diag = obj.context_diagram.render(None) + + assert {b.uuid for b in diag[fnc.uuid].children} >= expected_uuids