Fix bug where when deleting representations it could sometimes overreach and delete geometric, style, or layer data of other elements. See #1812.

src/ifcopenshell-python/ifcopenshell/api/geometry/remove_representation.py

@@ -9,35 +9,25 @@ class Usecase:
             self.settings[key] = value
 
     def execute(self):
-        if self.settings["representation"].RepresentationType == "MappedRepresentation":
-            return self.remove_mapped_representation_portion_only()
-        return self.remove_entire_representation_tree()
-
-    def remove_mapped_representation_portion_only(self):
-        for item in self.settings["representation"].Items:
-            if len(self.file.get_inverse(item.MappingTarget)) == 1:
-                ifcopenshell.util.element.remove_deep(self.file, item.MappingTarget)
-                self.file.remove(item.MappingTarget)
-            self.file.remove(item)
-        self.file.remove(self.settings["representation"])
-
-    def remove_entire_representation_tree(self):
-        dummy_context = self.file.create_entity("IfcRepresentationContext")
+        styled_items = set()
+        presentation_layer_assignments = set()
         for subelement in self.file.traverse(self.settings["representation"]):
             if subelement.is_a("IfcRepresentationItem") and subelement.StyledByItem:
-                [self.file.remove(s) for s in subelement.StyledByItem]
+                [styled_items.add(s) for s in subelement.StyledByItem]
             elif subelement.is_a("IfcRepresentation"):
-                subelement.ContextOfItems = dummy_context
-                self.purge_representation_inverses(subelement)
-        self.purge_representation_inverses(self.settings["representation"])
-        ifcopenshell.util.element.remove_deep(self.file, self.settings["representation"])
+                for inverse in self.file.get_inverse(subelement):
+                    if inverse.is_a("IfcPresentationLayerAssignment"):
+                        presentation_layer_assignments.add(inverse)
 
-    def purge_representation_inverses(self, element):
-        for inverse in self.file.get_inverse(element):
-            if inverse.is_a("IfcPresentationLayerAssignment"):
-                assigned_items = set(inverse.AssignedItems)
-                if len(assigned_items) == 1:
-                    self.file.remove(inverse)
-                else:
-                    assigned_items.remove(element)
-                    inverse.AssignedItems = list(assigned_items)
+        ifcopenshell.util.element.remove_deep2(
+            self.file,
+            self.settings["representation"],
+            extra_subgraph_elements=list(styled_items | presentation_layer_assignments),
+        )
+
+        for element in styled_items:
+            if not element.Item:
+                self.file.remove(element)
+        for element in presentation_layer_assignments:
+            if len(element.AssignedItems) == 0:
+                self.file.remove(element)

src/ifcopenshell-python/ifcopenshell/util/element.py

@@ -136,11 +136,12 @@ def remove_deep(ifc_file, element):
     ifc_file.unbatch()
 
 
-def remove_deep2(ifc_file, element):
+def remove_deep2(ifc_file, element, extra_subgraph_elements=[]):
     # Experimental remove deep proposal. No batch for now until this is more certain. See #1812.
     # ifc_file.batch()
     to_delete = set()
     subgraph = list(ifc_file.traverse(element, breadth_first=True))
+    subgraph.extend(extra_subgraph_elements)
     subgraph_set = set(subgraph)
     subelement_queue = ifc_file.traverse(element, max_levels=1)
     while subelement_queue:

src/ifcopenshell-python/test/api/geometry/test_remove_representation.py

