Skip to content

Commit

Permalink
Fix excessive cloning when deleting PCVs by ID
Browse files Browse the repository at this point in the history
The normalization of values to-be-deleted was done too lately,
leading to excessive cloning (basically at each mapping source
evaluation). This is now fixed. The normalization is done when
ODO is computed in the focus context.

This resolves MID-7057. However, it is only an approximate fix
for now. The more thorough analysis of the problem is needed.
  • Loading branch information
mederly committed May 26, 2021
1 parent 44ffb3d commit 988ba40
Show file tree
Hide file tree
Showing 5 changed files with 8,186 additions and 7 deletions.
Expand Up @@ -135,12 +135,14 @@ private void addSecondaryDeltas(List<ObjectDelta<O>> allDeltas) {
*/
@NotNull
public ObjectDeltaObject<O> getObjectDeltaObjectRelative() {
return new ObjectDeltaObject<>(objectCurrent, getCurrentDelta(), objectNew, getObjectDefinition());
return new ObjectDeltaObject<>(objectCurrent, getCurrentDelta(), objectNew, getObjectDefinition())
.normalizeValuesToDelete(true); // FIXME temporary solution
}

@NotNull
public ObjectDeltaObject<O> getObjectDeltaObjectAbsolute() {
return new ObjectDeltaObject<>(objectOld, getSummaryDelta(), objectNew, getObjectDefinition());
return new ObjectDeltaObject<>(objectOld, getSummaryDelta(), objectNew, getObjectDefinition())
.normalizeValuesToDelete(true); // FIXME temporary solution
}

// This method may be useful for hooks. E.g. if a hook wants to insert a special secondary delta to avoid
Expand Down
Expand Up @@ -6,9 +6,12 @@
*/
package com.evolveum.midpoint.model.intest.mapping;

import static org.assertj.core.api.Assertions.assertThat;
import static org.testng.AssertJUnit.assertNotNull;

import java.io.File;
import java.io.FileNotFoundException;
import java.net.ConnectException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.UUID;
Expand All @@ -32,6 +35,8 @@
import com.evolveum.midpoint.prism.util.PrismTestUtil;
import com.evolveum.midpoint.prism.xml.XmlTypeConverter;
import com.evolveum.midpoint.schema.constants.SchemaConstants;
import com.evolveum.midpoint.schema.internals.InternalCounters;
import com.evolveum.midpoint.schema.internals.InternalMonitor;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.schema.result.OperationResultStatus;
import com.evolveum.midpoint.schema.util.MiscSchemaUtil;
Expand Down Expand Up @@ -89,6 +94,11 @@ public class TestMapping extends AbstractMappingTest {
"resource-dummy-timed.xml", "567d9834-4f2c-4e5b-89a6-ebd804c7d469");
private static final String RESOURCE_DUMMY_TIMED_NAME = "timed";

private static final TestResource RESOURCE_DUMMY_MEGA_OUTBOUND = new TestResource(TEST_DIR,
"resource-dummy-mega-outbound.xml", "2b1c05f1-8b70-43e6-ac46-3e5ee621ee36");
private static final String RESOURCE_DUMMY_MEGA_OUTBOUND_NAME = "mega-outbound";
private static final int MEGA_ATTRIBUTES = 1000;

private static final File ROLE_ANTINIHILIST_FILE = new File(TEST_DIR, "role-antinihilist.xml");
private static final String ROLE_ANTINIHILIST_OID = "4c5c6c44-bd7d-11e7-99ef-9b82464da93d";

Expand Down Expand Up @@ -157,6 +167,8 @@ public void initSystem(Task initTask, OperationResult initResult) throws Excepti
initDummyResource(RESOURCE_DUMMY_SERVICES_INBOUND_PWD_GENERATE_NAME,
RESOURCE_DUMMY_SERVICES_INBOUND_PWD_GENERATE.file, RESOURCE_DUMMY_SERVICES_INBOUND_PWD_GENERATE.oid, initTask, initResult);
initDummyResource(RESOURCE_DUMMY_TIMED_NAME, RESOURCE_DUMMY_TIMED.file, RESOURCE_DUMMY_TIMED.oid, initTask, initResult);
dummyResourceCollection.initDummyResource(RESOURCE_DUMMY_MEGA_OUTBOUND_NAME, RESOURCE_DUMMY_MEGA_OUTBOUND.file,
RESOURCE_DUMMY_MEGA_OUTBOUND.oid, this::initMegaResource, initTask, initResult);

repoAddObjectFromFile(ROLE_ANTINIHILIST_FILE, initResult);
repoAddObjectFromFile(ROLE_BLUE_TITANIC_FILE, initResult);
Expand All @@ -171,6 +183,14 @@ public void initSystem(Task initTask, OperationResult initResult) throws Excepti
// setGlobalTracingOverride(createModelLoggingTracingProfile());
}

