diff --git a/src/bonsai/bonsai/bim/module/project/data.py b/src/bonsai/bonsai/bim/module/project/data.py index 115ef3376f..db64041a89 100644 --- a/src/bonsai/bonsai/bim/module/project/data.py +++ b/src/bonsai/bonsai/bim/module/project/data.py @@ -165,9 +165,6 @@ class ProjectLibraryData: root = tool.Project.get_root_context(library_file) results.append((str(root.id()), f"{root.is_a()} {root.Name or 'Unnamed'}", root.Description or "")) for library_id, data in cls.data["project_libraries"].items(): - # Defensive guard only; see tool.Project.ensure_project_context for the fix. - if library_id == root.id(): - continue results.append((str(library_id), data["Name"] or "Unnamed", data["Description"] or "")) return results diff --git a/src/bonsai/bonsai/bim/module/project/operator.py b/src/bonsai/bonsai/bim/module/project/operator.py index d0c87de2ea..b702762e6b 100644 --- a/src/bonsai/bonsai/bim/module/project/operator.py +++ b/src/bonsai/bonsai/bim/module/project/operator.py @@ -215,7 +215,7 @@ class SelectLibraryFile(bpy.types.Operator, IFCFileSelector, ImportHelper): filepath = self.get_filepath() ifc_file = tool.Ifc.get() library_file: ifcopenshell.file - library_file = tool.Project.open_library_file(filepath) + library_file = ifcopenshell.open(filepath) if library_file.schema_identifier != ifc_file.schema_identifier: self.report( {"ERROR"}, @@ -237,14 +237,14 @@ class SelectLibraryFile(bpy.types.Operator, IFCFileSelector, ImportHelper): def rollback(self, data): if data["old_filepath"]: IfcStore.library_path = data["old_filepath"] - IfcStore.library_file = tool.Project.open_library_file(data["old_filepath"]) + IfcStore.library_file = ifcopenshell.open(data["old_filepath"]) else: IfcStore.library_path = "" IfcStore.library_file = None def commit(self, data): IfcStore.library_path = data["filepath"] - IfcStore.library_file = tool.Project.open_library_file(data["filepath"]) + IfcStore.library_file = ifcopenshell.open(data["filepath"]) def draw(self, context): self.layout.prop(self, "append_all", text="Append Entire Library") @@ -809,10 +809,7 @@ class AddProjectLibrary(bpy.types.Operator): assert library_file root_context = tool.Project.get_root_context(library_file) project_library = ifcopenshell.api.root.create_entity(library_file, "IfcProjectLibrary") - if root_context.is_a("IfcProject"): - ifcopenshell.api.project.assign_declaration(library_file, [project_library], root_context) - else: - ifcopenshell.api.nest.assign_object(library_file, [project_library], root_context) + ifcopenshell.api.project.assign_declaration(library_file, [project_library], root_context) ProjectLibraryData.load() # Update enum. props.selected_project_library = str(project_library.id()) props.is_editing_project_library = True diff --git a/src/bonsai/bonsai/tool/project.py b/src/bonsai/bonsai/tool/project.py index bdf6b925af..2224effc4b 100644 --- a/src/bonsai/bonsai/tool/project.py +++ b/src/bonsai/bonsai/tool/project.py @@ -38,8 +38,6 @@ from typing import ( import bpy import ifcopenshell import ifcopenshell.api.document -import ifcopenshell.api.project -import ifcopenshell.api.root import ifcopenshell.util.element import ifcopenshell.util.representation import ifcopenshell.util.shape_builder @@ -392,10 +390,9 @@ class Project(bonsai.core.tool.Project): ) -> Union[ifcopenshell.entity_instance, None]: """Return the IfcContext that declares or nests ``project_library``. - Every IfcProjectLibrary should be either nested under another library or - declared to the file's IfcProject (see ensure_project_context()). Returns - ``None`` only as a defensive fallback for malformed data with neither - relationship, which should not occur on a normalized file. + Every IfcProjectLibrary in a supported (spec-valid) file is either nested + under another library or declared to the file's IfcProject. Returns ``None`` + only as a defensive fallback for malformed data with neither relationship. """ if nests := project_library.Nests: return nests[0].RelatingObject @@ -409,48 +406,11 @@ class Project(bonsai.core.tool.Project): Per the IFC Project Context concept template, every project data set (this includes library files) shall contain exactly one IfcProject; there is no - such thing as a spec-valid file rooted on IfcProjectLibrary alone. Files - opened through open_library_file() are normalized by ensure_project_context() - so an IfcProject is always present here. The IfcProjectLibrary fallback below - only guards callers that bypass that normalization (e.g. a file opened - directly with ifcopenshell.open); it is not a legitimate IFC structure and - should not be relied upon. Caller is responsible for the IFC2X3 guard; - IfcContext does not exist in that schema. + such thing as a spec-valid file rooted on IfcProjectLibrary alone. A file + without an IfcProject is not supported and this raises IndexError. Caller is + responsible for the IFC2X3 guard; IfcContext does not exist in that schema. """ - if projects := ifc_file.by_type("IfcProject"): - return projects[0] - return ifc_file.by_type("IfcProjectLibrary")[0] - - @classmethod - def ensure_project_context(cls, ifc_file: ifcopenshell.file) -> None: - """Repair a file that is missing the IfcProject the IFC spec requires. - - Some externally authored library files only contain IfcProjectLibrary, with - no IfcProject (see #8183). Per the Project Context concept template, all - project data sets shall contain a single IfcProject, and IfcProjectLibrary - instances are assigned to it via IfcRelDeclares. Rather than treating such a - file as though a bare IfcProjectLibrary were a legitimate root context, create - the missing IfcProject and declare the file's root-level IfcProjectLibrary - instances to it, so the in-memory model becomes spec-valid. - """ - if ifc_file.schema == "IFC2X3" or ifc_file.by_type("IfcProject"): - return - root_libraries = [lib for lib in ifc_file.by_type("IfcProjectLibrary") if not lib.Nests and not lib.HasContext] - if not root_libraries: - return - project = ifcopenshell.api.root.create_entity(ifc_file, ifc_class="IfcProject", name="Unnamed") - ifcopenshell.api.project.assign_declaration(ifc_file, definitions=root_libraries, relating_context=project) - - @classmethod - def open_library_file(cls, filepath: str) -> ifcopenshell.file: - """Open a library file, repairing a missing IfcProject if needed. - - See ensure_project_context() for why this repair is necessary rather than - treating a library-only file as spec-valid. - """ - library_file = ifcopenshell.open(filepath) - cls.ensure_project_context(library_file) - return library_file + return ifc_file.by_type("IfcProject")[0] @classmethod def get_project_hierarchy(cls, ifc_file: ifcopenshell.file) -> HiearchyDict: diff --git a/src/bonsai/test/bim/module/project/test_project_library_data.py b/src/bonsai/test/bim/module/project/test_project_library_data.py index ea8b8e3a55..a6bf12f86b 100644 --- a/src/bonsai/test/bim/module/project/test_project_library_data.py +++ b/src/bonsai/test/bim/module/project/test_project_library_data.py @@ -32,13 +32,14 @@ from test.bim.bootstrap import NewIfc pytestmark = pytest.mark.project -def _make_valid_library_file(*, with_child: bool = False) -> ifcopenshell.file: +def _make_library_file(*, with_child: bool = False) -> ifcopenshell.file: """Build a spec-valid IFC4 library file: IfcProject + IfcProjectLibrary declared to it. Per the IFC Project Context concept template, every project data set (library files included) shall contain exactly one IfcProject, and IfcProjectLibrary - instances are assigned to it via IfcRelDeclares. ``with_child=True`` also nests a - sub-library under the root via IfcRelNests, mirroring real authored library files. + instances are assigned to it via IfcRelDeclares. This matches how every library + file shipped in bonsai/bim/data/libraries is actually authored. ``with_child=True`` + also nests a sub-library under the root via IfcRelNests. """ library_file = ifcopenshell.api.project.create_file(version="IFC4") project = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProject", name="Demo Project") @@ -50,94 +51,37 @@ def _make_valid_library_file(*, with_child: bool = False) -> ifcopenshell.file: return library_file -def _make_malformed_library_file(*, with_child: bool = False) -> ifcopenshell.file: - """Build an IFC4 file containing only an IfcProjectLibrary, no IfcProject. +class TestLibraryFile(NewIfc): + """Project-library UI code operating on a spec-valid model (IfcProject root). - This is NOT spec-valid IFC (IfcProject is mandatory per the Project Context - concept template) but mirrors real externally authored files that omit it, such - as the one reported in #8183. Used to exercise the repair path - (tool.Project.ensure_project_context / open_library_file), not as an example of - a legitimate model. + A file containing only IfcProjectLibrary and no IfcProject is not valid IFC and + is not supported; see test_get_root_context_raises_for_a_file_without_a_project. """ - library_file = ifcopenshell.api.project.create_file(version="IFC4") - root = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="RootLib") - if with_child: - child = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="ChildLib") - ifcopenshell.api.nest.assign_object(library_file, [child], root) - return library_file - - -class TestEnsureProjectContext(NewIfc): - """tool.Project.ensure_project_context() repairs files missing the required IfcProject.""" - - def test_repairs_malformed_file_by_declaring_root_library_to_a_new_project(self): - library_file = _make_malformed_library_file() - root = library_file.by_type("IfcProjectLibrary")[0] - - tool.Project.ensure_project_context(library_file) - - projects = library_file.by_type("IfcProject") - assert len(projects) == 1 - assert root.HasContext - assert root.HasContext[0].RelatingContext == projects[0] - - def test_only_declares_root_level_libraries_not_nested_children(self): - library_file = _make_malformed_library_file(with_child=True) - root = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "RootLib") - child = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "ChildLib") - - tool.Project.ensure_project_context(library_file) - - assert root.HasContext - assert not child.HasContext - assert child.Nests and child.Nests[0].RelatingObject == root - - def test_is_a_noop_when_project_already_present(self): - library_file = _make_valid_library_file() - before = set(library_file.by_type("IfcProject")) - - tool.Project.ensure_project_context(library_file) - - assert set(library_file.by_type("IfcProject")) == before - - def test_is_a_noop_when_no_project_library_either(self): - library_file = ifcopenshell.api.project.create_file(version="IFC4") - - tool.Project.ensure_project_context(library_file) - - assert not library_file.by_type("IfcProject") - - def test_open_library_file_repairs_a_malformed_file_from_disk(self, tmp_path): - library_file = _make_malformed_library_file() - filepath = tmp_path / "malformed_library.ifc" - library_file.write(str(filepath)) - - opened = tool.Project.open_library_file(str(filepath)) - - assert len(opened.by_type("IfcProject")) == 1 - assert opened.by_type("IfcProjectLibrary")[0].HasContext - - -class TestValidLibraryFile(NewIfc): - """Downstream project-library UI code operating on a spec-valid model (IfcProject root).""" def test_get_root_context_returns_the_project(self): - library_file = _make_valid_library_file() + library_file = _make_library_file() project = library_file.by_type("IfcProject")[0] root = tool.Project.get_root_context(library_file) assert root == project + def test_get_root_context_raises_for_a_file_without_a_project(self): + library_file = ifcopenshell.api.project.create_file(version="IFC4") + ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="RootLib") + + with pytest.raises(IndexError): + tool.Project.get_root_context(library_file) + def test_get_parent_library_returns_project_for_declared_root_library(self): - library_file = _make_valid_library_file() + library_file = _make_library_file() project = library_file.by_type("IfcProject")[0] root = library_file.by_type("IfcProjectLibrary")[0] assert tool.Project.get_parent_library(root) == project def test_get_project_hierarchy_roots_libraries_under_the_project(self): - library_file = _make_valid_library_file(with_child=True) + library_file = _make_library_file(with_child=True) project = library_file.by_type("IfcProject")[0] root = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "RootLib") child = next(lib for lib in library_file.by_type("IfcProjectLibrary") if lib.Name == "ChildLib") @@ -147,12 +91,14 @@ class TestValidLibraryFile(NewIfc): assert root in hierarchy[project] assert child in hierarchy[root] - def test_project_library_data_loads_with_a_single_unique_root_entry(self): + def test_project_library_data_loads_with_unique_enum_keys(self): # Regression test for the ci-bonsai-daily failure: parent_libraries_enum() # must never emit two entries with the same STEP id (Blender's EnumProperty - # requires unique keys). Reproduced here on a repaired, spec-valid file rather - # than an invalid library-only one. - IfcStore.library_file = _make_valid_library_file() + # requires unique keys). The original failure only occurred because the root + # context could incorrectly resolve to an IfcProjectLibrary that was also + # collected by project_libraries(); on a spec-valid model root is always the + # IfcProject, whose id never collides with a library id. + IfcStore.library_file = _make_library_file(with_child=True) try: ProjectLibraryData.is_loaded = False ProjectLibraryData.load() @@ -165,10 +111,10 @@ class TestValidLibraryFile(NewIfc): IfcStore.library_file = None ProjectLibraryData.is_loaded = False - def test_refresh_library_succeeds_on_valid_library_file(self): + def test_refresh_library_succeeds(self): import bpy - IfcStore.library_file = _make_valid_library_file(with_child=True) + IfcStore.library_file = _make_library_file(with_child=True) try: result = bpy.ops.bim.refresh_library() assert result == {"FINISHED"} @@ -179,7 +125,7 @@ class TestValidLibraryFile(NewIfc): def test_add_project_library_declares_new_library_under_the_project_root(self): import bpy - IfcStore.library_file = _make_valid_library_file() + IfcStore.library_file = _make_library_file() library_file = IfcStore.library_file try: project = library_file.by_type("IfcProject")[0]