Do minor sync/classification code changes

The only behavior change here is when doing classification
in ProjectionContextKeyFactoryImpl, we try to get the resource object
with "futurePointInTime" and "doNotDiscovery" options.

All other changes are more-or-less cosmetic.

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/ProjectionContextKeyFactoryImpl.java

@@ -12,6 +12,9 @@
 
 import com.evolveum.midpoint.provisioning.api.ProvisioningService;
 import com.evolveum.midpoint.provisioning.api.Resource;
+import com.evolveum.midpoint.schema.GetOperationOptions;
+import com.evolveum.midpoint.schema.SchemaService;
+import com.evolveum.midpoint.schema.SelectorOptions;
 import com.evolveum.midpoint.schema.processor.ResourceObjectDefinition;
 import com.evolveum.midpoint.schema.processor.ResourceObjectTypeIdentification;
 import com.evolveum.midpoint.schema.processor.ResourceSchema;
@@ -31,6 +34,8 @@
 
 import javax.xml.namespace.QName;
 
+import java.util.Collection;
+
 import static com.evolveum.midpoint.schema.GetOperationOptions.createReadOnlyCollection;
 import static com.evolveum.midpoint.schema.constants.SchemaConstants.INTENT_DEFAULT;
 import static com.evolveum.midpoint.util.MiscUtil.stateCheck;
@@ -77,10 +82,21 @@ private ShadowType classify(@NotNull ShadowType shadow, @NotNull Task task, @Not
                         + "Please specify the kind/intent precisely for %s", shadow);
 
         try {
-            // TODO integrate with the context loader - full shadow should be used if fetched here
-            // The "get" operation includes a classification attempt.
+            // We use the "get" operation to classify the shadow.
+            //
+            // TODO integrate with the context loader - full shadow should be stored in the context, if the fetch
+            //  from resource is successful
+            //
+            // TODO reconsider the options to use
+            //   - We use "future point in time" to allow pending changes to be processed. But this is a bit questionable: if we
+            //     have any pending changes, doesn't it mean that the shadow is most probably already classified?
+            //   - We also don't do discovery - to avoid unexpected processing here. At least for now.
+            Collection<SelectorOptions<GetOperationOptions>> options = SchemaService.get().getOperationOptionsBuilder()
+                    .futurePointInTime()
+                    .doNotDiscovery()
+                    .build();
             ShadowType updatedShadow = provisioningService
-                    .getObject(ShadowType.class, shadowOid, null, task, result)
+                    .getObject(ShadowType.class, shadowOid, options, task, result)
                     .asObjectable();
             PERFORMANCE_ADVISOR.info("Fetched {} (resource: {}) in order to classify it (result: {})",
                     updatedShadow, resourceOid, ShadowUtil.getTypeIdentification(updatedShadow));

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/loader/ProjectionsLoadOperation.java

@@ -547,7 +547,7 @@ private void getOrCreateForValueToAdd(OperationResult result)
                     // Create account context from embedded object
                     LensProjectionContext projectionContext =
                             new ShadowLevelOperation(embeddedShadow.asObjectable())
-                                    .createNew( result);
+                                    .createNew(result);
                     projectionContext.setPrimaryDeltaAfterStart(embeddedShadow.createAddDelta());
                     projectionContext.setFullShadow(true);
                     projectionContext.setExists(false);

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/sync/ShadowUpdater.java

@@ -61,7 +61,7 @@ ShadowUpdater updateAllSyncMetadata() throws SchemaException {
         updateSyncSituationDescription(now);
         updateSyncTimestamps(now, syncCtx.isFullMode());
 
-        updateCoordinates(true);
+        updateCoordinatesIfUnknown();
 
         return this;
     }
@@ -98,11 +98,25 @@ ShadowUpdater updateBothSyncTimestamps() throws SchemaException {
                 true);
     }
 
+    /**
+     * Updates kind/intent if some of them are null/empty. This is used if synchronization is skipped.
+     */
     ShadowUpdater updateCoordinatesIfMissing() throws SchemaException {
         assert syncCtx.isComplete();
         return updateCoordinates(false);
     }
 
