diff --git a/src/bonsai/bonsai/bim/module/geometry/operator.py b/src/bonsai/bonsai/bim/module/geometry/operator.py index 05a7b43625..e21668679b 100644 --- a/src/bonsai/bonsai/bim/module/geometry/operator.py +++ b/src/bonsai/bonsai/bim/module/geometry/operator.py @@ -890,6 +890,16 @@ class OverrideDelete(bpy.types.Operator): # Track aggregates before deleting their parts aggregates_to_check = self.track_aggregates(objects_to_remove) + # Snapshot the set of IFC entity ids being deleted in this batch so the + # connection-rel cascade inside `delete_ifc_object` can suppress + # partner-side regenerate when the partner is also about to vanish. + batch_being_deleted_ids: set[int] = set() + for obj in objects_to_remove: + if not tool.Blender.is_valid_data_block(obj): + continue + if (entity := tool.Ifc.get_entity(obj)) is not None: + batch_being_deleted_ids.add(entity.id()) + clear_active_object = True for i, obj in enumerate(objects_to_remove, 1): @@ -931,7 +941,7 @@ class OverrideDelete(bpy.types.Operator): if tool.Drawing.is_auto_annotation(element): self.report({"INFO"}, "References cannot be deleted. Exclude the referenced element instead.") continue - tool.Geometry.delete_ifc_object(obj) + tool.Geometry.delete_ifc_object(obj, batch_being_deleted_ids=batch_being_deleted_ids) elif tool.Geometry.is_representation_item(obj): tool.Geometry.delete_ifc_item(obj) else: diff --git a/src/bonsai/bonsai/core/connection.py b/src/bonsai/bonsai/core/connection.py new file mode 100644 index 0000000000..71a2bb871c --- /dev/null +++ b/src/bonsai/bonsai/core/connection.py @@ -0,0 +1,99 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Shared post-disconnect cleanup dispatch. + +Used by both ``bim.disconnect_elements`` (explicit user disconnect) and the +connection cascade in ``tool.Geometry.delete_ifc_object`` (implicit +disconnect-on-delete). Each rel kind returned by +:py:meth:`bonsai.tool.connection.Connection.find_rels` / +:py:meth:`find_rels_for_element` maps to a single arm here, so adding a new +rel kind means extending one dispatch table — both call sites benefit +automatically and the AST forward-compat guard enforces coverage. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +import bonsai.core.geometry +from bonsai.core.model import regenerate_wall_to_underside + +if TYPE_CHECKING: + import bpy + import ifcopenshell + + import bonsai.tool as tool + + +def disconnect_rel( + ifc: "type[tool.Ifc]", + geometry: "type[tool.Geometry]", + model: "type[tool.Model]", + connection: "type[tool.Connection]", + rel: "ifcopenshell.entity_instance", + kind: str, + elem: "ifcopenshell.entity_instance", + partner: "ifcopenshell.entity_instance", + skip_elem_recreate: bool = False, + skip_partner_recreate: bool = False, +) -> None: + """Run the post-disconnect cleanup for one rel. + + ``elem`` and ``partner`` are the two endpoints. The ``skip_*_recreate`` + flags suppress per-side regenerate / recreate work — used by the + cascade-on-delete to avoid re-extruding entities that are about to be + removed by ``remove_product``. For the disconnect operator (where neither + endpoint is being deleted), both flags stay False and the full cleanup + runs on both sides. + """ + if kind == "path": + bonsai.core.geometry.remove_connection(geometry, connection=rel) + if not skip_elem_recreate: + elem_obj = ifc.get_object(elem) + if elem_obj is not None: + model.recreate_wall(elem, elem_obj) + if not skip_partner_recreate: + partner_obj = ifc.get_object(partner) + if partner_obj is not None: + model.recreate_wall(partner, partner_obj) + elif kind == "element-top": + wall, _slab = connection.orient_element_top(rel, elem, partner) + ifc.run( + "geometry.disconnect_element", + relating_element=rel.RelatingElement, + related_element=rel.RelatedElement, + ) + # Skip the wall-side regenerate when the wall is itself being deleted — + # either it's the elem of this cascade pass, or it's the partner that + # was queued earlier in the same batch. + if (wall is elem and skip_elem_recreate) or (wall is partner and skip_partner_recreate): + return + wall_obj = ifc.get_object(wall) + if wall_obj is not None: + regenerate_wall_to_underside(ifc, geometry, model, [wall_obj]) + elif kind == "element": + ifc.run( + "geometry.disconnect_element", + relating_element=rel.RelatingElement, + related_element=rel.RelatedElement, + ) + else: + raise ValueError(f"Unknown rel kind: {kind!r}") diff --git a/src/bonsai/bonsai/core/model.py b/src/bonsai/bonsai/core/model.py index 874675ea7f..30b1c9b515 100644 --- a/src/bonsai/bonsai/core/model.py +++ b/src/bonsai/bonsai/core/model.py @@ -167,12 +167,22 @@ def regenerate_wall_to_underside( model: type[tool.Model], wall_objs: list[bpy.types.Object], ) -> None: - """Re-clip walls to their connected underside objects after the slab has moved.""" + """Re-clip walls to their connected underside objects after the slab has moved. + + When a wall has no remaining slab connections — the case reached after the + last TOP rel is severed (via disconnect or via cascade-on-slab-delete) — the + stale trim booleans are cleaned up so the wall reverts to its pre-clip + extrusion instead of holding orphan ``IfcBooleanResult`` items and a dead + ``BBIM_Boolean`` pset. + """ clipped_objs = [] + reverted_objs = [] for obj in wall_objs: wall = ifc.get_entity(obj) slab_objs = model.get_connected_slab_objs(wall) if not slab_objs: + model.remove_wall_to_underside_booleans(wall) + reverted_objs.append(obj) continue if ifc.is_moved(obj): geometry.run_edit_object_placement(obj=obj) @@ -185,8 +195,9 @@ def regenerate_wall_to_underside( if clip: model.clip_wall_to_slab(wall, clip) clipped_objs.append(obj) - if clipped_objs: - model.reload_body_representation(clipped_objs) + refresh_objs = clipped_objs + reverted_objs + if refresh_objs: + model.reload_body_representation(refresh_objs) def extend_wall_to_slab( diff --git a/src/bonsai/bonsai/core/tool.py b/src/bonsai/bonsai/core/tool.py index f48165bd4c..91003174ad 100644 --- a/src/bonsai/bonsai/core/tool.py +++ b/src/bonsai/bonsai/core/tool.py @@ -195,6 +195,14 @@ class Collector: def assign(cls, obj, should_clean_users_collection=False): pass +@interface +class Connection: + def find_rel(cls, elem_a, elem_b): pass + def find_rels(cls, elem_a, elem_b): pass + def find_rels_for_element(cls, elem): pass + def orient_element_top(cls, rel, elem_a, elem_b): pass + + @interface class Context: def clear_context(cls): pass @@ -694,11 +702,13 @@ class Model: def load_openings(cls, openings): pass def purge_scene_openings(cls): pass def recalculate_walls(cls, objs): pass + def recreate_wall(cls, element, obj): pass def regenerate_array(cls, parent, data): pass def regenerate_profile(cls, obj): pass def regenerate_slab(cls, obj): pass def reload_body_representation(cls, obj_or_objects): pass def remove_wall_to_underside_booleans(cls, wall): pass + def strip_underside_booleans(cls, wall): pass def replace_object_ifc_representation(cls, ifc_file, ifc_context, obj, new_representation): pass diff --git a/src/bonsai/bonsai/tool/connection.py b/src/bonsai/bonsai/tool/connection.py index 055eb47a68..4ec154b573 100644 --- a/src/bonsai/bonsai/tool/connection.py +++ b/src/bonsai/bonsai/tool/connection.py @@ -93,6 +93,43 @@ class Connection: rels = cls.find_rels(elem_a, elem_b) return rels[0] if rels else (None, None) + @classmethod + def find_rels_for_element( + cls, + elem: "ifcopenshell.entity_instance", + ) -> "list[tuple[ifcopenshell.entity_instance, str, ifcopenshell.entity_instance]]": + """Return every supported rel touching ``elem`` as ``(rel, kind, partner)`` + triples. ``partner`` is the *other* element on the rel — the side cascade + cleanup must operate on when ``elem`` is being deleted. + + Mirrors :py:meth:`find_rels`'s kind taxonomy. The single-element entry + point lets the cascade-on-delete in ``tool.Geometry.delete_ifc_object`` + enumerate everything the disconnect operator would handle pairwise. + """ + result: list[tuple["ifcopenshell.entity_instance", str, "ifcopenshell.entity_instance"]] = [] + seen: set[int] = set() + + def _record(rel, kind, partner): + if partner is None or rel.id() in seen: + return + seen.add(rel.id()) + result.append((rel, kind, partner)) + + for rel in getattr(elem, "ConnectedTo", []) or (): + if rel.is_a("IfcRelConnectsPathElements"): + _record(rel, "path", getattr(rel, "RelatedElement", None)) + elif rel.is_a("IfcRelConnectsElements"): + kind = "element-top" if getattr(rel, "Description", None) == "TOP" else "element" + _record(rel, kind, getattr(rel, "RelatedElement", None)) + for rel in getattr(elem, "ConnectedFrom", []) or (): + if rel.is_a("IfcRelConnectsPathElements"): + _record(rel, "path", getattr(rel, "RelatingElement", None)) + elif rel.is_a("IfcRelConnectsElements"): + kind = "element-top" if getattr(rel, "Description", None) == "TOP" else "element" + _record(rel, kind, getattr(rel, "RelatingElement", None)) + + return result + @classmethod def orient_element_top( cls, diff --git a/src/bonsai/bonsai/tool/geometry.py b/src/bonsai/bonsai/tool/geometry.py index e0b719f065..e7de9c1f6a 100644 --- a/src/bonsai/bonsai/tool/geometry.py +++ b/src/bonsai/bonsai/tool/geometry.py @@ -65,6 +65,7 @@ from typing_extensions import TypeIs import bonsai.bim.helper import bonsai.bim.import_ifc +import bonsai.core.connection import bonsai.core.drawing import bonsai.core.geometry import bonsai.core.root @@ -271,12 +272,38 @@ class Geometry(bonsai.core.tool.Geometry): bpy.data.objects.remove(obj) @classmethod - def delete_ifc_object(cls, obj: bpy.types.Object) -> None: + def delete_ifc_object( + cls, + obj: bpy.types.Object, + batch_being_deleted_ids: Optional[set[int]] = None, + ) -> None: ifc_file = tool.Ifc.get() element = tool.Ifc.get_entity(obj) if not element: return - elif element.is_a("IfcAnnotation"): + # Cascade connection-rel teardown — symmetric to bim.disconnect_elements. + # When a slab connected to a wall via IfcRelConnectsElements(TOP) is deleted, + # the wall's trim booleans + BBIM_Boolean pset would otherwise be orphaned. + # skip_elem_recreate is always True here because we're inside delete: the + # element is about to vanish, so re-extruding it would be wasted work. + # skip_partner_recreate fires only when the partner is also queued in the + # same OverrideDelete batch. + if element.is_a("IfcRoot"): + skip_ids = batch_being_deleted_ids or set() + for rel, kind, partner in tool.Connection.find_rels_for_element(element): + bonsai.core.connection.disconnect_rel( + tool.Ifc, + tool.Geometry, + tool.Model, + tool.Connection, + rel=rel, + kind=kind, + elem=element, + partner=partner, + skip_elem_recreate=True, + skip_partner_recreate=(partner.id() in skip_ids), + ) + if element.is_a("IfcAnnotation"): if element.ObjectType == "DRAWING": return bonsai.core.drawing.remove_drawing(tool.Ifc, tool.Drawing, drawing=element) elif tool.Drawing.is_auto_annotation(element): @@ -2348,6 +2375,16 @@ class Geometry(bonsai.core.tool.Geometry): old_to_new[element] = [new] if new.is_a("IfcRelSpaceBoundary"): tool.Boundary.decorate_boundary(new_obj) + # Slab-trim booleans (from extend_walls_to_underside) belong to + # the source wall's connection, not the copy. Strip them so the + # duplicate reverts to its pre-clip extrusion — mirrors the way + # filling rels are dropped while manual booleans persist on copy. + # Reload the body when something was stripped so the viewport + # immediately shows the unclipped geometry; otherwise the user + # sees a stale mesh until they Shift+G, which is easy to miss. + if new.is_a("IfcWall"): + if tool.Model.strip_underside_booleans(new): + tool.Model.reload_body_representation(new_obj) # Remap Blender parent relationships for duplicated objects for old_obj_name, new_obj_name in old_obj_name_to_new_obj_name.items(): diff --git a/src/bonsai/bonsai/tool/model.py b/src/bonsai/bonsai/tool/model.py index d126657e36..8882d0c65f 100644 --- a/src/bonsai/bonsai/tool/model.py +++ b/src/bonsai/bonsai/tool/model.py @@ -909,6 +909,48 @@ class Model(bonsai.core.tool.Model): """Return True if element has an IfcRelConnectsElements(TOP) relationship.""" return any(rel.is_a("IfcRelConnectsElements") and rel.Description == "TOP" for rel in element.ConnectedFrom) + @classmethod + def strip_underside_booleans(cls, wall: ifcopenshell.entity_instance) -> bool: + """Remove slab-trim ``IfcBooleanResult`` items from a wall's body chain. + + Returns ``True`` if any boolean was removed, so the caller knows whether + a Blender-side body reload is needed to surface the geometry change. + + Hook for the duplicate path (Shift+D): the source wall's clip booleans + don't make sense on a copy pulled away from the slab. Booleans whose + ``SecondOperand.is_a("IfcTessellatedFaceSet")`` are removed — same + imprecise discriminator the rest of the wall-to-underside machinery + uses (manual cuts authored from tessellated meshes would also be + stripped, but most manual cuts use ``IfcExtrudedAreaSolid`` / CSG + primitives and are unaffected). + + Cannot reuse ``remove_wall_to_underside_booleans`` here because the + duplicate's ``BBIM_Boolean.Data`` holds the source wall's stale ids — + ``get_manual_booleans`` returns empty on the copy and the helper + early-returns. The duplicate hook works directly off the chain. + """ + representation = tool.Geometry.get_body_representation(wall) + if not representation: + return False + chain = cls.get_booleans(wall, representation) + to_remove = [ + b for b in chain if (sec := b.SecondOperand) is not None and sec.is_a("IfcTessellatedFaceSet") + ] + for b in to_remove: + tool.Geometry.remove_representation_item(b.SecondOperand, wall) + # Sweep the now-stale BBIM_Boolean entries on the copy (their ids point + # at booleans that were never in this wall's chain — they survived the + # ifcopenshell deep copy as JSON text in the pset payload). + pset_data = ifcopenshell.util.element.get_pset(wall, "BBIM_Boolean") + if pset_data: + representation = tool.Geometry.get_body_representation(wall) + chain_ids = {b.id() for b in cls.get_booleans(wall, representation)} if representation else set() + stored_ids = set(json.loads(pset_data["Data"])) + stale_ids = stored_ids - chain_ids + if stale_ids: + cls.unmark_manual_booleans(wall, list(stale_ids)) + return bool(to_remove) + @classmethod def remove_wall_to_underside_booleans(cls, wall: ifcopenshell.entity_instance) -> None: """Remove all IfcBooleanResult items previously added by extend_walls_to_underside.""" diff --git a/src/bonsai/test/bim/module/model/test_disconnect_elements.py b/src/bonsai/test/bim/module/model/test_disconnect_elements.py index 02464e1625..9eb37481de 100644 --- a/src/bonsai/test/bim/module/model/test_disconnect_elements.py +++ b/src/bonsai/test/bim/module/model/test_disconnect_elements.py @@ -121,6 +121,52 @@ def test_find_rels_dedups_by_id(): assert len(rels) == 1 +# --------------------------------------------------------------------------- +# tool.Connection.find_rels_for_element — single-element entry point +# --------------------------------------------------------------------------- + + +def test_find_rels_for_element_returns_kind_and_partner_per_rel(): + """Cascade-on-delete needs every rel touching one element plus the partner + element on the other side of each rel — that's the cleanup target.""" + elem = _elem() + partner_a = _elem() + partner_b = _elem() + rel_path = _rel("IfcRelConnectsPathElements", related=partner_a, rel_id=1) + rel_top = _rel("IfcRelConnectsElements", relating=partner_b, description="TOP", rel_id=2) + elem.ConnectedTo = [rel_path] + elem.ConnectedFrom = [rel_top] + + result = tool.Connection.find_rels_for_element(elem) + + assert (rel_path, "path", partner_a) in result + assert (rel_top, "element-top", partner_b) in result + assert len(result) == 2 + + +def test_find_rels_for_element_dedups_by_rel_id(): + elem = _elem() + partner = _elem() + rel = _rel("IfcRelConnectsPathElements", related=partner, relating=partner, rel_id=1) + elem.ConnectedTo = [rel] + elem.ConnectedFrom = [rel] + + result = tool.Connection.find_rels_for_element(elem) + + assert len(result) == 1 + + +def test_find_rels_for_element_skips_rels_without_partner(): + """Defensive: a malformed rel missing the opposite-side attribute should not + crash — record nothing for it rather than emit a (rel, kind, None) triple + that would later trip a None-deref in the dispatch.""" + elem = _elem() + bad = _rel("IfcRelConnectsPathElements", related=None, rel_id=1) + elem.ConnectedTo = [bad] + + assert tool.Connection.find_rels_for_element(elem) == [] + + # --------------------------------------------------------------------------- # tool.Connection.find_rel — first-match convenience # --------------------------------------------------------------------------- @@ -165,15 +211,16 @@ def _make_op(*, a_guid="A", b_guid="B"): return op -def test_disconnect_path_removes_all_rels_then_recreates_walls(): +def test_disconnect_dispatches_one_call_per_rel(): + """Operator forwards every rel returned by find_rels to disconnect_rel, + in order — the operator is a thin wrapper; per-kind cleanup logic lives + in core.connection.disconnect_rel and is tested separately.""" from bonsai.bim.module.model.wall import DisconnectElements elem_a = Mock() elem_b = Mock() rel1 = Mock() rel2 = Mock() - obj_a = Mock() - obj_b = Mock() ifc_file = MagicMock() ifc_file.by_guid.side_effect = lambda g: {"A": elem_a, "B": elem_b}[g] @@ -181,45 +228,114 @@ def test_disconnect_path_removes_all_rels_then_recreates_walls(): with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( "bonsai.bim.module.model.wall.tool.Connection.find_rels", - return_value=[(rel1, "path"), (rel2, "path")], - ), patch( - "bonsai.bim.module.model.wall.tool.Ifc.get_object", side_effect=lambda e: {elem_a: obj_a, elem_b: obj_b}[e] - ), patch("bonsai.bim.module.model.wall.bonsai.core.geometry.remove_connection") as remove, patch( - "bonsai.bim.module.model.wall.tool.Model.recreate_wall" - ) as recreate, patch("bonsai.bim.module.model.wall._resync_walls_after_mutation") as resync: + return_value=[(rel1, "path"), (rel2, "element-top")], + ), patch("bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel") as dispatch, patch( + "bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=Mock() + ), patch("bonsai.bim.module.model.wall._resync_walls_after_mutation"): DisconnectElements._perform(op, context=MagicMock()) - assert remove.call_count == 2 - assert recreate.call_count == 2 - resync.assert_called_once_with([obj_a, obj_b]) + assert dispatch.call_count == 2 + # Both rels dispatch with elem=elem_a, partner=elem_b regardless of orientation + # — orient_element_top inside disconnect_rel recovers the wall/slab roles. + for call, expected_rel, expected_kind in zip( + dispatch.call_args_list, [rel1, rel2], ["path", "element-top"] + ): + kw = call.kwargs + assert kw["rel"] is expected_rel + assert kw["kind"] == expected_kind + assert kw["elem"] is elem_a + assert kw["partner"] is elem_b op.report.assert_not_called() -def test_disconnect_element_top_calls_regenerate(): +def test_disconnect_resyncs_path_objs_once_for_path_kind(): + """For path rels the operator collects both endpoint objects and resyncs + drafts once at the end — a Blender-side concern that doesn't belong in + the core dispatch.""" from bonsai.bim.module.model.wall import DisconnectElements - wall = Mock() - slab = Mock() - wall_obj = Mock() + elem_a = Mock() + elem_b = Mock() + obj_a = Mock() + obj_b = Mock() rel = Mock() - rel.RelatedElement = wall ifc_file = MagicMock() - ifc_file.by_guid.side_effect = lambda g: {"A": wall, "B": slab}[g] + ifc_file.by_guid.side_effect = lambda g: {"A": elem_a, "B": elem_b}[g] + op = _make_op() + + with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( + "bonsai.bim.module.model.wall.tool.Connection.find_rels", return_value=[(rel, "path")] + ), patch( + "bonsai.bim.module.model.wall.tool.Ifc.get_object", + side_effect=lambda e: {elem_a: obj_a, elem_b: obj_b}[e], + ), patch("bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel"), patch( + "bonsai.bim.module.model.wall._resync_walls_after_mutation" + ) as resync: + DisconnectElements._perform(op, context=MagicMock()) + + resync.assert_called_once_with([obj_a, obj_b]) + + +def test_disconnect_skips_resync_for_non_path_kind(): + """element-top / element kinds don't need wall-draft resync — that's a + path-specific concern (DumbWallJoiner geometry refresh).""" + from bonsai.bim.module.model.wall import DisconnectElements + + elem_a = Mock() + elem_b = Mock() + rel = Mock() + + ifc_file = MagicMock() + ifc_file.by_guid.side_effect = lambda g: {"A": elem_a, "B": elem_b}[g] op = _make_op() with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( "bonsai.bim.module.model.wall.tool.Connection.find_rels", return_value=[(rel, "element-top")] - ), patch( - "bonsai.bim.module.model.wall.tool.Connection.orient_element_top", return_value=(wall, slab) - ), patch("bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=wall_obj), patch( - "bonsai.bim.module.model.wall.ifcopenshell.api.geometry.disconnect_element" - ) as disc, patch("bonsai.bim.module.model.wall.core.regenerate_wall_to_underside") as regen: + ), patch("bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=Mock()), patch( + "bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel" + ), patch("bonsai.bim.module.model.wall._resync_walls_after_mutation") as resync: DisconnectElements._perform(op, context=MagicMock()) - disc.assert_called_once_with(ifc_file, relating_element=slab, related_element=wall) - regen.assert_called_once() - op.report.assert_not_called() + resync.assert_not_called() + + +def test_disconnect_gizmo_direction_symmetry(): + """The wall-selected gizmo dispatches with element_a=wall, element_b=slab. + The slab-selected gizmo dispatches with element_a=slab, element_b=wall. + Both routes hit disconnect_rel with the same (rel, kind) pair — orientation + recovery happens inside the dispatch, not at the operator layer.""" + from bonsai.bim.module.model.wall import DisconnectElements + + wall = Mock(name="wall") + slab = Mock(name="slab") + rel = Mock() + + ifc_file = MagicMock() + op = _make_op() + + def _run_with_guids(a, b): + ifc_file.by_guid.side_effect = lambda g: {a: wall if a == "WALL" else slab, b: slab if b == "SLAB" else wall}[g] + op.element_a_guid = a + op.element_b_guid = b + with patch("bonsai.bim.module.model.wall.tool.Ifc.get", return_value=ifc_file), patch( + "bonsai.bim.module.model.wall.tool.Connection.find_rels", return_value=[(rel, "element-top")] + ), patch("bonsai.bim.module.model.wall.tool.Ifc.get_object", return_value=Mock()), patch( + "bonsai.bim.module.model.wall.bonsai.core.connection.disconnect_rel" + ) as dispatch, patch("bonsai.bim.module.model.wall._resync_walls_after_mutation"): + DisconnectElements._perform(op, context=MagicMock()) + return dispatch.call_args.kwargs + + wall_first = _run_with_guids("WALL", "SLAB") + slab_first = _run_with_guids("SLAB", "WALL") + + # disconnect_rel sees (rel, "element-top") in both runs; elem/partner swap + # by argument order but orient_element_top inside disconnect_rel resolves + # the wall/slab roles symmetrically. + assert wall_first["rel"] is rel and slab_first["rel"] is rel + assert wall_first["kind"] == slab_first["kind"] == "element-top" + assert {wall_first["elem"], wall_first["partner"]} == {wall, slab} + assert {slab_first["elem"], slab_first["partner"]} == {wall, slab} def test_disconnect_reports_on_unknown_guids(): diff --git a/src/bonsai/test/core/bootstrap.py b/src/bonsai/test/core/bootstrap.py index cd8371e1c3..6715fe8a94 100644 --- a/src/bonsai/test/core/bootstrap.py +++ b/src/bonsai/test/core/bootstrap.py @@ -60,6 +60,13 @@ def collector(): prophet.verify() +@pytest.fixture +def connection(): + prophet = Prophecy(bonsai.core.tool.Connection) + yield prophet + prophet.verify() + + @pytest.fixture def context(): prophet = Prophecy(bonsai.core.tool.Context) diff --git a/src/bonsai/test/core/test_connection.py b/src/bonsai/test/core/test_connection.py new file mode 100644 index 0000000000..5b09bc4dfa --- /dev/null +++ b/src/bonsai/test/core/test_connection.py @@ -0,0 +1,218 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Dispatch tests for ``core.connection.disconnect_rel``. + +The dispatch is the single source of truth for per-kind cleanup shared by the +explicit ``bim.disconnect_elements`` operator and the implicit cascade in +``tool.Geometry.delete_ifc_object``. Each kind has one test that pins which +helpers must be called; the AST forward-compat guard in +``test_connection_forward_compat.py`` then asserts the dispatch table covers +every kind ``Connection.find_rels`` can emit. + +Uses ``unittest.mock`` directly (rather than the Prophecy fixtures) because +the dispatch passes IFC rel entities with attribute access (``rel.RelatingElement``) +that Prophecy's JSON call recorder can't serialize. +""" + +from types import SimpleNamespace +from unittest.mock import Mock, patch + +import pytest + +import bonsai.core.connection as subject + + +def _rel(relating="slab", related="wall"): + return SimpleNamespace(RelatingElement=relating, RelatedElement=related) + + +def _ifc_with_objects(mapping): + ifc = Mock() + ifc.get_object.side_effect = lambda e: mapping.get(e) + ifc.run = Mock() + return ifc + + +class TestDisconnectRelPath: + def test_removes_connection_and_recreates_both_walls(self): + ifc = _ifc_with_objects({"elem_a": "obj_a", "elem_b": "obj_b"}) + geometry = Mock() + model = Mock() + connection = Mock() + + with patch("bonsai.core.connection.bonsai.core.geometry.remove_connection") as remove: + subject.disconnect_rel( + ifc, geometry, model, connection, + rel="rel", kind="path", elem="elem_a", partner="elem_b", + ) + + remove.assert_called_once_with(geometry, connection="rel") + model.recreate_wall.assert_any_call("elem_a", "obj_a") + model.recreate_wall.assert_any_call("elem_b", "obj_b") + assert model.recreate_wall.call_count == 2 + + def test_skip_elem_recreate_suppresses_elem_side(self): + """Cascade case: elem is being deleted — don't recreate it.""" + ifc = _ifc_with_objects({"elem": "elem_obj", "partner": "partner_obj"}) + geometry = Mock() + model = Mock() + connection = Mock() + + with patch("bonsai.core.connection.bonsai.core.geometry.remove_connection"): + subject.disconnect_rel( + ifc, geometry, model, connection, + rel="rel", kind="path", elem="elem", partner="partner", + skip_elem_recreate=True, + ) + + model.recreate_wall.assert_called_once_with("partner", "partner_obj") + + def test_skip_partner_recreate_suppresses_partner_side(self): + ifc = _ifc_with_objects({"elem": "elem_obj", "partner": "partner_obj"}) + geometry = Mock() + model = Mock() + connection = Mock() + + with patch("bonsai.core.connection.bonsai.core.geometry.remove_connection"): + subject.disconnect_rel( + ifc, geometry, model, connection, + rel="rel", kind="path", elem="elem", partner="partner", + skip_partner_recreate=True, + ) + + model.recreate_wall.assert_called_once_with("elem", "elem_obj") + + def test_both_skips_means_only_remove_rel(self): + ifc = _ifc_with_objects({}) + geometry = Mock() + model = Mock() + connection = Mock() + + with patch("bonsai.core.connection.bonsai.core.geometry.remove_connection") as remove: + subject.disconnect_rel( + ifc, geometry, model, connection, + rel="rel", kind="path", elem="elem", partner="partner", + skip_elem_recreate=True, + skip_partner_recreate=True, + ) + + remove.assert_called_once() + model.recreate_wall.assert_not_called() + + +class TestDisconnectRelElementTop: + def test_disconnects_then_regenerates_wall(self): + """Operator case (no skip flags): both sides survive, so the wall gets + re-clipped against currently-connected slabs.""" + rel = _rel() + ifc = _ifc_with_objects({"wall": "wall_obj"}) + geometry = Mock() + model = Mock() + connection = Mock() + connection.orient_element_top.return_value = ("wall", "slab") + + with patch("bonsai.core.connection.regenerate_wall_to_underside") as regen: + subject.disconnect_rel( + ifc, geometry, model, connection, + rel=rel, kind="element-top", elem="elem", partner="partner", + ) + + ifc.run.assert_called_once_with( + "geometry.disconnect_element", relating_element="slab", related_element="wall" + ) + regen.assert_called_once_with(ifc, geometry, model, ["wall_obj"]) + + def test_slab_delete_cascade_still_regenerates_wall(self): + """When slab is being deleted (elem=slab), wall survives and must + re-clip against remaining connections — the cascade's main purpose.""" + rel = _rel() + ifc = _ifc_with_objects({"wall": "wall_obj"}) + connection = Mock() + connection.orient_element_top.return_value = ("wall", "slab") + + with patch("bonsai.core.connection.regenerate_wall_to_underside") as regen: + subject.disconnect_rel( + ifc, Mock(), Mock(), connection, + rel=rel, kind="element-top", elem="slab", partner="wall", + skip_elem_recreate=True, # slab is being deleted + ) + + regen.assert_called_once() + + def test_wall_delete_cascade_skips_wall_regen(self): + """When the wall itself is being deleted, regenerating its body moments + before remove_product wipes it is wasted work — skip.""" + rel = _rel() + ifc = _ifc_with_objects({"wall": "wall_obj"}) + connection = Mock() + connection.orient_element_top.return_value = ("wall", "slab") + + with patch("bonsai.core.connection.regenerate_wall_to_underside") as regen: + subject.disconnect_rel( + ifc, Mock(), Mock(), connection, + rel=rel, kind="element-top", elem="wall", partner="slab", + skip_elem_recreate=True, # wall is being deleted + ) + + regen.assert_not_called() + ifc.run.assert_called_once() # rel still removed + + def test_both_in_batch_skips_wall_regen(self): + """Batch delete of both endpoints, processing slab first: partner (wall) + also queued for deletion → skip wall regen.""" + rel = _rel() + ifc = _ifc_with_objects({"wall": "wall_obj"}) + connection = Mock() + connection.orient_element_top.return_value = ("wall", "slab") + + with patch("bonsai.core.connection.regenerate_wall_to_underside") as regen: + subject.disconnect_rel( + ifc, Mock(), Mock(), connection, + rel=rel, kind="element-top", elem="slab", partner="wall", + skip_elem_recreate=True, + skip_partner_recreate=True, # wall also in batch + ) + + regen.assert_not_called() + + +class TestDisconnectRelElement: + def test_just_removes_the_rel(self): + rel = _rel(relating="A", related="B") + ifc = Mock() + + subject.disconnect_rel( + ifc, Mock(), Mock(), Mock(), + rel=rel, kind="element", elem="elem_a", partner="elem_b", + ) + + ifc.run.assert_called_once_with( + "geometry.disconnect_element", relating_element="A", related_element="B" + ) + + +class TestDisconnectRelUnknownKind: + def test_raises_value_error(self): + with pytest.raises(ValueError, match="Unknown rel kind"): + subject.disconnect_rel( + Mock(), Mock(), Mock(), Mock(), + rel="rel", kind="bogus", elem="a", partner="b", + ) diff --git a/src/bonsai/test/tool/test_connection_forward_compat.py b/src/bonsai/test/tool/test_connection_forward_compat.py new file mode 100644 index 0000000000..af830353b3 --- /dev/null +++ b/src/bonsai/test/tool/test_connection_forward_compat.py @@ -0,0 +1,123 @@ +# Bonsai - OpenBIM Blender Add-on +# Copyright (C) 2026 +# +# This file is part of Bonsai. +# +# Bonsai is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Bonsai 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 General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Bonsai. If not, see . +# +# This file was generated with the assistance of an AI coding tool. + +"""Forward-compat AST contract: ``core.connection.disconnect_rel`` must have a +branch for every rel ``kind`` emitted by ``tool.connection.Connection`` lookups. + +Adding a new rel kind (e.g. ``"void"``, ``"fill"``, ``"interferes"``) to +``find_rels`` / ``find_rels_for_element`` without extending ``disconnect_rel`` +would silently regress the disconnect operator and the cascade-on-delete: a new +kind would reach the dispatch, hit the ``raise ValueError("Unknown rel kind")`` +fallback, and either crash the operator or leave the cascade half-done. This +guard makes the symmetry mandatory at test time.""" + +import ast +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.model + + +BONSAI_ROOT = Path(__file__).parent.parent.parent / "bonsai" +TOOL_CONNECTION = BONSAI_ROOT / "tool" / "connection.py" +CORE_CONNECTION = BONSAI_ROOT / "core" / "connection.py" + + +def _find_function(tree: ast.Module, name: str) -> ast.FunctionDef: + for node in ast.walk(tree): + if isinstance(node, ast.FunctionDef) and node.name == name: + return node + raise AssertionError(f"Function {name!r} not found") + + +def _find_method(tree: ast.Module, class_name: str, method_name: str) -> ast.FunctionDef: + for node in ast.walk(tree): + if isinstance(node, ast.ClassDef) and node.name == class_name: + for child in node.body: + if isinstance(child, ast.FunctionDef) and child.name == method_name: + return child + raise AssertionError(f"Method {class_name}.{method_name} not found") + + +def _kinds_emitted_by(method: ast.FunctionDef) -> set[str]: + """Extract every kind label this method emits. + + Looks at exactly two narrow patterns to avoid false positives from + docstrings or type-annotation strings: + + - ``_record(rel, "", …)`` — positional string at index 1, the + conventional emit shape in ``find_rels`` / ``find_rels_for_element``. + - ``kind = "" if … else ""`` and chained variants — string + literals on either branch of an ``ast.IfExp`` assigned to ``kind``. + """ + kinds: set[str] = set() + for node in ast.walk(method): + if isinstance(node, ast.Call): + func = node.func + if isinstance(func, ast.Name) and func.id == "_record" and len(node.args) >= 2: + arg = node.args[1] + if isinstance(arg, ast.Constant) and isinstance(arg.value, str): + kinds.add(arg.value) + elif isinstance(arg, ast.IfExp): + for branch in (arg.body, arg.orelse): + if isinstance(branch, ast.Constant) and isinstance(branch.value, str): + kinds.add(branch.value) + elif isinstance(node, ast.Assign): + targets = [t for t in node.targets if isinstance(t, ast.Name) and t.id == "kind"] + if not targets or not isinstance(node.value, ast.IfExp): + continue + for branch in (node.value.body, node.value.orelse): + if isinstance(branch, ast.Constant) and isinstance(branch.value, str): + kinds.add(branch.value) + return kinds + + +def _kind_branches_in_disconnect_rel(tree: ast.Module) -> set[str]: + """Return every kind matched by ``disconnect_rel``'s ``kind == "…"`` branches.""" + fn = _find_function(tree, "disconnect_rel") + kinds: set[str] = set() + for node in ast.walk(fn): + if isinstance(node, ast.Compare) and len(node.ops) == 1 and isinstance(node.ops[0], ast.Eq): + left = node.left + right = node.comparators[0] + if isinstance(left, ast.Name) and left.id == "kind": + if isinstance(right, ast.Constant) and isinstance(right.value, str): + kinds.add(right.value) + return kinds + + +def test_disconnect_rel_handles_every_kind_emitted_by_connection_lookups() -> None: + tool_tree = ast.parse(TOOL_CONNECTION.read_text(encoding="utf-8")) + core_tree = ast.parse(CORE_CONNECTION.read_text(encoding="utf-8")) + + emitted = _kinds_emitted_by(_find_method(tool_tree, "Connection", "find_rels")) | _kinds_emitted_by( + _find_method(tool_tree, "Connection", "find_rels_for_element") + ) + handled = _kind_branches_in_disconnect_rel(core_tree) + + assert emitted, "Sanity check: no kinds extracted — emit pattern may have changed" + + missing = emitted - handled + assert not missing, ( + f"core.connection.disconnect_rel is missing branches for kinds {missing}. " + f"Every kind returned by Connection.find_rels / find_rels_for_element " + f"must have a matching if/elif branch in the dispatch." + )