From e4c2f077734ba7b733f44c9dfbb021be0d346c5b Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Wed, 14 Feb 2018 15:28:34 +0100 Subject: [PATCH] Fix MID-4407: Closure not recomputed correctly Add-with-overwrite repo method was (probably for a very long time) broken with respect for org closure maintenance. Hopefully, other methods (standard addObject, modifyObject, deleteObject) were not affected. --- .../midpoint/prism/PrismContainerValue.java | 4 +- .../midpoint/prism/delta/ObjectDelta.java | 1 + .../closure/OrgClosureOverwriteAddTest.java | 83 +++++++++++++++++++ .../repo-sql-impl-test/testng-integration.xml | 1 + .../repo/sql/helpers/ObjectUpdater.java | 2 +- 5 files changed, 88 insertions(+), 3 deletions(-) create mode 100644 repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/closure/OrgClosureOverwriteAddTest.java diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/PrismContainerValue.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/PrismContainerValue.java index 241bfaad4d1..f5f356b26f2 100644 --- a/infra/prism/src/main/java/com/evolveum/midpoint/prism/PrismContainerValue.java +++ b/infra/prism/src/main/java/com/evolveum/midpoint/prism/PrismContainerValue.java @@ -1096,7 +1096,7 @@ void diffItems(PrismContainerValue thisValue, PrismContainerValue other, if (thisValue.getItems() != null) { for (Item thisItem: thisValue.getItems()) { Item otherItem = other.findItem(thisItem.getElementName()); - if (!isLiteral) { + if (!isLiteral) { // here should be perhaps ignoreMetadata instead ItemDefinition itemDef = thisItem.getDefinition(); if (itemDef == null && other.getDefinition() != null) { itemDef = other.getDefinition().findItemDefinition(thisItem.getElementName()); @@ -1118,7 +1118,7 @@ void diffItems(PrismContainerValue thisValue, PrismContainerValue other, // Already processed in previous loop continue; } - if (!isLiteral) { + if (!isLiteral) { // here should be perhaps ignoreMetadata instead ItemDefinition itemDef = otherItem.getDefinition(); if (itemDef == null && thisValue.getDefinition() != null) { itemDef = thisValue.getDefinition().findItemDefinition(otherItem.getElementName()); diff --git a/infra/prism/src/main/java/com/evolveum/midpoint/prism/delta/ObjectDelta.java b/infra/prism/src/main/java/com/evolveum/midpoint/prism/delta/ObjectDelta.java index 701f3237f0d..2eb8953bcdf 100644 --- a/infra/prism/src/main/java/com/evolveum/midpoint/prism/delta/ObjectDelta.java +++ b/infra/prism/src/main/java/com/evolveum/midpoint/prism/delta/ObjectDelta.java @@ -221,6 +221,7 @@ public void setObjectToAdd(PrismObject objectToAdd) { } } + @NotNull public Collection> getModifications() { return modifications; } diff --git a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/closure/OrgClosureOverwriteAddTest.java b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/closure/OrgClosureOverwriteAddTest.java new file mode 100644 index 00000000000..e3840b47cd3 --- /dev/null +++ b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/closure/OrgClosureOverwriteAddTest.java @@ -0,0 +1,83 @@ +/* + * Copyright (c) 2010-2018 Evolveum + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.evolveum.midpoint.repo.sql.closure; + +import com.evolveum.midpoint.prism.PrismObject; +import com.evolveum.midpoint.repo.api.RepoAddOptions; +import com.evolveum.midpoint.schema.result.OperationResult; +import com.evolveum.midpoint.util.logging.Trace; +import com.evolveum.midpoint.util.logging.TraceManager; +import com.evolveum.midpoint.xml.ns._public.common.common_3.OrgType; +import org.springframework.test.annotation.DirtiesContext; +import org.springframework.test.context.ContextConfiguration; +import org.testng.annotations.Test; + +import static org.testng.AssertJUnit.assertFalse; + +/** + * MID-4407 + * + * @author mederly + */ +@ContextConfiguration(locations = {"../../../../../../ctx-test.xml"}) +@DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_CLASS) +public class OrgClosureOverwriteAddTest extends AbstractOrgClosureTest { + + private static final Trace LOGGER = TraceManager.getTrace(OrgClosureOverwriteAddTest.class); + + private static final int[] ORG_CHILDREN_IN_LEVEL = { 1, 1 }; + private static final int[] USER_CHILDREN_IN_LEVEL = null; + private static final int[] PARENTS_IN_LEVEL = { 0, 1 }; + + private OrgClosureTestConfiguration configuration; + + public OrgClosureOverwriteAddTest() { + configuration = new OrgClosureTestConfiguration(); + configuration.setCheckChildrenSets(true); + configuration.setCheckClosureMatrix(true); + configuration.setDeletionsToClosureTest(1); + configuration.setOrgChildrenInLevel(ORG_CHILDREN_IN_LEVEL); + configuration.setUserChildrenInLevel(USER_CHILDREN_IN_LEVEL); + configuration.setParentsInLevel(PARENTS_IN_LEVEL); + } + + @Test + public void test100AddWithOverwrite() throws Exception { + OperationResult opResult = new OperationResult("===[ test100AddWithOverwrite ]==="); + + _test100LoadOrgStructure(); + _test150CheckClosure(); + + String parentOid = orgsByLevels.get(0).get(0); + String childOid = orgsByLevels.get(1).get(0); + PrismObject childOrg = repositoryService.getObject(OrgType.class, childOid, null, opResult); + childOrg.asObjectable().getParentOrgRef().clear(); + + LOGGER.info("+++ adding object with 'overwrite' option +++"); + repositoryService.addObject(childOrg, RepoAddOptions.createOverwrite(), opResult); + + orgGraph.removeEdge(childOid, parentOid); + checkOrgGraph(); + boolean problem = checkClosureMatrix(); + assertFalse("Closure problem was detected", problem); + } + + @Override + public OrgClosureTestConfiguration getConfiguration() { + return configuration; + } +} diff --git a/repo/repo-sql-impl-test/testng-integration.xml b/repo/repo-sql-impl-test/testng-integration.xml index 6d4e6219aae..d6117a78dc5 100644 --- a/repo/repo-sql-impl-test/testng-integration.xml +++ b/repo/repo-sql-impl-test/testng-integration.xml @@ -39,6 +39,7 @@ + diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java index 8e8dfe4b185..efcfd656f0a 100644 --- a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java +++ b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectUpdater.java @@ -179,7 +179,7 @@ private String overwriteAddObjectAttempt(PrismObject o if (originalOid != null) { try { oldObject = objectRetriever.getObjectInternal(session, object.getCompileTimeClass(), originalOid, null, true, result); - ObjectDelta delta = object.diff(oldObject); + ObjectDelta delta = oldObject.diff(object, false, true); modifications = delta.getModifications(); LOGGER.trace("overwriteAddObjectAttempt: originalOid={}, modifications={}", originalOid, modifications);