Fix updating index-only attributes in provisioning

Index-only cached attributes were not correctly maintained in repo. This is now fixed: 1) ShadowUpdater#updateShadow now explicitly retrieves index-only attributes from the repository before computing update deltas (if needed). 2) ResourceAttributeContainer#convertFromContainer method now correctly preserves "incomplete" flag on individual attributes. This should fix MID-7162.
Evolveum · Aug 25, 2021 · 0f5ad9b · 0f5ad9b
1 parent e9a4124
commit 0f5ad9b
Show file tree

Hide file tree

Showing 12 changed files with 455 additions and 24 deletions.
diff --git a/...rc/main/java/com/evolveum/midpoint/schema/processor/ObjectClassComplexTypeDefinition.java b/...rc/main/java/com/evolveum/midpoint/schema/processor/ObjectClassComplexTypeDefinition.java
@@ -245,4 +245,9 @@ default ObjectQuery createShadowSearchQuery(String resourceOid) throws SchemaExc
     boolean matches(ShadowType shadow);
 
     MutableObjectClassComplexTypeDefinition toMutable();
+
+    default boolean hasIndexOnlyAttributes() {
+        return getAttributeDefinitions().stream()
+                .anyMatch(ItemDefinition::isIndexOnly);
+    }
 }
diff --git a/...hema/src/main/java/com/evolveum/midpoint/schema/processor/ResourceAttributeContainer.java b/...hema/src/main/java/com/evolveum/midpoint/schema/processor/ResourceAttributeContainer.java
@@ -47,6 +47,7 @@ static ResourceAttributeContainer convertFromContainer(PrismContainer<?> origAtt
                 }
                 attributesContainer.add(attribute);
                 attribute.applyDefinition(attributeDefinition);
+                attribute.setIncomplete(item.isIncomplete());
             } else {
                 throw new SchemaException("Cannot process item of type "+item.getClass().getSimpleName()+", attributes can only be properties");
             }

diff --git a/...sioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/GetHelper.java b/...sioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/GetHelper.java
@@ -15,6 +15,8 @@
 
 import com.evolveum.midpoint.provisioning.impl.resourceobjects.ResourceObjectConverter;
 
+import com.evolveum.midpoint.schema.*;
+
 import org.apache.commons.lang.Validate;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
@@ -33,10 +35,6 @@
 import com.evolveum.midpoint.provisioning.ucf.api.GenericFrameworkException;
 import com.evolveum.midpoint.provisioning.util.ProvisioningUtil;
 import com.evolveum.midpoint.repo.api.RepositoryService;
-import com.evolveum.midpoint.schema.GetOperationOptions;
-import com.evolveum.midpoint.schema.PointInTimeType;
-import com.evolveum.midpoint.schema.RefreshShadowOperation;
-import com.evolveum.midpoint.schema.SelectorOptions;
 import com.evolveum.midpoint.schema.internals.InternalCounters;
 import com.evolveum.midpoint.schema.internals.InternalMonitor;
 import com.evolveum.midpoint.schema.processor.ResourceAttribute;

diff --git a/...ain/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowDeltaComputer.java b/...ain/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowDeltaComputer.java
@@ -229,7 +229,7 @@ private void processAttributes(ProvisioningContext ctx,
                     }
                 }
             }
-            incompleteCacheableAttributes.clear();          // So we are OK regarding this. We can update caching timestamp.
+            incompleteCacheableAttributes.clear(); // So we are OK regarding this. We can update caching timestamp.
         }
     }
 

