Skip to content

Commit

Permalink
Tests and fixes for deadShadowRetentionPeriod=0 (MID-6435)
Browse files Browse the repository at this point in the history
  • Loading branch information
@semancik
semancik committed Sep 8, 2020
1 parent fb8964f commit 73a506d
Show file tree
Hide file tree
Showing 21 changed files with 474 additions and 115 deletions.
9 changes: 9 additions & 0 deletions infra/prism-api/src/main/java/com/evolveum/midpoint/prism/xml/XmlTypeConverter.java
View file
Expand Up @@ -219,6 +219,14 @@ public static Duration createDuration(Duration duration) {
toBigDecimal(duration.getField(DatatypeConstants.SECONDS)));
}

public static boolean isZero(Duration duration) {
if (duration == null) {
return true;
}
return duration.getSign() == 0;
}


// to be used from within createDuration only (for general use it should be rewritten!!)
private static BigDecimal toBigDecimal(Number number) {
if (number instanceof BigDecimal) {
Expand Down Expand Up @@ -510,4 +518,5 @@ private static PolyString polyStringToJava(Element polyStringElement) throws Sch
return new PolyString(orig, norm);
}
}

}
2 changes: 1 addition & 1 deletion pom.xml
View file
Expand Up @@ -867,7 +867,7 @@
<dependency>
<groupId>com.evolveum.polygon</groupId>
<artifactId>connector-ldap</artifactId>
<version>3.0</version>
<version>3.1-SNAPSHOT</version>
<exclusions>
<!-- Needed otherwise the JDK14 SLF4J binding can override the midpoint's logback binding -->
<exclusion>
Expand Down
4 changes: 3 additions & 1 deletion .../provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ShadowCache.java
View file
Expand Up @@ -1834,7 +1834,9 @@ private PrismObject<ShadowType> cleanUpDeadShadow(ProvisioningContext ctx,
}
}

