diff --git a/src/ifcopenshell-python/test/test_file.py b/src/ifcopenshell-python/test/test_file.py index c2c0aa54ea..0913ad2e6f 100644 --- a/src/ifcopenshell-python/test/test_file.py +++ b/src/ifcopenshell-python/test/test_file.py @@ -198,6 +198,54 @@ class TestFile(test.bootstrap.IFC4): result = self.file.add(element) assert result.is_a() == element.is_a() + def test_assigning_an_unowned_instance_adds_it_to_the_file(self): + user = ifcopenshell.create_entity("IfcPersonAndOrganization", schema="IFC4") + owner = self.file.createIfcOwnerHistory() + owner.OwningUser = user + assert user.id() != 0 + assert self.file.by_id(user.id()) == user + assert self.file.get_inverse(user) == {owner} + assert f"#{user.id()}" in self.file.to_string() + + def test_assigning_an_unowned_instance_adds_its_references_to_the_file(self): + person = ifcopenshell.create_entity("IfcPerson", schema="IFC4") + organization = ifcopenshell.create_entity("IfcOrganization", schema="IFC4", Name="o") + user = ifcopenshell.create_entity( + "IfcPersonAndOrganization", schema="IFC4", ThePerson=person, TheOrganization=organization + ) + owner = self.file.createIfcOwnerHistory() + owner.OwningUser = user + assert self.file.by_id(person.id()) == person + assert self.file.by_id(organization.id()) == organization + assert self.file.traverse(user) == [user, person, organization] + + def test_assigning_unowned_instances_in_an_aggregate_adds_them_to_the_file(self): + role = ifcopenshell.create_entity("IfcActorRole", schema="IFC4") + person = self.file.createIfcPerson() + person.Roles = [role] + assert role.id() != 0 + assert self.file.by_id(role.id()) == role + + def test_assigning_an_instance_from_another_file_copies_it_into_the_file(self): + g = ifcopenshell.file(schema="IFC4") + other_user = g.createIfcPersonAndOrganization() + owner = self.file.createIfcOwnerHistory() + owner.OwningUser = other_user + copied = owner.OwningUser + assert copied.wrapped_data.file_pointer() != other_user.wrapped_data.file_pointer() + assert self.file.by_id(copied.id()) == copied + assert g.by_id(other_user.id()) == other_user + + def test_assigning_an_instance_of_another_schema_raises(self): + g = ifcopenshell.file(schema="IFC2X3") + other_user = g.createIfcPersonAndOrganization() + owner = self.file.createIfcOwnerHistory() + with pytest.raises(Exception): + owner.OwningUser = other_user + unowned = ifcopenshell.create_entity("IfcPersonAndOrganization", schema="IFC2X3") + with pytest.raises(Exception): + owner.OwningUser = unowned + def test_getting_elements_by_type(self): wall = self.file.createIfcWall() slab = self.file.createIfcSlab() diff --git a/src/ifcparse/IfcParse.cpp b/src/ifcparse/IfcParse.cpp index bc6df697af..221d3d46b8 100644 --- a/src/ifcparse/IfcParse.cpp +++ b/src/ifcparse/IfcParse.cpp @@ -1180,12 +1180,45 @@ IfcUtil::IfcBaseClass::set_attribute_value(size_t i, const T& t) { } { void* const storage = file_ ? std::visit([](const auto& m) { return (void*)&m; }, file_->storage_) : nullptr; + // #414 #486 Propagate ownership of instances not yet owned by this file + // through the idempotent IfcFile::addEntity: unowned instances are + // registered in place, instances from another file are copied, and a + // schema mismatch throws instead of storing a dangling reference. + auto adopt_if_not_owned = [](IfcParse::IfcFile* file, IfcUtil::IfcBaseClass* instance) -> IfcUtil::IfcBaseClass* { + if (instance != nullptr && file != nullptr && instance->file_ != file) { + return file->addEntity(instance); + } + return instance; + }; if constexpr (std::is_pointer_v) { - if (t) { - data_.set_attribute_value(storage, &declaration(), id() ? id() : identity(), i, t); + IfcUtil::IfcBaseClass* to_write = adopt_if_not_owned(file_, t); + if (to_write) { + data_.set_attribute_value(storage, &declaration(), id() ? id() : identity(), i, to_write); } else { data_.set_attribute_value(storage, &declaration(), id() ? id() : identity(), i, Blank{}); } + } else if constexpr (std::is_same_v) { + aggregate_of_instance::ptr to_write(new aggregate_of_instance); + if (t) { + to_write->reserve(t->size()); + for (auto* instance : *t) { + to_write->push(adopt_if_not_owned(file_, instance)); + } + } + data_.set_attribute_value(storage, &declaration(), id() ? id() : identity(), i, to_write); + } else if constexpr (std::is_same_v) { + aggregate_of_aggregate_of_instance::ptr to_write(new aggregate_of_aggregate_of_instance); + if (t) { + for (auto outer = t->begin(); outer != t->end(); ++outer) { + std::vector inner; + inner.reserve(outer->size()); + for (auto* instance : *outer) { + inner.push_back(adopt_if_not_owned(file_, instance)); + } + to_write->push(inner); + } + } + data_.set_attribute_value(storage, &declaration(), id() ? id() : identity(), i, to_write); } else { data_.set_attribute_value(storage, &declaration(), id() ? id() : identity(),i, t); } diff --git a/src/ifctester/test/test_facet.py b/src/ifctester/test/test_facet.py index 3c5d91a6e4..f7a9c4c3e1 100644 --- a/src/ifctester/test/test_facet.py +++ b/src/ifctester/test/test_facet.py @@ -252,19 +252,28 @@ class TestEntity: ifc, identification="AWB", name="Architects Without Ballpens" ) user = ifcopenshell.api.owner.add_person_and_organisation(ifc, person=person, organisation=organisation) - ifcopenshell.api.owner.settings.get_user = lambda x: user - ifcopenshell.api.owner.settings.get_application = lambda x: application + # The owner-history settings are module-global. Save and restore them so the + # IFC2X3 user/application created here does not leak into later IFC4 tests, + # where assigning a cross-schema instance now raises (see #486). + original_get_user = ifcopenshell.api.owner.settings.get_user + original_get_application = ifcopenshell.api.owner.settings.get_application + try: + ifcopenshell.api.owner.settings.get_user = lambda x: user + ifcopenshell.api.owner.settings.get_application = lambda x: application - element = ifcopenshell.api.root.create_entity(ifc, "IfcFlowTerminal") - element_type = ifcopenshell.api.root.create_entity(ifc, "IfcAirTerminalType") - ifcopenshell.api.type.assign_type(ifc, related_objects=[element], relating_type=element_type) - facet = Entity(name="IFCAIRTERMINAL") - assert facet.filter(ifc) == [element] - run("In IFC2X3 the type class is checked instead 1/2", facet=facet, inst=element, expected=True) + element = ifcopenshell.api.root.create_entity(ifc, "IfcFlowTerminal") + element_type = ifcopenshell.api.root.create_entity(ifc, "IfcAirTerminalType") + ifcopenshell.api.type.assign_type(ifc, related_objects=[element], relating_type=element_type) + facet = Entity(name="IFCAIRTERMINAL") + assert facet.filter(ifc) == [element] + run("In IFC2X3 the type class is checked instead 1/2", facet=facet, inst=element, expected=True) - facet = Entity(name="IFCELECTRICAPPLIANCE") - assert facet.filter(ifc) == [] - run("In IFC2X3 the type class is checked instead 2/2", facet=facet, inst=element, expected=False) + facet = Entity(name="IFCELECTRICAPPLIANCE") + assert facet.filter(ifc) == [] + run("In IFC2X3 the type class is checked instead 2/2", facet=facet, inst=element, expected=False) + finally: + ifcopenshell.api.owner.settings.get_user = original_get_user + ifcopenshell.api.owner.settings.get_application = original_get_application def test_to_string_required_applicability(self): spec = ifctester.ids.Specification(name="Foo", minOccurs=1, maxOccurs="unbounded")