Optimise update owner history to speed up complex operations

src/ifcopenshell-python/Makefile

@@ -9,7 +9,7 @@ qa:
 
 .PHONY: coverage
 coverage:
-	coverage run --source ifcopenshell -m pytest
+	coverage run --source ifcopenshell -m pytest -p no:pytest-blender test
 	coverage html
 	xdg-open htmlcov/index.html
 

src/ifcopenshell-python/ifcopenshell/api/owner/settings.py

@@ -1,4 +1,5 @@
 # Note: it is the intent for you to override these with your own functions
+users = {}
 
 
 def get_person(ifc):
@@ -14,3 +15,20 @@ def get_organisation(ifc):
 def get_application(ifc):
     applications = ifc.by_type("IfcApplication") or [None]
     return applications[0]
+
+
+def get_user(ifc):
+    person = get_person(ifc)
+    organisation = get_organisation(ifc)
+    if not person or not organisation:
+        return
+    key = f"{person.id()}-{organisation.id()}"
+    user = users.get(key)
+    if not user:
+        for element in ifc.by_type("IfcPersonAndOrganization"):
+            if element.ThePerson == person and element.TheOrganization == organisation:
+                users[key] = element
+                user = element
+        if not user:
+            return ifc.create_entity("IfcPersonAndOrganization", ThePerson=person, TheOrganization=organisation)
+    return user

src/ifcopenshell-python/ifcopenshell/api/owner/update_owner_history.py

@@ -1,6 +1,7 @@
 import time
 import ifcopenshell
 import ifcopenshell.api
+import ifcopenshell.util.element
 
 
 class Usecase:
@@ -13,34 +14,20 @@ class Usecase:
     def execute(self):
         if not hasattr(self.settings["element"], "OwnerHistory"):
             return
-        self.settings["person"] = ifcopenshell.api.owner.settings.get_person(self.file)
-        self.settings["organisation"] = ifcopenshell.api.owner.settings.get_organisation(self.file)
+        user = ifcopenshell.api.owner.settings.get_user(self.file)
+        application = ifcopenshell.api.owner.settings.get_application(self.file)
+        if not user or not application:
+            return
         if not self.settings["element"].OwnerHistory:
             self.settings["element"].OwnerHistory = ifcopenshell.api.run(
                 "owner.create_owner_history", self.file, **self.settings
             )
             return self.settings["element"].OwnerHistory
         if len(self.file.get_inverse(self.settings["element"].OwnerHistory)) > 1:
-            old_history = self.settings["element"].OwnerHistory
-            self.settings["element"].OwnerHistory = self.file.create_entity("IfcOwnerHistory")
-            for i, attribute in enumerate(old_history):
-                self.settings["element"].OwnerHistory[i] = attribute
-        user = self.get_user()
-        application = ifcopenshell.api.owner.settings.get_application(self.file)
+            new = ifcopenshell.util.element.copy(self.file, self.settings["element"].OwnerHistory)
+            self.settings["element"].OwnerHistory = new
         self.settings["element"].OwnerHistory.ChangeAction = "MODIFIED"
         self.settings["element"].OwnerHistory.LastModifiedDate = int(time.time())
         self.settings["element"].OwnerHistory.LastModifyingUser = user
         self.settings["element"].OwnerHistory.LastModifyingApplication = application
         return self.settings["element"].OwnerHistory
-
-    def get_user(self):
-        for element in self.file.by_type("IfcPersonAndOrganization"):
-            if (
-                element.ThePerson == self.settings["person"]
-                and element.TheOrganization == self.settings["organisation"]
-            ):
-                return element
-        return self.file.create_entity(
-            "IfcPersonAndOrganization",
-            **{"ThePerson": self.settings["person"], "TheOrganization": self.settings["organisation"]},
-        )

src/ifcopenshell-python/test/api/owner/test_update_owner_history.py

