Skip to content

Commit

Permalink
Fixing handling of dead shadows. Improving the tests.
Browse files Browse the repository at this point in the history
  • Loading branch information
@semancik
semancik committed Apr 27, 2017
1 parent d69facf commit 2324924
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 96 deletions.
36 changes: 26 additions & 10 deletions ...pl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/ActivationProcessor.java
View file
Expand Up @@ -141,21 +141,25 @@ public <F extends FocusType> void processActivationUserCurrent(LensContext<F> co
throw new IllegalStateException("Decision "+decision+" already present for projection "+projCtxDesc);
}

if (projCtx.isThombstone()) {
// Let's keep thombstones linked until they expire. So we do not have shadows without owners.
// This is also needed for async delete operations.
// TODO: thombstone expiration
projCtx.setSynchronizationPolicyDecision(SynchronizationPolicyDecision.KEEP);
LOGGER.trace("Evaluated decision for {} to {} because it is thombstone, skipping further activation processing", projCtxDesc, SynchronizationPolicyDecision.KEEP);
return;
}

if (synchronizationIntent == SynchronizationIntent.UNLINK) {
projCtx.setSynchronizationPolicyDecision(SynchronizationPolicyDecision.UNLINK);
LOGGER.trace("Evaluated decision for {} to {} because of unlink synchronization intent, skipping further activation processing", projCtxDesc, SynchronizationPolicyDecision.UNLINK);
return;
}

if (projCtx.isThombstone()) {
if (shouldKeepThombstone(projCtx)) {
// Let's keep thombstones linked until they expire. So we do not have shadows without owners.
// This is also needed for async delete operations.
projCtx.setSynchronizationPolicyDecision(SynchronizationPolicyDecision.KEEP);
LOGGER.trace("Evaluated decision for {} to {} because it is thombstone, skipping further activation processing", projCtxDesc, SynchronizationPolicyDecision.KEEP);
} else {
projCtx.setSynchronizationPolicyDecision(SynchronizationPolicyDecision.UNLINK);
LOGGER.trace("Evaluated decision for {} to {} because it is thombstone, skipping further activation processing", projCtxDesc, SynchronizationPolicyDecision.UNLINK);
}
return;
}

