Skip to content

Commit

Permalink
Fix too optimistic fake resource object creation
Browse files Browse the repository at this point in the history 
ShadowedChange started to create fake (identifiers-only) resource
objects in 574d326. However, this was
too optimistic: in object-class-less DELETE events in wildcard LS such
objects cannot be constructed. This commit thus fixes things by avoiding
creation of these objects where it's not possible.

This fixes failing OpenDJ-related conntests.

(These tests are also adapted to changes of provisioning internals:
some redundant operations were eliminated).
  • Loading branch information
@mederly
mederly committed Nov 13, 2023
1 parent 8378497 commit 47fe059
Show file tree
Hide file tree
Showing 12 changed files with 188 additions and 30 deletions.
10 changes: 10 additions & 0 deletions ...schema/src/main/java/com/evolveum/midpoint/schema/processor/AttributeDefinitionStore.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,4 +171,14 @@ default Collection<? extends QName> getNamesOfAttributesWithInboundExpressions()
attribute.setIncomplete(property.isIncomplete());
return attribute;
}

@SuppressWarnings("unchecked")
default <T> @NotNull ResourceAttribute<T> instantiateAttribute(@NotNull QName attrName, @NotNull T... realValues)
throws SchemaException {
//noinspection unchecked
var attrDef = (ResourceAttributeDefinition<T>) findAttributeDefinitionRequired(attrName);
var attr = attrDef.instantiate();
attr.addRealValues(realValues);
return attr;
}
}
3 changes: 1 addition & 2 deletions ...in/java/com/evolveum/midpoint/provisioning/impl/resourceobjects/ResourceObjectChange.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,7 @@ public abstract class ResourceObjectChange extends AbstractResourceEntity {
this.ucfChangeStatus = AlreadyInitializedObject.of(initialErrorState);
}

