append_asset - fix duplicated applications/organizations/pao

Noticed when starting new project with demo library it had 29 duplicated IfcApplications...

src/bonsai/scripts/generate_demo_library.py

@@ -151,6 +151,7 @@ class LibraryGenerator:
             application = ifcopenshell.api.owner.add_application(
                 self.file, application_full_name="Bonsai", application_identifier="Bonsai"
             )
+            # Override Bonsai methods for duration of the script.
             ifcopenshell.api.owner.settings.get_user = lambda x: user
             ifcopenshell.api.owner.settings.get_application = lambda x: application
 

src/ifcopenshell-python/ifcopenshell/api/project/append_asset.py

@@ -179,6 +179,12 @@ class Usecase:
     assume_asset_uniqueness_by_name: bool
     whitelisted_inverse_attributes: dict[str, list[str]]
 
+    added_elements: dict[int, ifcopenshell.entity_instance]
+    """Elements added with ``add_element``."""
+
+    reuse_identities: dict[int, ifcopenshell.entity_instance]
+    """Mapping of old element ids to new elements, usually fiiled by ``file_add``."""
+
     def execute(self):
         # mapping of old element ids to new elements
         self.added_elements: dict[int, ifcopenshell.entity_instance] = {}
@@ -213,6 +219,16 @@ class Usecase:
             return None
 
     def get_existing_element(self, element: ifcopenshell.entity_instance) -> Union[ifcopenshell.entity_instance, None]:
+        """Get existing element for a library element.
+
+        Return element if it was already added with ``add_element``
+        or if it's not necessary (model already has a replacement for it).
+
+        Note that if element is returned, it will be accepted as-is,
+        it's subgraph inverses won't be checked.
+
+        Return ``None`` if element wasn't added before and needs to be added.
+        """
         if element.id() in self.added_elements:
             return self.added_elements[element.id()]
         if element.is_a("IfcRoot"):
@@ -232,6 +248,20 @@ class Usecase:
             if name is None:
                 return None
             return next((e for e in self.file.by_type(element.is_a()) if e.Name == name), None)
+
+        # Not really assets but if we don't check them here,
+        # their subgraph entities may be appended twice.
+        elif (ifc_class := element.is_a()) == "IfcOrganization":
+            attr_name = "Id" if self.file.schema == "IFC2X3" else "Identification"
+            org_id = getattr(element, attr_name)
+            if org_id is not None:
+                return next((e for e in self.file.by_type("IfcOrganization") if getattr(e, attr_name) == org_id), None)
+        elif ifc_class == "IfcPerson":
+            attr_name = "Id" if self.file.schema == "IFC2X3" else "Identification"
+            person_id = getattr(element, attr_name)
+            if person_id is not None:
+                return next((e for e in self.file.by_type("IfcPerson") if getattr(e, attr_name) == person_id), None)
+
         else:
             return None
 
@@ -318,6 +348,7 @@ class Usecase:
         return element
 
     def add_element(self, element: ifcopenshell.entity_instance) -> Union[ifcopenshell.entity_instance, None]:
+        """Add element and check all it's subgraph inverses."""
         if element.id() == 0:
             return
         existing_element = self.get_existing_element(element)
@@ -358,6 +389,7 @@ class Usecase:
         return False
 
     def check_inverses(self, element: ifcopenshell.entity_instance) -> None:
+        """Add inverse elements for the whitelisted inverse attributes."""
         for source_class, attributes in self.whitelisted_inverse_attributes.items():
             if not element.is_a(source_class):
                 continue
@@ -372,10 +404,15 @@ class Usecase:
                         self.add_inverse_element(inverse)
 
     def add_inverse_element(self, element: ifcopenshell.entity_instance) -> None:
-        # Inverse attributes are added manually because they are basically
+        """Add inverse element.
-        # relationships that can reference many other assets that we are not
-        # interested in.
 
+        Inverse elements are requiring different method than ``file_add``
+        because they can reference many other assets that we are not
+        interested in.
+
+        E.g. a IfcRelAssociatesMaterial referencing products unrelated
+        to the current asset.
+        """
         # For layer assignment we don't want to add it's items
         # to avoid adding representations / items that are not related to current append_asset.
         skip_not_reused_entities_attr_i = None
@@ -535,7 +572,32 @@ class Usecase:
             attributes_ = element.wrapped_data.declaration().as_entity().all_attributes()
             return attributes_
 
-        # Maybe element already exists.
+        def get_existing_element_(
+            subelement: ifcopenshell.entity_instance,
+        ) -> Union[ifcopenshell.entity_instance, None]:
+            # Check identity because `subelement` might not be the current `element`,
+            # e.g. for IfcPersonAndOrganization.
+            element_identity = subelement.wrapped_data.identity()
+            if subelement_ := reuse_identities.get(element_identity):
+                return subelement_
+
+            ifc_class = subelement.is_a()
+            assert ifc_class in ("IfcOrganization", "IfcPerson")
+            attr_name = "Id" if ifc_file.schema == "IFC2X3" else "Identification"
+            subelement_id = getattr(subelement, attr_name)
+
+            if subelement_id is not None:
+                existing_org = next(
+                    (e for e in ifc_file.by_type(ifc_class) if getattr(e, attr_name) == subelement_id), None
+                )
+                if existing_org is not None:
+                    reuse_identities[element_identity] = existing_org
+                    return existing_org
+
+        # Check if element already exists.
+        # NOTE: Ensure this part is in sync with `get_existing_element`,
+        # if some class is present here but not in `get_existing_element`,
+        # then it might create duplicated subelements.
         if element.is_a("IfcProfileDef"):
             profile_name = element.ProfileName
             if profile_name is not None:
