Fix assigned projections with focus iteration

When the assigned resource object constructions are evaluated repeatedly (for example, when focus iteration is engaged), midPoint may try to fetch not-yet-existing projection. It is because of limitations of the current LensProjectionContext#isAdd implementation (see the javadoc added). Either we would improve that method, or - as a quicker albeit maybe not so comprehensive fix - we simply avoid loading projections without OID when evaluating such constructions. The latter approach was selected for this commit. This resolves MID-8569. (cherry picked from commit 4a4067a)
Evolveum · Mar 9, 2023 · 16dcab7 · 16dcab7
1 parent 7892e2d
commit 16dcab7
Show file tree

Hide file tree

Showing 12 changed files with 312 additions and 17 deletions.
diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/LensElementContext.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/LensElementContext.java
@@ -15,11 +15,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
-
-import com.evolveum.midpoint.model.impl.lens.ElementState.CurrentObjectAdjuster;
-
-import com.evolveum.midpoint.model.impl.lens.ElementState.ObjectDefinitionRefiner;
-import com.evolveum.midpoint.prism.util.CloneUtil;
+import javax.xml.datatype.XMLGregorianCalendar;
 
 import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.lang.StringUtils;
@@ -29,14 +25,18 @@
 import com.evolveum.midpoint.model.api.context.EvaluatedPolicyRule;
 import com.evolveum.midpoint.model.api.context.EvaluatedPolicyRuleTrigger;
 import com.evolveum.midpoint.model.api.context.ModelElementContext;
+import com.evolveum.midpoint.model.impl.lens.ElementState.CurrentObjectAdjuster;
+import com.evolveum.midpoint.model.impl.lens.ElementState.ObjectDefinitionRefiner;
 import com.evolveum.midpoint.model.impl.lens.assignments.AssignmentSpec;
+import com.evolveum.midpoint.model.impl.lens.projector.ActivationProcessor;
 import com.evolveum.midpoint.prism.Objectable;
 import com.evolveum.midpoint.prism.PrismContext;
 import com.evolveum.midpoint.prism.PrismObject;
 import com.evolveum.midpoint.prism.PrismObjectDefinition;
 import com.evolveum.midpoint.prism.delta.ItemDelta;
 import com.evolveum.midpoint.prism.delta.ObjectDelta;
 import com.evolveum.midpoint.prism.delta.PlusMinusZero;
+import com.evolveum.midpoint.prism.util.CloneUtil;
 import com.evolveum.midpoint.schema.DeltaConvertor;
 import com.evolveum.midpoint.schema.ObjectDeltaOperation;
 import com.evolveum.midpoint.schema.result.OperationResult;
