From 0a273085324d051c86b5feb4e9cf961e4156eb38 Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Fri, 1 Oct 2021 11:05:35 +1000 Subject: [PATCH] Optimise create owner history to not rely on a person and organisation lookup, speeding up long procedures --- .../test/tool/test_context_editor.py | 1 - .../api/owner/create_owner_history.py | 18 +---- .../ifcopenshell/api/owner/settings.py | 27 +------ .../api/sequence/add_work_schedule.py | 6 +- .../test/api/context/__init__.py | 0 .../test/api/owner/test_add_organisation.py | 9 +++ .../test/api/owner/test_add_person.py | 16 +++++ .../test/api/owner/test_add_role.py | 18 +++++ .../api/owner/test_create_owner_history.py | 29 ++++++++ .../api/owner/test_update_owner_history.py | 72 ++++--------------- src/ifcopenshell-python/test/bootstrap.py | 3 +- 11 files changed, 92 insertions(+), 107 deletions(-) create mode 100644 src/ifcopenshell-python/test/api/context/__init__.py create mode 100644 src/ifcopenshell-python/test/api/owner/test_add_organisation.py create mode 100644 src/ifcopenshell-python/test/api/owner/test_add_person.py create mode 100644 src/ifcopenshell-python/test/api/owner/test_add_role.py create mode 100644 src/ifcopenshell-python/test/api/owner/test_create_owner_history.py diff --git a/src/blenderbim/test/tool/test_context_editor.py b/src/blenderbim/test/tool/test_context_editor.py index dcec919fc0..8a6af7b74d 100644 --- a/src/blenderbim/test/tool/test_context_editor.py +++ b/src/blenderbim/test/tool/test_context_editor.py @@ -116,5 +116,4 @@ class TestExportAttributes(test.bim.bootstrap.NewFile): "UserDefinedTargetView": "UserDefinedTargetView", "ContextIdentifier": None, "ContextType": None, - "CoordinateSpaceDimension": None, } diff --git a/src/ifcopenshell-python/ifcopenshell/api/owner/create_owner_history.py b/src/ifcopenshell-python/ifcopenshell/api/owner/create_owner_history.py index ad99aa9077..6f0eb1cd89 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/owner/create_owner_history.py +++ b/src/ifcopenshell-python/ifcopenshell/api/owner/create_owner_history.py @@ -11,13 +11,11 @@ class Usecase: self.settings[key] = value def execute(self): - 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 self.file.schema != "IFC2X3": - if not self.settings["person"] or not self.settings["organisation"] or not application: + if not user or not application: return - user = self.get_user() return self.file.create_entity( "IfcOwnerHistory", **{ @@ -31,15 +29,3 @@ class Usecase: "CreationDate": int(time.time()), }, ) - - 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/ifcopenshell/api/owner/settings.py b/src/ifcopenshell-python/ifcopenshell/api/owner/settings.py index 32519a0b6b..811d7d09f0 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/owner/settings.py +++ b/src/ifcopenshell-python/ifcopenshell/api/owner/settings.py @@ -1,15 +1,4 @@ # Note: it is the intent for you to override these with your own functions -users = {} - - -def get_person(ifc): - people = ifc.by_type("IfcPerson") or [None] - return people[0] - - -def get_organisation(ifc): - organisations = ifc.by_type("IfcOrganization") or [None] - return organisations[0] def get_application(ifc): @@ -18,17 +7,5 @@ def get_application(ifc): 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 + users = ifc.by_type("IfcPersonAndOrganization") or [None] + return users[0] diff --git a/src/ifcopenshell-python/ifcopenshell/api/sequence/add_work_schedule.py b/src/ifcopenshell-python/ifcopenshell/api/sequence/add_work_schedule.py index e75bb4dde0..9f9ca50b41 100644 --- a/src/ifcopenshell-python/ifcopenshell/api/sequence/add_work_schedule.py +++ b/src/ifcopenshell-python/ifcopenshell/api/sequence/add_work_schedule.py @@ -24,9 +24,9 @@ class Usecase: name=self.settings["name"], ) work_schedule.CreationDate = ifcopenshell.util.date.datetime2ifc(datetime.now(), "IfcDateTime") - person = ifcopenshell.api.owner.settings.get_person(self.file) - if person: - work_schedule.Creators = [person] + user = ifcopenshell.api.owner.settings.get_user(self.file) + if user: + work_schedule.Creators = [user.ThePerson] work_schedule.StartTime = ifcopenshell.util.date.datetime2ifc(self.settings["start_time"], "IfcDateTime") if self.settings["work_plan"]: diff --git a/src/ifcopenshell-python/test/api/context/__init__.py b/src/ifcopenshell-python/test/api/context/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/ifcopenshell-python/test/api/owner/test_add_organisation.py b/src/ifcopenshell-python/test/api/owner/test_add_organisation.py new file mode 100644 index 0000000000..48f07d381b --- /dev/null +++ b/src/ifcopenshell-python/test/api/owner/test_add_organisation.py @@ -0,0 +1,9 @@ +import test.bootstrap +import ifcopenshell.api + + +class TestAddOrganisation(test.bootstrap.IFC4): + def test_adding_an_organisation(self): + org = ifcopenshell.api.run("owner.add_organisation", self.file, identification="Id", name="Name") + assert org.Identification == "Id" + assert org.Name == "Name" diff --git a/src/ifcopenshell-python/test/api/owner/test_add_person.py b/src/ifcopenshell-python/test/api/owner/test_add_person.py new file mode 100644 index 0000000000..b4dcf184a0 --- /dev/null +++ b/src/ifcopenshell-python/test/api/owner/test_add_person.py @@ -0,0 +1,16 @@ +import test.bootstrap +import ifcopenshell.api + + +class TestAddPerson(test.bootstrap.IFC4): + def test_adding_a_person(self): + person = ifcopenshell.api.run( + "owner.add_person", + self.file, + identification="Identification", + family_name="FamilyName", + given_name="GivenName", + ) + assert person.Identification == "Identification" + assert person.FamilyName == "FamilyName" + assert person.GivenName == "GivenName" diff --git a/src/ifcopenshell-python/test/api/owner/test_add_role.py b/src/ifcopenshell-python/test/api/owner/test_add_role.py new file mode 100644 index 0000000000..a94ac7be88 --- /dev/null +++ b/src/ifcopenshell-python/test/api/owner/test_add_role.py @@ -0,0 +1,18 @@ +import test.bootstrap +import ifcopenshell.api + + +class TestAddRole(test.bootstrap.IFC4): + def test_adding_a_role_to_a_person(self): + person = self.file.createIfcPerson() + role = ifcopenshell.api.run("owner.add_role", self.file, assigned_object=person) + assert role.is_a("IfcActorRole") + assert role.Role == "ARCHITECT" + assert person.Roles == (role,) + + def test_adding_a_role_to_an_organisation(self): + organisation = self.file.createIfcOrganization() + role = ifcopenshell.api.run("owner.add_role", self.file, assigned_object=organisation) + assert role.is_a("IfcActorRole") + assert role.Role == "ARCHITECT" + assert organisation.Roles == (role,) diff --git a/src/ifcopenshell-python/test/api/owner/test_create_owner_history.py b/src/ifcopenshell-python/test/api/owner/test_create_owner_history.py new file mode 100644 index 0000000000..1c590b48fb --- /dev/null +++ b/src/ifcopenshell-python/test/api/owner/test_create_owner_history.py @@ -0,0 +1,29 @@ +import time +import test.bootstrap +import ifcopenshell.api + + +class TestCreateOwnerHistory(test.bootstrap.IFC4): + def test_creating_nothing_if_no_user_or_application_is_available(self): + history = ifcopenshell.api.run("owner.create_owner_history", self.file) + assert history is None + + def test_creating_a_history_using_a_specified_user_and_application(self): + old_get_user = ifcopenshell.api.owner.settings.get_user + old_get_application = ifcopenshell.api.owner.settings.get_application + user = self.file.createIfcPersonAndOrganization() + application = self.file.createIfcApplication() + ifcopenshell.api.owner.settings.get_user = lambda x : user + ifcopenshell.api.owner.settings.get_application = lambda x : application + history = ifcopenshell.api.run("owner.create_owner_history", self.file) + ifcopenshell.api.owner.settings.get_user = old_get_user + ifcopenshell.api.owner.settings.get_application = old_get_application + assert history.is_a("IfcOwnerHistory") + assert history.OwningUser == user + assert history.OwningApplication == application + assert history.State == "READWRITE" + assert history.ChangeAction == "ADDED" + assert abs(time.time() - history.LastModifiedDate) < 5 + assert history.LastModifyingUser == user + assert history.LastModifyingApplication == application + assert abs(time.time() - history.CreationDate) < 5 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 index 1d03c25628..dda8fcabae 100644 --- a/src/ifcopenshell-python/test/api/owner/test_update_owner_history.py +++ b/src/ifcopenshell-python/test/api/owner/test_update_owner_history.py @@ -5,18 +5,12 @@ 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_user = ifcopenshell.api.owner.settings.get_user 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 + application = self.file.createIfcApplication() + ifcopenshell.api.owner.settings.get_user = lambda x : user ifcopenshell.api.owner.settings.get_application = lambda x : application element = self.file.createIfcWall() @@ -28,24 +22,16 @@ class TestUpdateOwnerHistory(test.bootstrap.IFC4): 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_user = get_user 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_user = ifcopenshell.api.owner.settings.get_user 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 + application = self.file.createIfcApplication() + ifcopenshell.api.owner.settings.get_user = lambda x : user ifcopenshell.api.owner.settings.get_application = lambda x : application element = self.file.createIfcWall() @@ -60,24 +46,16 @@ class TestUpdateOwnerHistory(test.bootstrap.IFC4): 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_user = get_user 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_user = ifcopenshell.api.owner.settings.get_user 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 + application = self.file.createIfcApplication() + ifcopenshell.api.owner.settings.get_user = lambda x : user ifcopenshell.api.owner.settings.get_application = lambda x : application element = self.file.createIfcWall() @@ -94,35 +72,9 @@ class TestUpdateOwnerHistory(test.bootstrap.IFC4): 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_user = get_user 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/bootstrap.py b/src/ifcopenshell-python/test/bootstrap.py index eb47e8d573..ad1fb6599b 100644 --- a/src/ifcopenshell-python/test/bootstrap.py +++ b/src/ifcopenshell-python/test/bootstrap.py @@ -13,6 +13,5 @@ class IFC2X3: @pytest.fixture(autouse=True) def setup(self): self.file = ifcopenshell.api.run("project.create_file", version="IFC2X3") - ifcopenshell.api.owner.settings.get_person = lambda ifc: ifc.createIfcPerson() - ifcopenshell.api.owner.settings.get_organisation = lambda ifc: ifc.createIfcOrganization() + ifcopenshell.api.owner.settings.get_user = lambda ifc: ifc.createIfcPersonAndOrganization() ifcopenshell.api.owner.settings.get_application = lambda ifc: ifc.createIfcApplication()