@@ -545,12 +607,14 @@ class Usecase:
                 if existing_profile is not None:
                     reuse_identities[element_identity] = existing_profile
                     return existing_profile
+
         elif element.is_a("IfcMaterial"):
             material_name = element.Name
             existing_material = next((e for e in ifc_file.by_type("IfcMaterial") if e.Name == material_name), None)
             if existing_material is not None:
                 reuse_identities[element_identity] = existing_material
                 return existing_material
+
         elif element.is_a("IfcPresentationStyle"):
             style_name = element.Name
             if style_name is not None:
@@ -559,7 +623,38 @@ class Usecase:
                     reuse_identities[element_identity] = existing_style
                     return existing_style
 
-        attrs = {}
+        elif ifc_class == "IfcApplication":
+            app_id = element.ApplicationIdentifier
+            if app_id is not None:
+                existing_app = next(
+                    (e for e in ifc_file.by_type("IfcApplication") if e.ApplicationIdentifier == app_id), None
+                )
+                if existing_app is not None:
+                    reuse_identities[element_identity] = existing_app
+                    return existing_app
+
+        elif ifc_class == "IfcOrganization":
+            existing_org = get_existing_element_(element)
+            if existing_org is not None:
+                reuse_identities[element_identity] = existing_org
+                return existing_org
+
+        elif ifc_class == "IfcPerson":
+            existing_person = get_existing_element_(element)
+            if existing_person is not None:
+                reuse_identities[element_identity] = existing_person
+                return existing_person
+
+        elif ifc_class == "IfcPersonAndOrganization":
+            if (person := get_existing_element_(element.ThePerson)) and (
+                org := get_existing_element_(element.TheOrganization)
+            ):
+                for pao in ifc_file.by_type("IfcPersonAndOrganization"):
+                    if pao.ThePerson == person and pao.TheOrganization == org:
+                        reuse_identities[element_identity] = pao
+                        return pao
+
+        attrs: dict[int, Any] = {}
 
         # Utils method for the loop.
         def get_tuple_type(tuple_: tuple) -> type:

src/ifcopenshell-python/test/api/project/test_append_asset.py

@@ -31,6 +31,7 @@ import ifcopenshell.api.project
 import ifcopenshell.api.material
 import ifcopenshell.api.profile
 import ifcopenshell.api.unit
+import ifcopenshell.api.owner.settings
 import ifcopenshell.util.element
 import ifcopenshell.util.placement
 import ifcopenshell.util.unit
@@ -562,6 +563,42 @@ class TestAppendAssetIFC2X3(test.bootstrap.IFC2X3):
         representations = tuple(self.file.by_type("IfcRepresentation"))
         assert self.file.by_type("IfcPresentationLayerAssignment")[0].AssignedItems == representations
 
+    def test_append_owner_history_without_producing_duplicates(self):
+        ifc_file = ifcopenshell.file()
+        library = ifcopenshell.file()
+
+        # Setup owner history.
+        ifcopenshell.api.owner.settings.factory_reset()
+        person = ifcopenshell.api.owner.add_person(library)
+        ifcopenshell.api.owner.add_role(library, person, "CONTRIBUTOR")
+        organization = ifcopenshell.api.owner.add_organisation(library)
+        user = ifcopenshell.api.owner.add_person_and_organisation(library, person=person, organisation=organization)
+        application = ifcopenshell.api.owner.add_application(library)
+        assert len(library.by_type("IfcPerson")) == 1
+        assert len(library.by_type("IfcPersonAndOrganization")) == 1
+        assert len(library.by_type("IfcApplication")) == 1
+        assert len(library.by_type("IfcOrganization")) == 2
+
+        # IfcOrganization's attributes.
+        assert len(library.by_type("IfcTelecomAddress")) == 1
+        # IfcPerson's attributes.
+        assert len(library.by_type("IfcActorRole")) == 2
+
+        # Add 2 wall types.
+        wall_type_1 = ifcopenshell.api.root.create_entity(library, ifc_class="IfcWallType")
+        wall_type_2 = ifcopenshell.api.root.create_entity(library, ifc_class="IfcWallType")
+
+        ifcopenshell.api.project.append_asset(ifc_file, library, wall_type_1)
+        ifcopenshell.api.project.append_asset(ifc_file, library, wall_type_2)
+
+        assert len(ifc_file.by_type("IfcPerson")) == 1
+        assert len(ifc_file.by_type("IfcPersonAndOrganization")) == 1
+        assert len(ifc_file.by_type("IfcApplication")) == 1
+        assert len(ifc_file.by_type("IfcOrganization")) == 2
+        # Ensure their attributes are also not duplicated.
+        assert len(ifc_file.by_type("IfcTelecomAddress")) == 1
+        assert len(ifc_file.by_type("IfcActorRole")) == 2
+
 
 class TestAppendAssetIFC4(test.bootstrap.IFC4, TestAppendAssetIFC2X3):
     # NOTE: breaks in IFC2X3 since IfcProfileDef doesn't have "HasProperties" inverse in ifc2x3