Merge pull request #411 from gaphor/fix-issue-with-copied-associations

Fix issue where copy/paste + delete of original association causes association ends to be removed

gaphor/UML/classes/association.py

@@ -90,6 +90,8 @@ def __init__(self, id=None, model=None):
             f"{base}.type[Interface].ownedAttribute", self.on_association_end_value
         ).watch(
             f"{base}.appliedStereotype.classifier", self.on_association_end_value
+        ).watch(
+            "subject[Association].memberEnd"
         ).watch(
             "subject[Association].ownedEnd"
         ).watch(
@@ -387,6 +389,16 @@ def __init__(self, owner, end=None):
 
     name_bounds = property(lambda s: s._name_bounds)
 
+    @property
+    def owner(self):
+        """ Override Element.owner. """
+        return self._owner
+
+    @property
+    def owner_handle(self):
+        # handle(event) is the event handler method
+        return self._owner.head if self is self._owner.head_end else self._owner.tail
+
     def request_update(self):
         self._owner.request_update()
 

gaphor/UML/classes/classconnect.py

@@ -100,7 +100,23 @@ def allow(self, handle, port):
         if not isinstance(element.subject, UML.Classifier):
             return None
 
-        return super().allow(handle, port)
+        if not self.line.subject:
+            return True
+
+        line = self.line
+        is_head = handle is line.head
+        end = line.head_end if is_head else line.tail_end
+        assert end.subject
+
+        def is_connection_allowed(h):
+            if h is handle:
+                return True
+            connected = self.get_connected(h)
+            return (not connected) or connected.subject is element.subject
+
+        return all(
+            is_connection_allowed(p.owner_handle) for p in end.subject.presentation
+        )
 
     def connect_subject(self, handle):
         element = self.element
@@ -121,29 +137,8 @@ def connect_subject(self, handle):
                 # Set subject last so that event handlers can trigger
                 line.subject = relation
 
-            else:
-                assert isinstance(line.subject, UML.Association)
-                end1 = line.subject.memberEnd[0]
-                end2 = line.subject.memberEnd[1]
-                if (end1.type is c1.subject and end2.type is c2.subject) or (
-                    end2.type is c1.subject and end1.type is c2.subject
-                ):
-                    return
-
-            line.subject.memberEnd[0].type = c1.subject  # type: ignore[assignment]
-            line.subject.memberEnd[1].type = c2.subject  # type: ignore[assignment]
-            UML.model.set_navigability(
-                line.subject,
-                line.head_end.subject,
-                line.subject.memberEnd[0].navigability,
-            )
-            line.head_end.subject.aggregation = line.subject.memberEnd[0].aggregation
-            UML.model.set_navigability(
-                line.subject,
-                line.tail_end.subject,
-                line.subject.memberEnd[1].navigability,
-            )
-            line.tail_end.subject.aggregation = line.subject.memberEnd[1].aggregation
+            line.head_end.subject.type = c1.subject  # type: ignore[assignment]
+            line.tail_end.subject.type = c2.subject  # type: ignore[assignment]
 
     def reconnect(self, handle, port):
         line = self.line
@@ -171,8 +166,12 @@ def disconnect_subject(self, handle: Handle) -> None:
         On connect, we pair association member ends with the element they
         connect to. On disconnect, we remove this relation.
         """
-        for e in list(self.line.subject.memberEnd):
-            e.type = None
+        association = self.line.subject
+        if association and len(association.presentation) <= 1:
+            for e in list(association.memberEnd):
+                UML.model.set_navigability(association, e, None)
+            for e in list(association.memberEnd):
+                e.type = None
 
 
 @Connector.register(Named, ImplementationItem)

gaphor/UML/classes/classespropertypages.py

@@ -493,17 +493,24 @@ def construct(self):
         head_signal_handlers = self.construct_end(builder, "head", head)
         tail_signal_handlers = self.construct_end(builder, "tail", tail)
 
-        def handler(event):
+        def name_handler(event):
             end_name = "head" if event.element is head.subject else "tail"
             self.update_end_name(builder, end_name, event.element)
 
+        def restore_nav_handler(event):
+            prop = event.element
+            if prop.type and prop.opposite and prop.opposite.type:
+                for end_name, end in (("head", head), ("tail", tail)):
+                    combo = builder.get_object(f"{end_name}-navigation")
+                    self._on_end_navigability_change(combo, end)
+
         # Watch on association end:
-        self.watcher.watch("memberEnd[Property].name", handler).watch(
-            "memberEnd[Property].aggregation", handler
-        ).watch("memberEnd[Property].visibility", handler).watch(
-            "memberEnd[Property].lowerValue", handler
+        self.watcher.watch("memberEnd[Property].name", name_handler).watch(
+            "memberEnd[Property].visibility", name_handler
+        ).watch("memberEnd[Property].lowerValue", name_handler).watch(
+            "memberEnd[Property].upperValue", name_handler
         ).watch(
-            "memberEnd[Property].upperValue", handler
+            "memberEnd[Property].type", restore_nav_handler,
         ).subscribe_all()
 
         builder.connect_signals(

gaphor/UML/classes/tests/test_association.py

@@ -30,19 +30,24 @@ def test_create(self):
         self.assoc.show_direction = True
         assert self.assoc.show_direction
 
-    def test_invert_direction(self):
+    def test_direction(self):
         """Test association direction inverting
         """
         self.connect(self.assoc, self.assoc.head, self.class1)
         self.connect(self.assoc, self.assoc.tail, self.class2)
 
-        head_subject = self.assoc.subject.memberEnd[0]
-        tail_subject = self.assoc.subject.memberEnd[1]
+        assert self.assoc.head_end.subject is self.assoc.subject.memberEnd[0]
+        assert self.assoc.tail_end.subject is self.assoc.subject.memberEnd[1]
+
+    def test_invert_direction(self):
+        self.connect(self.assoc, self.assoc.head, self.class1)
+        self.connect(self.assoc, self.assoc.tail, self.class2)
 
         self.assoc.invert_direction()
 
-        assert head_subject is self.assoc.subject.memberEnd[1]
-        assert tail_subject is self.assoc.subject.memberEnd[0]
+        assert self.assoc.subject.memberEnd
+        assert self.assoc.head_end.subject is self.assoc.subject.memberEnd[1]
+        assert self.assoc.tail_end.subject is self.assoc.subject.memberEnd[0]
 
     def test_association_end_updates(self):
         """Test association end navigability connected to a class"""
@@ -89,3 +94,7 @@ def test_association_orthogonal(self):
             pass  # Expected, hanve only 2 handles, need 3 or more
         else:
             assert False, "Can not set line to orthogonal with less than 3 handles"
+
+    def test_association_end_owner_handles(self):
+        assert self.assoc.head_end.owner_handle is self.assoc.head
+        assert self.assoc.tail_end.owner_handle is self.assoc.tail

gaphor/UML/classes/tests/test_associationconnect.py

@@ -0,0 +1,201 @@
+import pytest
+
+from gaphor import UML
+from gaphor.diagram.tests.fixtures import allow, connect, disconnect
+from gaphor.UML.classes.association import AssociationItem
+from gaphor.UML.classes.klass import ClassItem
+
+
+@pytest.fixture
+def create(diagram, element_factory):
+    def _create(item_class, element_class=None):
+        return diagram.create(
+            item_class,
+            subject=(element_factory.create(element_class) if element_class else None),
+        )
+
+    return _create
+
+
+@pytest.fixture
+def connected_association(create):
+    asc = create(AssociationItem)
+    c1 = create(ClassItem, UML.Class)
+    c2 = create(ClassItem, UML.Class)
+
+    connect(asc, asc.head, c1)
+    assert asc.subject is None  # no UML metaclass yet
+
+    connect(asc, asc.tail, c2)
+    assert asc.subject is not None
+
+    return asc, c1, c2
+
+
+@pytest.fixture
+def clone(create):
+    def _clone(item):
+        new = create(type(item))
+        new.subject = item.subject
+        new.head_end.subject = item.head_end.subject
+        new.tail_end.subject = item.tail_end.subject
+        return new
+
+    return _clone
+
+
+def get_connected(item, handle):
+    cinfo = item.canvas.get_connection(handle)
+    if cinfo:
+        return cinfo.connected  # type: ignore[no-any-return] # noqa: F723
+    return None
+
+
+def test_glue_to_class(connected_association):
+    asc, c1, c2 = connected_association
+
+    glued = allow(asc, asc.head, c1)
+    assert glued
+
+    connect(asc, asc.head, c1)
+
+    glued = allow(asc, asc.tail, c2)
+    assert glued
+
+
+def test_association_item_connect(connected_association, element_factory):
+    asc, c1, c2 = connected_association
+
+    # Diagram, Class *2, Property *2, Association, StyleSheet
+    assert len(element_factory.lselect()) == 7
+    assert asc.head_end.subject is not None
+    assert asc.tail_end.subject is not None
+
+
+def test_association_item_reconnect(connected_association, create):
+    asc, c1, c2 = connected_association
+    c3 = create(ClassItem, UML.Class)
+
+    UML.model.set_navigability(asc.subject, asc.tail_end.subject, True)
+
+    a = asc.subject
+
+    connect(asc, asc.tail, c3)
+
+    assert a is asc.subject
+    ends = [p.type for p in asc.subject.memberEnd]
+    assert c1.subject in ends
+    assert c3.subject in ends
+    assert c2.subject not in ends
+    assert asc.tail_end.subject.navigability is True
+
+
+def test_disconnect_should_disconnect_model(connected_association):
+    asc, c1, c2 = connected_association
+
+    disconnect(asc, asc.head)
+    disconnect(asc, asc.tail)
+    assert c1 is not get_connected(asc, asc.head)
+    assert c2 is not get_connected(asc, asc.tail)
+
+    assert asc.subject
+    assert len(asc.subject.memberEnd) == 2
+    assert asc.subject.memberEnd[0].type is None
+    assert asc.subject.memberEnd[1].type is None
+
+
+def test_disconnect_of_second_association_should_leave_model_in_tact(
+    connected_association, clone
+):
+    asc, c1, c2 = connected_association
+    new = clone(asc)
+
+    disconnect(new, new.head)
+    assert asc.subject.memberEnd[0].type is c1.subject
+    assert asc.subject.memberEnd[1].type is c2.subject
+    assert new.subject is asc.subject
+
+
+def test_disconnect_of_navigable_end_should_remove_owner_relationship(
+    connected_association,
+):
+    asc, c1, c2 = connected_association
+
+    UML.model.set_navigability(asc.subject, asc.head_end.subject, True)
+
+    assert asc.head_end.subject in c2.subject.ownedAttribute
+
+    disconnect(asc, asc.head)
+
+    assert asc.subject
+    assert len(asc.subject.memberEnd) == 2
+    assert asc.subject.memberEnd[0].type is None
+    assert asc.head_end.subject not in c2.subject.ownedAttribute
+    assert asc.tail_end.subject not in c1.subject.ownedAttribute
+    assert asc.head_end.subject.type is None
+    assert asc.tail_end.subject.type is None
+
+
+def test_allow_reconnect_for_single_presentation(connected_association, create):
+    asc, c1, c2 = connected_association
+    c3 = create(ClassItem, UML.Class)
+
+    assert allow(asc, asc.head, c3)
+
+
+def test_allow_reconnect_on_same_class_for_multiple_presentations(
+    connected_association, clone, create
+):
+    asc, c1, c2 = connected_association
+    new = clone(asc)
+
+    assert allow(new, new.head, c1)
+    assert allow(new, new.tail, c2)
+
+
+def test_allow_reconnect_if_only_one_connected_presentations(
+    connected_association, clone, create
+):
+    asc, c1, c2 = connected_association
+    clone(asc)
+
+    c3 = create(ClassItem, UML.Class)
+
+    assert allow(asc, asc.head, c3)
+
+
+def test_disallow_connect_if_already_connected_with_presentations(
+    connected_association, clone, create
+):
+    asc, c1, c2 = connected_association
+    new = clone(asc)
+
+    c3 = create(ClassItem, UML.Class)
+
+    assert not allow(new, new.head, c3)
+
+
+def test_disallow_reconnect_if_multiple_connected_presentations(
+    connected_association, clone, create
+):
+    asc, c1, c2 = connected_association
+    new = clone(asc)
+    connect(new, new.head, c1)
+
+    c3 = create(ClassItem, UML.Class)
+
+    assert not allow(asc, asc.head, c3)
+
+
+def test_allow_both_ends_connected_to_the_same_class(create, clone):
+    asc = create(AssociationItem)
+    c1 = create(ClassItem, UML.Class)
+    connect(asc, asc.head, c1)
+    connect(asc, asc.tail, c1)
+
+    new = clone(asc)
+    c2 = create(ClassItem, UML.Class)
+    assert allow(new, new.head, c1)
+    assert allow(new, new.tail, c1)
+    assert not allow(new, new.head, c2)
+    assert not allow(new, new.tail, c2)