mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-08-13 19:07:57 +00:00
Bonsai: repair library files missing the required IfcProject, not just the symptom
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. There is no such thing as a
spec-valid file rooted on IfcProjectLibrary alone.
get_root_context() (added in 260a387069, #8184) treated a missing IfcProject
as license to use the top-level IfcProjectLibrary as the file's root context
instead. That invalid premise is why project_libraries() (which walks every
IfcProjectLibrary, root included) then re-added that same entity, producing
the duplicate, colliding enum key this PR originally papered over with a
dedup guard.
Add tool.Project.ensure_project_context(), which repairs a file missing
IfcProject by creating one and declaring the file's root-level
IfcProjectLibrary instances to it, and tool.Project.open_library_file(),
which opens a library file through that repair. Route all three
IfcStore.library_file load sites in SelectLibraryFile through it. Downstream
code (get_root_context, ProjectLibraryData, RefreshLibrary,
AddProjectLibrary) now always operates on a spec-valid model, so the
duplicate enum entry cannot occur; the previous one-line dedup guard in
parent_libraries_enum() is kept only as cheap defense in depth for callers
that bypass the load-time repair, not as the fix.
Rework test_project_library_data.py: the previous _make_library_only_file()
fixture built an invalid library-only model and asserted that as correct
behaviour. Replace it with a spec-valid fixture (IfcProject + IfcProjectLibrary
declared to it) for the downstream tests, and a malformed fixture used only to
exercise the new repair path.
Verified live in headless Blender (isolated profile): reproduced the original
duplicate-enum-key failure mode, then confirmed ensure_project_context/
open_library_file repair a malformed file and ProjectLibraryData,
refresh_library and add_project_library all operate correctly on the result,
with no duplicate keys and no regression on already-valid files or IFC2X3.
This change was made with the assistance of an AI tool.
This commit is contained in:
Petru Conduraru
@@ -165,10 +165,7 @@ 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():
|
||||
# Library-only files have an IfcProjectLibrary as their root context, and
|
||||
# project_libraries() collects every IfcProjectLibrary in the file (root
|
||||
# included). Skip it here since it was already added above, otherwise the
|
||||
# root library is listed twice with colliding enum keys.
|
||||
# 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 ""))
|
||||
|
||||
@@ -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 = ifcopenshell.open(filepath)
|
||||
library_file = tool.Project.open_library_file(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 = ifcopenshell.open(data["old_filepath"])
|
||||
IfcStore.library_file = tool.Project.open_library_file(data["old_filepath"])
|
||||
else:
|
||||
IfcStore.library_path = ""
|
||||
IfcStore.library_file = None
|
||||
|
||||
def commit(self, data):
|
||||
IfcStore.library_path = data["filepath"]
|
||||
IfcStore.library_file = ifcopenshell.open(data["filepath"])
|
||||
IfcStore.library_file = tool.Project.open_library_file(data["filepath"])
|
||||
|
||||
def draw(self, context):
|
||||
self.layout.prop(self, "append_all", text="Append Entire Library")
|
||||
|
||||
@@ -38,6 +38,8 @@ 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
|
||||
@@ -390,8 +392,10 @@ class Project(bonsai.core.tool.Project):
|
||||
) -> Union[ifcopenshell.entity_instance, None]:
|
||||
"""Return the IfcContext that declares or nests ``project_library``.
|
||||
|
||||
Returns ``None`` when ``project_library`` is itself the root of a
|
||||
library-only file (no IfcRelNests, no IfcRelDeclares).
|
||||
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.
|
||||
"""
|
||||
if nests := project_library.Nests:
|
||||
return nests[0].RelatingObject
|
||||
@@ -401,16 +405,53 @@ class Project(bonsai.core.tool.Project):
|
||||
|
||||
@classmethod
|
||||
def get_root_context(cls, ifc_file: ifcopenshell.file) -> ifcopenshell.entity_instance:
|
||||
"""Return the file's root IfcContext.
|
||||
"""Return the file's IfcProject.
|
||||
|
||||
Prefers IfcProject if present, otherwise falls back to IfcProjectLibrary —
|
||||
library-only files are valid per IFC4+ and contain no IfcProject. Caller is
|
||||
responsible for the IFC2X3 guard; IfcContext does not exist in that schema.
|
||||
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.
|
||||
"""
|
||||
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
|
||||
|
||||
@classmethod
|
||||
def get_project_hierarchy(cls, ifc_file: ifcopenshell.file) -> HiearchyDict:
|
||||
"""Get project hierarchy in the following form:
|
||||
|
||||
@@ -32,12 +32,32 @@ from test.bim.bootstrap import NewIfc
|
||||
pytestmark = pytest.mark.project
|
||||
|
||||
|
||||
def _make_library_only_file(*, with_child: bool = False) -> ifcopenshell.file:
|
||||
"""Build a minimal IFC4 file containing only an IfcProjectLibrary (no IfcProject).
|
||||
def _make_valid_library_file(*, with_child: bool = False) -> ifcopenshell.file:
|
||||
"""Build a spec-valid IFC4 library file: IfcProject + IfcProjectLibrary declared to it.
|
||||
|
||||
Per IFC4+, a file must contain at least one IfcContext; IfcProjectLibrary is a
|
||||
valid root on its own. ``with_child=True`` nests a sub-library under the root via
|
||||
IfcRelNests, mirroring real authored library files.
|
||||
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.
|
||||
"""
|
||||
library_file = ifcopenshell.api.project.create_file(version="IFC4")
|
||||
project = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProject", name="Demo Project")
|
||||
root = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="RootLib")
|
||||
ifcopenshell.api.project.assign_declaration(library_file, definitions=[root], relating_context=project)
|
||||
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
|
||||
|
||||
|
||||
def _make_malformed_library_file(*, with_child: bool = False) -> ifcopenshell.file:
|
||||
"""Build an IFC4 file containing only an IfcProjectLibrary, no IfcProject.
|
||||
|
||||
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.
|
||||
"""
|
||||
library_file = ifcopenshell.api.project.create_file(version="IFC4")
|
||||
root = ifcopenshell.api.root.create_entity(library_file, ifc_class="IfcProjectLibrary", name="RootLib")
|
||||
@@ -47,49 +67,108 @@ def _make_library_only_file(*, with_child: bool = False) -> ifcopenshell.file:
|
||||
return library_file
|
||||
|
||||
|
||||
class TestLibraryOnlyFile(NewIfc):
|
||||
def test_get_root_context_returns_project_library_when_no_project(self):
|
||||
library_file = _make_library_only_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()
|
||||
project = library_file.by_type("IfcProject")[0]
|
||||
|
||||
root = tool.Project.get_root_context(library_file)
|
||||
|
||||
assert root.is_a("IfcProjectLibrary")
|
||||
assert root.Name == "RootLib"
|
||||
assert root == project
|
||||
|
||||
def test_get_parent_library_returns_none_for_root_library(self):
|
||||
library_file = _make_library_only_file()
|
||||
def test_get_parent_library_returns_project_for_declared_root_library(self):
|
||||
library_file = _make_valid_library_file()
|
||||
project = library_file.by_type("IfcProject")[0]
|
||||
root = library_file.by_type("IfcProjectLibrary")[0]
|
||||
|
||||
assert tool.Project.get_parent_library(root) is None
|
||||
assert tool.Project.get_parent_library(root) == project
|
||||
|
||||
def test_get_project_hierarchy_skips_root_library(self):
|
||||
library_file = _make_library_only_file(with_child=True)
|
||||
def test_get_project_hierarchy_roots_libraries_under_the_project(self):
|
||||
library_file = _make_valid_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")
|
||||
|
||||
hierarchy = tool.Project.get_project_hierarchy(library_file)
|
||||
|
||||
assert root in hierarchy
|
||||
assert root in hierarchy[project]
|
||||
assert child in hierarchy[root]
|
||||
|
||||
def test_project_library_data_loads_without_crash(self):
|
||||
IfcStore.library_file = _make_library_only_file()
|
||||
def test_project_library_data_loads_with_a_single_unique_root_entry(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()
|
||||
try:
|
||||
ProjectLibraryData.is_loaded = False
|
||||
ProjectLibraryData.load()
|
||||
assert ProjectLibraryData.is_loaded
|
||||
enum = ProjectLibraryData.data["parent_libraries_enum"]
|
||||
assert len(enum) == 1
|
||||
assert enum[0][1].startswith("IfcProjectLibrary ")
|
||||
keys = [entry[0] for entry in enum]
|
||||
assert len(keys) == len(set(keys))
|
||||
assert enum[0][1].startswith("IfcProject ")
|
||||
finally:
|
||||
IfcStore.library_file = None
|
||||
ProjectLibraryData.is_loaded = False
|
||||
|
||||
def test_refresh_library_succeeds_on_library_only_file(self):
|
||||
def test_refresh_library_succeeds_on_valid_library_file(self):
|
||||
import bpy
|
||||
|
||||
IfcStore.library_file = _make_library_only_file(with_child=True)
|
||||
IfcStore.library_file = _make_valid_library_file(with_child=True)
|
||||
try:
|
||||
result = bpy.ops.bim.refresh_library()
|
||||
assert result == {"FINISHED"}
|
||||
@@ -97,13 +176,13 @@ class TestLibraryOnlyFile(NewIfc):
|
||||
IfcStore.library_file = None
|
||||
ProjectLibraryData.is_loaded = False
|
||||
|
||||
def test_add_project_library_nests_under_root_when_no_project(self):
|
||||
def test_add_project_library_declares_new_library_under_the_project_root(self):
|
||||
import bpy
|
||||
|
||||
IfcStore.library_file = _make_library_only_file()
|
||||
IfcStore.library_file = _make_valid_library_file()
|
||||
library_file = IfcStore.library_file
|
||||
try:
|
||||
root = library_file.by_type("IfcProjectLibrary")[0]
|
||||
project = library_file.by_type("IfcProject")[0]
|
||||
before = set(library_file.by_type("IfcProjectLibrary"))
|
||||
|
||||
result = bpy.ops.bim.add_project_library()
|
||||
@@ -113,9 +192,9 @@ class TestLibraryOnlyFile(NewIfc):
|
||||
new_libraries = after - before
|
||||
assert len(new_libraries) == 1
|
||||
new_library = next(iter(new_libraries))
|
||||
assert new_library.Nests
|
||||
assert new_library.Nests[0].RelatingObject == root
|
||||
assert not new_library.HasContext
|
||||
assert new_library.HasContext
|
||||
assert new_library.HasContext[0].RelatingContext == project
|
||||
assert not new_library.Nests
|
||||
finally:
|
||||
IfcStore.library_file = None
|
||||
ProjectLibraryData.is_loaded = False
|
||||
|
||||
Reference in New Issue
Block a user