From b8136d47621c911a230ff457c89189229c5c34fc Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Sun, 22 Mar 2026 15:06:10 +1100 Subject: [PATCH] Prevent cyclic references when assigning nesting or aggregation Walk up the full hierarchy via get_parent() in can_nest() and can_aggregate() to reject assignments that would create a cycle. Also reject self-assignment. Fix #7248 Co-Authored-By: Claude Opus 4.6 --- src/bonsai/bonsai/tool/aggregate.py | 37 +++++++++++++++++++------- src/bonsai/bonsai/tool/nest.py | 20 +++++++++++--- src/bonsai/test/tool/test_aggregate.py | 37 ++++++++++++++++++++++++++ src/bonsai/test/tool/test_nest.py | 37 ++++++++++++++++++++++++++ 4 files changed, 119 insertions(+), 12 deletions(-) diff --git a/src/bonsai/bonsai/tool/aggregate.py b/src/bonsai/bonsai/tool/aggregate.py index 43b155a5aa..1f7f601862 100644 --- a/src/bonsai/bonsai/tool/aggregate.py +++ b/src/bonsai/bonsai/tool/aggregate.py @@ -49,21 +49,40 @@ class Aggregate(bonsai.core.tool.Aggregate): related_object = tool.Ifc.get_entity(related_obj) if not relating_object or not related_object: return False + if relating_object == related_object: + return False + + is_compatible_class = False if (relating_object.is_a("IfcElement") or relating_object.is_a("IfcElementType")) and related_object.is_a( "IfcElement" ): - return True - if tool.Ifc.get_schema() == "IFC2X3": + is_compatible_class = True + elif tool.Ifc.get_schema() == "IFC2X3": if relating_object.is_a("IfcSpatialStructureElement") and related_object.is_a("IfcSpatialStructureElement"): - return True - if relating_object.is_a("IfcProject") and related_object.is_a("IfcSpatialStructureElement"): - return True + is_compatible_class = True + elif relating_object.is_a("IfcProject") and related_object.is_a("IfcSpatialStructureElement"): + is_compatible_class = True else: if relating_object.is_a("IfcSpatialElement") and related_object.is_a("IfcSpatialElement"): - return True - if relating_object.is_a("IfcProject") and related_object.is_a("IfcSpatialElement"): - return True - return False + is_compatible_class = True + elif relating_object.is_a("IfcProject") and related_object.is_a("IfcSpatialElement"): + is_compatible_class = True + + if not is_compatible_class: + return False + + # Prevent cyclic references: walk up the full hierarchy from the + # proposed parent and reject if we encounter the proposed child. + ancestor = ifcopenshell.util.element.get_parent(relating_object) + seen = {relating_object} + while ancestor: + if ancestor == related_object: + return False + if ancestor in seen: + break + seen.add(ancestor) + ancestor = ifcopenshell.util.element.get_parent(ancestor) + return True @classmethod def has_physical_body_representation(cls, element: ifcopenshell.entity_instance) -> bool: diff --git a/src/bonsai/bonsai/tool/nest.py b/src/bonsai/bonsai/tool/nest.py index 37d0fa678f..5a1b2c83b2 100644 --- a/src/bonsai/bonsai/tool/nest.py +++ b/src/bonsai/bonsai/tool/nest.py @@ -45,9 +45,23 @@ class Nest(bonsai.core.tool.Nest): related_object = tool.Ifc.get_entity(related_obj) if not relating_object or not related_object: return False - if relating_object.is_a("IfcElement") and related_object.is_a("IfcElement"): - return True - return False + if relating_object == related_object: + return False + is_compatible_class = relating_object.is_a("IfcElement") and related_object.is_a("IfcElement") + if not is_compatible_class: + return False + # Prevent cyclic references: walk up the full hierarchy from the + # proposed parent and reject if we encounter the proposed child. + ancestor = ifcopenshell.util.element.get_parent(relating_object) + seen = {relating_object} + while ancestor: + if ancestor == related_object: + return False + if ancestor in seen: + break + seen.add(ancestor) + ancestor = ifcopenshell.util.element.get_parent(ancestor) + return True @classmethod def disable_editing(cls, obj: bpy.types.Object) -> None: diff --git a/src/bonsai/test/tool/test_aggregate.py b/src/bonsai/test/tool/test_aggregate.py index bab71b2c4a..4fd9388c13 100644 --- a/src/bonsai/test/tool/test_aggregate.py +++ b/src/bonsai/test/tool/test_aggregate.py @@ -18,6 +18,7 @@ import bpy import ifcopenshell +import ifcopenshell.api.aggregate import ifcopenshell.api.context import ifcopenshell.api.geometry import ifcopenshell.api.root @@ -99,6 +100,42 @@ class TestCanAggregate(NewFile): subelement_obj = bpy.data.objects.new("Object", None) assert subject.can_aggregate(element_obj, subelement_obj) is False + def test_element_cannot_aggregate_to_itself(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + element = ifc.createIfcElementAssembly() + element_obj = bpy.data.objects.new("Object", None) + tool.Ifc.link(element, element_obj) + assert subject.can_aggregate(element_obj, element_obj) is False + + def test_cyclic_aggregation_is_prevented(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + assembly_a = ifc.createIfcElementAssembly() + assembly_a_obj = bpy.data.objects.new("AssemblyA", None) + tool.Ifc.link(assembly_a, assembly_a_obj) + beam = ifc.createIfcBeam() + beam_obj = bpy.data.objects.new("Beam", None) + tool.Ifc.link(beam, beam_obj) + ifcopenshell.api.aggregate.assign_object(ifc, products=[beam], relating_object=assembly_a) + assert subject.can_aggregate(beam_obj, assembly_a_obj) is False + + def test_deep_cyclic_aggregation_is_prevented(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + assembly_a = ifc.createIfcElementAssembly() + assembly_a_obj = bpy.data.objects.new("AssemblyA", None) + tool.Ifc.link(assembly_a, assembly_a_obj) + assembly_b = ifc.createIfcElementAssembly() + assembly_b_obj = bpy.data.objects.new("AssemblyB", None) + tool.Ifc.link(assembly_b, assembly_b_obj) + beam = ifc.createIfcBeam() + beam_obj = bpy.data.objects.new("Beam", None) + tool.Ifc.link(beam, beam_obj) + ifcopenshell.api.aggregate.assign_object(ifc, products=[assembly_b], relating_object=assembly_a) + ifcopenshell.api.aggregate.assign_object(ifc, products=[beam], relating_object=assembly_b) + assert subject.can_aggregate(beam_obj, assembly_a_obj) is False + class TestHasPhysicalBodyRepresentation(NewFile): def test_run(self): diff --git a/src/bonsai/test/tool/test_nest.py b/src/bonsai/test/tool/test_nest.py index 14acfa18bc..368ea9a923 100644 --- a/src/bonsai/test/tool/test_nest.py +++ b/src/bonsai/test/tool/test_nest.py @@ -19,6 +19,7 @@ import bpy import ifcopenshell import ifcopenshell.api +import ifcopenshell.api.nest import ifcopenshell.api.spatial import bonsai.core.tool @@ -51,6 +52,42 @@ class TestCanNest(NewFile): subelement_obj = bpy.data.objects.new("Object", None) assert subject.can_nest(element_obj, subelement_obj) is False + def test_element_cannot_nest_to_itself(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + element = ifc.createIfcWall() + element_obj = bpy.data.objects.new("Object", None) + tool.Ifc.link(element, element_obj) + assert subject.can_nest(element_obj, element_obj) is False + + def test_cyclic_nesting_is_prevented(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + wall_a = ifc.createIfcWall() + wall_a_obj = bpy.data.objects.new("WallA", None) + tool.Ifc.link(wall_a, wall_a_obj) + wall_b = ifc.createIfcWall() + wall_b_obj = bpy.data.objects.new("WallB", None) + tool.Ifc.link(wall_b, wall_b_obj) + ifcopenshell.api.nest.assign_object(ifc, related_objects=[wall_b], relating_object=wall_a) + assert subject.can_nest(wall_b_obj, wall_a_obj) is False + + def test_deep_cyclic_nesting_is_prevented(self): + ifc = ifcopenshell.file() + tool.Ifc.set(ifc) + wall_a = ifc.createIfcWall() + wall_a_obj = bpy.data.objects.new("WallA", None) + tool.Ifc.link(wall_a, wall_a_obj) + wall_b = ifc.createIfcWall() + wall_b_obj = bpy.data.objects.new("WallB", None) + tool.Ifc.link(wall_b, wall_b_obj) + wall_c = ifc.createIfcWall() + wall_c_obj = bpy.data.objects.new("WallC", None) + tool.Ifc.link(wall_c, wall_c_obj) + ifcopenshell.api.nest.assign_object(ifc, related_objects=[wall_b], relating_object=wall_a) + ifcopenshell.api.nest.assign_object(ifc, related_objects=[wall_c], relating_object=wall_b) + assert subject.can_nest(wall_c_obj, wall_a_obj) is False + class TestDisableEditing(NewFile): def test_run(self):