mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-09 09:21:46 +00:00
Bonsai: unit tests for link filter helpers + refactor notes
Adds test/tool coverage for the pure link helpers: encode/decode_link_filter (plain round-trip, JSON promotion for exclude and loaded, legacy and malformed decode) and get_link_cache_paths (legacy names, include-only hash pinned to the pre-exclude formula so existing caches stay valid, and the same-include/different-exclude collision case the key exists to prevent). 12 tests, verified passing under Blender python. Documents the deliberate undo-system exemption on the link transform autosave handler, and records the deferred refactors in the dev note: an upstream exclude= parameter for filter_elements (separate ifcopenshell-python PR) and the skipped core/tool interface ceremony. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -295,6 +295,26 @@ stay compatible.
|
||||
- Debugging note: merged cut groups carry member guids as CSS *classes*, not as the
|
||||
`ifcopenshell:guid` attribute — inspect both when checking cut output.
|
||||
|
||||
## Deferred refactors (deliberate)
|
||||
|
||||
- **Upstream `exclude=` on `filter_elements`** — the include−exclude set difference
|
||||
is hand-rolled twice (links, drawings) because the selector grammar has no
|
||||
difference operator and `parent` negation is broken by design (its `!=`/regex
|
||||
paths also match GlobalIds, so negation strips everything that has a parent).
|
||||
The right home is an `exclude=` parameter on
|
||||
`ifcopenshell.util.selector.filter_elements`, documented in
|
||||
`selector_syntax.rst` together with the `parent`-negation limitation. Deferred
|
||||
to a separate ifcopenshell-python PR (different review audience; would widen
|
||||
this PR mid-review). Once it lands, both Bonsai call sites collapse.
|
||||
- **Core/tool ceremony skipped** — the new `tool.Project` methods have no
|
||||
`core/tool.py` interface declarations and no `bonsai/core` orchestration
|
||||
functions, matching the pre-existing linked-model code (which bypasses the
|
||||
core layer wholesale; `LoadLinkedProject` is flagged "prototyping" upstream).
|
||||
Interfaces nobody calls through wouldn't add testability — the pure helpers
|
||||
(`encode_link_filter`/`decode_link_filter`, `get_link_cache_paths`) are
|
||||
covered directly in `test/tool/test_project.py` instead. Revisit if the
|
||||
linked-model subsystem is ever promoted out of prototype status.
|
||||
|
||||
## Review round 1 (PR #8242, falken10vdl) — decisions
|
||||
|
||||
- **Path-form mismatch → duplicate documents (confirmed bug, fixed).**
|
||||
|
||||
@@ -112,7 +112,14 @@ addon_keymaps = []
|
||||
|
||||
@bpy.app.handlers.persistent
|
||||
def _autosave_link_transforms(scene, depsgraph):
|
||||
"""Persist link transformations whenever an editing link's handle is moved."""
|
||||
"""Persist link transformations whenever an editing link's handle is moved.
|
||||
|
||||
Deliberate exemption from the transaction rule in
|
||||
docs/guides/development/undo_system.rst: a handler cannot run inside
|
||||
execute_ifc_operator, so this IFC write is not undo-tracked. It stays
|
||||
consistent anyway because undoing the move fires another depsgraph
|
||||
update, which re-saves the reverted matrix.
|
||||
"""
|
||||
import bonsai.tool as tool
|
||||
|
||||
props = tool.Project.get_project_props()
|
||||
|
||||
@@ -501,3 +501,67 @@ class TestGettingLinkedElementGeomSlice:
|
||||
obj = cast(bpy.types.Object, obj)
|
||||
slice_ = subject.Link.get_linked_element_geom_slice(obj, "aaa")
|
||||
assert range(15)[slice_] == range(5)
|
||||
|
||||
|
||||
class TestEncodeDecodeLinkFilter:
|
||||
def test_plain_include_round_trip(self):
|
||||
assert subject.encode_link_filter("IfcWall", "") == "IfcWall"
|
||||
assert subject.decode_link_filter("IfcWall") == ("IfcWall", "", False)
|
||||
|
||||
def test_empty_filter_encodes_to_none(self):
|
||||
assert subject.encode_link_filter("", "") is None
|
||||
assert subject.decode_link_filter(None) == ("", "", False)
|
||||
assert subject.decode_link_filter("") == ("", "", False)
|
||||
|
||||
def test_exclude_promotes_to_json(self):
|
||||
encoded = subject.encode_link_filter('IfcElement, group="X"', 'IfcSlab, parent="Y"')
|
||||
assert encoded.startswith("{")
|
||||
assert subject.decode_link_filter(encoded) == ('IfcElement, group="X"', 'IfcSlab, parent="Y"', False)
|
||||
|
||||
def test_loaded_promotes_to_json(self):
|
||||
encoded = subject.encode_link_filter("IfcWall", "", loaded=True)
|
||||
assert encoded.startswith("{")
|
||||
assert subject.decode_link_filter(encoded) == ("IfcWall", "", True)
|
||||
|
||||
def test_loaded_without_filter(self):
|
||||
encoded = subject.encode_link_filter("", "", loaded=True)
|
||||
assert subject.decode_link_filter(encoded) == ("", "", True)
|
||||
|
||||
def test_legacy_non_json_decodes_as_include(self):
|
||||
legacy = 'IfcElement, location="House - Type B"'
|
||||
assert subject.decode_link_filter(legacy) == (legacy, "", False)
|
||||
|
||||
def test_malformed_json_decodes_as_include(self):
|
||||
assert subject.decode_link_filter("{not json") == ("{not json", "", False)
|
||||
|
||||
|
||||
class TestGetLinkCachePaths:
|
||||
def test_empty_filter_keeps_legacy_names(self):
|
||||
blend, json_ = subject.get_link_cache_paths("/x/File A.ifc", "")
|
||||
assert blend.name == "File A.ifc.cache.blend"
|
||||
assert json_.name == "File A.ifc.cache.json"
|
||||
|
||||
def test_include_only_hash_matches_pre_exclude_formula(self):
|
||||
# Existing caches were keyed by md5(query)[:8]; they must stay valid.
|
||||
import hashlib
|
||||
|
||||
blend, _ = subject.get_link_cache_paths("/x/File A.ifc", "IfcWall")
|
||||
expected = hashlib.md5(b"IfcWall").hexdigest()[:8]
|
||||
assert blend.name == f"File A.ifc.cache.{expected}.blend"
|
||||
|
||||
def test_blend_and_json_share_a_suffix(self):
|
||||
blend, json_ = subject.get_link_cache_paths("/x/File A.ifc", "IfcWall", "IfcDoor")
|
||||
assert blend.name.removesuffix("blend") == json_.name.removesuffix("json")
|
||||
|
||||
def test_same_include_different_exclude_do_not_collide(self):
|
||||
# The reason the cache key hashes both strings: same-include links
|
||||
# with different excludes must not serve each other's geometry.
|
||||
a, _ = subject.get_link_cache_paths("/x/f.ifc", "IfcElement", "IfcSlab")
|
||||
b, _ = subject.get_link_cache_paths("/x/f.ifc", "IfcElement", "IfcDoor")
|
||||
c, _ = subject.get_link_cache_paths("/x/f.ifc", "IfcElement", "")
|
||||
assert len({a.name, b.name, c.name}) == 3
|
||||
|
||||
def test_exclude_only_distinct_from_empty_filter(self):
|
||||
a, _ = subject.get_link_cache_paths("/x/f.ifc", "", "IfcDoor")
|
||||
b, _ = subject.get_link_cache_paths("/x/f.ifc", "", "")
|
||||
assert a.name != b.name
|
||||
|
||||
Reference in New Issue
Block a user