@@ -509,12 +509,24 @@ public void triggerRule(@NotNull EvaluatedPolicyRule rule, Collection<EvaluatedP
     //endregion
 
     //region Kinds of operations
+
+    /**
+     * Be cautious when using this method for {@link LensProjectionContext}. The projection may be called into existence because
+     * a construction is assigned - i.e., no primary delta exists in this case. But the policy decision can also be null: until
+     * {@link ActivationProcessor#processProjectionsActivation(LensContext, String, XMLGregorianCalendar, Task, OperationResult)}
+     * is started - e.g. during the whole focus projection! See also MID-8569.
+     */
     public abstract boolean isAdd();
 
+    /**
+     * See also limitations for {@link #isAdd()}.
+     */
     public abstract boolean isDelete();
 
     /**
      * TODO description
+     *
+     * See also limitations for {@link #isAdd()}.
      */
     public boolean isModify() {
         return ObjectDelta.isModify(getCurrentDelta());

diff --git a/.../main/java/com/evolveum/midpoint/model/impl/lens/construction/ConstructionEvaluation.java b/.../main/java/com/evolveum/midpoint/model/impl/lens/construction/ConstructionEvaluation.java
@@ -74,7 +74,8 @@ class ConstructionEvaluation<AH extends AssignmentHolderType, ROC extends Resour
      */
     private boolean evaluated;
 
-    public ConstructionEvaluation(@NotNull EvaluatedResourceObjectConstructionImpl<AH, ROC> evaluatedConstruction,
+    ConstructionEvaluation(
+            @NotNull EvaluatedResourceObjectConstructionImpl<AH, ROC> evaluatedConstruction,
             @NotNull Task task, @NotNull OperationResult result) {
         this.evaluatedConstruction = evaluatedConstruction;
         this.construction = evaluatedConstruction.getConstruction();

diff --git a/...olveum/midpoint/model/impl/lens/construction/EvaluatedResourceObjectConstructionImpl.java b/...olveum/midpoint/model/impl/lens/construction/EvaluatedResourceObjectConstructionImpl.java
@@ -188,7 +188,7 @@ public String toString() {
                 "discriminator=" + rsd +
                 ", construction=" + construction +
                 ", projectionContext='" + projectionContext +
-                ')';
+                "')";
     }
     //endregion
 
@@ -249,6 +249,7 @@ public NextRecompute evaluate(Task task, OperationResult parentResult) throws Co
                 result.recordNotApplicable();
                 return null;
             } else {
+                LOGGER.trace("Starting evaluation of {}", this);
                 evaluation = new ConstructionEvaluation<>(this, task, result);
                 evaluation.evaluate();
                 return evaluation.getNextRecompute();
@@ -263,6 +264,8 @@ public NextRecompute evaluate(Task task, OperationResult parentResult) throws Co
 
     /**
      * Sets up the projection context. The implementation differs for assigned and plain constructions.
+     *
+     * It is sometimes possible that the projection context does not exist yet.
      */
     protected abstract void initializeProjectionContext();
 
@@ -285,22 +288,36 @@ public NextRecompute evaluate(Task task, OperationResult parentResult) throws Co
      */
     String getFullShadowLoadReason(MappingType outboundMappingBean) {
         if (projectionContext == null) {
+            LOGGER.trace("We will not load full shadow, because we have no projection context");
             return null;
         }
         if (projectionContext.isFullShadow()) {
+            LOGGER.trace("We will not load full shadow, because we already have one");
             return null;
         }
         if (projectionContext.isDelete()) {
+            LOGGER.trace("We will not load full shadow, because the context is being deleted");
+            return null;
+        }
+        if (projectionContext.getOid() == null) {
+            // Normally, this is checked in the context loader along with "isAdd" condition.
+            // However, in some situations (before the projection activation is run), we don't have enough information
+            // to evaluate "isAdd" condition. This approach is the most safe.
+            LOGGER.trace("We will not load full shadow, because we don't have shadow OID (yet?)");
             return null;
         }
         MappingStrengthType strength = outboundMappingBean.getStrength();
         if (strength == MappingStrengthType.STRONG) {
+            LOGGER.trace("We will load full shadow, because of strong outbound mapping");
             return "strong outbound mapping";
         } else if (strength == MappingStrengthType.WEAK) {
+            LOGGER.trace("We will load full shadow, because of weak outbound mapping");
             return "weak outbound mapping";
         } else if (outboundMappingBean.getTarget() != null && outboundMappingBean.getTarget().getSet() != null) {
+            LOGGER.trace("We will load full shadow, because of outbound mapping with target set specified");
             return "outbound mapping target set specified";
         } else {
+            LOGGER.trace("We will not load full shadow, because we don't have any specific reason to do so now");
             return null;
         }
     }

diff --git a/...impl/src/main/java/com/evolveum/midpoint/model/impl/lens/construction/ItemEvaluation.java b/...impl/src/main/java/com/evolveum/midpoint/model/impl/lens/construction/ItemEvaluation.java
@@ -123,6 +123,8 @@ public void evaluate() throws CommunicationException, ObjectNotFoundException, S
             SecurityViolationException, ConfigurationException, ExpressionEvaluationException {
         checkNotYetEvaluated();
 
+        LOGGER.trace("Starting evaluation of (item-level) {}", this);
+
         constructionEvaluation.loadFullShadowIfNeeded(this);
 
         try {
@@ -144,7 +146,7 @@ public void evaluate() throws CommunicationException, ObjectNotFoundException, S
         }
     }
 
-    protected void checkNotYetEvaluated() {
+    private void checkNotYetEvaluated() {
         if (evaluatedMapping != null) {
             throw new IllegalStateException();
         }
@@ -298,4 +300,11 @@ private String getTypedItemName() {
      */
     protected abstract String getItemType();
 
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + "{" +
+                "itemName=" + itemName +
+                ", itemPath=" + itemPath +
+                '}';
+    }
 }
diff --git a/...pl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/ActivationProcessor.java b/...pl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/ActivationProcessor.java
@@ -387,7 +387,8 @@ private <F extends FocusType> void processActivationMappingsCurrent(LensContext<
         }
     }
 
-    private void setSynchronizationPolicyDecision(LensProjectionContext projCtx, SynchronizationPolicyDecision decision, OperationResult result) {
+    private void setSynchronizationPolicyDecision(
+            LensProjectionContext projCtx, SynchronizationPolicyDecision decision, OperationResult result) {
         projCtx.setSynchronizationPolicyDecision(decision);
         result.addReturn("decision", String.valueOf(decision));
     }
@@ -742,15 +743,17 @@ private <T, F extends FocusType> void evaluateOutboundMapping(final LensContext<
                 DebugUtil.debugDumpLazily(outputTripleMap, 1));
 
         if (projCtx.isDoReconciliation()) {
-            reconcileOutboundValue(context, projCtx, outputTripleMap, desc);
+            reconcileOutboundValue(projCtx, outputTripleMap, desc);
         }
     }
 
     /**
      * TODO: can we align this with ReconciliationProcessor?
      */
-    private <T, F extends FocusType> void reconcileOutboundValue(LensContext<F> context, LensProjectionContext projCtx,
-            Map<UniformItemPath, MappingOutputStruct<PrismPropertyValue<T>>> outputTripleMap, String desc) throws SchemaException {
+    private <T> void reconcileOutboundValue(
+            LensProjectionContext projCtx,
+            Map<UniformItemPath, MappingOutputStruct<PrismPropertyValue<T>>> outputTripleMap,
+            String desc) throws SchemaException {
 
         // TODO: check for full shadow?
 

diff --git a/...a/com/evolveum/midpoint/model/impl/lens/projector/loader/ProjectionFullLoadOperation.java b/...a/com/evolveum/midpoint/model/impl/lens/projector/loader/ProjectionFullLoadOperation.java
@@ -87,8 +87,6 @@ public void loadFullShadow(OperationResult parentResult)
         createTraceIfNeeded(result);
 
         try {
-            String oid = projCtx.getOid();
-
             if (projCtx.isHigherOrder()) {
                 // It may be just too early to load the projection
                 if (LensUtil.hasLowerOrderContext(context, projCtx) && context.getExecutionWave() < projCtx.getWave()) {
@@ -99,6 +97,7 @@ public void loadFullShadow(OperationResult parentResult)
             }
 
             Collection<SelectorOptions<GetOperationOptions>> options = createOptions();
+            String oid = projCtx.getOid();
             try {
                 if (oid == null) {
                     throw new IllegalStateException("Trying to load shadow with null OID (reason for load: " + reason + ") for "

diff --git a/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestIteration.java b/model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestIteration.java
@@ -13,6 +13,10 @@
 import java.util.Collection;
 import java.util.List;
 
+import com.evolveum.midpoint.test.DummyTestResource;
+
+import com.evolveum.midpoint.test.TestResource;
+
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.annotation.DirtiesContext.ClassMode;
 import org.springframework.test.context.ContextConfiguration;
@@ -99,6 +103,10 @@ public class TestIteration extends AbstractInitializedModelIntegrationTest {
     protected static final File USER_TEMPLATE_ITERATION_ASSOCIATE_FILE = new File(TEST_DIR, "user-template-iteration-associate.xml");
     protected static final String USER_TEMPLATE_ITERATION_ASSOCIATE_OID = "c0ee8964-0d2a-45d5-8a8e-6ee4f31e1c12";
 
+    // Used e.g. in test910
+    private static final TestResource<ObjectTemplateType> USER_TEMPLATE_SIMPLE_ITERATION = new TestResource<>(
+            TEST_DIR, "user-template-simple-iteration.xml", "1f104a6b-f7f1-4a69-a68b-c71fa2f9f1ac");
+
     private static final String USER_ANGELICA_NAME = "angelica";
     private static final String ACCOUNT_SPARROW_NAME = "sparrow";
 
@@ -149,7 +157,15 @@ public class TestIteration extends AbstractInitializedModelIntegrationTest {
 
     private static final String EMAIL_SUFFIX = "@example.com";
 
-    protected String jupiterUserOid;
+    private static final DummyTestResource RESOURCE_DUMMY_ASSOCIATIONS = new DummyTestResource(
+            TEST_DIR, "resource-dummy-associations.xml", "64ae70db-2b2c-418e-b2bd-d167a28cfbd3",
+            "associations");
+    private static final TestResource<RoleType> METAROLE_DUMMY_ASSOCIATIONS = new TestResource<>(
+            TEST_DIR, "metarole-dummy-associations.xml", "fc07a007-fdd4-44d3-99cf-d57b4df9509d");
+    private static final TestResource<RoleType> ROLE_CS_101 = new TestResource<>(
+            TEST_DIR, "role-cs-101.xml", "4e8170d7-31e6-4c5f-b3c6-ad9f7c4bbb62");
+
+    private String jupiterUserOid;
 
     String iterationTokenDiplomatico;
     String iterationTokenMillonario;
@@ -184,6 +200,12 @@ public void initSystem(Task initTask, OperationResult initResult) throws Excepti
         addObject(USER_LARGO_FILE);
 
         assumeAssignmentPolicy(AssignmentPolicyEnforcementType.RELATIVE);
+
+        addObject(USER_TEMPLATE_SIMPLE_ITERATION, initTask, initResult);
+
+        initAndTestDummyResource(RESOURCE_DUMMY_ASSOCIATIONS, initTask, initResult);
+        addObject(METAROLE_DUMMY_ASSOCIATIONS, initTask, initResult);
+        addObject(ROLE_CS_101, initTask, initResult);
     }
 
     /**
@@ -2208,4 +2230,45 @@ public void test900SkippingConstraintsCheckForCaseType() throws Exception {
         dummyAuditService.assertHasDelta(ChangeType.MODIFY, CaseType.class);
         dummyAuditService.assertExecutionSuccess();
     }
+
+    /** Simply adding (conflicting) user with an association. MID-8569. */
+    @Test
+    public void test910AddUserWithAssociation() throws Exception {
+        Task task = getTestTask();
+        OperationResult result = task.getResult();
+
+        setDefaultUserTemplate(USER_TEMPLATE_SIMPLE_ITERATION.oid);
+        try {
+            given("existing user 'Joe Smith' with CS-101 assignment");
+            UserType existingJoe = new UserType()
+                    .givenName("Joe")
+                    .familyName("Smith")
+                    .assignment(ROLE_CS_101.assignmentTo());
+            addObject(existingJoe.asPrismObject(), task, result);
+            assertUserWithAssociation("smith1");
+
+            when("conflicting 'John Smith' with CS-101 assignment is added");
+            UserType newJoe = new UserType()
+                    .givenName("John")
+                    .familyName("Smith")
+                    .assignment(ROLE_CS_101.assignmentTo());
+            addObject(newJoe.asPrismObject(), task, result);
+
+            then("second user is created");
+            assertUserWithAssociation("smith2");
+        } finally {
+            setDefaultUserTemplate(null);
+        }
+    }
+
+    private void assertUserWithAssociation(String name) throws Exception {
+        String shadowOid = assertUserAfterByUsername(name)
+                .links()
+                .singleLive()
+                .getOid();
+
+        assertShadow(getShadowModel(shadowOid), "after")
+                .associations()
+                .assertSize(1);
+    }
 }
diff --git a/model/model-intest/src/test/resources/iteration/metarole-dummy-associations.xml b/model/model-intest/src/test/resources/iteration/metarole-dummy-associations.xml
@@ -0,0 +1,42 @@
+<!--
+  ~ Copyright (C) 2010-2023 Evolveum and contributors
+  ~
+  ~ This work is dual-licensed under the Apache License 2.0
+  ~ and European Union Public License. See LICENSE file for details.
+  -->
+
+<role xmlns="http://midpoint.evolveum.com/xml/ns/public/common/common-3"
+        xmlns:ri="http://midpoint.evolveum.com/xml/ns/public/resource/instance-3"
+        oid="fc07a007-fdd4-44d3-99cf-d57b4df9509d">
+    <name>metarole-dummy-associations</name>
+    <inducement>
+        <construction>
+            <resourceRef oid="64ae70db-2b2c-418e-b2bd-d167a28cfbd3"/>
+            <kind>entitlement</kind>
+            <intent>group</intent>
+        </construction>
+    </inducement>
+    <inducement>
+        <construction>
+            <resourceRef oid="64ae70db-2b2c-418e-b2bd-d167a28cfbd3"/>
+            <kind>account</kind>
+            <intent>default</intent>
+            <association>
+                <ref>ri:group</ref>
+                <outbound>
+                    <strength>strong</strength>
+                    <expression>
+                        <associationFromLink>
+                            <projectionDiscriminator>
+                                <kind>entitlement</kind>
+                                <intent>group</intent>
+                            </projectionDiscriminator>
+                        </associationFromLink>
+                    </expression>
+                </outbound>
+            </association>
+        </construction>
+        <order>2</order>
+        <focusType>UserType</focusType>
+    </inducement>
+</role>