From 20302b00c67784bef82921a376fd3285ba35fa50 Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Thu, 20 Apr 2023 21:32:36 +0200 Subject: [PATCH] Improve applySchemasAndSecurity perf test The test is now more realistic, as there's an archetype plus template with some definition overriding. Moreover, we introduced three modes of definition updating: NONE, ROOT_ONLY, and FULL. The first two would allow use pre-computed definitions for archetypes without the need to touch the objects (except for cloning immutable ones!). Hence, we'll aim for NONE definition update mode, if possible. --- .../schema/DefinitionUpdateOption.java | 39 +++++ .../midpoint/schema/GetOperationOptions.java | 25 +++ .../schema/ParsedGetOperationOptions.java | 7 + .../impl/controller/SchemaTransformer.java | 63 +++++--- .../model/impl/perf/TestPerformance.java | 148 +++++++++++++----- .../test/resources/perf/archetype-person.xml | 11 ++ .../resources/perf/object-template-person.xml | 72 +++++++++ .../test/asserter/AssignmentGenerator.java | 26 ++- 8 files changed, 323 insertions(+), 68 deletions(-) create mode 100644 infra/schema/src/main/java/com/evolveum/midpoint/schema/DefinitionUpdateOption.java create mode 100644 model/model-impl/src/test/resources/perf/archetype-person.xml create mode 100644 model/model-impl/src/test/resources/perf/object-template-person.xml diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/DefinitionUpdateOption.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/DefinitionUpdateOption.java new file mode 100644 index 00000000000..19723a76396 --- /dev/null +++ b/infra/schema/src/main/java/com/evolveum/midpoint/schema/DefinitionUpdateOption.java @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2010-2017 Evolveum and contributors + * + * This work is dual-licensed under the Apache License 2.0 + * and European Union Public License. See LICENSE file for details. + */ + +package com.evolveum.midpoint.schema; + +import com.evolveum.midpoint.util.annotation.Experimental; + +/** + * Should be definitions updated after an object is retrieved via model API? + * See `applySchemasAndSecurity` method. + * + * Applicable only on the root level. + * + * TEMPORARY, mainly for the purposes of experiments with various approaches during midPoint 4.8 development. + */ +@Experimental +public enum DefinitionUpdateOption { + + /** + * The object or container definition is not to be updated at all. + */ + NONE, + + /** + * Only the definition at the root level (object or container) is updated. + * Definitions of the child items (if any) are not touched. + * For example, the client may apply the root definition if needed. + */ + ROOT_ONLY, + + /** + * Updated definition is propagated onto all items in the returned structure. + */ + DEEP; +} diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/GetOperationOptions.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/GetOperationOptions.java index f0b545aa78f..fcb1d913480 100644 --- a/infra/schema/src/main/java/com/evolveum/midpoint/schema/GetOperationOptions.java +++ b/infra/schema/src/main/java/com/evolveum/midpoint/schema/GetOperationOptions.java @@ -212,6 +212,14 @@ public class GetOperationOptions extends AbstractOptions implements Serializable */ private DefinitionProcessingOption definitionProcessing; + /** + * Probably temporary. Default is {@link DefinitionUpdateOption#NONE}. + * + * @see DefinitionUpdateOption + */ + @Experimental + private DefinitionUpdateOption definitionUpdate; + /** * Whether to override default iteration method (in searchObjectsIterative) configured for particular DBMS. * @@ -920,6 +928,23 @@ public static GetOperationOptions createDefinitionProcessing(DefinitionProcessin return opts; } + public DefinitionUpdateOption getDefinitionUpdate() { + return definitionUpdate; + } + + public void setDefinitionUpdate(DefinitionUpdateOption value) { + this.definitionUpdate = value; + } + + public GetOperationOptions definitionUpdate(DefinitionUpdateOption value) { + this.definitionUpdate = value; + return this; + } + + public static DefinitionUpdateOption getDefinitionUpdate(GetOperationOptions options) { + return options != null ? options.definitionUpdate : null; + } + public IterationMethodType getIterationMethod() { return iterationMethod; } diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/ParsedGetOperationOptions.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/ParsedGetOperationOptions.java index dc6f1123834..d43ab7b22ee 100644 --- a/infra/schema/src/main/java/com/evolveum/midpoint/schema/ParsedGetOperationOptions.java +++ b/infra/schema/src/main/java/com/evolveum/midpoint/schema/ParsedGetOperationOptions.java @@ -12,6 +12,7 @@ import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Objects; import com.evolveum.midpoint.util.annotation.Experimental; @@ -85,4 +86,10 @@ public boolean isEmpty() { public Map getDefinitionProcessingMap() { return SelectorOptions.extractOptionValues(originalCollection, (o) -> o.getDefinitionProcessing()); } + + public @NotNull DefinitionUpdateOption getDefinitionUpdate() { + return Objects.requireNonNullElse( + GetOperationOptions.getDefinitionUpdate(getRootOptions()), + DefinitionUpdateOption.DEEP); // temporary default + } } diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/SchemaTransformer.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/SchemaTransformer.java index 23a2f4cad79..8f1777885a1 100644 --- a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/SchemaTransformer.java +++ b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/SchemaTransformer.java @@ -242,23 +242,31 @@ SearchResultList applySchemasAndSecurityToContainers( object = object.cloneIfImmutable(); // TODO clone only if really needed + DefinitionUpdateOption definitionUpdateOption = parsedOptions.getDefinitionUpdate(); + boolean raw = GetOperationOptions.isRaw(rootOptions); + + // we must determine the template before items are stripped off due to authorizations + ObjectTemplateType objectTemplate = !raw && definitionUpdateOption != DefinitionUpdateOption.NONE ? + determineObjectTemplate(object, result) : null; + if (!GetOperationOptions.isExecutionPhase(rootOptions)) { applySchemasAndSecurityPhase(object, securityConstraints, AuthorizationPhaseType.REQUEST); } applySchemasAndSecurityPhase(object, securityConstraints, AuthorizationPhaseType.EXECUTION); - // TODO is this needed? - transform(object, new DefinitionsToTransformable()); + if (definitionUpdateOption != DefinitionUpdateOption.NONE) { + // TODO is this needed? + transform(object, new DefinitionsToTransformable()); - // we do not need to process object template when processing in raw mode - // TODO cache the definitions - if (!GetOperationOptions.isRaw(rootOptions)) { - ObjectTemplateType objectTemplate = determineObjectTemplate(object, result); - applyObjectTemplateToObject(object, objectTemplate, task, result); - } + // we do not need to process object template when processing in raw mode + // TODO cache the definitions + if (objectTemplate != null) { + applyObjectTemplateToObject(object, objectTemplate, definitionUpdateOption, task, result); + } - // TODO consider removing this as it shouldn't be needed after definitions caching is implemented - applyDefinitionProcessingOption(object, parsedOptions); + // TODO consider removing this as it shouldn't be needed after definitions caching is implemented + applyDefinitionProcessingOption(object, parsedOptions); + } } catch (Throwable t) { result.recordException(t); throw t; @@ -759,19 +767,20 @@ void applyObjectTemplateToDefinition( applyObjectTemplateToDefinition(objectDefinition, subTemplate.asObjectable(), task, result); } for (ObjectTemplateItemDefinitionType templateItemDef: objectTemplate.getItem()) { - ItemPath itemPath = ItemRefinedDefinitionTypeUtil.getRef(templateItemDef); - ItemDefinition itemDef = objectDefinition.findItemDefinition(itemPath); - if (itemDef != null) { - applyObjectTemplateItem(itemDef, templateItemDef, "item " + itemPath + " in object type " + objectDefinition.getTypeName() + " as specified in item definition in " + objectTemplate); - } else { - OperationResult subResult = result.createMinorSubresult(SchemaTransformer.class.getName() + ".applyObjectTemplateToDefinition"); - subResult.recordPartialError("No definition for item " + itemPath + " in object type " + objectDefinition.getTypeName() + " as specified in item definition in " + objectTemplate); - } + ItemPath itemPath = ItemRefinedDefinitionTypeUtil.getRef(templateItemDef); + ItemDefinition itemDef = objectDefinition.findItemDefinition(itemPath); + if (itemDef != null) { + applyObjectTemplateItem(itemDef, templateItemDef, "item " + itemPath + " in object type " + objectDefinition.getTypeName() + " as specified in item definition in " + objectTemplate); + } else { + OperationResult subResult = result.createMinorSubresult(SchemaTransformer.class.getName() + ".applyObjectTemplateToDefinition"); + subResult.recordPartialError("No definition for item " + itemPath + " in object type " + objectDefinition.getTypeName() + " as specified in item definition in " + objectTemplate); + } } } private void applyObjectTemplateToObject( - PrismObject object, ObjectTemplateType objectTemplate, Task task, OperationResult result) + PrismObject object, ObjectTemplateType objectTemplate, DefinitionUpdateOption option, + Task task, OperationResult result) throws ObjectNotFoundException, SchemaException, ConfigurationException { if (objectTemplate == null) { return; @@ -783,7 +792,7 @@ private void applyObjectTemplateToObject( for (ObjectReferenceType includeRef: objectTemplate.getIncludeRef()) { PrismObject subTemplate = cacheRepositoryService.getObject( ObjectTemplateType.class, includeRef.getOid(), createReadOnlyCollection(), result); - applyObjectTemplateToObject(object, subTemplate.asObjectable(), task, result); + applyObjectTemplateToObject(object, subTemplate.asObjectable(), option, task, result); } for (ObjectTemplateItemDefinitionType templateItemDef: objectTemplate.getItem()) { ItemPath itemPath = ItemRefinedDefinitionTypeUtil.getRef(templateItemDef); @@ -797,12 +806,14 @@ private void applyObjectTemplateToObject( + " as specified in item definition in " + objectTemplate); continue; } - Collection> items = object.getAllItems(itemPath); - for (Item item : items) { - ItemDefinition itemDef = item.getDefinition(); - if (itemDef != itemDefFromObject) { - applyObjectTemplateItem(itemDef, templateItemDef, "item "+itemPath+" in " + object - + " as specified in item definition in "+objectTemplate); + if (option == DefinitionUpdateOption.DEEP) { + Collection> items = object.getAllItems(itemPath); + for (Item item : items) { + ItemDefinition itemDef = item.getDefinition(); + if (itemDef != itemDefFromObject) { + applyObjectTemplateItem(itemDef, templateItemDef, "item " + itemPath + " in " + object + + " as specified in item definition in " + objectTemplate); + } } } } diff --git a/model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/perf/TestPerformance.java b/model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/perf/TestPerformance.java index e2222f46992..c0c2dd1dca8 100644 --- a/model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/perf/TestPerformance.java +++ b/model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/perf/TestPerformance.java @@ -11,8 +11,16 @@ import java.io.File; +import com.evolveum.midpoint.prism.PrismObjectDefinition; +import com.evolveum.midpoint.schema.DefinitionUpdateOption; +import com.evolveum.midpoint.schema.GetOperationOptions; import com.evolveum.midpoint.schema.internals.InternalsConfig; +import com.evolveum.midpoint.util.exception.*; +import com.evolveum.midpoint.xml.ns._public.common.common_3.ArchetypeType; + +import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectTemplateType; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; @@ -30,7 +38,6 @@ import com.evolveum.midpoint.test.asserter.AssignmentGenerator; import com.evolveum.midpoint.test.util.MidPointTestConstants; import com.evolveum.midpoint.util.CheckedConsumer; -import com.evolveum.midpoint.util.exception.CommonException; import com.evolveum.midpoint.xml.ns._public.common.common_3.RoleType; import com.evolveum.midpoint.xml.ns._public.common.common_3.UserType; @@ -47,6 +54,10 @@ public class TestPerformance extends AbstractEmptyInternalModelTest { private static final File TEST_DIR = new File(MidPointTestConstants.TEST_RESOURCES_DIR, "perf"); + private static final TestObject OBJECT_TEMPLATE_PERSON = TestObject.file( + TEST_DIR, "object-template-person.xml", "202c9a5e-b876-4009-8b87-25688b12f7b8"); + private static final TestObject ARCHETYPE_PERSON = TestObject.file( + TEST_DIR, "archetype-person.xml", "3bb58e36-04b3-4601-a57a-4aaaa3b64458"); private static final TestObject ROLE_CAN_READ_ALL = TestObject.file( TEST_DIR, "role-can-read-all.xml", "8d79c980-0999-49f7-ba11-6776dad41770"); private static final TestObject USER_CAN_READ_ALL = TestObject.file( @@ -69,6 +80,7 @@ public class TestPerformance extends AbstractEmptyInternalModelTest { public void initSystem(Task initTask, OperationResult initResult) throws Exception { super.initSystem(initTask, initResult); initTestObjects(initTask, initResult, + OBJECT_TEMPLATE_PERSON, ARCHETYPE_PERSON, ROLE_CAN_READ_ALL, USER_CAN_READ_ALL, ROLE_CAN_READ_ALMOST_ALL, USER_CAN_READ_ALMOST_ALL, ROLE_CAN_READ_FEW, USER_CAN_READ_FEW); @@ -83,89 +95,134 @@ protected boolean isAvoidLoggingChange() { /** Tests schema/security application for full autz (superuser). */ @Test public void test100ApplyFullAutz() throws CommonException { - executeAutzTest("full", null); + executeAutzTest("full", user -> assertReadAll(user)); } /** Tests schema/security application for "read all" autz. */ @Test public void test110ApplyReadAll() throws CommonException { login(USER_CAN_READ_ALL.get()); - executeAutzTest("read all", user -> { - assertUserAfter(user.asPrismObject()) - .extension() - .assertSize(50) - .end(); - var assignment = user.getAssignment().get(0); - assertThat(assignment.getTargetRef()).isNotNull(); - assertThat(assignment.getDescription()).isNotNull(); - }); + executeAutzTest("read all", user -> assertReadAll(user)); + } + + private void assertReadAll(UserType user) { + assertUser(user.asPrismObject(), "after") + .extension() + .assertSize(50) + .end(); + var assignment = user.getAssignment().get(0); + assertThat(assignment.getTargetRef()).isNotNull(); + assertThat(assignment.getDescription()).isNotNull(); } /** Tests schema/security application for "read almost all" autz. */ @Test public void test120ApplyReadAlmostAll() throws CommonException { login(USER_CAN_READ_ALMOST_ALL.get()); - executeAutzTest("read almost all", user -> { - assertUserAfter(user.asPrismObject()) - .extension() - .assertSize(49) - .end(); - var assignment = user.getAssignment().get(0); - assertThat(assignment.getTargetRef()).isNotNull(); - assertThat(assignment.getDescription()).isNotNull(); - }); + executeAutzTest("read almost all", user -> assertReadAlmostAll(user)); + } + + private void assertReadAlmostAll(UserType user) throws CommonException { + assertUser(user.asPrismObject(), "after") + .extension() + .assertSize(49) + .end(); + var assignment = user.getAssignment().get(0); + assertThat(assignment.getTargetRef()).isNotNull(); + assertThat(assignment.getDescription()).isNotNull(); } /** Tests schema/security application for "read few" autz. */ @Test public void test130ApplyReadFew() throws CommonException { login(USER_CAN_READ_FEW.get()); - executeAutzTest("read few", user -> { - assertUserAfter(user.asPrismObject()) - .extension() - .assertSize(1) - .end(); - var assignment = user.getAssignment().get(0); - assertThat(assignment.getTargetRef()).isNull(); - assertThat(assignment.getDescription()).isNotNull(); - }); + executeAutzTest("read few", user -> assertReadFew(user)); } - private void executeAutzTest(String label, CheckedConsumer asserter) + private void assertReadFew(UserType user) throws CommonException { + assertUser(user.asPrismObject(), "after") + .extension() + .assertSize(1) + .end(); + var assignment = user.getAssignment().get(0); + assertThat(assignment.getTargetRef()).isNull(); + assertThat(assignment.getDescription()).isNotNull(); + } + + private void executeAutzTest( + String autzLabel, CheckedConsumer autzAsserter) + throws CommonException { + + executeAutzTest1(autzLabel, DefinitionUpdateOption.DEEP, autzAsserter); + executeAutzTest1(autzLabel, DefinitionUpdateOption.ROOT_ONLY, autzAsserter); + executeAutzTest1(autzLabel, DefinitionUpdateOption.NONE, autzAsserter); + } + + private void executeAutzTest1( + String autzLabel, DefinitionUpdateOption definitionUpdateOption, CheckedConsumer autzAsserter) throws CommonException { Task task = getTestTask(); OperationResult result = task.getResult(); - int iterations = 20; - int numUsers = 2000; + int iterations = 5; + int numUsers = 4000; + + String label = autzLabel + "/" + definitionUpdateOption; + + when("heating up: " + label); + int heatUpExecutions = numUsers * 2; + long heatUpDuration = executeSingleAutzTestIteration( + heatUpExecutions, definitionUpdateOption, + user -> { + if (autzAsserter != null) { + autzAsserter.accept(user); + } + checkDefinition(user, definitionUpdateOption); + }, task, result); + + display(String.format( + "Heat-up %s: Applied to a single user in %.3f ms (%,d executions)", + label, (double) heatUpDuration / heatUpExecutions, heatUpExecutions)); + + when("testing: " + label); long start = System.currentTimeMillis(); long netDuration = 0; for (int i = 0; i < iterations; i++) { netDuration += executeSingleAutzTestIteration( - numUsers, i == 0 ? asserter : null, task, result); + numUsers, definitionUpdateOption, null, task, result); } long grossDuration = System.currentTimeMillis() - start; int executions = numUsers * iterations; display(String.format( - "Testing %s: Applied a security to a single user in %.3f ms (%,d ms gross and %,d ms net duration, %,d executions)", + "Testing %s: Applied to a single user in %.3f ms (%,d ms gross and %,d ms net duration, %,d executions)", label, (double) netDuration / executions, grossDuration, netDuration, executions)); } private long executeSingleAutzTestIteration( - int numUsers, CheckedConsumer asserter, Task task, OperationResult result) + int numUsers, DefinitionUpdateOption definitionUpdateOption, + CheckedConsumer asserter, Task task, OperationResult result) throws CommonException { SearchResultList> users = new SearchResultList<>(); for (int i = 0; i < numUsers; i++) { UserType user = new UserType() - .name("user " + i); - extensionValueGenerator.populateExtension(user.asPrismObject(), 50); + .name("user " + i) + .assignment(ARCHETYPE_PERSON.assignmentTo() + .description("just-to-make-test-happy")); + PrismObject userPrismObject = user.asPrismObject(); + extensionValueGenerator.populateExtension(userPrismObject, 50); assignmentGenerator.populateAssignments(user, 50); - users.add(user.asPrismObject()); + assignmentGenerator.createRoleRefs(user); + userPrismObject.freeze(); + users.add(userPrismObject); } + var parsedOptions = ParsedGetOperationOptions.of( + GetOperationOptions.createReadOnly() + .definitionUpdate(definitionUpdateOption)); + long start = System.currentTimeMillis(); var usersAfter = - schemaTransformer.applySchemasAndSecurityToObjects(users, ParsedGetOperationOptions.empty(), task, result); + schemaTransformer.applySchemasAndSecurityToObjects(users, parsedOptions, task, result); long duration = System.currentTimeMillis() - start; if (asserter != null) { @@ -174,4 +231,17 @@ private long executeSingleAutzTestIteration( return duration; } + + private void checkDefinition(UserType user, DefinitionUpdateOption option) { + PrismObjectDefinition rootDef = user.asPrismObject().getDefinition(); + assertThat(rootDef.findPropertyDefinition(UserType.F_DESCRIPTION).getDisplayName()) + .as("description displayName in root def") + .isEqualTo(option == DefinitionUpdateOption.NONE ? "ObjectType.description" : "X-DESCRIPTION"); + if (user.getDescription() != null) { + var localDef = user.asPrismObject().findProperty(UserType.F_DESCRIPTION).getDefinition(); + assertThat(localDef.getDisplayName()) + .as("description displayName (local)") + .isEqualTo(option != DefinitionUpdateOption.DEEP ? "ObjectType.description" : "X-DESCRIPTION"); + } + } } diff --git a/model/model-impl/src/test/resources/perf/archetype-person.xml b/model/model-impl/src/test/resources/perf/archetype-person.xml new file mode 100644 index 00000000000..619e6363ff6 --- /dev/null +++ b/model/model-impl/src/test/resources/perf/archetype-person.xml @@ -0,0 +1,11 @@ + + person + + + + honorificSuffix + hidden + + + diff --git a/model/model-impl/src/test/resources/perf/object-template-person.xml b/model/model-impl/src/test/resources/perf/object-template-person.xml new file mode 100644 index 00000000000..91e3edf96f6 --- /dev/null +++ b/model/model-impl/src/test/resources/perf/object-template-person.xml @@ -0,0 +1,72 @@ + + person + + description + X-DESCRIPTION + + 1 + + + + emailAddress + X-EMAIL + + 1 + + + + assignment/description + X-A-DESCRIPTION + + 1 + + + + assignment/construction + 100 + + 0 + + + + extension/prop0001 + 101 + + + extension/prop0002 + 102 + + + extension/prop0003 + 103 + + + extension/prop0004 + 104 + + + extension/prop0005 + 105 + + + extension/prop0006 + 106 + + + extension/prop0007 + 107 + + + extension/prop0008 + 108 + + + extension/prop0009 + 109 + + + extension/prop0010 + 110 + + diff --git a/repo/repo-test-util/src/main/java/com/evolveum/midpoint/test/asserter/AssignmentGenerator.java b/repo/repo-test-util/src/main/java/com/evolveum/midpoint/test/asserter/AssignmentGenerator.java index 812bb195d10..0b91a6f9e19 100644 --- a/repo/repo-test-util/src/main/java/com/evolveum/midpoint/test/asserter/AssignmentGenerator.java +++ b/repo/repo-test-util/src/main/java/com/evolveum/midpoint/test/asserter/AssignmentGenerator.java @@ -12,9 +12,7 @@ import java.util.function.Function; import javax.xml.namespace.QName; -import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentHolderType; -import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentType; -import com.evolveum.midpoint.xml.ns._public.common.common_3.RoleType; +import com.evolveum.midpoint.xml.ns._public.common.common_3.*; /** * Generates assignments (in memory). @@ -42,4 +40,26 @@ public void populateAssignments(AssignmentHolderType object, int numberOfAssignm .targetRef(targetOidFunction.apply(i), targetTypeFunction.apply(i))); } } + + /** + * For objects that are not going through the clockwork, we (very simplistically) create + * `roleMembershipRef` and `archetypeRef` values from assignments. + * + * Assuming all assignments are valid. + * + * TODO improve if needed + */ + public void createRoleRefs(AssignmentHolderType object) { + List roleMembershipRefList = object.getRoleMembershipRef(); + List archetypeRefList = object.getArchetypeRef(); + for (AssignmentType assignment : object.getAssignment()) { + ObjectReferenceType targetRef = assignment.getTargetRef(); + if (targetRef != null) { + roleMembershipRefList.add(targetRef.clone()); + if (ArchetypeType.COMPLEX_TYPE.equals(targetRef.getType())) { + archetypeRefList.add(targetRef.clone()); + } + } + } + } }