Skip to content

Commit

Permalink
Consider reaped account as being "almost gone"
Browse files Browse the repository at this point in the history
Accounts in "reaping" lifecycle state were mostly treated like living
ones. However, this brings some problems when such an account is
re-created (see e.g. MID-8069): model tries to somehow revive such
account, leading to inconsistencies in its repository shadow.

This commit fixes the situation by introducing the following changes:

1. In clockwork, accounts in "reaping" state are considered just like
if they were gone. This means that when a new account for the object
type is to be created, a new projection context is allocated for it.
We no longer reuse the original context.

2. Accounts in this state have their primaryIdentifierValue cleared.
This allows us to create a "live" version of such account.

3. We ignore accounts being reaped during constraint violation checks
(just like we do for accounts that are gone).

4. Projection of accounts being reaped (in clockwork) is skipped.

This should resolve MID-8069.
  • Loading branch information
mederly committed Sep 21, 2022
1 parent ba1b93d commit dc42c96
Show file tree
Hide file tree
Showing 30 changed files with 284 additions and 230 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1571,10 +1571,10 @@
</p>
<p>
The relation in linkRef has the following meaning: org:default means that
that the shadow the link is pointing to is "live", i.e. the corresponding
the shadow the link is pointing to is "live", i.e. the corresponding
object exists on the resource. On the other hand, org:related means that
the shadow exists in repo, but with dead = true, i.e. the corresponding
object is not existing on the resource any more.
object is not existing on the resource anymore.
</p>
<p>
Especially, when the shadow is in the Reaping state (see
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ public LensContext(Class<F> focusClass) {

/**
* TODO not sure if this method should be here or in {@link ProjectionsLoadOperation} (that is currently the only client)
* The reason for being here is the similarity to {@link #getOrCreateProjectionContext(LensContext, ConstructionTargetKey)}
* The reason for being here is the similarity to {@link #getOrCreateProjectionContext(LensContext, ConstructionTargetKey, boolean)}
* and coupling with {@link #getOrCreateProjectionContext(LensContext, HumanReadableDescribable, Supplier, Supplier)}.
*/
public @NotNull static <F extends ObjectType> GetOrCreateProjectionContextResult getOrCreateProjectionContext(
Expand All @@ -288,11 +288,11 @@ public LensContext(Class<F> focusClass) {
}

public @NotNull static <F extends ObjectType> GetOrCreateProjectionContextResult getOrCreateProjectionContext(
LensContext<F> context, ConstructionTargetKey targetKey) {
LensContext<F> context, ConstructionTargetKey targetKey, boolean acceptReaping) {
return getOrCreateProjectionContext(
context,
targetKey,
() -> context.findFirstProjectionContext(targetKey),
() -> context.findFirstProjectionContext(targetKey, acceptReaping),
() -> context.createProjectionContext(targetKey.toProjectionContextKey()));
}

Expand Down Expand Up @@ -494,10 +494,13 @@ public LensProjectionContext findFirstNotCompletedProjectionContext(@NotNull Con
/**
* TODO
*/
public LensProjectionContext findFirstProjectionContext(@NotNull ConstructionTargetKey targetKey) {
public LensProjectionContext findFirstProjectionContext(
@NotNull ConstructionTargetKey targetKey,
boolean acceptReaping) {
return getProjectionContexts().stream()
.filter(p -> !p.isGone())
.filter(p -> p.matches(targetKey))
.filter(p -> !p.isReaping() || acceptReaping)
.min(Comparator.comparing(LensProjectionContext::getOrder))
.orElse(null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,10 @@ public boolean isGone() {
return key.isGone();
}

public boolean isGoneOrReaping() {
return isGone() || isReaping();
}

public boolean isReaping() {
return getCurrentShadowState() == ShadowLifecycleStateType.REAPING;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public class EvaluatedAssignedResourceObjectConstructionImpl<AH extends Assignme
protected void initializeProjectionContext() {
// projection context may not exist yet (existence might not be yet decided)
setProjectionContext(
construction.getLensContext().findFirstProjectionContext(targetKey));
construction.getLensContext().findFirstProjectionContext(targetKey, false));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -625,8 +625,6 @@ private <T> PropertyDelta<T> consolidatePropertyWithSync(LensProjectionContext a
/**
* This method consolidate property delta against account absolute state which came from sync (not as delta)
*
* @param accCtx
* @param delta
* @return method return updated delta, or null if delta was empty after filtering (removing unnecessary values).
*/
private <T> PropertyDelta<T> consolidateWithSyncAbsolute(LensProjectionContext accCtx, PropertyDelta<T> delta,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ private <F extends FocusType> void processProjectionValues(LensContext<F> contex
if (policyDecision == SynchronizationPolicyDecision.UNLINK) {
// We will not update accounts that are being unlinked.
// we cannot skip deleted accounts here as the delete delta will be skipped as well
LOGGER.trace("Skipping processing of value for {} because the decision is {}", projContext.getHumanReadableName(), policyDecision);
LOGGER.trace("Skipping processing of values for {} because the decision is {}",
projContext.getHumanReadableName(), policyDecision);
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,14 @@ private <F extends ObjectType> void projectProjection(LensContext<F> context, Le
parentResult.addParam("resourceName", projectionContext.getResourceName());

if (projectionContext.getWave() != context.getProjectionWave()) {
LOGGER.trace("Skipping projection of {} because its wave ({}) is different from current projection wave ({})",
projectionContext, projectionContext.getWave(), context.getProjectionWave());
parentResult.recordStatus(OperationResultStatus.NOT_APPLICABLE, "Wave of the projection context differs from current projector wave");
recordSkipReason(parentResult,
"Skipping projection because the wave of the projection context (" + projectionContext.getWave() +
") differs from current projector wave (" + context.getProjectionWave() + ")");
return;
}

if (projectionContext.isCompleted()) {
LOGGER.trace("Skipping projection of {} because it's already completed", projectionContext);
parentResult.recordStatus(OperationResultStatus.NOT_APPLICABLE, "Projection context is already completed");
recordSkipReason(parentResult, "Skipping projection because the projection context is already completed");
return;
}

Expand All @@ -277,18 +276,24 @@ private <F extends ObjectType> void projectProjection(LensContext<F> context, Le
context.checkAbortRequested();

if (!projectionContext.isCanProject()) {
result.recordStatus(OperationResultStatus.NOT_APPLICABLE, "Skipping projection because of limited propagation");
recordSkipReason(result, "Skipping projection because of limited propagation");
return;
}

if (projectionContext.getSynchronizationPolicyDecision() == SynchronizationPolicyDecision.BROKEN ||
projectionContext.getSynchronizationPolicyDecision() == SynchronizationPolicyDecision.IGNORE) {
result.recordStatus(OperationResultStatus.NOT_APPLICABLE, "Skipping projection because it is " + projectionContext.getSynchronizationPolicyDecision());
recordSkipReason(result,
"Skipping projection because it is " + projectionContext.getSynchronizationPolicyDecision());
return;
}

if (projectionContext.isGone()) {
result.recordStatus(OperationResultStatus.NOT_APPLICABLE, "Skipping projection because it is gone");
recordSkipReason(result, "Skipping projection because it is gone");
return;
}

if (projectionContext.isReaping()) {
recordSkipReason(result, "Skipping projection because it is being reaped");
return;
}

Expand All @@ -300,7 +305,7 @@ private <F extends ObjectType> void projectProjection(LensContext<F> context, Le
context.checkConsistenceIfNeeded();

if (!dependencyProcessor.checkDependencies(projectionContext)) {
result.recordNotApplicable("Skipping projection because it has unsatisfied dependencies");
recordSkipReason(result, "Skipping projection because it has unsatisfied dependencies");
return;
}

Expand All @@ -313,7 +318,7 @@ private <F extends ObjectType> void projectProjection(LensContext<F> context, Le
Projector.class, context, projectionContext, activityDescription, now, task, result);

if (projectionContext.isGone()) {
result.recordStatus(OperationResultStatus.NOT_APPLICABLE, "Skipping projection because it is gone");
recordSkipReason(result, "Skipping projection because it is gone");
return;
}

Expand Down Expand Up @@ -351,6 +356,11 @@ private <F extends ObjectType> void projectProjection(LensContext<F> context, Le
}
}

private static void recordSkipReason(OperationResult result, String message) {
LOGGER.trace("{}", message);
result.recordStatus(OperationResultStatus.NOT_APPLICABLE, message);
}

private <F extends ObjectType> void addConflictingContexts(LensContext<F> context) {
for (LensProjectionContext conflictingContext : context.getConflictingProjectionContexts()) {
LOGGER.trace("Adding conflicting projection context {}", conflictingContext.getHumanReadableName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,12 @@ public void check(Task task, OperationResult result)
LOGGER.trace("No contexts found for {}; this looks like a real constraint violation", candidateOid);
return true;
}
if (matchingContexts.stream().allMatch(LensProjectionContext::isGone)) {
LOGGER.trace("All {} context(s) of {} are gone. This looks like a phantom constraint violation",
if (matchingContexts.stream().allMatch(LensProjectionContext::isGoneOrReaping)) {
LOGGER.trace("All {} context(s) of {} are gone or being reaped. This looks like a phantom constraint violation",
matchingContexts.size(), candidateOid);
return false;
} else {
LOGGER.trace("There are {} context(s) for {}, not all gone. Confirming the constraint violation.",
LOGGER.trace("There are {} context(s) for {}, not all gone or being reaped. Confirming the constraint violation.",
matchingContexts.size(), candidateOid);
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,8 @@ public boolean before(@NotNull ConstructionTargetKey key) {
@Override
public void onAssigned(@NotNull ConstructionTargetKey key, String desc)
throws SchemaException, ConfigurationException {
LensProjectionContext projectionContext = LensContext.getOrCreateProjectionContext(context, key).context;
LensProjectionContext projectionContext =
LensContext.getOrCreateProjectionContext(context, key, false).context;
projectionContext.setAssigned(true);
projectionContext.setAssignedOldIfUnknown(false);
projectionContext.setLegalOldIfUnknown(false);
Expand All @@ -477,7 +478,7 @@ public void onAssigned(@NotNull ConstructionTargetKey key, String desc)
@Override
public void onUnchangedValid(@NotNull ConstructionTargetKey key, String desc)
throws SchemaException, ConfigurationException {
LensProjectionContext projectionContext = context.findFirstProjectionContext(key);
LensProjectionContext projectionContext = context.findFirstProjectionContext(key, false);
if (projectionContext == null) {
if (processOnlyExistingProjContexts) {
LOGGER.trace("Projection {} skip: unchanged (valid), processOnlyExistingProjContexts", desc);
Expand All @@ -486,7 +487,8 @@ public void onUnchangedValid(@NotNull ConstructionTargetKey key, String desc)
// The projection should exist before the change but it does not
// This happens during reconciliation if there is an inconsistency.
// Pretend that the assignment was just added. That should do.
projectionContext = LensContext.getOrCreateProjectionContext(context, key).context;
projectionContext =
LensContext.getOrCreateProjectionContext(context, key, false).context;
}
LOGGER.trace("Projection {} legal: unchanged (valid)", desc);
projectionContext.setAssigned(true);
Expand All @@ -503,7 +505,7 @@ public void onUnchangedValid(@NotNull ConstructionTargetKey key, String desc)
@Override
public void onUnchangedInvalid(@NotNull ConstructionTargetKey key, String desc)
throws SchemaException, ConfigurationException {
LensProjectionContext projectionContext = context.findFirstProjectionContext(key);
LensProjectionContext projectionContext = context.findFirstProjectionContext(key, true);
if (projectionContext == null) {
if (processOnlyExistingProjContexts) {
LOGGER.trace("Projection {} skip: unchanged (invalid), processOnlyExistingProjContexts", desc);
Expand Down Expand Up @@ -532,7 +534,7 @@ public void onUnchangedInvalid(@NotNull ConstructionTargetKey key, String desc)
public void onUnassigned(@NotNull ConstructionTargetKey key, String desc)
throws SchemaException, ConfigurationException {
// Note we look only at wave >= current wave here
LensProjectionContext projectionContext = context.findFirstProjectionContext(key);
LensProjectionContext projectionContext = context.findFirstProjectionContext(key, true);
if (projectionContext != null && projectionContext.getObjectCurrent() != null) {
projectionContext.setAssigned(false);
projectionContext.setAssignedOldIfUnknown(true);
Expand Down Expand Up @@ -566,7 +568,7 @@ public void after(
getConstructions(constructionMapTriple.getZeroMap().get(key), true),
getConstructions(constructionMapTriple.getPlusMap().get(key), true),
getConstructions(constructionMapTriple.getMinusMap().get(key), false));
LensProjectionContext projectionContext = context.findFirstProjectionContext(key);
LensProjectionContext projectionContext = context.findFirstProjectionContext(key, true);
if (projectionContext != null) {
// This can be null in a exotic case if we delete already deleted account
LOGGER.trace("Construction delta set triple for {}:\n{}", key,
Expand Down

0 comments on commit dc42c96

Please sign in to comment.