mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-09 17:31:45 +00:00
Tolerate stale array child/parent GUIDs (#8177)
* Tolerate stale array child/parent GUIDs A real-world IFC project (an arrayed door whose host got deleted externally) crashed Bonsai's project load with "Instance with GlobalId not found" inside setup_arrays. tool.Blender.get_object_from_guid declared Optional return but let RuntimeError propagate; callers iterating BBIM_Array child lists then crashed instead of skipping. Honour the documented contract by returning None on miss, matching the convention used by every other by_guid lookup helper in tool/array.py, tool/ifc.py, tool/geometry.py. Sweep the four user-action sites that resolve array child/parent GUIDs without a guard - they shared the same bug class but were reachable from different operators (regenerate_array, RegenerateArray clear, duplicate_ifc_objects, process_arrays). An already-missing entity is the desired terminal state for each, so the fix is try/except RuntimeError: continue/skip. setup_arrays now also collects each parent with at least one stale child GUID into IfcImporter.broken_arrays, surfaced via a new Project panel banner mirroring the existing pending_opening_recut UX. The banner reports the count and offers "Select Elements" to navigate to the affected array parents and a Dismiss button. constrain_children_to_parent was being called once per layer inside setup_arrays' for loop even though it always iterates all layers internally - lifted out of the loop (pre-existing N x perf bug that the stale-GUID print exposed). Regression tests: - test_returns_none_when_guid_not_in_file pins the get_object_from_guid Optional contract. - test_remove_array_tolerates_stale_child_guid injects a fake child GUID into BBIM_Array.Data and asserts bim.remove_array completes cleanly. Generated with the assistance of an AI coding tool. * Black: wrap long bl_description in dismiss_pending_array_repair Generated with the assistance of an AI coding tool.
This commit is contained in:
@@ -223,6 +223,7 @@ class IfcImporter:
|
||||
self.elements: set[ifcopenshell.entity_instance] = set()
|
||||
self.annotations: set[ifcopenshell.entity_instance] = set()
|
||||
self.gross_elements: set[ifcopenshell.entity_instance] = set()
|
||||
self.broken_arrays: set[ifcopenshell.entity_instance] = set()
|
||||
self.element_types: set[ifcopenshell.entity_instance] = set()
|
||||
self.spatial_elements: set[ifcopenshell.entity_instance] = set()
|
||||
self.meshes: dict[str, OBJECT_DATA_TYPE] = {}
|
||||
@@ -1220,7 +1221,17 @@ class IfcImporter:
|
||||
continue
|
||||
for i in range(len(data)):
|
||||
tool.Array.set_children_lock_state(element, i, True)
|
||||
tool.Array.constrain_children_to_parent(element)
|
||||
tool.Array.constrain_children_to_parent(element)
|
||||
for layer in data:
|
||||
for child_guid in layer.get("children", ()):
|
||||
try:
|
||||
self.file.by_guid(child_guid)
|
||||
except RuntimeError:
|
||||
print(
|
||||
f"setup_arrays: array parent {element.GlobalId} references missing "
|
||||
f"child GUID {child_guid!r}."
|
||||
)
|
||||
self.broken_arrays.add(element)
|
||||
|
||||
def update_linked_aggregates(self):
|
||||
# TODO Remove this after a while. See commit 17d6b8a
|
||||
|
||||
@@ -1040,7 +1040,10 @@ class OverrideDelete(bpy.types.Operator):
|
||||
pset = ifcopenshell.util.element.get_pset(element, "BBIM_Array")
|
||||
if not pset:
|
||||
continue
|
||||
array_parents.add(ifc_file.by_guid(pset["Parent"]))
|
||||
try:
|
||||
array_parents.add(ifc_file.by_guid(pset["Parent"]))
|
||||
except RuntimeError:
|
||||
continue
|
||||
|
||||
for array_parent in array_parents:
|
||||
array_parent_obj = tool.Ifc.get_object(array_parent)
|
||||
|
||||
@@ -423,7 +423,11 @@ class RegenerateArray(bpy.types.Operator, tool.Ifc.Operator):
|
||||
pset = tool.Ifc.get().by_id(pset["id"])
|
||||
for array in arrays:
|
||||
for child in set(array["children"]):
|
||||
if child_obj := tool.Ifc.get_object(tool.Ifc.get().by_guid(child)):
|
||||
try:
|
||||
child_element = tool.Ifc.get().by_guid(child)
|
||||
except RuntimeError:
|
||||
continue
|
||||
if child_obj := tool.Ifc.get_object(child_element):
|
||||
tool.Geometry.delete_ifc_object(child_obj)
|
||||
array["children"].clear()
|
||||
# Always operate on the parent — this operator can be invoked with
|
||||
|
||||
@@ -30,7 +30,9 @@ classes = (
|
||||
operator.BIM_FH_import_ifc,
|
||||
operator.BIM_OT_apply_pending_opening_cuts,
|
||||
operator.BIM_OT_dismiss_multi_instance_warning,
|
||||
operator.BIM_OT_dismiss_pending_array_repair,
|
||||
operator.BIM_OT_dismiss_pending_opening_cuts,
|
||||
operator.BIM_OT_select_pending_array_repair,
|
||||
operator.BIM_OT_select_pending_opening_cuts,
|
||||
operator.BIM_OT_load_clipping_planes,
|
||||
operator.BIM_OT_save_clipping_planes,
|
||||
@@ -86,6 +88,7 @@ classes = (
|
||||
prop.FilterCategory,
|
||||
prop.Link,
|
||||
prop.EditedObj,
|
||||
prop.PendingArrayRepair,
|
||||
prop.PendingOpeningRecut,
|
||||
prop.BIMProjectProperties,
|
||||
prop.MeasureToolSettings,
|
||||
|
||||
@@ -1236,6 +1236,17 @@ class LoadProjectElements(bpy.types.Operator):
|
||||
f"Apply manually from the Project panel.",
|
||||
)
|
||||
|
||||
props.pending_array_repair.clear()
|
||||
if ifc_importer.broken_arrays:
|
||||
for element in ifc_importer.broken_arrays:
|
||||
item = props.pending_array_repair.add()
|
||||
item.ifc_definition_id = element.id()
|
||||
self.report(
|
||||
{"WARNING"},
|
||||
f"{len(ifc_importer.broken_arrays)} array parent(s) reference missing child GUIDs. "
|
||||
f"Inspect from the Project panel.",
|
||||
)
|
||||
|
||||
tool.Project.load_default_thumbnails()
|
||||
tool.Project.set_default_context()
|
||||
tool.Project.set_default_modeling_dimensions()
|
||||
@@ -3539,3 +3550,44 @@ class BIM_OT_select_pending_opening_cuts(bpy.types.Operator):
|
||||
tool.Blender.set_objects_selection(context, active_object=objects[0], selected_objects=objects)
|
||||
self.report({"INFO"}, f"Selected {len(objects)} element(s).")
|
||||
return {"FINISHED"}
|
||||
|
||||
|
||||
class BIM_OT_select_pending_array_repair(bpy.types.Operator):
|
||||
bl_idname = "bim.select_pending_array_repair"
|
||||
bl_label = "Select Array Parents With Missing Children"
|
||||
bl_description = "Select the Blender objects of array parents whose BBIM_Array.Data references children that don't resolve in the file."
|
||||
bl_options = {"REGISTER", "UNDO"}
|
||||
|
||||
def execute(self, context: bpy.types.Context) -> set[str]:
|
||||
ifc_file = tool.Ifc.get()
|
||||
if ifc_file is None:
|
||||
self.report({"INFO"}, "No IFC file loaded.")
|
||||
return {"CANCELLED"}
|
||||
objects: list[bpy.types.Object] = []
|
||||
for item in tool.Project.get_project_props().pending_array_repair:
|
||||
try:
|
||||
element = ifc_file.by_id(item.ifc_definition_id)
|
||||
except RuntimeError:
|
||||
continue
|
||||
obj = tool.Ifc.get_object(element)
|
||||
if obj is not None:
|
||||
objects.append(obj)
|
||||
if not objects:
|
||||
self.report({"INFO"}, "No matching Blender objects found for the pending list.")
|
||||
return {"CANCELLED"}
|
||||
tool.Blender.set_objects_selection(context, active_object=objects[0], selected_objects=objects)
|
||||
self.report({"INFO"}, f"Selected {len(objects)} array parent(s).")
|
||||
return {"FINISHED"}
|
||||
|
||||
|
||||
class BIM_OT_dismiss_pending_array_repair(bpy.types.Operator):
|
||||
bl_idname = "bim.dismiss_pending_array_repair"
|
||||
bl_label = "Dismiss Pending Array Repair"
|
||||
bl_description = (
|
||||
"Clear the pending array-repair list without acting on it. The underlying BBIM_Array.Data stays unchanged."
|
||||
)
|
||||
bl_options = {"REGISTER", "UNDO"}
|
||||
|
||||
def execute(self, context: bpy.types.Context) -> set[str]:
|
||||
tool.Project.get_project_props().pending_array_repair.clear()
|
||||
return {"FINISHED"}
|
||||
|
||||
@@ -306,6 +306,17 @@ class PendingOpeningRecut(PropertyGroup):
|
||||
ifc_definition_id: int
|
||||
|
||||
|
||||
class PendingArrayRepair(PropertyGroup):
|
||||
"""One array parent whose ``BBIM_Array.Data`` references at least one
|
||||
child GUID that does not resolve in the current IFC file. The user can
|
||||
select these parents from the Project panel banner to inspect them."""
|
||||
|
||||
ifc_definition_id: IntProperty(name="IFC Definition ID")
|
||||
|
||||
if TYPE_CHECKING:
|
||||
ifc_definition_id: int
|
||||
|
||||
|
||||
class BIMProjectProperties(PropertyGroup):
|
||||
is_editing: BoolProperty(name="Is Editing", default=False)
|
||||
is_loading: BoolProperty(name="Is Loading", default=False)
|
||||
@@ -372,6 +383,7 @@ class BIMProjectProperties(PropertyGroup):
|
||||
description="Maxium number of openings that object can have. If object has more openings, it will be loaded without openings",
|
||||
)
|
||||
pending_opening_recut: CollectionProperty(name="Pending Opening Recut", type=PendingOpeningRecut)
|
||||
pending_array_repair: CollectionProperty(name="Pending Array Repair", type=PendingArrayRepair)
|
||||
style_limit: IntProperty(
|
||||
name="Style Limit",
|
||||
default=300,
|
||||
@@ -538,6 +550,7 @@ class BIMProjectProperties(PropertyGroup):
|
||||
angular_tolerance: float
|
||||
void_limit: int
|
||||
pending_opening_recut: bpy.types.bpy_prop_collection_idprop[PendingOpeningRecut]
|
||||
pending_array_repair: bpy.types.bpy_prop_collection_idprop[PendingArrayRepair]
|
||||
style_limit: int
|
||||
distance_limit: float
|
||||
false_origin_mode: Literal["AUTOMATIC", "MANUAL", "DISABLED"]
|
||||
|
||||
@@ -205,6 +205,20 @@ class BIM_PT_project(Panel):
|
||||
row.operator("bim.apply_pending_opening_cuts", text="Apply Openings", icon="PLAY")
|
||||
row.operator("bim.dismiss_pending_opening_cuts", text="", icon="CANCEL")
|
||||
|
||||
if pending := pprops.pending_array_repair:
|
||||
box = self.layout.box()
|
||||
box.alert = True
|
||||
box.label(text="Arrays With Missing Children", icon="ERROR")
|
||||
draw_multiline_text(
|
||||
box.column(align=True),
|
||||
f"{len(pending)} array parent(s) reference child GUIDs that don't exist in this file. "
|
||||
f"The arrays loaded incomplete. Select to inspect, or dismiss.",
|
||||
context=context,
|
||||
)
|
||||
row = box.row(align=True)
|
||||
row.operator("bim.select_pending_array_repair", text="Select Elements", icon="RESTRICT_SELECT_OFF")
|
||||
row.operator("bim.dismiss_pending_array_repair", text="", icon="CANCEL")
|
||||
|
||||
if props.ifc_file:
|
||||
self.draw_loaded_project_ui(context)
|
||||
else:
|
||||
|
||||
@@ -1274,7 +1274,10 @@ class Blender(bonsai.core.tool.Blender):
|
||||
|
||||
@classmethod
|
||||
def get_object_from_guid(cls, guid: str) -> Union[bpy.types.Object, None]:
|
||||
element = tool.Ifc.get().by_guid(guid)
|
||||
try:
|
||||
element = tool.Ifc.get().by_guid(guid)
|
||||
except RuntimeError:
|
||||
return None
|
||||
obj = tool.Ifc.get_object(element)
|
||||
if obj:
|
||||
return obj
|
||||
|
||||
@@ -2478,7 +2478,10 @@ class Geometry(bonsai.core.tool.Geometry):
|
||||
pset = ifcopenshell.util.element.get_pset(element, "BBIM_Array")
|
||||
if not pset:
|
||||
continue
|
||||
array_parents.add(tool.Ifc.get().by_guid(pset["Parent"]))
|
||||
try:
|
||||
array_parents.add(tool.Ifc.get().by_guid(pset["Parent"]))
|
||||
except RuntimeError:
|
||||
continue
|
||||
|
||||
for array_parent in array_parents:
|
||||
array_parent_obj = tool.Ifc.get_object(array_parent)
|
||||
|
||||
@@ -1320,7 +1320,10 @@ class Model(bonsai.core.tool.Model):
|
||||
# handle elements unused in the array after regeneration
|
||||
removed_children = set(existing_children) - set(array["children"])
|
||||
for removed_child in removed_children:
|
||||
element = tool.Ifc.get().by_guid(removed_child)
|
||||
try:
|
||||
element = tool.Ifc.get().by_guid(removed_child)
|
||||
except RuntimeError:
|
||||
continue
|
||||
# Strip any wall/slab opening cut by this child before deletion,
|
||||
# so the host's HasOpenings shrinks symmetrically with count.
|
||||
if getattr(element, "FillsVoids", None):
|
||||
|
||||
@@ -183,3 +183,16 @@ class TestNpFrombufferLegacy(NewFile):
|
||||
result = subject.np_frombuffer_legacy(data, n)
|
||||
assert result.shape == (n,)
|
||||
np.testing.assert_allclose(result, np.arange(n))
|
||||
|
||||
|
||||
class TestGetObjectFromGuidMissing(NewFile):
|
||||
"""``get_object_from_guid`` must honour its ``Optional[Object]`` return
|
||||
contract: a GUID that does not resolve in the current IFC file yields
|
||||
``None``, not a ``RuntimeError``. Callers iterate stored GUID lists
|
||||
(array children, library refs, …) and rely on the falsy return to
|
||||
skip stale entries."""
|
||||
|
||||
def test_returns_none_when_guid_not_in_file(self):
|
||||
bpy.ops.bim.create_project()
|
||||
assert tool.Ifc.get() is not None
|
||||
assert subject.get_object_from_guid("3iyt7r$Hf4_hQYNhBIDJI4") is None
|
||||
|
||||
@@ -672,6 +672,30 @@ class TestUsingArrays(NewFile):
|
||||
pset = ifcopenshell.util.element.get_pset(element, "BBIM_Array")
|
||||
assert pset is None, (obj, pset)
|
||||
|
||||
def test_remove_array_tolerates_stale_child_guid(self):
|
||||
"""``bim.remove_array`` and the underlying ``regenerate_array`` must
|
||||
survive a child GUID in ``BBIM_Array.Data`` that no longer resolves
|
||||
in the file. Real-world IFC files can carry dangling array refs
|
||||
from external edits — the remove path is meant to delete those
|
||||
children, so an already-missing entity is the desired terminal
|
||||
state, not a fatal error."""
|
||||
self.setup_array()
|
||||
parent_obj = bpy.context.active_object
|
||||
parent_element = tool.Ifc.get_entity(parent_obj)
|
||||
ifc_file = tool.Ifc.get()
|
||||
|
||||
pset = ifcopenshell.util.element.get_pset(parent_element, "BBIM_Array")
|
||||
data = json.loads(pset["Data"])
|
||||
data[0]["children"].append("3iyt7r$Hf4_hQYNhBIDJI4")
|
||||
ifcopenshell.api.pset.edit_pset(
|
||||
ifc_file,
|
||||
pset=ifc_file.by_id(pset["id"]),
|
||||
properties={"Data": json.dumps(data)},
|
||||
)
|
||||
|
||||
bpy.ops.bim.remove_array(item=0)
|
||||
assert ifcopenshell.util.element.get_pset(parent_element, "BBIM_Array") is None
|
||||
|
||||
|
||||
class TestApplyIfcMaterialChanges(NewFile):
|
||||
def get_used_styles(self, obj: bpy.types.Object) -> set[ifcopenshell.entity_instance]:
|
||||
|
||||
Reference in New Issue
Block a user