ResourceObjectChange(
@NotNull UcfChange ucfChange, @NotNull ProvisioningContext originalContext) {
ResourceObjectChange(@NotNull UcfChange ucfChange, @NotNull ProvisioningContext originalContext) {
this(ucfChange.getLocalSequenceNumber(),
ucfChange.getPrimaryIdentifierValue(),
ucfChange.getResourceObjectDefinition(),
Expand Down
19 changes: 11 additions & 8 deletions ...src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/AbstractShadowedEntity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,15 @@ public void initializeInternalCommon(Task task, OperationResult result) {

//region Shadow acquisition

/** Looks up and creates (if needed) a shadow for the resource object (in {@link #getResourceObject()}). Deals with errors. */
/**
* Looks up and creates (if needed) a shadow for the resource object (in {@link #getResourceObjectRequired()}).
* Deals with errors.
*/
@NotNull ShadowType acquireRepoShadow(OperationResult result) throws SchemaException, ConfigurationException,
ObjectNotFoundException, CommunicationException, ExpressionEvaluationException, EncryptionException,
SecurityViolationException {

var resourceObjectBean = getResourceObject().getBean();
var resourceObjectBean = getResourceObjectRequired().getBean();

// The resource object does not have any kind or intent at this point.
// But let us at least apply the definition using object class(es) in the fetched object.
Expand Down Expand Up @@ -115,7 +118,7 @@ void acquireAndSetRepoShadowInEmergency(OperationResult result)
getLogger().trace("Acquiring repo shadow in emergency:\n{}", debugDumpLazily( 1));
try {
return ShadowAcquisition.acquireRepoShadow(
globalCtx, getResourceObject().getBean(), true, result);
globalCtx, getResourceObjectRequired().getBean(), true, result);
} catch (Exception e) {
setAcquiredRepoShadowInEmergency(
acquireRepoShadowInUltraEmergency(result));
Expand All @@ -131,7 +134,7 @@ void acquireAndSetRepoShadowInEmergency(OperationResult result)
throws SchemaException, ConfigurationException, ObjectNotFoundException,
CommunicationException, ExpressionEvaluationException, EncryptionException, SecurityViolationException {
ShadowType minimalResourceObject = ShadowsUtil.minimize(
getResourceObject().getBean(),
getResourceObjectRequired().getBean(),
globalCtx.getObjectDefinitionRequired());
getLogger().trace("Minimal resource object to acquire a shadow for:\n{}",
DebugUtil.debugDumpLazily(minimalResourceObject, 1));
Expand All @@ -150,7 +153,7 @@ ShadowType updateShadowInRepository(
// TODO [from ShadowedChange]:
// should we update the repo even if we obtained current resource object from the cache? (except for e.g. metadata)
return b.shadowUpdater.updateShadowInRepository(
shadowCtx, getResourceObject(), getResourceObjectDelta(), repoShadow, result);
shadowCtx, getResourceObjectRequired(), getResourceObjectDelta(), repoShadow, result);
}

@NotNull ShadowType createShadowedObject(
Expand All @@ -159,14 +162,14 @@ ShadowType updateShadowInRepository(
SecurityViolationException, ExpressionEvaluationException, EncryptionException {
// TODO do we want also to futurize the shadow like in getObject?
// TODO [from ShadowedChange] should we bother merging if we obtained resource object from the cache?
return ShadowedObjectConstruction.construct(shadowCtx, repoShadow, getResourceObject(), result);
return ShadowedObjectConstruction.construct(shadowCtx, repoShadow, getResourceObjectRequired(), result);
}
//endregion

//region "SPI"

/** Underlying resource object. */
abstract @NotNull ResourceObject getResourceObject();
/** Underlying resource object. Be sure to avoid calling it when there's none. */
abstract @NotNull ResourceObject getResourceObjectRequired();

/** The delta - always null for search, null or non-null for changes. */
abstract @Nullable ObjectDelta<ShadowType> getResourceObjectDelta();
Expand Down
33 changes: 21 additions & 12 deletions ...ng-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/ShadowedChange.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ public abstract class ShadowedChange<ROC extends ResourceObjectChange>

/**
* Resource object: the best that is known at given time instant.
* May be even null for DELETE-only, no-OC changes in wildcard LS mode.
*
* @see #determineCurrentResourceObjectBeforeShadow()
* @see #determineCurrentResourceObjectAfterShadow(ProvisioningContext, ShadowType, OperationResult)
Expand Down Expand Up @@ -116,6 +117,7 @@ public abstract class ShadowedChange<ROC extends ResourceObjectChange>
public void initializeInternalCommon(Task task, OperationResult result) {
super.initializeInternalCommon(task, result);
// Delta is not cloned in the constructor, because it may be changed during resource change initialization.
// We need to get those changes here.
resourceObjectDelta = CloneUtil.clone(resourceObjectChange.getObjectDelta());
resourceObject = determineCurrentResourceObjectBeforeShadow();
}
Expand Down Expand Up @@ -220,7 +222,7 @@ public void initializeInternalForPrerequisiteNotApplicable(Task task, OperationR
}

@Override
public @NotNull ResourceObject getResourceObject() {
public @NotNull ResourceObject getResourceObjectRequired() {
return Objects.requireNonNull(resourceObject, "No resource object");
}

Expand Down Expand Up @@ -250,7 +252,8 @@ public void checkConsistence() {
}
}

private @NotNull ResourceObject determineCurrentResourceObjectBeforeShadow() {
/** Null value can be returned only for (some) delete events in wildcard context. */
private @Nullable ResourceObject determineCurrentResourceObjectBeforeShadow() {
CompleteResourceObject completeResourceObject = resourceObjectChange.getCompleteResourceObject();
if (completeResourceObject != null) {
return completeResourceObject.resourceObject().clone();
Expand All @@ -268,31 +271,37 @@ public void checkConsistence() {
}
}

private @NotNull ResourceObject createIdentifiersOnlyFakeResourceObject() {
private @Nullable ResourceObject createIdentifiersOnlyFakeResourceObject() {
ResourceObjectDefinition objectDefinition = resourceObjectChange.getCurrentResourceObjectDefinition();
if (objectDefinition == null) {
if (isDelete()) {
// This can happen for wildcard live sync, with no-object-class DELETE event. We have to deal with it.
return null;
} else {
throw new IllegalStateException(
"Could not create shadow from change description. Object definition is not specified: " + this);
}
}
ShadowType fakeResourceObject = new ShadowType();
fakeResourceObject.setObjectClass(objectDefinition.getTypeName());
ResourceAttributeContainer attributeContainer = objectDefinition.toResourceAttributeContainerDefinition().instantiate();
try {
ResourceObjectDefinition objectDefinition =
MiscUtil.stateNonNull(
resourceObjectChange.getCurrentResourceObjectDefinition(),
"Could not create shadow from change description. Object definition is not specified.");
ShadowType fakeResourceObject = new ShadowType();
fakeResourceObject.setObjectClass(objectDefinition.getTypeName());
ResourceAttributeContainer attributeContainer = objectDefinition.toResourceAttributeContainerDefinition().instantiate();
fakeResourceObject.asPrismObject().add(attributeContainer);
for (ResourceAttribute<?> identifier : resourceObjectChange.getIdentifiers()) {
attributeContainer.add(identifier.clone());
}
return ResourceObject.fromBean(fakeResourceObject, getPrimaryIdentifierValue());
} catch (SchemaException e) {
// All the operations are schema-safe, so this is really a kind of internal error.
throw SystemException.unexpected(e, "when creating fake resource object");
}
return ResourceObject.fromBean(fakeResourceObject, getPrimaryIdentifierValue());
}

private @NotNull ResourceObject determineCurrentResourceObjectAfterShadow(
@NotNull ProvisioningContext shadowCtx, @NotNull ShadowType repoShadow, @NotNull OperationResult result)
throws SchemaException, CommunicationException, ConfigurationException, ExpressionEvaluationException,
SecurityViolationException, EncryptionException, NotApplicableException {
if (!resourceObjectIsTemporary) {
if (resourceObject != null && !resourceObjectIsTemporary) {
return resourceObject;
}
LOGGER.trace("Going to determine current resource object, as the previous one was temporary");
Expand Down
8 changes: 4 additions & 4 deletions ...pl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/ShadowedObjectFound.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void initializeInternalForPrerequisiteOk(Task task, OperationResult resul
// No need to log stack trace now. It will be logged at the place where the exception is processed.
// It is questionable whether to log anything at all.
LOGGER.warn("Couldn't initialize {}. Continuing with previously acquired repo shadow: {}. Error: {}",
getResourceObject(), repoShadow, getClassWithMessage(e));
getResourceObjectRequired(), repoShadow, getClassWithMessage(e));
shadowedObject = repoShadow;
throw e;
}
Expand Down Expand Up @@ -139,12 +139,12 @@ public void checkConsistence() {
if (shadowedObject != null) {
ProvisioningUtil.validateShadow(shadowedObject, true);
} else {
ProvisioningUtil.validateShadow(getResourceObject().getBean(), false);
ProvisioningUtil.validateShadow(getResourceObjectRequired().getBean(), false);
}
}

private @NotNull ShadowType getAdoptedOrOriginalObject() {
return MoreObjects.firstNonNull(shadowedObject, getResourceObject().getBean());
return MoreObjects.firstNonNull(shadowedObject, getResourceObjectRequired().getBean());
}

/**
Expand Down Expand Up @@ -204,7 +204,7 @@ public void checkConsistence() {
* @see ResourceObjectFound#resourceObject
*/
@Override
public @NotNull ResourceObject getResourceObject() {
public @NotNull ResourceObject getResourceObjectRequired() {
return resourceObjectFound.getResourceObject();
}

Expand Down
3 changes: 2 additions & 1 deletion ...g-impl/src/test/java/com/evolveum/midpoint/provisioning/impl/MockLiveSyncTaskHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public class MockLiveSyncTaskHandler {
@Autowired private ProvisioningService provisioningService;
@Autowired private SynchronizationServiceMock syncServiceMock;

public void synchronize(ResourceOperationCoordinates coords, LiveSyncTokenStorage tokenStorage,
public void synchronize(
ResourceOperationCoordinates coords, LiveSyncTokenStorage tokenStorage,
Task task, OperationResult result)
throws ObjectNotFoundException, CommunicationException, SchemaException, ConfigurationException,
SecurityViolationException, ExpressionEvaluationException, PolicyViolationException, PreconditionViolationException {
Expand Down
47 changes: 47 additions & 0 deletions ...sioning-impl/src/test/java/com/evolveum/midpoint/provisioning/impl/opendj/TestOpenDj.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@
import javax.xml.datatype.XMLGregorianCalendar;
import javax.xml.namespace.QName;

import com.evolveum.midpoint.provisioning.api.LiveSyncTokenStorage;
import com.evolveum.midpoint.provisioning.api.ResourceObjectShadowChangeDescription;
import com.evolveum.midpoint.provisioning.impl.DummyTokenStorageImpl;
import com.evolveum.midpoint.schema.constants.TestResourceOpNames;
import com.evolveum.midpoint.test.util.MidPointTestConstants;
import com.evolveum.midpoint.util.exception.*;
Expand Down Expand Up @@ -3423,6 +3426,50 @@ public void test730IntegerOver32Bits() throws Exception {
.assertValue(QNAME_UID_NUMBER, overLong);
}

/** Tests the wildcard LS with DELETE event. It has no OC information, requiring very careful treatment. */
@Test
public void test740WildcardLiveSyncWithDeleteEvent() throws Exception {
Task task = getTestTask();
OperationResult result = task.getResult();

LiveSyncTokenStorage tokenStorage = new DummyTokenStorageImpl();
ResourceOperationCoordinates coords = ResourceOperationCoordinates.ofResource(resource.getOid());

var accountDef = Resource.of(resource)
.getCompleteSchemaRequired()
.findObjectClassDefinitionRequired(OBJECT_CLASS_INETORGPERSON_QNAME);

given("LDAP account");
var accountToAdd = new ShadowType()
.resourceRef(resource.getOid(), ResourceType.COMPLEX_TYPE)
.objectClass(OBJECT_CLASS_INETORGPERSON_QNAME);
var attributes = ShadowUtil.getOrCreateAttributesContainer(accountToAdd, accountDef);
attributes.add(accountDef.instantiateAttribute(QNAME_DN, "uid=test740,ou=People,dc=example,dc=com"));
attributes.add(accountDef.instantiateAttribute(QNAME_UID, "test740"));
attributes.add(accountDef.instantiateAttribute(QNAME_CN, "Test740"));
attributes.add(accountDef.instantiateAttribute(QNAME_SN, "Test"));
provisioningService.addObject(accountToAdd.asPrismObject(), null, null, task, result);

and("livesync token is fetched");
mockLiveSyncTaskHandler.synchronize(coords, tokenStorage, task, result);
syncServiceMock.reset();

when("account is deleted on LDAP");
openDJController.delete("uid=test740,ou=People,dc=example,dc=com");

and("livesync is executed");
mockLiveSyncTaskHandler.synchronize(coords, tokenStorage, task, result);

then("operation is successful and there was a delete event");
assertSuccess(result);

ResourceObjectShadowChangeDescription lastChange = syncServiceMock.getLastChange();
displayDumpable("The change", lastChange);

and("there was a model notification");
syncServiceMock.assertNotifyChange();
}

/**
* Look insite OpenDJ logs to check for clues of undesirable behavior.
* MID-7091
Expand Down

0 comments on commit 47fe059

Please sign in to comment.