optimize create_unique_element_association()

src/sgraph/modelapi.py

@@ -229,12 +229,11 @@ def create_assoc(x: SElement, other: SElement, is_outgoing: bool, ea: SElementAs
             new_or_existing_referred_elem, this_is_new = sub_graph.create_or_get_element(x)
 
             if is_outgoing:
-                SElementAssociation(other, new_or_existing_referred_elem, ea.deptype,
-                                    ea.attrs).initElems()
+                SElementAssociation.create_unique_element_association(
+                    other, new_or_existing_referred_elem, ea.deptype, ea.attrs)
             else:
-
-                SElementAssociation(new_or_existing_referred_elem, other, ea.deptype,
-                                    ea.attrs).initElems()
+                SElementAssociation.create_unique_element_association(
+                    new_or_existing_referred_elem, other, ea.deptype, ea.attrs)
 
             return new_or_existing_referred_elem, this_is_new
 

src/sgraph/selement.py

@@ -11,7 +11,7 @@
 
 class SElement:
     __slots__ = 'name', 'parent', 'children', 'childrenDict', 'outgoing', 'incoming', 'attrs', \
-        'human_readable_name'
+        'human_readable_name', '_incoming_index'
 
     name: str
     parent: Optional["SElement"]
@@ -21,6 +21,8 @@ class SElement:
     incoming: list["SElementAssociation"]
     attrs: dict[str, str | int | list[str]]
     human_readable_name: str
+    # Index for O(1) duplicate association lookup: (id(fromElement), deptype) -> assoc
+    _incoming_index: dict[tuple[int, str], "SElementAssociation"]
 
     def __init__(self, parent: Optional['SElement'], name: str):
         """
@@ -69,6 +71,7 @@ def __init__(self, parent: Optional['SElement'], name: str):
         self.childrenDict = {}
         self.outgoing = []
         self.incoming = []
+        self._incoming_index = {}
         self.attrs = {}
         # self.num = '0'
 

src/sgraph/selementassociation.py

@@ -36,23 +36,14 @@ def create_unique_element_association(
         :returns: Return tuple of the existing
             or new element and a boolean indicating if a new element was
             created (true if new was created, false otherwise)"""
-        def filter_existing(incoming: "SElementAssociation"):
-            from_element_matches = incoming.fromElement == from_element
-            if dependency_type:
-                return dependency_type == incoming.deptype and \
-                    from_element_matches
-            else:
-                return from_element_matches
-
-        filtered_associations = filter(filter_existing, to_element.incoming)
-        existing_associations = list(filtered_associations)
-
-        # Do not create association if the same association already exists
-        if len(existing_associations) > 0:
+        # O(1) lookup using the index
+        key = (id(from_element), dependency_type)
+        existing = to_element._incoming_index.get(key)
+
+        if existing is not None:
             # Combine attributes to the existing association
-            existing_associations[0].attrs.update(dependency_attributes)
-            # return {'existingOrNewAssociation': existing_associations[0], 'isNew': False}
-            return existing_associations[0], False
+            existing.attrs.update(dependency_attributes)
+            return existing, False
 
         new_association = SElementAssociation(from_element, to_element, dependency_type,
                                               dependency_attributes)
@@ -114,10 +105,16 @@ def getAttributes(self):
     def initElems(self):
         self.fromElement.outgoing.append(self)
         self.toElement.incoming.append(self)
+        # Maintain index for O(1) duplicate lookup
+        key = (id(self.fromElement), self.deptype)
+        self.toElement._incoming_index[key] = self
 
     def remove(self):
         self.fromElement.outgoing.remove(self)
         self.toElement.incoming.remove(self)
+        # Maintain index for O(1) duplicate lookup
+        key = (id(self.fromElement), self.deptype)
+        self.toElement._incoming_index.pop(key, None)
 
     def addAttribute(self, attr_name: str, attr_val: str | int | list[str]):
         self.attrs[attr_name] = attr_val

tests/modelapi_test.py

@@ -190,3 +190,47 @@ def test_filter_model_with_attributes_ref():
     assert (original_model.createOrGetElementFromPath(e).attrs == m2.createOrGetElementFromPath(e).attrs)
     print(m2.to_deps(None))
 
+
+def test_filter_model_no_duplicate_associations():
+    """Test that filter_model does not create duplicate associations.
+
+    This test verifies the fix for issue #37 - using create_unique_element_association
+    to prevent duplicate associations when the same dependency is encountered multiple
+    times during traversal.
+    """
+    from sgraph import SGraph, SElement, SElementAssociation
+
+    # Create a model where the same association could be encountered multiple times
+    model = SGraph(SElement(None, ''))
+    a = model.createOrGetElementFromPath('/root/a')
+    b = model.createOrGetElementFromPath('/root/b')
+    c = model.createOrGetElementFromPath('/root/c')
+    external = model.createOrGetElementFromPath('/external/lib')
+
+    # Both a and b depend on external
+    SElementAssociation(a, external, 'use').initElems()
+    SElementAssociation(b, external, 'use').initElems()
+    # a also depends on b
+    SElementAssociation(a, b, 'use').initElems()
+    # c depends on a
+    SElementAssociation(c, a, 'use').initElems()
+
+    model_api = ModelApi(model=model)
+    root_elem = model.createOrGetElementFromPath('/root')
+
+    # Filter with DirectAndIndirect to traverse through multiple paths
+    subgraph = model_api.filter_model(
+        root_elem, model,
+        filter_outgoing=FilterAssocations.DirectAndIndirect,
+        filter_incoming=FilterAssocations.Ignore)
+
+    # Find the external element in the subgraph
+    external_in_subgraph = subgraph.findElementFromPath('/external/lib')
+    assert external_in_subgraph is not None
+
+    # Verify no duplicate incoming associations to external
+    # Each unique (from, to, type) combination should only appear once
+    incoming_signatures = [(ea.fromElement.getPath(), ea.deptype) for ea in external_in_subgraph.incoming]
+    assert len(incoming_signatures) == len(set(incoming_signatures)), \
+        f"Duplicate associations found: {incoming_signatures}"
+