diff --git a/src/bonsai/scripts/generate_demo_library.py b/src/bonsai/scripts/generate_demo_library.py index 3c8642a1e0..219f0f74db 100644 --- a/src/bonsai/scripts/generate_demo_library.py +++ b/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 diff --git a/src/ifcopenshell-python/ifcopenshell/api/project/append_asset.py b/src/ifcopenshell-python/ifcopenshell/api/project/append_asset.py index ff9097c2cc..123f4f38e9 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/project/append_asset.py +++ b/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 - # relationships that can reference many other assets that we are not - # interested in. + """Add inverse element. + 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: diff --git a/src/ifcopenshell-python/test/api/project/test_append_asset.py b/src/ifcopenshell-python/test/api/project/test_append_asset.py index 9251f10592..a1185b8ebb 100644 --- a/src/ifcopenshell-python/test/api/project/test_append_asset.py +++ b/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