Skip to content

Commit

Permalink
Stop using shadow lifecycle for proposed shadows
Browse files Browse the repository at this point in the history
The "technical" shadow lifecycle state is now determined purely from
the "dead" and "exists" properties, and the pending operations,
according to
https://docs.evolveum.com/midpoint/reference/resources/shadow/dead/.

This resolves MID-4833.
  • Loading branch information
mederly committed Nov 10, 2022
1 parent 1aee57e commit b7d9c55
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 39 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright (C) 2010-2022 Evolveum and contributors
*
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
*/

package com.evolveum.midpoint.schema.util;

import com.evolveum.midpoint.xml.ns._public.common.common_3.PendingOperationExecutionStatusType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.PendingOperationType;

import com.evolveum.prism.xml.ns._public.types_3.ChangeTypeType;
import com.evolveum.prism.xml.ns._public.types_3.ObjectDeltaType;

import org.jetbrains.annotations.NotNull;

import static com.evolveum.midpoint.xml.ns._public.common.common_3.PendingOperationExecutionStatusType.EXECUTING;
import static com.evolveum.midpoint.xml.ns._public.common.common_3.PendingOperationExecutionStatusType.EXECUTION_PENDING;

public class PendingOperationTypeUtil {

public static boolean isAdd(@NotNull PendingOperationType operation) {
ObjectDeltaType delta = operation.getDelta();
return delta != null && delta.getChangeType() == ChangeTypeType.ADD;
}

public static boolean isPendingOrExecuting(PendingOperationType operation) {
PendingOperationExecutionStatusType status = operation.getExecutionStatus();
return status == EXECUTION_PENDING || status == EXECUTING;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -590,11 +590,13 @@ public AttributesToReturn createAttributesToReturn() {

// Methods delegated to shadow caretaker (convenient to be here, but not sure if it's ok...)

/** Beware! Creates a new context based on the shadow kind/intent/OC. */
public ProvisioningContext applyAttributesDefinition(@NotNull PrismObject<ShadowType> shadow)
throws SchemaException, ConfigurationException {
return getCaretaker().applyAttributesDefinitionInNewContext(this, shadow);
}

/** Beware! Creates a new context based on the shadow kind/intent/OC. */
public ProvisioningContext applyAttributesDefinition(@NotNull ShadowType shadow)
throws SchemaException, ConfigurationException {
return getCaretaker().applyAttributesDefinitionInNewContext(this, shadow);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

import com.evolveum.midpoint.schema.processor.ResourceObjectDefinition;

import com.evolveum.midpoint.schema.util.PendingOperationTypeUtil;

import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
import org.springframework.beans.factory.annotation.Autowired;
Expand Down Expand Up @@ -248,7 +250,7 @@ public ProvisioningContext reapplyDefinitions(
continue;
}
if (PendingOperationExecutionStatusType.COMPLETED.equals(executionStatus)
&& ProvisioningUtil.isOverPeriod(now, gracePeriod, pendingOperation)) {
&& ProvisioningUtil.isCompletedAndOverPeriod(now, gracePeriod, pendingOperation)) {
// Completed operations over grace period. They have already affected current state. They are already "applied".
continue;
}
Expand Down Expand Up @@ -315,7 +317,8 @@ public List<PendingOperationType> sortPendingOperations(List<PendingOperationTyp
return sortedList;
}

public ChangeTypeType findPreviousPendingLifecycleOperationInGracePeriod(
/** Returns {@link ChangeTypeType#ADD}, {@link ChangeTypeType#DELETE}, or null. */
public ChangeTypeType findPendingLifecycleOperationInGracePeriod(
@Nullable ProvisioningContext ctx,
@NotNull ShadowType shadow,
@NotNull XMLGregorianCalendar now) {
Expand All @@ -332,9 +335,9 @@ public ChangeTypeType findPreviousPendingLifecycleOperationInGracePeriod(
}
ChangeTypeType changeType = delta.getChangeType();
if (ChangeTypeType.MODIFY.equals(changeType)) {
continue;
continue; // MODIFY is not a lifecycle operation
}
if (ProvisioningUtil.isOverPeriod(now, gracePeriod, pendingOperation)) {
if (ProvisioningUtil.isCompletedAndOverPeriod(now, gracePeriod, pendingOperation)) {
continue;
}
if (changeType == ChangeTypeType.DELETE) {
Expand All @@ -348,6 +351,13 @@ public ChangeTypeType findPreviousPendingLifecycleOperationInGracePeriod(
return found;
}

private boolean hasPendingOrExecutingAdd(@NotNull ShadowType shadow) {
return shadow.getPendingOperation().stream()
.anyMatch(p ->
PendingOperationTypeUtil.isAdd(p)
&& PendingOperationTypeUtil.isPendingOrExecuting(p));
}

// NOTE: detection of quantum states (gestation, corpse) might not be precise. E.g. the shadow may already be
// tombstone because it is not in the snapshot. But as long as the pending operation is in grace we will still
// detect it as corpse. But that should not cause any big problems.
Expand All @@ -356,12 +366,16 @@ public ChangeTypeType findPreviousPendingLifecycleOperationInGracePeriod(
return determineShadowStateInternal(ctx, shadow, clock.currentTimeXMLGregorianCalendar());
}

/** If emergency situations the context can be null. */
/**
* Determines the shadow lifecycle state according to https://docs.evolveum.com/midpoint/reference/resources/shadow/dead/.
*
* @param ctx Used to know the grace period. In emergency situations it can be null.
*/
private @NotNull ShadowLifecycleStateType determineShadowStateInternal(
@Nullable ProvisioningContext ctx,
@NotNull ShadowType shadow,
@NotNull XMLGregorianCalendar now) {
ChangeTypeType pendingLifecycleOperation = findPreviousPendingLifecycleOperationInGracePeriod(ctx, shadow, now);
ChangeTypeType pendingLifecycleOperation = findPendingLifecycleOperationInGracePeriod(ctx, shadow, now);
if (ShadowUtil.isDead(shadow)) {
if (pendingLifecycleOperation == ChangeTypeType.DELETE) {
return ShadowLifecycleStateType.CORPSE;
Expand All @@ -378,12 +392,10 @@ public ChangeTypeType findPreviousPendingLifecycleOperationInGracePeriod(
return ShadowLifecycleStateType.LIVE;
}
}
// FIXME This is wrong. Lifecycle is used for two independent purposes, see MID-4833.
// TODO we should use pending ADD instead
if (SchemaConstants.LIFECYCLE_PROPOSED.equals(shadow.getLifecycleState())) {
return ShadowLifecycleStateType.PROPOSED;
} else {
if (hasPendingOrExecutingAdd(shadow)) {
return ShadowLifecycleStateType.CONCEIVED;
} else {
return ShadowLifecycleStateType.PROPOSED;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ private void expirePendingOperations(
Duration pendingOperationRetentionPeriod = ProvisioningUtil.getPendingOperationRetentionPeriod(ctx);
Duration expirePeriod = XmlTypeConverter.longerDuration(gracePeriod, pendingOperationRetentionPeriod);
for (PendingOperationType pendingOperation: repoShadow.getPendingOperation()) {
if (ProvisioningUtil.isOverPeriod(now, expirePeriod, pendingOperation)) {
if (ProvisioningUtil.isCompletedAndOverPeriod(now, expirePeriod, pendingOperation)) {
LOGGER.trace("Deleting pending operation because it is completed '{}' and expired: {}",
pendingOperation.getResultStatus(), pendingOperation);
shadowDelta.addModificationDeleteContainer(ShadowType.F_PENDING_OPERATION, pendingOperation.clone());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ private boolean hasDeadShadowWithDeleteOperation(

XMLGregorianCalendar now = clock.currentTimeXMLGregorianCalendar();
for (PrismObject<ShadowType> previousDeadShadow : previousDeadShadows) {
if (shadowCaretaker.findPreviousPendingLifecycleOperationInGracePeriod(ctx, previousDeadShadow.asObjectable(), now) == ChangeTypeType.DELETE) {
if (shadowCaretaker.findPendingLifecycleOperationInGracePeriod(ctx, previousDeadShadow.asObjectable(), now) == ChangeTypeType.DELETE) {
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,7 @@ void addNewProposedShadow(
return;
}

// This is wrong: MID-4833
ShadowType newRepoShadow = createRepositoryShadow(ctx, shadowToAdd);
newRepoShadow.setLifecycleState(SchemaConstants.LIFECYCLE_PROPOSED);
opState.setExecutionStatus(PendingOperationExecutionStatusType.REQUESTED);
pendingOperationsHelper.addPendingOperationAdd(
newRepoShadow, shadowToAdd, opState, ctx.getTask().getTaskIdentifier());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,16 +367,6 @@ ShadowType recordDeleteResult(
}
}

// TODO: this is wrong. Provisioning should not change lifecycle states. Just for compatibility. MID-4833
if (ctx.shouldUseProposedShadows()) {
String currentLifecycleState = repoShadow.getLifecycleState();
if (currentLifecycleState != null && !currentLifecycleState.equals(SchemaConstants.LIFECYCLE_ACTIVE)) {
shadowModifications.add(
createShadowPropertyReplaceDelta(
repoShadow, ShadowType.F_LIFECYCLE_STATE, SchemaConstants.LIFECYCLE_ACTIVE));
}
}

return shadowModifications;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,8 @@ public static Duration getPendingOperationRetentionPeriod(ProvisioningContext ct
return period;
}

public static boolean isOverPeriod(XMLGregorianCalendar now, Duration period, PendingOperationType pendingOperation) {
public static boolean isCompletedAndOverPeriod(
XMLGregorianCalendar now, Duration period, PendingOperationType pendingOperation) {
if (!isCompleted(pendingOperation.getResultStatus())) {
return false;
}
Expand Down Expand Up @@ -527,7 +528,9 @@ public static int getMaxRetryAttempts(ProvisioningContext ctx) {
}

public static boolean isCompleted(OperationResultStatusType statusType) {
return statusType != null && statusType != OperationResultStatusType.IN_PROGRESS && statusType != OperationResultStatusType.UNKNOWN;
return statusType != null
&& statusType != OperationResultStatusType.IN_PROGRESS
&& statusType != OperationResultStatusType.UNKNOWN;
}

public static boolean hasPendingAddOperation(ShadowType shadow) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import com.evolveum.midpoint.schema.constants.MidPointConstants;
import com.evolveum.midpoint.schema.processor.*;

import com.evolveum.midpoint.util.exception.ConfigurationException;
import com.evolveum.midpoint.util.exception.*;

import com.evolveum.prism.xml.ns._public.types_3.RawType;

Expand Down Expand Up @@ -75,7 +75,6 @@
import com.evolveum.midpoint.test.IntegrationTestTools;
import com.evolveum.midpoint.test.ObjectChecker;
import com.evolveum.midpoint.test.util.TestUtil;
import com.evolveum.midpoint.util.exception.SchemaException;
import com.evolveum.midpoint.xml.ns._public.common.api_types_3.ObjectModificationType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
import com.evolveum.midpoint.xml.ns._public.resource.capabilities_3.*;
Expand Down Expand Up @@ -1452,7 +1451,7 @@ protected void assertWillRepoShadowAfterCreate(PrismObject<ShadowType> repoShado
}

protected void checkRepoAccountShadowWillBasic(PrismObject<ShadowType> accountRepo,
XMLGregorianCalendar start, XMLGregorianCalendar end, Integer expectedNumberOfAttributes) {
XMLGregorianCalendar start, XMLGregorianCalendar end, Integer expectedNumberOfAttributes) throws CommonException {
display("Will account repo", accountRepo);
ShadowType accountTypeRepo = accountRepo.asObjectable();
assertShadowName(accountRepo, ACCOUNT_WILL_USERNAME);
Expand Down Expand Up @@ -1480,16 +1479,13 @@ protected void assertPrimaryIdentifierValue(PrismObject<ShadowType> shadow, Stri
}
}

// FIXME MID-4833
private boolean isProposedShadow(PrismObject<ShadowType> shadow) {
String lifecycleState = shadow.asObjectable().getLifecycleState();
if (lifecycleState == null) {
return false;
}
return SchemaConstants.LIFECYCLE_PROPOSED.equals(lifecycleState);
private boolean isProposedShadow(PrismObject<ShadowType> shadow) throws CommonException {
provisioningService.determineShadowState(shadow, getTestTask(), getTestOperationResult());
return shadow.asObjectable().getShadowLifecycleState() == ShadowLifecycleStateType.PROPOSED;
}

protected void checkRepoAccountShadowWill(PrismObject<ShadowType> accountRepo, XMLGregorianCalendar start, XMLGregorianCalendar end) {
protected void checkRepoAccountShadowWill(
PrismObject<ShadowType> accountRepo, XMLGregorianCalendar start, XMLGregorianCalendar end) throws CommonException {
checkRepoAccountShadowWillBasic(accountRepo, start, end, 2);
assertRepoShadowCacheActivation(accountRepo, null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.evolveum.midpoint.schema.constants.SchemaConstants;
import com.evolveum.midpoint.schema.processor.ResourceAttributeDefinition;

import com.evolveum.midpoint.util.exception.CommonException;
import com.evolveum.midpoint.util.exception.ConfigurationException;

import org.springframework.test.annotation.DirtiesContext;
Expand Down Expand Up @@ -400,7 +401,8 @@ private void updateAndCheckMultivaluedAttribute(DummyAccount account, boolean us
}

@Override
protected void checkRepoAccountShadowWill(PrismObject<ShadowType> shadowRepo, XMLGregorianCalendar start, XMLGregorianCalendar end) {
protected void checkRepoAccountShadowWill(
PrismObject<ShadowType> shadowRepo, XMLGregorianCalendar start, XMLGregorianCalendar end) throws CommonException {
// Sometimes there are 6 and sometimes 7 attributes. Treasure is not returned by default. It is not normally in the cache.
// So do not check for number of attributes here. Check for individual values.
checkRepoAccountShadowWillBasic(shadowRepo, start, end, null);
Expand Down

0 comments on commit b7d9c55

Please sign in to comment.