From ae09adf8546c5237a7f020f49b92cda9881600f3 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Fri, 24 Feb 2023 13:01:14 +1100 Subject: [PATCH] Fix #2424. Bug where indirect assets (e.g. importing a type using a material) with relationships had their relationships duplicated --- .../ifcopenshell/api/project/append_asset.py | 67 ++++++++++++------- .../test/api/project/test_append_asset.py | 51 ++++++++++++++ 2 files changed, 95 insertions(+), 23 deletions(-) diff --git a/src/ifcopenshell-python/ifcopenshell/api/project/append_asset.py b/src/ifcopenshell-python/ifcopenshell/api/project/append_asset.py index 681e70e9d8..50e29994fd 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/project/append_asset.py +++ b/src/ifcopenshell-python/ifcopenshell/api/project/append_asset.py @@ -106,40 +106,38 @@ class Usecase: self.target_class = "IfcProfileDef" return self.append_profile_def() - def get_existing_element(self): + def get_existing_element(self, element): + if element.id() in self.added_elements: + return self.added_elements[element.id()] try: - return self.file.by_guid(self.settings["element"].GlobalId) + if element.is_a("IfcRoot"): + return self.file.by_guid(element.GlobalId) + elif element.is_a("IfcMaterial"): + return [e for e in self.file.by_type("IfcMaterial") if e.Name == element.Name][0] + elif element.is_a("IfcProfileDef"): + return [e for e in self.file.by_type("IfcProfileDef") if e.ProfileName == element.ProfileName][0] except: return False def append_material(self): - if [e for e in self.file.by_type("IfcMaterial") if e.Name == self.settings["element"].Name]: - return - self.whitelisted_inverse_attributes = {"IfcMaterial": ["HasProperties", "HasRepresentation"]} + self.whitelisted_inverse_attributes = { + "IfcMaterial": ["HasExternalReferences", "HasProperties", "HasRepresentation"] + } self.existing_contexts = self.file.by_type("IfcGeometricRepresentationContext") element = self.add_element(self.settings["element"]) - if not element.HasRepresentation: - return element - self.reuse_existing_contexts() + if element.HasRepresentation: + self.reuse_existing_contexts() return element def append_cost_schedule(self): - element = self.get_existing_element() - if element: - return element self.whitelisted_inverse_attributes = {"IfcCostSchedule": ["Controls"], "IfcCostItem": ["IsNestedBy"]} return self.add_element(self.settings["element"]) def append_profile_def(self): - if [e for e in self.file.by_type("IfcProfileDef") if e.ProfileName == self.settings["element"].ProfileName]: - return self.whitelisted_inverse_attributes = {"IfcProfileDef": ["HasProperties"]} return self.add_element(self.settings["element"]) def append_type_product(self): - element = self.get_existing_element() - if element: - return element self.whitelisted_inverse_attributes = { "IfcObjectDefinition": ["HasAssociations"], "IfcMaterialDefinition": ["HasExternalReferences", "HasProperties", "HasRepresentation"], @@ -151,14 +149,11 @@ class Usecase: return element def append_product(self): - element = self.get_existing_element() - if element: - return element self.whitelisted_inverse_attributes = { "IfcObjectDefinition": ["HasAssociations"], "IfcObject": ["IsDefinedBy.IfcRelDefinesByProperties"], "IfcElement": ["HasOpenings"], - "IfcMaterialDefinition": ["HasExternalReferences", "HasProperties"], + "IfcMaterialDefinition": ["HasExternalReferences", "HasProperties", "HasRepresentation"], "IfcRepresentationItem": ["StyledByItem"], } self.existing_contexts = self.file.by_type("IfcGeometricRepresentationContext") @@ -185,15 +180,41 @@ class Usecase: def add_element(self, element): if element.id() == 0: return - if element.id() in self.added_elements: - return self.added_elements[element.id()] + existing_element = self.get_existing_element(element) + if existing_element: + return existing_element + # if element.id() in self.added_elements: + # return self.added_elements[element.id()] new = self.file.add(element) self.added_elements[element.id()] = new - for subelement in self.settings["library"].traverse(element): + self.check_inverses(element) + for subelement in self.settings["library"].traverse(element)[1:]: + existing_element = self.get_existing_element(subelement) + if existing_element: + self.added_elements[subelement.id()] = existing_element + if not self.has_whitelisted_inverses(existing_element): + self.check_inverses(subelement) + break self.added_elements[subelement.id()] = self.file.add(subelement) self.check_inverses(subelement) return new + def has_whitelisted_inverses(self, element): + for source_class, attributes in self.whitelisted_inverse_attributes.items(): + if not element.is_a(source_class): + continue + for attribute in attributes: + attribute_class = None + if "." in attribute: + attribute, attribute_class = attribute.split(".") + value = getattr(element, attribute, []) + if attribute_class: + for subvalue in value: + if subvalue.is_a(attribute_class): + return True + elif value: + return True + def check_inverses(self, element): for source_class, attributes in self.whitelisted_inverse_attributes.items(): if not element.is_a(source_class): 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 a66110b2de..db67461438 100644 --- a/src/ifcopenshell-python/test/api/project/test_append_asset.py +++ b/src/ifcopenshell-python/test/api/project/test_append_asset.py @@ -62,6 +62,57 @@ class TestAppendAsset(test.bootstrap.IFC4): ifcopenshell.api.run("project.append_asset", self.file, library=library, element=element) assert self.file.by_type("IfcWallType")[0].HasAssociations[0].RelatingMaterial.Name == "Material" + def test_append_two_type_products_sharing_the_same_material_with_properties(self): + library = ifcopenshell.api.run("project.create_file") + element1 = ifcopenshell.api.run("root.create_entity", library, ifc_class="IfcWallType") + element2 = ifcopenshell.api.run("root.create_entity", library, ifc_class="IfcWallType") + material = ifcopenshell.api.run("material.add_material", library, name="Material") + + pset = ifcopenshell.api.run("pset.add_pset", library, product=material, name="Foo_Bar") + ifcopenshell.api.run("pset.edit_pset", library, pset=pset, properties={"Foo": "Bar"}) + + ifcopenshell.api.run("material.assign_material", library, product=element1, material=material) + ifcopenshell.api.run("material.assign_material", library, product=element2, material=material) + new1 = ifcopenshell.api.run("project.append_asset", self.file, library=library, element=element1) + new2 = ifcopenshell.api.run("project.append_asset", self.file, library=library, element=element2) + + assert len(self.file.by_type("IfcMaterialProperties")) == 1 + material = self.file.by_type("IfcMaterial")[0] + assert ifcopenshell.util.element.get_material(new1) == material + assert ifcopenshell.util.element.get_material(new2) == material + assert ifcopenshell.util.element.get_psets(material)["Foo_Bar"]["Foo"] == "Bar" + + def test_append_two_type_products_sharing_the_same_material_indirectly_via_a_material_set(self): + library = ifcopenshell.api.run("project.create_file") + element1 = ifcopenshell.api.run("root.create_entity", library, ifc_class="IfcWallType") + element2 = ifcopenshell.api.run("root.create_entity", library, ifc_class="IfcWallType") + + material = ifcopenshell.api.run("material.add_material", library, name="Material") + pset = ifcopenshell.api.run("pset.add_pset", library, product=material, name="Foo_Bar") + ifcopenshell.api.run("pset.edit_pset", library, pset=pset, properties={"Foo": "Bar"}) + + layer_set1 = ifcopenshell.api.run("material.add_material_set", library, set_type="IfcMaterialLayerSet") + ifcopenshell.api.run("material.add_layer", library, layer_set=layer_set1, material=material) + ifcopenshell.api.run("material.add_layer", library, layer_set=layer_set1, material=material) + + layer_set2 = ifcopenshell.api.run("material.add_material_set", library, set_type="IfcMaterialLayerSet") + ifcopenshell.api.run("material.add_layer", library, layer_set=layer_set2, material=material) + ifcopenshell.api.run("material.add_layer", library, layer_set=layer_set2, material=material) + + ifcopenshell.api.run("material.assign_material", library, product=element1, material=layer_set1) + ifcopenshell.api.run("material.assign_material", library, product=element2, material=layer_set2) + + new1 = ifcopenshell.api.run("project.append_asset", self.file, library=library, element=element1) + new2 = ifcopenshell.api.run("project.append_asset", self.file, library=library, element=element2) + + assert len(self.file.by_type("IfcMaterialProperties")) == 1 + material = self.file.by_type("IfcMaterial")[0] + assert ifcopenshell.util.element.get_material(new1).MaterialLayers[0].Material == material + assert ifcopenshell.util.element.get_material(new1).MaterialLayers[1].Material == material + assert ifcopenshell.util.element.get_material(new2).MaterialLayers[0].Material == material + assert ifcopenshell.util.element.get_material(new2).MaterialLayers[1].Material == material + assert ifcopenshell.util.element.get_psets(material)["Foo_Bar"]["Foo"] == "Bar" + def test_append_a_type_product_with_its_styles(self): library = ifcopenshell.api.run("project.create_file") element = ifcopenshell.api.run("root.create_entity", library, ifc_class="IfcWallType")