@@ -0,0 +1,79 @@
+import test.bootstrap
+import ifcopenshell.api
+
+
+class TestRemoveRepresentation(test.bootstrap.IFC4):
+    def test_removing_a_single_unused_shape_representation(self):
+        representation = self.file.createIfcShapeRepresentation()
+        ifcopenshell.api.run("geometry.remove_representation", self.file, representation=representation)
+        assert len(self.file.by_type("IfcShapeRepresentation")) == 0
+
+    def test_not_removing_a_shape_representation_in_use(self):
+        representation = self.file.createIfcShapeRepresentation()
+        self.file.createIfcProductRepresentation(Representations=[representation])
+        ifcopenshell.api.run("geometry.remove_representation", self.file, representation=representation)
+        assert len(self.file.by_type("IfcShapeRepresentation")) == 1
+
+    def test_removing_a_mapped_representation_fully(self):
+        representation = self.file.createIfcShapeRepresentation(
+            RepresentationType="MappedRepresentation",
+            Items=[
+                self.file.createIfcMappedItem(
+                    MappingTarget=self.file.createIfcRepresentationMap(
+                        MappedRepresentation=self.file.createIfcShapeRepresentation()
+                    )
+                )
+            ],
+        )
+        assert len(self.file.by_type("IfcShapeRepresentation")) == 2
+        ifcopenshell.api.run("geometry.remove_representation", self.file, representation=representation)
+        assert len(self.file.by_type("IfcShapeRepresentation")) == 0
+
+    def test_removing_only_the_representation_mapping_if_the_map_has_other_users(self):
+        representation_map = self.file.createIfcRepresentationMap(
+            MappedRepresentation=self.file.createIfcShapeRepresentation()
+        )
+        self.file.createIfcWallType(RepresentationMaps=[representation_map])
+        representation = self.file.createIfcShapeRepresentation(
+            RepresentationType="MappedRepresentation",
+            Items=[
+                self.file.createIfcMappedItem(
+                    MappingTarget=representation_map
+                )
+            ],
+        )
+        assert len(self.file.by_type("IfcShapeRepresentation")) == 2
+        ifcopenshell.api.run("geometry.remove_representation", self.file, representation=representation)
+        assert len(self.file.by_type("IfcShapeRepresentation")) == 1
+        assert self.file.by_type("IfcShapeRepresentation")[0].RepresentationType != "MappedRepresentation"
+        assert len(self.file.by_type("IfcRepresentationMap")) == 1
+
+    def test_purging_styled_items(self):
+        item = self.file.createIfcExtrudedAreaSolid()
+        representation = self.file.createIfcShapeRepresentation(Items=[item])
+        styled_item = self.file.createIfcStyledItem(Item=item)
+        ifcopenshell.api.run("geometry.remove_representation", self.file, representation=representation)
+        assert len(self.file.by_type("IfcStyledItem")) == 0
+
+    def test_not_purging_styled_items_if_used_elsewhere(self):
+        item = self.file.createIfcExtrudedAreaSolid()
+        representation = self.file.createIfcShapeRepresentation(Items=[item])
+        styled_item = self.file.createIfcStyledItem(Item=item)
+        representation2 = self.file.createIfcShapeRepresentation(Items=[item])
+        ifcopenshell.api.run("geometry.remove_representation", self.file, representation=representation)
+        assert len(self.file.by_type("IfcStyledItem")) == 1
+
+    def test_purging_presentation_layers(self):
+        representation = self.file.createIfcShapeRepresentation()
+        layer = self.file.createIfcPresentationLayerAssignment(AssignedItems=[representation])
+        ifcopenshell.api.run("geometry.remove_representation", self.file, representation=representation)
+        assert len(self.file.by_type("IfcPresentationLayerAssignment")) == 0
+        assert len(self.file.by_type("IfcShapeRepresentation")) == 0
+
+    def test_not_purging_presentation_layers_still_in_use(self):
+        representation = self.file.createIfcShapeRepresentation()
+        representation2 = self.file.createIfcShapeRepresentation()
+        layer = self.file.createIfcPresentationLayerAssignment(AssignedItems=[representation, representation2])
+        ifcopenshell.api.run("geometry.remove_representation", self.file, representation=representation)
+        assert len(self.file.by_type("IfcPresentationLayerAssignment")) == 1
+        assert len(self.file.by_type("IfcShapeRepresentation")) == 1