diff --git a/.../src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowManager.java b/.../src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowManager.java
@@ -317,6 +317,8 @@ public boolean isRepositoryOnlyModification(Collection<? extends ItemDelta> modi
      * Updates repository shadow based on object or delta from resource.
      * Updates: cached attributes and activation, shadow name, aux object classes, exists flag, caching metadata.
      *
+     * Retrieves index-only attributes from repo if needed.
+     *
      * @param currentResourceObject Current state of the resource object. Not shadowized yet.
      * @param resourceObjectDelta Delta coming from the resource (if known).
      *

diff --git a/.../src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowUpdater.java b/.../src/main/java/com/evolveum/midpoint/provisioning/impl/shadows/manager/ShadowUpdater.java
@@ -24,13 +24,14 @@
 import com.evolveum.midpoint.provisioning.api.ProvisioningOperationOptions;
 import com.evolveum.midpoint.provisioning.api.ProvisioningService;
 import com.evolveum.midpoint.provisioning.api.ShadowDeathEvent;
+import com.evolveum.midpoint.provisioning.impl.ShadowCaretaker;
 import com.evolveum.midpoint.provisioning.impl.shadows.ConstraintsChecker;
 import com.evolveum.midpoint.provisioning.impl.ProvisioningContext;
 import com.evolveum.midpoint.provisioning.impl.ProvisioningOperationState;
 import com.evolveum.midpoint.provisioning.impl.ShadowState;
 import com.evolveum.midpoint.provisioning.util.ProvisioningUtil;
 import com.evolveum.midpoint.repo.api.*;
-import com.evolveum.midpoint.schema.DeltaConvertor;
+import com.evolveum.midpoint.schema.*;
 import com.evolveum.midpoint.schema.constants.SchemaConstants;
 import com.evolveum.midpoint.schema.processor.ResourceAttribute;
 import com.evolveum.midpoint.schema.result.AsynchronousOperationResult;
@@ -89,6 +90,7 @@ class ShadowUpdater {
     @Autowired private ShadowDeltaComputer shadowDeltaComputer;
     @Autowired private ShadowFinder shadowFinder;
     @Autowired private ShadowCreator shadowCreator;
+    @Autowired private ShadowCaretaker shadowCaretaker;
     @Autowired private Helper helper;
     @Autowired private CreatorUpdaterHelper creatorUpdaterHelper;
     @Autowired private PendingOperationsHelper pendingOperationsHelper;
@@ -940,8 +942,15 @@ PrismObject<ShadowType> updateShadow(@NotNull ProvisioningContext ctx,
             throws SchemaException, ObjectNotFoundException, ConfigurationException, CommunicationException,
             ExpressionEvaluationException {
 
-        ObjectDelta<ShadowType> computedShadowDelta = shadowDeltaComputer.computeShadowDelta(ctx, repoShadow, currentResourceObject,
-                resourceObjectDelta, shadowState);
+        if (resourceObjectDelta == null) {
+            repoShadow = retrieveIndexOnlyAttributesIfNeeded(ctx, repoShadow, result);
+        } else {
+            LOGGER.trace("Resource object delta is present. We assume we will be able to update the shadow without "
+                    + "explicitly reading index-only attributes first."); // TODO check if this assumption is correct
+        }
+
+        ObjectDelta<ShadowType> computedShadowDelta = shadowDeltaComputer.computeShadowDelta(ctx, repoShadow,
+                currentResourceObject, resourceObjectDelta, shadowState);
 
         if (!computedShadowDelta.isEmpty()) {
             LOGGER.trace("Updating repo shadow {} with delta:\n{}", repoShadow, computedShadowDelta.debugDumpLazily(1));
@@ -955,6 +964,44 @@ PrismObject<ShadowType> updateShadow(@NotNull ProvisioningContext ctx,
         }
     }
 
+    private PrismObject<ShadowType> retrieveIndexOnlyAttributesIfNeeded(ProvisioningContext shadowCtx,
+            PrismObject<ShadowType> repoShadow, OperationResult result)
+            throws SchemaException, ExpressionEvaluationException, CommunicationException, ConfigurationException,
+            ObjectNotFoundException {
+        RefinedObjectClassDefinition objectClassDefinition = shadowCtx.getObjectClassDefinition();
+        if (objectClassDefinition == null) {
+            // TODO consider throwing an exception
+            LOGGER.warn("No object class definition for {}", shadowCtx);
+            return repoShadow;
+        }
+
+        if (!objectClassDefinition.hasIndexOnlyAttributes()) {
+            LOGGER.trace("No index only attributes -> nothing to retrieve");
+            return repoShadow;
+        }
+
+        if (ShadowUtil.getAttributes(repoShadow).stream()
+                .noneMatch(Item::isIncomplete)) {
+            LOGGER.trace("All repo attributes are complete -> nothing to retrieve");
+            return repoShadow;
+        }
+
+        LOGGER.debug("Re-reading the shadow, retrieving all attributes (including index-only ones): {}", repoShadow);
+        Collection<SelectorOptions<GetOperationOptions>> options =
+                SchemaService.get().getOperationOptionsBuilder()
+                        .item(ShadowType.F_ATTRIBUTES).retrieve(RetrieveOption.INCLUDE)
+                        .build();
+
+        PrismObject<ShadowType> retrievedRepoShadow =
+                repositoryService.getObject(ShadowType.class, repoShadow.getOid(), options, result);
+
+        shadowCaretaker.applyAttributesDefinition(shadowCtx, retrievedRepoShadow);
+
+        LOGGER.trace("Full repo shadow:\n{}", retrievedRepoShadow.debugDumpLazily(1));
+
+        return retrievedRepoShadow;
+    }
+
     public void deleteShadow(ProvisioningContext ctx, PrismObject<ShadowType> oldRepoShadow, OperationResult result)
             throws ObjectNotFoundException, SchemaException, CommunicationException, ConfigurationException,
             ExpressionEvaluationException {

diff --git a/...l/src/test/java/com/evolveum/midpoint/provisioning/impl/dummy/AbstractBasicDummyTest.java b/...l/src/test/java/com/evolveum/midpoint/provisioning/impl/dummy/AbstractBasicDummyTest.java
@@ -6,6 +6,7 @@
  */
 package com.evolveum.midpoint.provisioning.impl.dummy;
 
+import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.testng.AssertJUnit.*;
 
@@ -20,7 +21,10 @@
 import javax.xml.datatype.XMLGregorianCalendar;
 import javax.xml.namespace.QName;
 
+import com.evolveum.midpoint.schema.processor.*;
+
 import org.apache.commons.lang.StringUtils;
+import org.jetbrains.annotations.NotNull;
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.context.ContextConfiguration;
 import org.testng.AssertJUnit;
@@ -53,10 +57,6 @@
 import com.evolveum.midpoint.schema.constants.ConnectorTestOperation;
 import com.evolveum.midpoint.schema.constants.SchemaConstants;
 import com.evolveum.midpoint.schema.internals.InternalCounters;
-import com.evolveum.midpoint.schema.processor.ResourceAttribute;
-import com.evolveum.midpoint.schema.processor.ResourceAttributeDefinition;
-import com.evolveum.midpoint.schema.processor.ResourceSchema;
-import com.evolveum.midpoint.schema.processor.ResourceSchemaImpl;
 import com.evolveum.midpoint.schema.result.OperationResult;
 import com.evolveum.midpoint.schema.result.OperationResultStatus;
 import com.evolveum.midpoint.schema.statistics.ConnectorOperationalStatus;
@@ -1433,7 +1433,7 @@ protected ObjectChecker<ShadowType> createShadowChecker(final boolean fullShadow
         };
     }
 
-    protected <T> void assertRepoShadowCachedAttributeValue(PrismObject<ShadowType> shadowRepo, String attrName, T... attrValues) {
+    protected void assertRepoShadowCachedAttributeValue(PrismObject<ShadowType> shadowRepo, String attrName, Object... attrValues) {
         PrismAsserts.assertNoItem(shadowRepo, ItemPath.create(ShadowType.F_ATTRIBUTES,
                 new ItemName(ResourceTypeUtil.getResourceNamespace(resource), attrName)));
     }
@@ -1468,4 +1468,22 @@ protected void assertRepoShadowPasswordValue(PrismObject<ShadowType> shadowRepo,
         assertNull("Unexpected password value in repo shadow " + shadowRepo, passwordValue);
     }
 
+    protected ResourceAttributeDefinition<?> getAccountAttrDef(String name) throws SchemaException {
+        return requireNonNull(
+                getAccountObjectClassDefinition().findAttributeDefinition(name));
+    }
+
+    protected ResourceAttributeDefinition<?> getAccountAttrDef(QName name) throws SchemaException {
+        return requireNonNull(
+                getAccountObjectClassDefinition().findAttributeDefinition(name));
+    }
+
+    @NotNull
+    private ObjectClassComplexTypeDefinition getAccountObjectClassDefinition() throws SchemaException {
+        ResourceSchema resourceSchema =
+                requireNonNull(
+                        RefinedResourceSchema.getResourceSchema(resource, prismContext));
+        return requireNonNull(
+                resourceSchema.findObjectClassDefinition(SchemaTestConstants.ACCOUNT_OBJECT_CLASS_LOCAL_NAME));
+    }
 }