Refuse cyclic aggregation in aggregate.assign_object (#7154)

Aggregating a product under relating_object makes the product an ancestor
of relating_object. If a product is relating_object itself, or is already
an ancestor of it, the new IfcRelAggregates closes a loop in the
decomposition tree. Traversing that loop later (for example resolving a
relative placement via util.placement.get_local_placement) recurses without
end and raises RecursionError; without placements it silently writes an
invalid cyclic model.

Before writing anything, walk the aggregate ancestor chain of
relating_object (including itself, with a visited set so a pre-existing bad
cycle is still safe) and raise a clear ValueError if any product is in it,
rather than proceeding. Fixing this in the api protects every caller,
including the Bonsai operator.

Verified: self-aggregation, a 2-node cycle, and a deeper cycle now raise a
clean ValueError instead of RecursionError; a normal aggregate of a distinct
element under a distinct aggregate still works, and the existing
test/api/aggregate suite (20 tests) passes.

Generated with the assistance of an AI coding tool.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Petru Conduraru
2026-07-11 15:27:43 +03:00
parent 256d5a63f1
commit bfc950a948
@@ -88,6 +88,24 @@ def assign_object(
return
products_set = set(products)
# Guard against creating a cyclic aggregation. Aggregating a product under
# relating_object makes the product an ancestor of relating_object, so if a
# product is relating_object itself or already an ancestor of it, the
# relationship would form a loop. Such a cycle later causes infinite
# recursion (e.g. RecursionError) when placements or the decomposition tree
# are traversed, so refuse it up front. See issue #7154.
ancestor = relating_object
ancestors: set[ifcopenshell.entity_instance] = set()
while ancestor is not None and ancestor not in ancestors:
ancestors.add(ancestor)
ancestor = ifcopenshell.util.element.get_aggregate(ancestor)
cycling = products_set & ancestors
if cycling:
raise ValueError(
"Cannot aggregate an object under itself or one of its descendants "
f"(would create a cyclic aggregation): {cycling}"
)
is_decomposed_by = next((i for i in relating_object.IsDecomposedBy if i.is_a("IfcRelAggregates")), None)
previous_aggregates_rels: set[ifcopenshell.entity_instance] = set()