From 90e481ce2c7e943afc701c5e863a5254fd135dda Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Mon, 20 Sep 2021 10:17:16 +1000 Subject: [PATCH] Optimise update owner history to speed up complex operations --- src/ifcopenshell-python/Makefile | 2 +- .../ifcopenshell/api/owner/settings.py | 18 +++ .../api/owner/update_owner_history.py | 27 +--- .../test/api/geometry/__init__.py | 0 .../test/api/owner/__init__.py | 0 .../api/owner/test_update_owner_history.py | 128 ++++++++++++++++++ .../test/api/spatial/__init__.py | 0 7 files changed, 154 insertions(+), 21 deletions(-) create mode 100644 src/ifcopenshell-python/test/api/geometry/__init__.py create mode 100644 src/ifcopenshell-python/test/api/owner/__init__.py create mode 100644 src/ifcopenshell-python/test/api/owner/test_update_owner_history.py create mode 100644 src/ifcopenshell-python/test/api/spatial/__init__.py diff --git a/src/ifcopenshell-python/Makefile b/src/ifcopenshell-python/Makefile index 3ef57ccd12..7d38c58657 100644 --- a/src/ifcopenshell-python/Makefile +++ b/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 diff --git a/src/ifcopenshell-python/ifcopenshell/api/owner/settings.py b/src/ifcopenshell-python/ifcopenshell/api/owner/settings.py index 4e7b48dda1..32519a0b6b 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/owner/settings.py +++ b/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 diff --git a/src/ifcopenshell-python/ifcopenshell/api/owner/update_owner_history.py b/src/ifcopenshell-python/ifcopenshell/api/owner/update_owner_history.py index f6354abe2f..e3e469f169 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/owner/update_owner_history.py +++ b/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"]}, - ) diff --git a/src/ifcopenshell-python/test/api/geometry/__init__.py b/src/ifcopenshell-python/test/api/geometry/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/ifcopenshell-python/test/api/owner/__init__.py b/src/ifcopenshell-python/test/api/owner/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/ifcopenshell-python/test/api/owner/test_update_owner_history.py b/src/ifcopenshell-python/test/api/owner/test_update_owner_history.py new file mode 100644 index 0000000000..1d03c25628 --- /dev/null +++ b/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 = {} diff --git a/src/ifcopenshell-python/test/api/spatial/__init__.py b/src/ifcopenshell-python/test/api/spatial/__init__.py new file mode 100644 index 0000000000..e69de29bb2