Fix #414: propagate file ownership when assigning instance attributes

Root cause: IfcUtil::IfcBaseClass::set_attribute_value stored the raw
instance pointer without checking which file owns it, and serialization
writes references as the instance's own id. An instance never added to
the file has id 0 and serializes as the invalid reference #0, and an
instance owned by another file serializes as its id in that other file,
which silently aliases an unrelated instance in the target file. The
inverse reference map was corrupted the same way through
register_inverse. aothms reported the unowned case in #414 in 2018 as a
regression against the pre 0.5 behaviour where ownership was propagated
automatically on assignment.

Ownership is now propagated through the idempotent IfcFile::addEntity
before the value is stored, for scalar instance attributes and both
aggregate forms. An unowned instance is registered in this file in
place, together with its forward references, and receives a fresh id.
An instance owned by another file of the same schema is recursively
copied, the same behaviour file.add() gives. A schema mismatch throws
inside addEntity instead of storing a dangling reference. Same file
assignments are unchanged.

This extends the approach of the #486 fix (PR #8406), which adopts
foreign file instances on assignment but leaves unowned instances
untouched. The test_facet.py change is the same owner history settings
isolation fix PR #8406 carries, needed because a cross schema user
leaking from the IFC2X3 test now raises in later IFC4 tests instead of
being stored silently.

Generated with the assistance of an AI coding tool.

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()

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<T>) {
-            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<T, aggregate_of_instance::ptr>) {
+            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<T, aggregate_of_aggregate_of_instance::ptr>) {
+            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<IfcUtil::IfcBaseClass*> 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);
         }

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")