@@ -0,0 +1,128 @@
+import time
+import test.bootstrap
+import ifcopenshell.api
+
+
+class TestUpdateOwnerHistory(test.bootstrap.IFC4):
+    def test_creating_an_owner_history_when_there_is_no_existing_history(self):
+        get_person = ifcopenshell.api.owner.settings.get_person
+        get_organisation = ifcopenshell.api.owner.settings.get_organisation
+        get_application = ifcopenshell.api.owner.settings.get_application
+
+        person = self.file.createIfcPerson()
+        organisation = self.file.createIfcOrganization()
+        application = self.file.createIfcApplication()
+        user = self.file.createIfcPersonAndOrganization()
+        user.ThePerson = person
+        user.TheOrganization = organisation
+        ifcopenshell.api.owner.settings.get_person = lambda x : person
+        ifcopenshell.api.owner.settings.get_organisation = lambda x : organisation
+        ifcopenshell.api.owner.settings.get_application = lambda x : application
+
+        element = self.file.createIfcWall()
+        history = ifcopenshell.api.run("owner.update_owner_history", self.file, element=element)
+        assert history.is_a("IfcOwnerHistory")
+        assert element.OwnerHistory == history
+        assert history.ChangeAction == "ADDED"
+        assert abs(history.LastModifiedDate - time.time()) < 5
+        assert history.LastModifyingApplication == application
+        assert history.LastModifyingUser == user
+
+        ifcopenshell.api.owner.settings.get_person = get_person
+        ifcopenshell.api.owner.settings.get_organisation = get_organisation
+        ifcopenshell.api.owner.settings.get_application = get_application
+        ifcopenshell.api.owner.settings.users = {}
+
+    def test_updating_an_existing_history(self):
+        get_person = ifcopenshell.api.owner.settings.get_person
+        get_organisation = ifcopenshell.api.owner.settings.get_organisation
+        get_application = ifcopenshell.api.owner.settings.get_application
+
+        person = self.file.createIfcPerson()
+        organisation = self.file.createIfcOrganization()
+        application = self.file.createIfcApplication()
+        user = self.file.createIfcPersonAndOrganization()
+        user.ThePerson = person
+        user.TheOrganization = organisation
+        ifcopenshell.api.owner.settings.get_person = lambda x : person
+        ifcopenshell.api.owner.settings.get_organisation = lambda x : organisation
+        ifcopenshell.api.owner.settings.get_application = lambda x : application
+
+        element = self.file.createIfcWall()
+        old_history = ifcopenshell.api.run("owner.create_owner_history", self.file)
+        element.OwnerHistory = old_history
+
+        new_history = ifcopenshell.api.run("owner.update_owner_history", self.file, element=element)
+        assert new_history == old_history
+        assert element.OwnerHistory == new_history
+        assert new_history.ChangeAction == "MODIFIED"
+        assert abs(new_history.LastModifiedDate - time.time()) < 5
+        assert new_history.LastModifyingApplication == application
+        assert new_history.LastModifyingUser == user
+
+        ifcopenshell.api.owner.settings.get_person = get_person
+        ifcopenshell.api.owner.settings.get_organisation = get_organisation
+        ifcopenshell.api.owner.settings.get_application = get_application
+        ifcopenshell.api.owner.settings.users = {}
+
+    def test_updating_an_existing_history_shared_by_multiple_elements(self):
+        get_person = ifcopenshell.api.owner.settings.get_person
+        get_organisation = ifcopenshell.api.owner.settings.get_organisation
+        get_application = ifcopenshell.api.owner.settings.get_application
+
+        person = self.file.createIfcPerson()
+        organisation = self.file.createIfcOrganization()
+        application = self.file.createIfcApplication()
+        user = self.file.createIfcPersonAndOrganization()
+        user.ThePerson = person
+        user.TheOrganization = organisation
+        ifcopenshell.api.owner.settings.get_person = lambda x : person
+        ifcopenshell.api.owner.settings.get_organisation = lambda x : organisation
+        ifcopenshell.api.owner.settings.get_application = lambda x : application
+
+        element = self.file.createIfcWall()
+        element2 = self.file.createIfcWall()
+        old_history = ifcopenshell.api.run("owner.create_owner_history", self.file)
+        element.OwnerHistory = old_history
+        element2.OwnerHistory = old_history
+
+        new_history = ifcopenshell.api.run("owner.update_owner_history", self.file, element=element)
+        assert new_history != old_history
+        assert element.OwnerHistory == new_history
+        assert new_history.ChangeAction == "MODIFIED"
+        assert abs(new_history.LastModifiedDate - time.time()) < 5
+        assert new_history.LastModifyingApplication == application
+        assert new_history.LastModifyingUser == user
+
+        ifcopenshell.api.owner.settings.get_person = get_person
+        ifcopenshell.api.owner.settings.get_organisation = get_organisation
+        ifcopenshell.api.owner.settings.get_application = get_application
+        ifcopenshell.api.owner.settings.users = {}
+
+    def test_doing_nothing_if_no_history_can_be_updated(self):
+        person = self.file.createIfcPerson()
+        assert ifcopenshell.api.run("owner.update_owner_history", self.file, element=person) == None
+
+    def test_creating_a_user_if_one_does_not_exist(self):
+        get_person = ifcopenshell.api.owner.settings.get_person
+        get_organisation = ifcopenshell.api.owner.settings.get_organisation
+        get_application = ifcopenshell.api.owner.settings.get_application
+
+        person = self.file.createIfcPerson()
+        organisation = self.file.createIfcOrganization()
+        application = self.file.createIfcApplication()
+        ifcopenshell.api.owner.settings.get_person = lambda x : person
+        ifcopenshell.api.owner.settings.get_organisation = lambda x : organisation
+        ifcopenshell.api.owner.settings.get_application = lambda x : application
+
+        element = self.file.createIfcWall()
+        element.OwnerHistory = ifcopenshell.api.run("owner.create_owner_history", self.file)
+
+        history = ifcopenshell.api.run("owner.update_owner_history", self.file, element=element)
+        assert history.LastModifyingUser.ThePerson == person
+        assert history.LastModifyingUser.TheOrganization == organisation
+
+        ifcopenshell.api.owner.settings.get_person = get_person
+        ifcopenshell.api.owner.settings.get_organisation = get_organisation
+        ifcopenshell.api.owner.settings.get_application = get_application
+        ifcopenshell.api.owner.settings.users = {}