From 03a66d2806dafa76e0c317d78c5771af798dd5bc Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Tue, 21 Feb 2023 10:59:47 +1100 Subject: [PATCH] Fix #2787. Fix creating invalid orphaned property and group relationships when removing systems / groups / elements. --- .../ifcopenshell/api/group/remove_group.py | 22 +++++++++- .../ifcopenshell/api/root/remove_product.py | 13 +++++- .../ifcopenshell/api/system/remove_system.py | 22 +++++++++- .../test/api/group/__init__.py | 0 .../test/api/group/test_remove_group.py | 43 +++++++++++++++++++ .../test/api/root/test_remove_product.py | 7 +++ .../test/api/system/test_remove_system.py | 16 +++++++ 7 files changed, 118 insertions(+), 5 deletions(-) create mode 100644 src/ifcopenshell-python/test/api/group/__init__.py create mode 100644 src/ifcopenshell-python/test/api/group/test_remove_group.py diff --git a/src/ifcopenshell-python/ifcopenshell/api/group/remove_group.py b/src/ifcopenshell-python/ifcopenshell/api/group/remove_group.py index 83ddf23851..b7c7dfb38f 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/group/remove_group.py +++ b/src/ifcopenshell-python/ifcopenshell/api/group/remove_group.py @@ -16,6 +16,9 @@ # You should have received a copy of the GNU Lesser General Public License # along with IfcOpenShell. If not, see . +import ifcopenshell +import ifcopenshell.api + class Usecase: def __init__(self, file, group=None): @@ -40,6 +43,21 @@ class Usecase: self.settings = {"group": group} def execute(self): - for rel in self.settings["group"].IsGroupedBy or []: - self.file.remove(rel) + for inverse_id in [i.id() for i in self.file.get_inverse(self.settings["group"])]: + try: + inverse = self.file.by_id(inverse_id) + except: + continue + if inverse.is_a("IfcRelDefinesByProperties"): + ifcopenshell.api.run( + "pset.remove_pset", + self.file, + product=self.settings["group"], + pset=inverse.RelatingPropertyDefinition, + ) + elif inverse.is_a("IfcRelAssignsToGroup"): + if inverse.RelatingGroup == self.settings["group"]: + self.file.remove(inverse) + elif len(inverse.RelatedObjects) == 1: + self.file.remove(inverse) self.file.remove(self.settings["group"]) diff --git a/src/ifcopenshell-python/ifcopenshell/api/root/remove_product.py b/src/ifcopenshell-python/ifcopenshell/api/root/remove_product.py index cb98c7979f..af10d9b149 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/root/remove_product.py +++ b/src/ifcopenshell-python/ifcopenshell/api/root/remove_product.py @@ -27,12 +27,20 @@ class Usecase: product, but also all of its relationships. It is always recommended to use this function to prevent orphaned data in your IFC model. + This is intended to be used for removing: + + - IfcAnnotation + - IfcElement + - IfcElementType + - IfcSpatialElement + - IfcSpatialElementType + For example, geometric representations are removed. Placement coordinates are also removed. Properties are removed. Material, type, containment, aggregation, and nesting relationships are removed (but naturally, the materials, types, containers, etc themselves remain). - :param product: The IfcProduct to remove. + :param product: The element to remove. :type product: ifcopenshell.entity_instance.entity_instance :return: None :rtype: None @@ -119,4 +127,7 @@ class Usecase: self.file.remove(inverse) elif inverse.is_a("IfcRelConnectsPathElements"): self.file.remove(inverse) + elif inverse.is_a("IfcRelAssignsToGroup"): + if len(inverse.RelatedObjects) == 1: + self.file.remove(inverse) self.file.remove(self.settings["product"]) diff --git a/src/ifcopenshell-python/ifcopenshell/api/system/remove_system.py b/src/ifcopenshell-python/ifcopenshell/api/system/remove_system.py index bbde6b5c58..d135e9f0ad 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/system/remove_system.py +++ b/src/ifcopenshell-python/ifcopenshell/api/system/remove_system.py @@ -16,6 +16,9 @@ # You should have received a copy of the GNU Lesser General Public License # along with IfcOpenShell. If not, see . +import ifcopenshell +import ifcopenshell.api + class Usecase: def __init__(self, file, system=None): @@ -42,6 +45,21 @@ class Usecase: self.settings = {"system": system} def execute(self): - for rel in self.settings["system"].IsGroupedBy or []: - self.file.remove(rel) + for inverse_id in [i.id() for i in self.file.get_inverse(self.settings["system"])]: + try: + inverse = self.file.by_id(inverse_id) + except: + continue + if inverse.is_a("IfcRelDefinesByProperties"): + ifcopenshell.api.run( + "pset.remove_pset", + self.file, + product=self.settings["system"], + pset=inverse.RelatingPropertyDefinition, + ) + elif inverse.is_a("IfcRelAssignsToGroup"): + if inverse.RelatingGroup == self.settings["system"]: + self.file.remove(inverse) + elif len(inverse.RelatedObjects) == 1: + self.file.remove(inverse) self.file.remove(self.settings["system"]) diff --git a/src/ifcopenshell-python/test/api/group/__init__.py b/src/ifcopenshell-python/test/api/group/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/ifcopenshell-python/test/api/group/test_remove_group.py b/src/ifcopenshell-python/test/api/group/test_remove_group.py new file mode 100644 index 0000000000..c5b55f3006 --- /dev/null +++ b/src/ifcopenshell-python/test/api/group/test_remove_group.py @@ -0,0 +1,43 @@ +# IfcOpenShell - IFC toolkit and geometry engine +# Copyright (C) 2022 Dion Moult +# +# This file is part of IfcOpenShell. +# +# IfcOpenShell is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# IfcOpenShell is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with IfcOpenShell. If not, see . + +import test.bootstrap +import ifcopenshell.api + + +class TestRemoveGroup(test.bootstrap.IFC4): + def test_removing_a_group(self): + group = ifcopenshell.api.run("group.add_group", self.file) + ifcopenshell.api.run("group.remove_group", self.file, group=group) + assert len(self.file.by_type("IfcGroup")) == 0 + + def test_removing_orphaned_group_relationships(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcPump") + group = ifcopenshell.api.run("group.add_group", self.file) + ifcopenshell.api.run("group.assign_group", self.file, products=[element], group=group) + ifcopenshell.api.run("group.remove_group", self.file, group=group) + assert not self.file.by_type("IfcRelAssignsToGroup") + + def test_removing_orphaned_property_relationships(self): + group = ifcopenshell.api.run("group.add_group", self.file) + pset = ifcopenshell.api.run("pset.add_pset", self.file, product=group, name="Foo_Bar") + ifcopenshell.api.run("pset.edit_pset", self.file, pset=pset, properties={"Foo": "Bar"}) + ifcopenshell.api.run("group.remove_group", self.file, group=group) + assert not self.file.by_type("IfcRelDefinesByProperties") + assert not self.file.by_type("IfcPropertySet") + assert not self.file.by_type("IfcPropertySingleValue") diff --git a/src/ifcopenshell-python/test/api/root/test_remove_product.py b/src/ifcopenshell-python/test/api/root/test_remove_product.py index 48b4a81305..c66ab9686b 100644 --- a/src/ifcopenshell-python/test/api/root/test_remove_product.py +++ b/src/ifcopenshell-python/test/api/root/test_remove_product.py @@ -238,3 +238,10 @@ class TestRemoveProduct(test.bootstrap.IFC4): ) ifcopenshell.api.run("root.remove_product", self.file, product=element) assert not self.file.by_type("IfcRelSpaceBoundary") + + def test_removing_orphaned_group_relationships(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcWall") + group = ifcopenshell.api.run("group.add_group", self.file, Name="Unit 1A") + ifcopenshell.api.run("group.assign_group", self.file, products=[element], group=group) + ifcopenshell.api.run("root.remove_product", self.file, product=element) + assert not self.file.by_type("IfcRelAssignsToGroup") diff --git a/src/ifcopenshell-python/test/api/system/test_remove_system.py b/src/ifcopenshell-python/test/api/system/test_remove_system.py index 34d99189af..86118cf943 100644 --- a/src/ifcopenshell-python/test/api/system/test_remove_system.py +++ b/src/ifcopenshell-python/test/api/system/test_remove_system.py @@ -25,3 +25,19 @@ class TestRemoveSystem(test.bootstrap.IFC4): system = ifcopenshell.api.run("system.add_system", self.file, ifc_class="IfcSystem") ifcopenshell.api.run("system.remove_system", self.file, system=system) assert len(self.file.by_type("IfcSystem")) == 0 + + def test_removing_orphaned_group_relationships(self): + element = ifcopenshell.api.run("root.create_entity", self.file, ifc_class="IfcPump") + system = ifcopenshell.api.run("system.add_system", self.file, ifc_class="IfcSystem") + ifcopenshell.api.run("system.assign_system", self.file, product=element, system=system) + ifcopenshell.api.run("system.remove_system", self.file, system=system) + assert not self.file.by_type("IfcRelAssignsToGroup") + + def test_removing_orphaned_property_relationships(self): + system = ifcopenshell.api.run("system.add_system", self.file, ifc_class="IfcSystem") + pset = ifcopenshell.api.run("pset.add_pset", self.file, product=system, name="Foo_Bar") + ifcopenshell.api.run("pset.edit_pset", self.file, pset=pset, properties={"Foo": "Bar"}) + ifcopenshell.api.run("system.remove_system", self.file, system=system) + assert not self.file.by_type("IfcRelDefinesByProperties") + assert not self.file.by_type("IfcPropertySet") + assert not self.file.by_type("IfcPropertySingleValue")