diff --git a/src/capellambse/model/_descriptors.py b/src/capellambse/model/_descriptors.py index ec244067..40d29cb5 100644 --- a/src/capellambse/model/_descriptors.py +++ b/src/capellambse/model/_descriptors.py @@ -1531,9 +1531,13 @@ def __set__( " make sure that __set_name__ gets called" ) - self.__delete__(obj) - for v in value: - self.__create_link(obj, v) + elmlist = self.__get__(obj) + assert isinstance(elmlist, _obj.ElementListCouplingMixin) + i = -1 + for i, v in enumerate(value): + self.insert(elmlist, i, v) + for o in elmlist[i + 1 :]: + self.delete(elmlist, o) def __delete__(self, obj: _obj.ModelObject) -> None: refobjs = list(self.__find_refs(obj)) @@ -1611,13 +1615,7 @@ def __create_link( with model._loader.new_uuid(parent._element) as obj_id: link = model._loader.create_link(parent._element, target._element) refobj = parent._element.makeelement(self.tag) - if before is None: - parent._element.append(refobj) - else: - before_elm = self.__backref(parent, before) - assert before_elm is not None - assert before_elm in parent._element - before_elm.addprevious(refobj) + self.__insert_refobj(parent, refobj, before=before) try: refobj.set(helpers.ATT_XT, xtype) refobj.set("id", obj_id) @@ -1633,6 +1631,21 @@ def __create_link( raise return refobj + def __insert_refobj( + self, + parent: _obj.ModelObject, + refobj: etree._Element, + *, + before: _obj.ModelObject | None, + ) -> None: + if before is None: + parent._element.append(refobj) + else: + before_elm = self.__backref(parent, before) + assert before_elm is not None + assert before_elm in parent._element + before_elm.addprevious(refobj) + def insert( self, elmlist: _obj.ElementListCouplingMixin, @@ -1657,11 +1670,26 @@ def insert( f" not {type(value)}" ) - self.__create_link( - elmlist._parent, - value, - before=elmlist[index] if index < len(elmlist) else None, - ) + try: + refobj = next( + r + for r in self.__find_refs(elmlist._parent) + if self.__follow_ref(elmlist._parent, r) is value._element + ) + except StopIteration: + self.__create_link( + elmlist._parent, + value, + before=elmlist[index] if index < len(elmlist) else None, + ) + else: + if index < len(elmlist): + self.__insert_refobj( + elmlist._parent, refobj, before=elmlist[index] + ) + else: + self.__insert_refobj(elmlist._parent, refobj, before=None) + elmlist._elements = list(self.__find_refs(elmlist._parent)) return t.cast(T_co, value) def delete( diff --git a/tests/test_model_layers.py b/tests/test_model_layers.py index ebf6570f..e0c84a4e 100644 --- a/tests/test_model_layers.py +++ b/tests/test_model_layers.py @@ -1218,20 +1218,22 @@ def test_lists_of_links_can_be_removed_from(model: m.MelodyModel): assert protect_students not in hogwarts.allocated_functions -def test_lists_of_links_disallow_insertion_of_duplicate_members( +def test_lists_of_links_reorder_existing_members_when_appending_duplicates( model: m.MelodyModel, ): parent = model.by_uuid("3b83b4ba-671a-4de8-9c07-a5c6b1d3c422") target = model.by_uuid("dfaf473d-257f-4455-90fd-fe9489dac617") assert target in parent.involved_activities + assert target != parent.involved_activities[-1] + links_before = set(parent.involved_activities._elements) - with pytest.raises(m.NonUniqueMemberError) as catch: - parent.involved_activities.append(target) + parent.involved_activities.append(target) - assert catch.value.args == (parent, "involved_activities", target) - assert "involved_activities" in str(catch.value) - assert parent.uuid in str(catch.value) - assert target.uuid in str(catch.value) + count = sum(1 for ia in parent.involved_activities if ia == target) + assert count == 1 + assert parent.involved_activities[-1] == target + links_after = set(parent.involved_activities._elements) + assert links_before == links_after @pytest.mark.parametrize(