if (synchronizationIntent == SynchronizationIntent.DELETE || projCtx.isDelete()) {
// TODO: is this OK?
projCtx.setSynchronizationPolicyDecision(SynchronizationPolicyDecision.DELETE);
Expand Down Expand Up @@ -318,7 +322,19 @@ public <F extends FocusType> void processActivationUserCurrent(LensContext<F> co

}

public <F extends FocusType> void processActivationMetadata(LensContext<F> context, LensProjectionContext accCtx,
private boolean shouldKeepThombstone(LensProjectionContext projCtx) {
PrismObject<ShadowType> objectCurrent = projCtx.getObjectCurrent();
if (objectCurrent != null) {
ShadowType objectCurrentType = objectCurrent.asObjectable();
if (!objectCurrentType.getPendingOperation().isEmpty()) {
return true;
}
}
// TODO: thombstone expiration
return false;
}

public <F extends FocusType> void processActivationMetadata(LensContext<F> context, LensProjectionContext accCtx,
XMLGregorianCalendar now, OperationResult result) throws ExpressionEvaluationException, ObjectNotFoundException, SchemaException, PolicyViolationException {
ObjectDelta<ShadowType> projDelta = accCtx.getDelta();
if (projDelta == null) {
Expand Down
29 changes: 17 additions & 12 deletions model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/lens/TestReconScript.java
View file
Expand Up @@ -41,6 +41,7 @@
import com.evolveum.midpoint.test.util.TestUtil;
import com.evolveum.midpoint.util.QNameUtil;
import com.evolveum.midpoint.util.exception.ObjectNotFoundException;
import com.evolveum.midpoint.xml.ns._public.common.common_3.FocusType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.TaskType;
Expand Down Expand Up @@ -104,9 +105,6 @@ public void test002testReconcileScriptsWhenReconciling() throws Exception{
final String TEST_NAME = "test002testReconcileScriptsWhenReconciling";
TestUtil.displayTestTile(this, TEST_NAME);

// Task task = taskManager.createTaskInstance(TEST_NAME);
// OperationResult parentResult = new OperationResult(TEST_NAME);

getDummyResource().getScriptHistory().clear();

importObjectFromFile(new File(TASK_RECON_DUMMY_FILENAME));
Expand Down Expand Up @@ -148,10 +146,8 @@ public void test003testReconcileScriptsAddUserAction() throws Exception{

getDummyResource().getScriptHistory().clear();

// importObjectFromFile(new File(TASK_RECON_DUMMY_FILENAME));
//
waitForTaskStart(TASK_RECON_DUMMY_OID, false, DEFAULT_TASK_WAIT_TIMEOUT);
//

waitForTaskNextRunAssertSuccess(TASK_RECON_DUMMY_OID, false, DEFAULT_TASK_WAIT_TIMEOUT);

waitForTaskFinish(TASK_RECON_DUMMY_OID, true);
Expand All @@ -168,7 +164,7 @@ public void test003testReconcileScriptsAddUserAction() throws Exception{
AssertJUnit.fail("Operation in shadow not null, recocniliation failed. ");
}

PrismObject<UserType> user = repositoryService.listAccountShadowOwner(ACCOUNT_BEFORE_SCRIPT_OID, parentResult);
PrismObject<FocusType> user = repositoryService.searchShadowOwner(ACCOUNT_BEFORE_SCRIPT_OID, null, parentResult);
AssertJUnit.assertNotNull("Owner for account " + shadow.asPrismObject() + " not found. Some probelm in recon occured.", user);


Expand Down Expand Up @@ -199,7 +195,7 @@ public void test005TestDryRunDelete() throws Exception{
OperationResult parentResult = new OperationResult(TEST_NAME);

PropertyDelta dryRunDelta = PropertyDelta.createModificationReplaceProperty(new ItemPath(TaskType.F_EXTENSION, SchemaConstants.MODEL_EXTENSION_DRY_RUN), task.getDefinition(), true);
Collection<PropertyDelta> modifications = new ArrayList<PropertyDelta>();
Collection<PropertyDelta> modifications = new ArrayList<>();
modifications.add(dryRunDelta);

repositoryService.modifyObject(TaskType.class, TASK_RECON_DUMMY_OID, modifications, parentResult);
Expand All @@ -216,7 +212,7 @@ public void test005TestDryRunDelete() throws Exception{
PrismObject<ShadowType> shadow = repositoryService.getObject(ShadowType.class, ACCOUNT_BEFORE_SCRIPT_OID, null, parentResult);
AssertJUnit.assertNotNull(shadow);

PrismObject<UserType> user = repositoryService.listAccountShadowOwner(ACCOUNT_BEFORE_SCRIPT_OID, parentResult);
PrismObject<FocusType> user = repositoryService.searchShadowOwner(ACCOUNT_BEFORE_SCRIPT_OID, null, parentResult);
AssertJUnit.assertNotNull("Owner for account " + shadow + " not found. Some probelm in dry run occured.", user);


Expand All @@ -231,27 +227,36 @@ public void test006TestReconDelete() throws Exception{
OperationResult parentResult = new OperationResult(TEST_NAME);

PropertyDelta dryRunDelta = PropertyDelta.createModificationReplaceProperty(new ItemPath(TaskType.F_EXTENSION, SchemaConstants.MODEL_EXTENSION_DRY_RUN), task.getDefinition(), false);
Collection<PropertyDelta> modifications = new ArrayList<PropertyDelta>();
Collection<PropertyDelta> modifications = new ArrayList<>();
modifications.add(dryRunDelta);

repositoryService.modifyObject(TaskType.class, TASK_RECON_DUMMY_OID, modifications, parentResult);

// dummyResource.deleteAccount("beforeScript");


// WHEN
TestUtil.displayWhen(TEST_NAME);

waitForTaskStart(TASK_RECON_DUMMY_OID, false);

waitForTaskNextRunAssertSuccess(TASK_RECON_DUMMY_OID, false);

waitForTaskFinish(TASK_RECON_DUMMY_OID, false);

// THEN
TestUtil.displayThen(TEST_NAME);

try{
PrismObject<ShadowType> shadow = repositoryService.getObject(ShadowType.class, ACCOUNT_BEFORE_SCRIPT_OID, null, parentResult);

display("Unexpected shadow", shadow);
AssertJUnit.fail("Expected object not found, but haven't got one");
} catch (ObjectNotFoundException ex){
//this is ok
}

PrismObject<UserType> user = repositoryService.listAccountShadowOwner(ACCOUNT_BEFORE_SCRIPT_OID, parentResult);
PrismObject<FocusType> user = repositoryService.searchShadowOwner(ACCOUNT_BEFORE_SCRIPT_OID, null, parentResult);
display("Unexpected owner", user);
AssertJUnit.assertNull("Owner for account " + ACCOUNT_BEFORE_SCRIPT_OID + " was found, but it should be not.", user);


Expand Down
2 changes: 1 addition & 1 deletion ...del-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java
View file
Expand Up @@ -252,7 +252,7 @@ public abstract class AbstractModelIntegrationTest extends AbstractIntegrationTe

protected DummyAuditService dummyAuditService;

protected boolean verbose = true;
protected boolean verbose = false;

private static final Trace LOGGER = TraceManager.getTrace(AbstractModelIntegrationTest.class);

Expand Down
106 changes: 54 additions & 52 deletions .../provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ShadowCache.java
View file
Expand Up @@ -96,6 +96,8 @@
*
*/
public abstract class ShadowCache {

public static String OP_PROCESS_SYNCHRONIZATION = ShadowCache.class.getName() + ".processSynchronization";

@Autowired(required = true)
@Qualifier("cacheRepositoryService")
Expand Down Expand Up @@ -379,13 +381,9 @@ private PrismObject<ShadowType> processNoFetchGet(ProvisioningContext ctx, Prism
shadowDelta.applyTo(repositoryShadow);
}

LOGGER.info("XXXXXXXX1\n{}", repositoryShadow.debugDumpLazily());

PrismObject<ShadowType> resultShadow = futurizeShadow(repositoryShadow, options, resource);
applyAttributesDefinition(ctx, resultShadow);

LOGGER.info("XXXXXXXX2\n{}", resultShadow.debugDumpLazily());

return resultShadow;
}

Expand Down Expand Up @@ -1753,59 +1751,63 @@ public int synchronize(ResourceShadowDiscriminator shadowCoordinates, PrismPrope
}

@SuppressWarnings("rawtypes")
boolean processSynchronization(ProvisioningContext ctx, Change change, OperationResult result)
boolean processSynchronization(ProvisioningContext ctx, Change change, OperationResult parentResult)
throws SchemaException, ObjectNotFoundException, ObjectAlreadyExistsException,
CommunicationException, ConfigurationException {

ResourceObjectShadowChangeDescription shadowChangeDescription = createResourceShadowChangeDescription(
change, ctx.getResource(), ctx.getChannel());

if (LOGGER.isTraceEnabled()) {
LOGGER.trace("**PROVISIONING: Created resource object shadow change description {}",
SchemaDebugUtil.prettyPrint(shadowChangeDescription));
}
OperationResult notifyChangeResult = new OperationResult(
ShadowCache.class.getName() + "notifyChange");
notifyChangeResult.addParam("resourceObjectShadowChangeDescription", shadowChangeDescription);

try {
notifyResourceObjectChangeListeners(shadowChangeDescription, ctx.getTask(), notifyChangeResult);
notifyChangeResult.recordSuccess();
} catch (RuntimeException ex) {
// recordFatalError(LOGGER, notifyChangeResult, "Synchronization
// error: " + ex.getMessage(), ex);
saveAccountResult(shadowChangeDescription, change, notifyChangeResult, result);
throw new SystemException("Synchronization error: " + ex.getMessage(), ex);
}

notifyChangeResult.computeStatus("Error in notify change operation.");

OperationResult result = parentResult.createSubresult(OP_PROCESS_SYNCHRONIZATION);

boolean successfull = false;
if (notifyChangeResult.isSuccess() || notifyChangeResult.isHandledError()) {
deleteShadowFromRepo(change, result);
successfull = true;
// // get updated token from change,
// // create property modification from new token
// // and replace old token with the new one
// PrismProperty<?> newToken = change.getToken();
// task.setExtensionProperty(newToken);
// processedChanges++;

} else {
successfull = false;
saveAccountResult(shadowChangeDescription, change, notifyChangeResult, result);
try {
ResourceObjectShadowChangeDescription shadowChangeDescription = createResourceShadowChangeDescription(
change, ctx.getResource(), ctx.getChannel());

if (LOGGER.isTraceEnabled()) {
LOGGER.trace("**PROVISIONING: Created resource object shadow change description {}",
SchemaDebugUtil.prettyPrint(shadowChangeDescription));
}
OperationResult notifyChangeResult = new OperationResult(
ShadowCache.class.getName() + "notifyChange");
notifyChangeResult.addParam("resourceObjectShadowChangeDescription", shadowChangeDescription);

try {
notifyResourceObjectChangeListeners(shadowChangeDescription, ctx.getTask(), notifyChangeResult);
notifyChangeResult.recordSuccess();
} catch (RuntimeException ex) {
// recordFatalError(LOGGER, notifyChangeResult, "Synchronization
// error: " + ex.getMessage(), ex);
saveAccountResult(shadowChangeDescription, change, notifyChangeResult, result);
throw new SystemException("Synchronization error: " + ex.getMessage(), ex);
}

notifyChangeResult.computeStatus("Error in notify change operation.");

if (notifyChangeResult.isSuccess() || notifyChangeResult.isHandledError()) {
deleteShadowFromRepoIfNeeded(change, result);
successfull = true;
// // get updated token from change,
// // create property modification from new token
// // and replace old token with the new one
// PrismProperty<?> newToken = change.getToken();
// task.setExtensionProperty(newToken);
// processedChanges++;

} else {
successfull = false;
saveAccountResult(shadowChangeDescription, change, notifyChangeResult, result);
}

if (result.isUnknown()) {
result.computeStatus();
}

} catch (SchemaException | ObjectNotFoundException | ObjectAlreadyExistsException |
CommunicationException | ConfigurationException | RuntimeException | Error e) {
result.recordFatalError(e);
throw e;
}

return successfull;
// }
// // also if no changes was detected, update token
// if (changes.isEmpty() && tokenProperty != null) {
// LOGGER.trace("No changes to synchronize on " +
// ObjectTypeUtil.toShortString(resourceType));
// task.setExtensionProperty(tokenProperty);
// }
// task.savePendingModifications(result);
// return processedChanges;
}

private void notifyResourceObjectChangeListeners(ResourceObjectShadowChangeDescription change, Task task,
Expand Down Expand Up @@ -1899,7 +1901,7 @@ private String getOidFromChange(Change change) {
return shadowOid;
}

private void deleteShadowFromRepo(Change change, OperationResult parentResult)
private void deleteShadowFromRepoIfNeeded(Change change, OperationResult parentResult)
throws ObjectNotFoundException {
if (change.getObjectDelta() != null && change.getObjectDelta().getChangeType() == ChangeType.DELETE
&& change.getOldShadow() != null) {
Expand Down
37 changes: 16 additions & 21 deletions testing/sanity/src/test/java/com/evolveum/midpoint/testing/sanity/TestSanity.java
View file
Expand Up @@ -3932,48 +3932,43 @@ public void test502NotifyChangeModifyAccountPassword() throws Exception{

@Test
public void test503NotifyChangeDeleteAccount() throws Exception{
TestUtil.displayTestTile("test503NotifyChangeDeleteAccount");

OperationResult parentResult = new OperationResult("test500notifyChange.addAngelicaAccount");
PrismObject<UserType> userAngelika = findUserByUsername(ANGELIKA_NAME);
assertNotNull("User with the name angelika must exist.", userAngelika);

UserType user = userAngelika.asObjectable();
assertNotNull("User with the name angelika must have one link ref.", user.getLinkRef());

assertEquals("Expected one account ref in user", 1, user.getLinkRef().size());
String oid = user.getLinkRef().get(0).getOid();

final String TEST_NAME = "test503NotifyChangeDeleteAccount";
TestUtil.displayTestTile(TEST_NAME);

PrismObject<UserType> userAngelika = findUserByUsername(ANGELIKA_NAME);
assertNotNull("User with the name angelika must exist.", userAngelika);

UserType user = userAngelika.asObjectable();
assertNotNull("User with the name angelika must have one link ref.", user.getLinkRef());

assertEquals("Expected one account ref in user", 1, user.getLinkRef().size());
String oid = user.getLinkRef().get(0).getOid();

ResourceObjectShadowChangeDescriptionType changeDescription = new ResourceObjectShadowChangeDescriptionType();
ObjectDeltaType delta = new ObjectDeltaType();
delta.setChangeType(ChangeTypeType.DELETE);
delta.setObjectType(ShadowType.COMPLEX_TYPE);

delta.setOid(oid);

changeDescription.setObjectDelta(delta);

changeDescription.setOldShadowOid(oid);
changeDescription.setChannel(SchemaConstants.CHANNEL_WEB_SERVICE_URI);

// WHEN
TaskType task = modelWeb.notifyChange(changeDescription);

// THEN
OperationResult result = OperationResult.createOperationResult(task.getResult());
display(result);
assertSuccess(result);
assertTrue(result.isAcceptable());

PrismObject<UserType> userAngelikaAfterSync = findUserByUsername(ANGELIKA_NAME);
display("User after", userAngelikaAfterSync);
assertNotNull("User with the name angelika must exist.", userAngelikaAfterSync);

UserType userType = userAngelikaAfterSync.asObjectable();
assertNotNull("User with the name angelika must have one link ref.", userType.getLinkRef());

assertEquals("Expected no account ref in user", 0, userType.getLinkRef().size());
// String oid = userType.getLinkRef().get(0).getOid();

// UserType userAfterSync = userAngelikaAfterSync.asObjectable();

// PrismAsserts.assertEqualsPolyString("wrong given name in user angelika", PrismTestUtil.createPolyStringType("newAngelika"), userAfterSync.getGivenName());

}

Expand Down

0 comments on commit 2324924

Please sign in to comment.