mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-10 17:58:20 +00:00
Bonsai: stop supporting library-only files, do not repair them
Per Moult's feedback: if the IFC is invalid, our default position is to not support it, not to patch around it. A library file with no IfcProject is invalid IFC (Project Context concept template requires exactly one IfcProject), and it is not ubiquitous: every library file bonsai ships under bim/data/libraries has an IfcProject with the IfcProjectLibrary declared to it via IfcRelDeclares. The single #8183 report is an outlier, not a common authoring pattern worth accommodating. Remove tool.Project.ensure_project_context() and open_library_file() (the load-time repair added in the previous commit here) and revert SelectLibraryFile's three load sites to plain ifcopenshell.open. Simplify get_root_context() back to returning ifc_file.by_type("IfcProject")[0] directly, no IfcProjectLibrary fallback: a file without IfcProject now raises IndexError instead of being silently treated as valid. AddProjectLibrary's nest-under-library branch is now dead code (root_context is always an IfcProject) and is removed. The one-line enum dedup guard from the original commit here is also removed: since get_root_context can only return an IfcProject or raise, an IfcProject id can never collide with a library id, so the guard has nothing left to guard against. Rework test_project_library_data.py: drop the invalid _make_library_only_file fixture and its tests, which asserted an unsupported model as correct behaviour. Replace with a single spec-valid fixture matching bonsai's own shipped library files (IfcProject + IfcProjectLibrary declared to it), used for the ci-bonsai-daily regression test and the refresh/add-library operators, plus one explicit test that get_root_context raises for a file without IfcProject, documenting that this input is intentionally unsupported rather than silently tolerated. Verified live in headless Blender (isolated profile, source-loaded, never the real profile): confirmed the removed methods are gone, that a library-only file now raises instead of being handled, that ProjectLibraryData/refresh_library/add_project_library all work correctly on a spec-valid model with unique enum keys, and spot-checked that every library file under bim/data/libraries already has an IfcProject. This change was made with the assistance of an AI tool.
This commit is contained in:
Petru Conduraru
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user