mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-06 07:51:47 +00:00
Cascade connection cleanup on element delete
Deleting a slab that was connected to a wall via IfcRelConnectsElements(TOP) left the wall holding orphan IfcBooleanResult items + a stale BBIM_Boolean pset. The disconnect operator already runs the right cleanup; element delete just never invoked it. Extract the per-kind cleanup into core.connection.disconnect_rel so the operator (bim.disconnect_elements) and a new cascade in tool.Geometry.delete_ifc_object share one dispatch table. Adding a future rel kind to tool.Connection.find_rels now flows into both call sites automatically; an AST forward-compat guard enforces coverage. Other adjustments: - regenerate_wall_to_underside zero-slab branch now removes stale clip booleans instead of silently skipping, so disconnecting the last TOP slab also reverts the wall correctly. - duplicate_ifc_objects (Shift+D) calls strip_underside_booleans on copied walls so the duplicate doesn't carry over the source's slab trim, then reloads the body representation when something was stripped so the viewport reflects the change without waiting on Shift+G. - batch_being_deleted_ids threads through OverrideDelete so the cascade can suppress partner-side regenerate when both endpoints are queued for deletion in the same batch. This file was generated with the assistance of an AI coding tool.
This commit is contained in:
Gorgious56
@@ -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:
|
||||
|
||||
@@ -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 <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
# 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}")
|
||||
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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():
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
# 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",
|
||||
)
|
||||
@@ -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 <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
# 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, "<kind>", …)`` — positional string at index 1, the
|
||||
conventional emit shape in ``find_rels`` / ``find_rels_for_element``.
|
||||
- ``kind = "<a>" if … else "<b>"`` 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."
|
||||
)
|
||||
Reference in New Issue
Block a user