fix: keep Model and Element consistent when adding relationships

Author: yt-ms

src/structurizr/model/element.py

@@ -99,14 +99,21 @@ def get_afferent_relationships(self) -> Iterator[Relationship]:
             r for r in self.get_model().get_relationships() if self is r.destination
         )
 
-    def add_relationship(self, relationship: Optional[Relationship] = None, **kwargs) -> Relationship:
+    def add_relationship(
+        self, relationship: Optional[Relationship] = None, **kwargs
+    ) -> Relationship:
         if relationship is None:
             relationship = Relationship(**kwargs)
+        elif relationship in self.relationships:
+            return relationship  # Nothing more to do
         if relationship.source is None:
             relationship.source = self
         elif relationship.source is not self:
-            raise ValueError(f"Cannot add relationship {relationship} to element {self} that is not its source.")
+            raise ValueError(
+                f"Cannot add relationship {relationship} to element {self} that is not its source."
+            )
         self.relationships.add(relationship)
+        self.get_model().add_relationship(relationship)
         return relationship
 
     @classmethod

src/structurizr/model/model.py

@@ -411,6 +411,8 @@ def _add_element(self, element: Element) -> None:
 
     def _add_relationship(self, relationship: Relationship) -> bool:
         """"""
+        if relationship in self.get_relationships():
+            return True  # Nothing to do
         if not relationship.id:
             relationship.id = self._id_generator.generate_id()
         elif relationship.id in self._elements_by_id:

tests/integration/test_model_element_relationships.py

@@ -27,7 +27,6 @@
 DEFINITIONS = Path(__file__).parent / "data" / "workspace_definition"
 
 
-@pytest.mark.xfail(strict=True)
 def test_adding_relationship_to_element_adds_to_model():
     """
     Make sure that when a relationship is added via Element.add_relationship it also
@@ -39,10 +38,10 @@ def test_adding_relationship_to_element_adds_to_model():
 
     r = sys1.add_relationship(source=sys1, destination=sys2, description="uses")
     assert list(sys1.relationships) == [r]
-    assert list(sys1.get_relationships()) == [r]  # Currently will fail
-    assert list(model.get_relationships()) == [r]  # Currently will fail
+    assert list(sys1.get_relationships()) == [r]
+    assert list(model.get_relationships()) == [r]
     assert list(sys2.relationships) == []  # relationships only contains outbound
-    assert list(sys2.get_relationships()) == [r]  # Currently will fail
+    assert list(sys2.get_relationships()) == [r]
 
 
 def test_adding_relationship_to_model_adds_to_element():

tests/unit/model/test_element.py

@@ -19,6 +19,7 @@
 import pytest
 
 from structurizr.model.element import Element
+from structurizr.model.model import Model
 
 
 class ConcreteElement(Element):
@@ -60,7 +61,8 @@ def test_element_can_only_add_relationship_to_source():
     element1 = ConcreteElement(name="elt1")
     element2 = ConcreteElement(name="elt1")
     with pytest.raises(
-        ValueError, match="Cannot add relationship .* to element .* that is not its source"
+        ValueError,
+        match="Cannot add relationship .* to element .* that is not its source",
     ):
         element1.add_relationship(source=element2)
 
@@ -72,5 +74,20 @@ def test_element_add_relationship_can_omit_source():
     """
     element1 = ConcreteElement(name="elt1")
     element2 = ConcreteElement(name="elt1")
+    model = Model()
+    model._add_element(element1)
     r = element1.add_relationship(destination=element2)
     assert r.source is element1
+
+
+def test_element_add_relationship_twice_is_ok():
+    """
+    Defensive test that adding the same relationship twice is fine.
+    """
+    element1 = ConcreteElement(name="elt1")
+    element2 = ConcreteElement(name="elt1")
+    model = Model()
+    model._add_element(element1)
+    r = element1.add_relationship(destination=element2)
+    element1.add_relationship(r)
+    assert list(element1.relationships) == [r]