Skip to content

Commit

Permalink
Make shadow lookup by secondary ID more robust
Browse files Browse the repository at this point in the history 
The operation used to fail if there were some dead shadows. Now it fails
only if there are multiple live shadows, or multiple dead shadows
AND no live shadow.

This resolves MID-9038.
  • Loading branch information
@mederly
mederly committed Sep 18, 2023
1 parent c7f388e commit 415cff6
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 21 deletions.
31 changes: 23 additions & 8 deletions .../evolveum/midpoint/provisioning/impl/resourceobjects/ResourceObjectReferenceResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,20 @@ class ResourceObjectReferenceResolver {

/**
* Resolve primary identifier from a collection of identifiers that may contain only secondary identifiers.
*
* We accept also dead shadows, but only if there is only one. (This is a bit inconsistent, should be fixed somehow.)
* Actually, we could be more courageous, and reject dead shadows altogether, as we use the result for object modification;
* but there is a theoretical chance that the shadow is dead in the repo but alive on the resource.
* See also {@link #resolvePrimaryIdentifiers(ProvisioningContext, ResourceObjectIdentification, OperationResult)}.
*/
@SuppressWarnings({ "unchecked", "rawtypes" })
Collection<? extends ResourceAttribute<?>> resolvePrimaryIdentifier(ProvisioningContext ctx,
Collection<? extends ResourceAttribute<?>> identifiers, final String desc, OperationResult result)
throws ObjectNotFoundException, SchemaException, CommunicationException, ConfigurationException,
ExpressionEvaluationException {
Collection<? extends ResourceAttribute<?>> resolvePrimaryIdentifier(
ProvisioningContext ctx,
Collection<? extends ResourceAttribute<?>> identifiers,
final String desc,
OperationResult result)
throws ObjectNotFoundException, SchemaException, CommunicationException, ConfigurationException,
ExpressionEvaluationException {
if (identifiers == null) {
return null;
}
Expand Down Expand Up @@ -202,7 +210,8 @@ Collection<? extends ResourceAttribute<?>> resolvePrimaryIdentifier(Provisioning
/**
* @param repoShadow Used when read capability is "caching only"
*/
PrismObject<ShadowType> fetchResourceObject(ProvisioningContext ctx,
PrismObject<ShadowType> fetchResourceObject(
ProvisioningContext ctx,
Collection<? extends ResourceAttribute<?>> identifiers,
AttributesToReturn attributesToReturn,
@Nullable PrismObject<ShadowType> repoShadow,
Expand Down Expand Up @@ -262,10 +271,15 @@ PrismObject<ShadowType> fetchResourceObject(ProvisioningContext ctx,

/**
* Resolve primary identifier from a collection of identifiers that may contain only secondary identifiers.
*
* We accept also dead shadows, but only if there is only one. (This is a bit inconsistent, should be fixed somehow.)
* Actually, we could be more courageous, and reject dead shadows altogether, as we use the result for object fetching;
* but there is a theoretical chance that the shadow is dead in the repo but alive on the resource.
* See also {@link #resolvePrimaryIdentifier(ProvisioningContext, Collection, String, OperationResult)}.
*/
@SuppressWarnings("unchecked")
private ResourceObjectIdentification resolvePrimaryIdentifiers(ProvisioningContext ctx,
ResourceObjectIdentification identification, OperationResult result)
private ResourceObjectIdentification resolvePrimaryIdentifiers(
ProvisioningContext ctx, ResourceObjectIdentification identification, OperationResult result)
throws ObjectNotFoundException, SchemaException, CommunicationException, ConfigurationException,
ExpressionEvaluationException {
if (identification == null) {
Expand All @@ -274,7 +288,8 @@ private ResourceObjectIdentification resolvePrimaryIdentifiers(ProvisioningConte
if (identification.hasPrimaryIdentifiers()) {
return identification;
}
Collection<ResourceAttribute<?>> secondaryIdentifiers = (Collection<ResourceAttribute<?>>) identification.getSecondaryIdentifiers();
Collection<ResourceAttribute<?>> secondaryIdentifiers =
(Collection<ResourceAttribute<?>>) identification.getSecondaryIdentifiers();
PrismObject<ShadowType> repoShadow = shadowFinder.lookupShadowBySecondaryIds(ctx, secondaryIdentifiers, result);
if (repoShadow == null) {
// TODO: we should attempt resource search here
Expand Down
7 changes: 6 additions & 1 deletion ...a/com/evolveum/midpoint/provisioning/impl/shadows/errors/MaintenanceExceptionHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.schema.result.OperationResultStatus;
import com.evolveum.midpoint.schema.util.ShadowUtil;
import com.evolveum.midpoint.util.DebugUtil;
import com.evolveum.midpoint.util.exception.*;
import com.evolveum.midpoint.util.logging.Trace;
import com.evolveum.midpoint.util.logging.TraceManager;
Expand Down Expand Up @@ -85,7 +86,11 @@ public OperationResultStatus handleAddError(
LOGGER.trace("Going to find matching shadows using the query:\n{}", query.debugDumpLazily(1));
List<PrismObject<ShadowType>> matchingShadows = shadowFinder.searchShadows(ctx, query, null, result);
LOGGER.trace("Found {}: {}", matchingShadows.size(), matchingShadows);
ShadowType liveShadow = asObjectable(selectLiveShadow(matchingShadows));
ShadowType liveShadow =
asObjectable(
selectLiveShadow(
matchingShadows,
DebugUtil.lazy(() -> "when looking by secondary identifier: " + query)));
LOGGER.trace("Live shadow found: {}", liveShadow);

if (liveShadow != null) {
Expand Down
5 changes: 3 additions & 2 deletions ...ava/com/evolveum/midpoint/provisioning/impl/shadows/errors/ObjectAlreadyExistHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ private void discoverConflictingShadow(

ObjectQuery query = createQueryBySecondaryIdentifier(newShadow);
List<PrismObject<ShadowType>> conflictingRepoShadows = shadowFinder.searchShadows(ctx, query, null, result);
PrismObject<ShadowType> oldShadow = selectLiveShadow(conflictingRepoShadows);
PrismObject<ShadowType> oldShadow = selectLiveShadow(conflictingRepoShadows, "(conflicting repo shadows)");
if (oldShadow != null) {
ctx.applyAttributesDefinition(oldShadow);
}
Expand All @@ -115,7 +115,8 @@ private void discoverConflictingShadow(

final List<PrismObject<ShadowType>> conflictingResourceShadows =
findConflictingShadowsOnResource(query, ctx.getTask(), result);
PrismObject<ShadowType> conflictingShadow = selectLiveShadow(conflictingResourceShadows);
PrismObject<ShadowType> conflictingShadow =
selectLiveShadow(conflictingResourceShadows, "(conflicting shadows)");

LOGGER.trace("DISCOVERY: found conflicting shadow for {}:\n{}", newShadow,
conflictingShadow == null ? " no conflicting shadow" : conflictingShadow.debugDumpLazily(1));
Expand Down
8 changes: 5 additions & 3 deletions ...l/src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowFinder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ public ShadowType lookupLiveShadowByPrimaryId(
List<PrismObject<ShadowType>> shadowsFound = searchRepoShadows(query, zeroStalenessOptions(), result);
LOGGER.trace("Found {} shadows (live or dead)", shadowsFound.size());

PrismObject<ShadowType> liveShadow = selectLiveShadow(shadowsFound);
PrismObject<ShadowType> liveShadow =
selectLiveShadow(shadowsFound, "when looking by primary identifier " + primaryIdentifier);
checkConsistency(liveShadow);
return asObjectable(liveShadow);
}
Expand Down Expand Up @@ -178,7 +179,8 @@ public ShadowType lookupLiveShadowByAllIds(

List<PrismObject<ShadowType>> shadows = searchRepoShadows(query, null, result);

PrismObject<ShadowType> singleShadow = ProvisioningUtil.selectLiveShadow(shadows);
PrismObject<ShadowType> singleShadow =
ProvisioningUtil.selectLiveShadow(shadows, "when looking by IDs: " + identifierContainer);
checkConsistency(singleShadow);
return asObjectable(singleShadow);
}
Expand Down Expand Up @@ -214,7 +216,7 @@ public PrismObject<ShadowType> lookupShadowBySecondaryIds(ProvisioningContext ct
Collection<ResourceAttribute<?>> secondaryIdentifiers, OperationResult result)
throws SchemaException {
List<PrismObject<ShadowType>> shadows = searchShadowsBySecondaryIds(ctx, secondaryIdentifiers, result);
return ProvisioningUtil.selectSingleShadow(shadows, lazy(() -> "secondary identifiers: " + secondaryIdentifiers));
return ProvisioningUtil.selectSingleShadowRelaxed(shadows, lazy(() -> "secondary identifiers: " + secondaryIdentifiers));
}

private List<PrismObject<ShadowType>> searchShadowsBySecondaryIds(
Expand Down
38 changes: 31 additions & 7 deletions ...isioning-impl/src/main/java/com/evolveum/midpoint/provisioning/util/ProvisioningUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ public static boolean isDoDiscovery(@NotNull ResourceType resource, Provisioning
public static PrismObject<ShadowType> selectSingleShadow(@NotNull List<PrismObject<ShadowType>> shadows, Object context) {
LOGGER.trace("Selecting from {} objects", shadows.size());

if (shadows.size() == 0) {
if (shadows.isEmpty()) {
return null;
} else if (shadows.size() > 1) {
LOGGER.error("Too many shadows ({}) for {}", shadows.size(), context);
Expand All @@ -581,24 +581,48 @@ public static PrismObject<ShadowType> selectSingleShadow(@NotNull List<PrismObje
}
}

/**
* As {@link #selectSingleShadow(List, Object)} but allows the existence of multiple dead shadows
* (if single live shadow exists). Not very nice! Transitional solution until better one is found.
*/
@Nullable
public static PrismObject<ShadowType> selectSingleShadowRelaxed(
@NotNull List<PrismObject<ShadowType>> shadows, Object context) {
var singleLive = selectLiveShadow(shadows, context);
if (singleLive != null) {
return singleLive;
}

// all remaining shadows (if any) are dead
if (shadows.isEmpty()) {
return null;
} else if (shadows.size() > 1) {
LOGGER.error("Cannot select from dead shadows ({}) for {}", shadows.size(), context);
LOGGER.debug("Shadows:\n{}", DebugUtil.debugDumpLazily(shadows));
throw new IllegalStateException("More than one [dead] shadow for " + context);
} else {
return shadows.get(0);
}
}

// TODO better place?
@Nullable
public static PrismObject<ShadowType> selectLiveShadow(List<PrismObject<ShadowType>> shadows) {
public static PrismObject<ShadowType> selectLiveShadow(List<PrismObject<ShadowType>> shadows, Object context) {
if (shadows == null || shadows.isEmpty()) {
return null;
}

List<PrismObject<ShadowType>> liveShadows = shadows.stream()
.filter(ShadowUtil::isNotDead)
.collect(Collectors.toList());
.toList();

if (liveShadows.isEmpty()) {
return null;
} else if (liveShadows.size() > 1) {
LOGGER.trace("More than one live shadow found ({} out of {}):\n{}",
liveShadows.size(), shadows.size(), DebugUtil.debugDumpLazily(shadows, 1));
LOGGER.trace("More than one live shadow found ({} out of {}) {}\n{}",
liveShadows.size(), shadows.size(), context, DebugUtil.debugDumpLazily(shadows, 1));
// TODO: handle "more than one shadow" case for conflicting shadows - MID-4490
throw new IllegalStateException("Found more than one live shadow: " + liveShadows);
throw new IllegalStateException("Found more than one live shadow " + context + ": " + liveShadows);
} else {
return liveShadows.get(0);
}
Expand All @@ -611,7 +635,7 @@ public static PrismObject<ShadowType> selectLiveShadow(List<PrismObject<ShadowTy
* TODO better place?
*/
public static ShadowType selectLiveOrAnyShadow(List<PrismObject<ShadowType>> shadows) {
PrismObject<ShadowType> liveShadow = ProvisioningUtil.selectLiveShadow(shadows);
PrismObject<ShadowType> liveShadow = ProvisioningUtil.selectLiveShadow(shadows, "");
if (liveShadow != null) {
return liveShadow.asObjectable();
} else if (shadows.isEmpty()) {
Expand Down

0 comments on commit 415cff6

Please sign in to comment.