+    /**
+     * Updates kind/intent if some of them are null/empty/unknown. This is used if synchronization is NOT skipped.
+     *
+     * TODO why the difference from {@link #updateCoordinatesIfMissing()}? Is there any reason for it?
+     * TODO this behavior should be made configurable
+     */
+    private void updateCoordinatesIfUnknown() throws SchemaException {
+        assert syncCtx.isComplete();
+        updateCoordinates(true);
+    }
+
     private ShadowUpdater updateCoordinates(boolean overwriteUnknownValues) throws SchemaException {
         assert syncCtx.isComplete();
         SynchronizationContext.Complete<?> completeCtx = (SynchronizationContext.Complete<?>) syncCtx;

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/sync/SynchronizationContext.java

@@ -437,11 +437,11 @@ boolean isFullMode() {
         return change;
     }
 
-    void recordSyncExclusion(SynchronizationExclusionReasonType reason) {
+    void recordSyncExclusionInTask(SynchronizationExclusionReasonType reason) {
         task.onSynchronizationExclusion(itemProcessingIdentifier, reason);
     }
 
-    void recordSyncStart() {
+    void recordSyncStartInTask() {
         task.onSynchronizationStart(itemProcessingIdentifier, shadowedResourceObject.getOid(), situation);
     }
 

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/sync/SynchronizationContextLoader.java

@@ -20,13 +20,11 @@
 import com.evolveum.midpoint.repo.common.SystemObjectCache;
 import com.evolveum.midpoint.model.impl.ModelBeans;
 import com.evolveum.midpoint.model.impl.ResourceObjectProcessingContextImpl;
-import com.evolveum.midpoint.prism.PrismObject;
 import com.evolveum.midpoint.provisioning.api.ResourceObjectShadowChangeDescription;
 import com.evolveum.midpoint.schema.result.OperationResult;
 import com.evolveum.midpoint.schema.util.ResourceTypeUtil;
 import com.evolveum.midpoint.schema.util.ShadowUtil;
 import com.evolveum.midpoint.task.api.Task;
-import com.evolveum.midpoint.util.MiscUtil;
 import com.evolveum.midpoint.util.exception.*;
 import com.evolveum.midpoint.util.logging.Trace;
 import com.evolveum.midpoint.util.logging.TraceManager;
@@ -58,16 +56,8 @@ SynchronizationContext<FocusType> loadSynchronizationContextFromChange(
             throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException,
             CommunicationException, ConfigurationException, SecurityViolationException {
 
-        PrismObject<ResourceType> resource =
-                MiscUtil.requireNonNull(
-                        change.getResource(),
-                        () -> new IllegalStateException("No resource in change description: " + change));
-
-        SynchronizationContext<FocusType> syncCtx = loadSynchronizationContext(
-                change,
-                resource.asObjectable(),
-                task,
-                result);
+        // TODO consider inlining this method
+        SynchronizationContext<FocusType> syncCtx = loadSynchronizationContext(change, task, result);
 
         if (Boolean.FALSE.equals(change.getShadowExistsInRepo())) {
             syncCtx.setShadowExistsInRepo(false);
@@ -79,7 +69,6 @@ SynchronizationContext<FocusType> loadSynchronizationContextFromChange(
 
     private <F extends FocusType> SynchronizationContext<F> loadSynchronizationContext(
             @NotNull ResourceObjectShadowChangeDescription change,
-            @NotNull ResourceType originalResource,
             @NotNull Task task,
             @NotNull OperationResult result)
             throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException,
@@ -89,9 +78,9 @@ private <F extends FocusType> SynchronizationContext<F> loadSynchronizationConte
 
         ResourceType updatedResource;
         if (SynchronizationContext.isSkipMaintenanceCheck()) {
-            updatedResource = originalResource;
+            updatedResource = change.getResource().asObjectable();
         } else {
-            updatedResource = checkNotInMaintenance(originalResource, task, result);
+            updatedResource = checkNotInMaintenance(change.getResource().asObjectable(), task, result);
         }
 
         SystemConfigurationType systemConfiguration = systemObjectCache.getSystemConfigurationBean(result);
@@ -170,19 +159,21 @@ private <F extends FocusType> SynchronizationContext<F> loadSynchronizationConte
             @NotNull OperationResult result) throws CommunicationException, ObjectNotFoundException, SchemaException,
             SecurityViolationException, ConfigurationException, ExpressionEvaluationException {
         ShadowType shadow = processingContext.getShadowedResourceObject();
-        ShadowKindType kind = shadow.getKind();
-        String intent = shadow.getIntent();
-        if (ShadowUtil.isKnown(kind) && ShadowUtil.isKnown(intent)) {
+        if (ShadowUtil.isClassified(shadow)) {
             if (isClassificationInSorterResult(sorterResult)) {
-                // Sorter result overrides any stored classification information
+                // Sorter result overrides any classification information stored in the shadow
+                // (and it is also applied to the shadow by SynchronizationContext#forceClassificationUpdate)
                 return TypeAndDefinition.of(schema, sorterResult.getKind(), sorterResult.getIntent());
             } else {
-                return TypeAndDefinition.of(schema, kind, intent);
+                return TypeAndDefinition.of(schema, shadow.getKind(), shadow.getIntent());
             }
         } else {
+            LOGGER.debug("Attempting to classify {} (most probably needless, as the shadow should have been "
+                    + "tried-to-be-classified before getting here)", shadow);
+            // The sorter result is used here (if it contains the classification)
             ResourceObjectClassification classification = beans.provisioningService
                     .classifyResourceObject(
-                            processingContext.getShadowedResourceObject(),
+                            shadow,
                             processingContext.getResource(),
                             sorterResult,
                             processingContext.getTask(),

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/sync/SynchronizationServiceImpl.java

@@ -80,18 +80,16 @@ public void notifyChange(
             logStart(change);
             checkConsistence(change);
 
-            // The sorter is evaluated in the following method call
+            // Object type and synchronization policy are determined here. Sorter is evaluated, if present.
             SynchronizationContext<?> syncCtx = syncContextLoader.loadSynchronizationContextFromChange(change, task, result);
 
             if (shouldSkipSynchronization(syncCtx, result)) {
                 return; // sync metadata are saved by the above method
             }
             SynchronizationContext.Complete<?> completeCtx = (SynchronizationContext.Complete<?>) syncCtx;
-
             setupLinkedOwnerAndSituation(completeCtx, change, result);
 
-            completeCtx.recordSyncStart();
-
+            completeCtx.recordSyncStartInTask();
             completeCtx.getUpdater()
                     .updateAllSyncMetadata()
                     .commit(result);
@@ -129,10 +127,11 @@ private void logStart(@NotNull ResourceObjectShadowChangeDescription change) {
      * TODO: Consider situations when one account belongs to two different users. It should correspond to
      *  the {@link SynchronizationSituationType#DISPUTED} situation.
      */
-    @Nullable
-    private PrismObject<FocusType> findShadowOwner(String shadowOid, OperationResult result) throws SchemaException {
+    private <F extends FocusType> @Nullable F findShadowOwner(SynchronizationContext.Complete<F> syncCtx, OperationResult result)
+            throws SchemaException {
+        ShadowType shadow = syncCtx.getShadowedResourceObject();
         ObjectQuery query = prismContext.queryFor(FocusType.class)
-                .item(FocusType.F_LINK_REF).ref(shadowOid, null, PrismConstants.Q_ANY)
+                .item(FocusType.F_LINK_REF).ref(shadow.getOid(), null, PrismConstants.Q_ANY)
                 .build();
         // TODO read-only later
         SearchResultList<PrismObject<FocusType>> owners =
@@ -141,16 +140,28 @@ private PrismObject<FocusType> findShadowOwner(String shadowOid, OperationResult
         if (owners.isEmpty()) {
             return null;
         }
+
         if (owners.size() > 1) {
-            LOGGER.warn("Found {} owners for shadow oid {}, returning first owner.", owners.size(), shadowOid);
+            LOGGER.warn("Found {} owners for {}, returning first owner: {}", owners.size(), shadow, owners);
+        }
+
+        FocusType owner = asObjectable(owners.get(0));
+
+        Class<F> expectedClass = syncCtx.getFocusClass();
+        if (expectedClass.isAssignableFrom(owner.getClass())) {
+            //noinspection unchecked
+            return (F) owner;
+        } else {
+            throw new SchemaException(
+                    String.format("Expected owner of type %s but %s was found instead; for %s",
+                            expectedClass.getSimpleName(), owner, shadow));
         }
-        return owners.get(0);
     }
 
     /**
      * Checks for common reasons to skip synchronization:
      *
-     * - no applicable synchronization policy,
+     * - no applicable synchronization policy (~ incomplete context),
      * - synchronization disabled,
      * - protected resource object.
      */
@@ -171,7 +182,7 @@ private boolean shouldSkipSynchronization(SynchronizationContext<?> syncCtx, Ope
                     .updateBothSyncTimestamps() // TODO should we really record this as full synchronization?
                     .commit(result);
             result.recordNotApplicable(message);
-            syncCtx.recordSyncExclusion(NO_SYNCHRONIZATION_POLICY);
+            syncCtx.recordSyncExclusionInTask(NO_SYNCHRONIZATION_POLICY);
             return true;
         }
 
@@ -185,7 +196,7 @@ private boolean shouldSkipSynchronization(SynchronizationContext<?> syncCtx, Ope
                     .updateCoordinatesIfMissing()
                     .commit(result);
             result.recordNotApplicable(message);
-            syncCtx.recordSyncExclusion(SYNCHRONIZATION_DISABLED);
+            syncCtx.recordSyncExclusionInTask(SYNCHRONIZATION_DISABLED);
             return true;
         }
 
@@ -199,7 +210,7 @@ private boolean shouldSkipSynchronization(SynchronizationContext<?> syncCtx, Ope
                     .updateCoordinatesIfMissing()
                     .commit(result);
             result.recordNotApplicable(message);
-            syncCtx.recordSyncExclusion(PROTECTED);
+            syncCtx.recordSyncExclusionInTask(PROTECTED);
             return true;
         }
 
@@ -216,8 +227,10 @@ private void checkConsistence(ResourceObjectShadowChangeDescription change) {
         }
     }
 
-    private <F extends FocusType> void setupLinkedOwnerAndSituation(SynchronizationContext.Complete<F> syncCtx,
-            ResourceObjectShadowChangeDescription change, OperationResult parentResult) throws SchemaException {
+    private <F extends FocusType> void setupLinkedOwnerAndSituation(
+            SynchronizationContext.Complete<F> syncCtx,
+            ResourceObjectShadowChangeDescription change,
+            OperationResult parentResult) throws SchemaException {
 
         OperationResult result = parentResult.subresult(OP_SETUP_SITUATION)
                 .setMinor()
@@ -229,7 +242,8 @@ private <F extends FocusType> void setupLinkedOwnerAndSituation(SynchronizationC
                 syncCtx.getFocusClass(), syncCtx.getPolicyName());
 
         try {
-            findLinkedOwner(syncCtx, change, result);
+            F linkedOwner = findShadowOwner(syncCtx, result);
+            syncCtx.setLinkedOwner(linkedOwner);
 
             if (syncCtx.getLinkedOwner() == null || syncCtx.isCorrelatorsUpdateRequested()) {
                 determineSituationWithCorrelators(syncCtx, change, result); // TODO change the name (if sorter is used)
@@ -247,19 +261,6 @@ private <F extends FocusType> void setupLinkedOwnerAndSituation(SynchronizationC
         }
     }
 
-    private <F extends FocusType> void findLinkedOwner(SynchronizationContext<F> syncCtx,
-            ResourceObjectShadowChangeDescription change, OperationResult result) throws SchemaException {
-
-        if (syncCtx.getLinkedOwner() != null) {
-            // TODO This never occurs. Clarify!
-            return;
-        }
-
-        PrismObject<FocusType> owner = findShadowOwner(change.getShadowOid(), result);
-        //noinspection unchecked
-        syncCtx.setLinkedOwner((F) asObjectable(owner));
-    }
-
     private <F extends FocusType> void determineSituationWithoutCorrelators(SynchronizationContext<F> syncCtx,
             ResourceObjectShadowChangeDescription change, OperationResult result) throws ConfigurationException {
 

provisioning/provisioning-api/src/main/java/com/evolveum/midpoint/provisioning/api/ProvisioningService.java

@@ -81,6 +81,8 @@ public interface ProvisioningService {
      *
      * ... TODO ...
      *
+     * These objects are classified, if they were not (and if they were fetched from the resource).
+     *
      * Notes:
      *
      * 1. The operation result is cleaned up before returning.

provisioning/provisioning-api/src/main/java/com/evolveum/midpoint/provisioning/api/ResourceObjectShadowChangeDescription.java

@@ -19,6 +19,7 @@
 import org.jetbrains.annotations.NotNull;
 
 import java.io.Serializable;
+import java.util.Objects;
 
 import static com.evolveum.midpoint.util.MiscUtil.stateCheck;
 
@@ -90,7 +91,7 @@ public void setObjectDelta(ObjectDelta<ShadowType> objectDelta) {
     }
 
     public @NotNull PrismObject<ShadowType> getShadowedResourceObject() {
-        return java.util.Objects.requireNonNull(shadowedResourceObject);
+        return Objects.requireNonNull(shadowedResourceObject);
     }
 
     public void setShadowedResourceObject(@NotNull PrismObject<ShadowType> shadowedResourceObject) {
@@ -106,7 +107,7 @@ public void setSourceChannel(String sourceChannel) {
     }
 
     public @NotNull PrismObject<ResourceType> getResource() {
-        return resource;
+        return Objects.requireNonNull(resource, "no resource");
     }
 
     public void setResource(PrismObject<ResourceType> resource) {