Skip to content

Commit

Permalink
Fix MID-4407: Closure not recomputed correctly
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mederly committed Feb 14, 2018
1 parent 732a8c6 commit e4c2f07
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 3 deletions.
Expand Up @@ -1096,7 +1096,7 @@ void diffItems(PrismContainerValue<C> thisValue, PrismContainerValue<C> 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());
Expand All @@ -1118,7 +1118,7 @@ void diffItems(PrismContainerValue<C> thisValue, PrismContainerValue<C> 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());
Expand Down
Expand Up @@ -221,6 +221,7 @@ public void setObjectToAdd(PrismObject<T> objectToAdd) {
}
}

@NotNull
public Collection<? extends ItemDelta<?,?>> getModifications() {
return modifications;
}
Expand Down
@@ -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<OrgType> 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;
}
}
1 change: 1 addition & 0 deletions repo/repo-sql-impl-test/testng-integration.xml
Expand Up @@ -39,6 +39,7 @@
<class name="com.evolveum.midpoint.repo.sql.ListAccountShadowOwnerTest"/>
<class name="com.evolveum.midpoint.repo.sql.OrgStructTest"/>
<class name="com.evolveum.midpoint.repo.sql.closure.OrgClosureCorrectnessTest"/>
<class name="com.evolveum.midpoint.repo.sql.closure.OrgClosureOverwriteAddTest"/>
<class name="com.evolveum.midpoint.repo.sql.SearchTest"/>
<class name="com.evolveum.midpoint.repo.sql.CleanupTest"/>
<class name="com.evolveum.midpoint.repo.sql.SearchShadowOwnerTest"/>
Expand Down
Expand Up @@ -179,7 +179,7 @@ private <T extends ObjectType> String overwriteAddObjectAttempt(PrismObject<T> o
if (originalOid != null) {
try {
oldObject = objectRetriever.getObjectInternal(session, object.getCompileTimeClass(), originalOid, null, true, result);
ObjectDelta<T> delta = object.diff(oldObject);
ObjectDelta<T> delta = oldObject.diff(object, false, true);
modifications = delta.getModifications();

LOGGER.trace("overwriteAddObjectAttempt: originalOid={}, modifications={}", originalOid, modifications);
Expand Down

0 comments on commit e4c2f07

Please sign in to comment.