private void initMegaResource(DummyResourceContoller controller) throws ConflictException,
FileNotFoundException, SchemaViolationException, InterruptedException, ConnectException {
DummyObjectClass objectClass = controller.getDummyResource().getAccountObjectClass();
for (int i = 0; i < MEGA_ATTRIBUTES; i++) {
controller.addAttrDef(objectClass, String.format("a-single-%04d", i), String.class, false, false);
}
}

/**
* Blue dummy has WEAK mappings. Let's play a bit with that.
*/
Expand Down Expand Up @@ -3167,6 +3187,65 @@ public void test520DeleteUserAssignment() throws Exception {
assertAssignments(userAfter, 1);
}

/**
* MID-4863 + MID-7057
*/
@Test
public void test530DeleteAssignmentByIdWithMegaMappings() throws Exception {
given();

Task task = getTestTask();
OperationResult result = getTestOperationResult();

InternalMonitor.reset();
InternalMonitor.setTrace(InternalCounters.PRISM_OBJECT_CLONE_COUNT, true);

final String userName = "test530";
UserType user = new UserType(prismContext)
.name(userName)
.beginAssignment()
.targetRef(ROLE_SUPERUSER_OID, RoleType.COMPLEX_TYPE)
.<UserType>end()
.beginAssignment()
.beginConstruction()
.resourceRef(RESOURCE_DUMMY_MEGA_OUTBOUND.oid, ResourceType.COMPLEX_TYPE)
.<AssignmentType>end()
.end();
String oid = addObject(user.asPrismObject(), null, task, result);

PrismObject<UserType> userCreated = assertUser(oid, "after creation")
.display()
.assertAssignments(2)
.assertLinks(1, 0)
.getObject();
DummyAccount account = assertDummyAccount(RESOURCE_DUMMY_MEGA_OUTBOUND_NAME, userName);
assertThat(account.getAttributeValue("a-single-0555")).as("attribute value").isEqualTo(userName);

when();

AssignmentType roleAssignment = findAssignment(userCreated, ROLE_SUPERUSER_OID, SchemaConstants.ORG_DEFAULT);
assertNotNull("role assignment not found", roleAssignment);
PrismContainerValue<Containerable> roleAssignmentIdOnlyPcv = prismContext.itemFactory().createContainerValue();
roleAssignmentIdOnlyPcv.setId(roleAssignment.getId());
ObjectDelta<UserType> delta = prismContext.deltaFor(UserType.class)
.item(UserType.F_ASSIGNMENT).delete(roleAssignmentIdOnlyPcv)
.asObjectDeltaCast(oid);

rememberCounter(InternalCounters.PRISM_OBJECT_CLONE_COUNT);
executeChanges(delta, null, task, result);

then();
assertSuccess(result);

// we will be happy to get a number significantly lower than ~2000 (2x1000 mappings)
assertCounterIncrement(InternalCounters.PRISM_OBJECT_CLONE_COUNT, 0, 100);

assertUser(oid, "after assignment deletion")
.display()
.assertAssignments(1)
.assertLinks(1, 0);
}

/**
* MID-6025
*/
Expand Down

0 comments on commit 988ba40

Please sign in to comment.