if (ProvisioningUtil.isOverPeriod(now, expirationPeriod, lastActivityTimestamp)) {
// Explicitly check for zero deadRetentionPeriod to avoid some split-millisecond issues with dead shadow deletion.
// If we have zero deadRetentionPeriod, we should get rid of all dead shadows immediately.
if (XmlTypeConverter.isZero(deadRetentionPeriod) || ProvisioningUtil.isOverPeriod(now, expirationPeriod, lastActivityTimestamp)) {
// Perish you stinking corpse!
LOGGER.debug("Deleting dead {} because it is expired", repoShadow);
shadowManager.deleteShadow(ctx, repoShadow, parentResult);
Expand Down
14 changes: 14 additions & 0 deletions ...pl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadowmanager/ShadowManager.java
View file
Expand Up @@ -10,13 +10,15 @@
import java.util.*;
import java.util.Objects;

import javax.xml.datatype.Duration;
import javax.xml.datatype.XMLGregorianCalendar;
import javax.xml.namespace.QName;

import com.evolveum.midpoint.prism.*;
import com.evolveum.midpoint.prism.delta.*;
import com.evolveum.midpoint.prism.path.ItemPath;
import com.evolveum.midpoint.prism.path.ItemName;
import com.evolveum.midpoint.prism.xml.XmlTypeConverter;
import com.evolveum.midpoint.provisioning.api.ProvisioningService;
import com.evolveum.midpoint.provisioning.impl.ConstraintsChecker;
import com.evolveum.midpoint.provisioning.impl.ProvisioningContext;
Expand Down Expand Up @@ -952,6 +954,18 @@ public void recordOperationException(
if (delta.isAdd()) {
// This means we have failed add operation here. We tried to add object,
// but we have failed. Which means that this shadow is now dead.

Duration deadRetentionPeriod = ProvisioningUtil.getDeadShadowRetentionPeriod(ctx);
if (XmlTypeConverter.isZero(deadRetentionPeriod)) {
// Do not bother with marking the shadow as dead. It should be gone immediately.
// Deleting it now saves one modify operation.
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("Deleting repository shadow (after error handling)\n{}", DebugUtil.debugDump(shadowChanges, 1));
}
repositoryService.deleteObject(ShadowType.class, opState.getRepoShadow().getOid(), parentResult);
return;
}

shadowChanges.addAll(
prismContext.deltaFor(ShadowType.class)
.item(ShadowType.F_DEAD).replace(true)
Expand Down
2 changes: 1 addition & 1 deletion ...l/src/test/java/com/evolveum/midpoint/provisioning/impl/dummy/AbstractBasicDummyTest.java
View file
Expand Up @@ -1116,7 +1116,7 @@ public void test100AddAccountWill() throws Exception {
}
assertWillRepoShadowAfterCreate(accountRepo);

syncServiceMock.assertNotifySuccessOnly();
syncServiceMock.assertSingleNotifySuccessOnly();

PrismObject<ShadowType> accountProvisioning = provisioningService.getObject(ShadowType.class,
ACCOUNT_WILL_OID, null, task, result);
Expand Down
88 changes: 44 additions & 44 deletions ...visioning-impl/src/test/java/com/evolveum/midpoint/provisioning/impl/dummy/TestDummy.java
View file

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions ...impl/src/test/java/com/evolveum/midpoint/provisioning/impl/dummy/TestDummyCaseIgnore.java
View file
Expand Up @@ -133,7 +133,7 @@ public void test280EntitleAccountWillPiratesAlreadyThere() throws Exception {
DummyGroup group = getDummyGroupAssert(GROUP_PIRATES_NAME, piratesIcfUid);
assertMember(group, getWillRepoIcfName());

syncServiceMock.assertNotifySuccessOnly();
syncServiceMock.assertSingleNotifySuccessOnly();

PrismObject<ShadowType> shadow = provisioningService.getObject(ShadowType.class, ACCOUNT_WILL_OID, null, task, result);
display("Shadow after", shadow);
Expand Down Expand Up @@ -172,7 +172,7 @@ public void test282DetitleAccountWillPirates() throws Exception {
DummyAccount dummyAccount = getDummyAccountAssert(ACCOUNT_WILL_USERNAME, willIcfUid);
assertNotNull("Account will is gone!", dummyAccount);

syncServiceMock.assertNotifySuccessOnly();
syncServiceMock.assertSingleNotifySuccessOnly();
assertSteadyResource();
}

Expand Down Expand Up @@ -208,7 +208,7 @@ public void test285EntitleAccountWillPiratesAlreadyThereCaseIgnore() throws Exce
DummyGroup group = getDummyGroupAssert(GROUP_PIRATES_NAME, piratesIcfUid);
IntegrationTestTools.assertGroupMember(group, getWillRepoIcfName(),true);

syncServiceMock.assertNotifySuccessOnly();
syncServiceMock.assertSingleNotifySuccessOnly();

PrismObject<ShadowType> shadow = provisioningService.getObject(ShadowType.class, ACCOUNT_WILL_OID, null, task, result);
display("Shadow after", shadow);
Expand Down Expand Up @@ -249,7 +249,7 @@ public void test289DetitleAccountWillPirates() throws Exception {
DummyAccount dummyAccount = getDummyAccountAssert(ACCOUNT_WILL_USERNAME, willIcfUid);
assertNotNull("Account will is gone!", dummyAccount);

syncServiceMock.assertNotifySuccessOnly();
syncServiceMock.assertSingleNotifySuccessOnly();
assertSteadyResource();
}

Expand Down
60 changes: 32 additions & 28 deletions ...mpl/src/test/java/com/evolveum/midpoint/provisioning/impl/dummy/TestDummyConsistency.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010-2019 Evolveum and contributors
* Copyright (c) 2010-2020 Evolveum and contributors
*
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
Expand Down Expand Up @@ -71,7 +71,7 @@ public class TestDummyConsistency extends AbstractDummyTest {
private XMLGregorianCalendar lastRequestEndTs;
private XMLGregorianCalendar lastAttemptStartTs;
private XMLGregorianCalendar lastAttemptEndTs;
private String shadowMorganOid = ACCOUNT_MORGAN_OID;
protected String shadowMorganOid = ACCOUNT_MORGAN_OID;

@Override
public void initSystem(Task initTask, OperationResult initResult) throws Exception {
Expand Down Expand Up @@ -122,7 +122,7 @@ public void test050AddAccountWill() throws Exception {
then();
assertSuccess(result);
assertEquals(ACCOUNT_WILL_OID, addedObjectOid);
syncServiceMock.assertNotifySuccessOnly();
syncServiceMock.assertSingleNotifySuccessOnly();

PrismObject<ShadowType> accountProvisioning = provisioningService.getObject(ShadowType.class,
ACCOUNT_WILL_OID, null, task, result);
Expand Down Expand Up @@ -187,7 +187,7 @@ public void test100AddAccountMorganCommunicationFailure() throws Exception {
assertEquals(ACCOUNT_MORGAN_OID, addedObjectOid);
account.checkConsistence();
lastRequestEndTs = lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyInProgressOnly();
syncServiceMock.assertSingleNotifyInProgressOnly();

assertUncreatedMorgan(1);

Expand Down Expand Up @@ -319,7 +319,7 @@ public void test106RefreshAccountMorganCommunicationFailureRetry() throws Except
result.computeStatus();
TestUtil.assertResultStatus(result, OperationResultStatus.HANDLED_ERROR);
lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyInProgressOnly();
syncServiceMock.assertSingleNotifyInProgressOnly();

assertUncreatedMorgan(2);

Expand Down Expand Up @@ -357,12 +357,12 @@ public void test108RefreshAccountMorganCommunicationFailureRetryAgain() throws E
result.computeStatus();
TestUtil.assertResultStatus(result, OperationResultStatus.HANDLED_ERROR);
lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyFailureOnly();
syncServiceMock.assertSingleNotifyFailureOnly();

assertMorganDead();
}

private void assertMorganDead() throws Exception {
protected void assertMorganDead() throws Exception {
PrismObject<ShadowType> repoShadow = getShadowRepo(ACCOUNT_MORGAN_OID);
assertNotNull("Shadow was not created in the repository", repoShadow);

Expand Down Expand Up @@ -515,7 +515,7 @@ public void test110AddAccountMorganAgainCommunicationFailure() throws Exception
assertInProgress(result);
account.checkConsistence();
lastRequestEndTs = lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyInProgressOnly();
syncServiceMock.assertSingleNotifyInProgressOnly();

assertUncreatedMorgan(1);

Expand Down Expand Up @@ -583,7 +583,7 @@ public void test116RefreshAccountMorganRetrySuccess() throws Exception {
display("Result", result);
assertSuccess(result);
lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifySuccessOnly();
syncServiceMock.assertSingleNotifySuccessOnly();

assertCreatedMorgan(2);

Expand Down Expand Up @@ -614,7 +614,7 @@ public void test120ModifyMorganFullNameCommunicationFailure() throws Exception {
then();
assertInProgress(result);
lastRequestEndTs = lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyInProgressOnly();
syncServiceMock.assertSingleNotifyInProgressOnly();

assertUnmodifiedMorgan(1, 2, ACCOUNT_MORGAN_FULLNAME_HM);

Expand Down Expand Up @@ -683,7 +683,7 @@ public void test126RefreshAccountMorganCommunicationFailureRetry() throws Except
result.computeStatus();
TestUtil.assertResultStatus(result, OperationResultStatus.HANDLED_ERROR);
lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyInProgressOnly();
syncServiceMock.assertSingleNotifyInProgressOnly();

assertUnmodifiedMorgan(2, 2, ACCOUNT_MORGAN_FULLNAME_HM);

Expand Down Expand Up @@ -721,7 +721,7 @@ public void test128RefreshAccountMorganCommunicationFailureRetryAgain() throws E
result.computeStatus();
TestUtil.assertResultStatus(result, OperationResultStatus.HANDLED_ERROR);
lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyFailureOnly();
syncServiceMock.assertSingleNotifyFailureOnly();

assertMorganModifyFailed();

Expand Down Expand Up @@ -779,7 +779,7 @@ public void test130ModifyMorganFullNameAgainCommunicationFailure() throws Except
then();
assertInProgress(result);
lastRequestEndTs = lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyInProgressOnly();
syncServiceMock.assertSingleNotifyInProgressOnly();

assertUnmodifiedMorgan(1, 3, ACCOUNT_MORGAN_FULLNAME_CHM);

Expand Down Expand Up @@ -888,7 +888,7 @@ public void test134GetAccountMorganForceRefreshRetryCommunicationFailure() throw
display("Result", result);
assertPartialError(result);
lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyInProgressOnly();
syncServiceMock.assertSingleNotifyInProgressOnly();

assertUnmodifiedMorgan(2, 3, ACCOUNT_MORGAN_FULLNAME_CHM);

Expand Down Expand Up @@ -924,7 +924,7 @@ public void test136RefreshAccountMorganRetrySuccess() throws Exception {
display("Result", result);
assertSuccess(result);
lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifySuccessOnly();
syncServiceMock.assertSingleNotifySuccessOnly();

assertModifiedMorgan(3, 3, ACCOUNT_MORGAN_FULLNAME_CHM);

Expand Down Expand Up @@ -954,7 +954,7 @@ public void test170DeleteMorganCommunicationFailure() throws Exception {
then();
assertInProgress(result);
lastRequestEndTs = lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyInProgressOnly();
syncServiceMock.assertSingleNotifyInProgressOnly();

assertUndeletedMorgan(1, 4);

Expand Down Expand Up @@ -1023,7 +1023,7 @@ public void test176RefreshAccountMorganCommunicationFailureRetry() throws Except
result.computeStatus();
TestUtil.assertResultStatus(result, OperationResultStatus.HANDLED_ERROR);
lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyInProgressOnly();
syncServiceMock.assertSingleNotifyInProgressOnly();

assertUndeletedMorgan(2, 4);

Expand Down Expand Up @@ -1061,7 +1061,7 @@ public void test178RefreshAccountMorganCommunicationFailureRetryAgain() throws E
result.computeStatus();
TestUtil.assertResultStatus(result, OperationResultStatus.HANDLED_ERROR);
lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyFailureOnly();
syncServiceMock.assertSingleNotifyFailureOnly();

assertMorganDeleteFailed();
}
Expand Down Expand Up @@ -1117,7 +1117,7 @@ public void test180DeleteMorganCommunicationFailureAgain() throws Exception {
then();
assertInProgress(result);
lastRequestEndTs = lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifyInProgressOnly();
syncServiceMock.assertSingleNotifyInProgressOnly();

assertUndeletedMorgan(1, 5);

Expand Down Expand Up @@ -1156,7 +1156,7 @@ public void test186RefreshAccountMorganRetrySuccess() throws Exception {
display("Result", result);
assertSuccess(result);
lastAttemptEndTs = clock.currentTimeXMLGregorianCalendar();
syncServiceMock.assertNotifySuccessOnly();
assertDeadShadowNotify();

assertDeletedMorgan(2, 5);

Expand All @@ -1165,6 +1165,10 @@ public void test186RefreshAccountMorganRetrySuccess() throws Exception {
assertSteadyResources();
}

protected void assertDeadShadowNotify() {
syncServiceMock.assertSingleNotifySuccessOnly();
}

/**
* Refreshing dead shadow after pending operation is expired.
* Pending operation should be gone.
Expand Down Expand Up @@ -1346,7 +1350,7 @@ public void test194AccountMorganDeadExpireShadow() throws Exception {
then();
display("Result", result);
assertSuccess(result);
syncServiceMock.assertNotifySuccessOnly();
syncServiceMock.assertSingleNotifySuccessOnly();

assertNoRepoObject(ShadowType.class, ACCOUNT_MORGAN_OID);

Expand Down Expand Up @@ -1381,7 +1385,7 @@ public void test196AccountMorganSecondDeadExpireShadow() throws Exception {
then();
display("Result", result);
assertSuccess(result);
syncServiceMock.assertNotifySuccessOnly();
syncServiceMock.assertSingleNotifySuccessOnly();

assertNoRepoObject(ShadowType.class, shadowMorganOid);

Expand Down Expand Up @@ -1440,7 +1444,7 @@ public void test800AddAccountMorganAlreadyExists() throws Exception {
.assertNone();

syncServiceMock
.assertNotifyFailureOnly()
.assertSingleNotifyFailureOnly()
.assertNotifyChange()
.assertNotifyChangeCalls(1)
.lastNotifyChange()
Expand Down Expand Up @@ -1517,7 +1521,7 @@ public void test802AddAccountMorganAlreadyExistsAgain() throws Exception {
.assertNone();

syncServiceMock
.assertNotifyFailureOnly()
.assertSingleNotifyFailureOnly()
.assertNotifyChange()
.assertNotifyChangeCalls(1)
.lastNotifyChange()
Expand Down Expand Up @@ -1634,7 +1638,7 @@ public void test806RenameAccountElizabethAlreadyExists() throws Exception {
.assertNone();

syncServiceMock
.assertNotifyFailureOnly()
.assertSingleNotifyFailureOnly()
.assertNotifyChange()
.assertNotifyChangeCalls(1)
.lastNotifyChange()
Expand Down Expand Up @@ -1712,7 +1716,7 @@ public void test808RenameAccountElizabethAlreadyExistsAgain() throws Exception {
.assertNone();

syncServiceMock
.assertNotifyFailureOnly()
.assertSingleNotifyFailureOnly()
.assertNotifyChange()
.assertNotifyChangeCalls(1)
.lastNotifyChange()
Expand Down Expand Up @@ -2000,7 +2004,7 @@ public void test816AddAccountElizabethAfterDeathAlreadyExists() throws Exception

// @formatter:off
syncServiceMock
.assertNotifyFailureOnly()
.assertSingleNotifyFailureOnly()
.assertNotifyChange()
.assertNotifyChangeCalls(1)
.lastNotifyChange()
Expand Down Expand Up @@ -2748,7 +2752,7 @@ private void assertMorganDeleteFailed() throws Exception {
.assertPassword(ACCOUNT_MORGAN_PASSWORD);
}

private void assertDeletedMorgan(int expectedAttemptNumber, int expectedNumberOfPendingOperations) throws Exception {
protected void assertDeletedMorgan(int expectedAttemptNumber, int expectedNumberOfPendingOperations) throws Exception {

PrismObject<ShadowType> repoShadow = getShadowRepo(shadowMorganOid);
assertNotNull("Shadow was not created in the repository", repoShadow);
Expand Down

0 comments on commit 73a506d

Please sign in to comment.