Add "push changes" option

Since 4.2 the expression evaluation algorithm filters out phantom changes. But these are sometimes necessary. So we introduced a special execution option that requests the projector to "push" even phantom changes through. This resolves MID-6420. Beware: experimental/preliminary implementation.
Evolveum · Sep 14, 2020 · 8a66052 · 8a66052
1 parent 42684b3
commit 8a66052
Show file tree

Hide file tree

Showing 18 changed files with 192 additions and 90 deletions.
diff --git a/...n/java/com/evolveum/midpoint/schema/traces/operations/ValueTupleTransformationOpNode.java b/...n/java/com/evolveum/midpoint/schema/traces/operations/ValueTupleTransformationOpNode.java
@@ -38,16 +38,10 @@ public String getValueTupleTransformationDescription() {
                 sb.append("for ").append(trace.getLocalContextDescription()).append(" ");
             }
             sb.append("(");
-            if (Boolean.TRUE.equals(trace.isHasPlus())) {
-                sb.append("+");
+            if (trace.getInputOrigin() != null) {
+                sb.append(trace.getInputOrigin()).append(" → ");
             }
-            if (Boolean.TRUE.equals(trace.isHasMinus())) {
-                sb.append("-");
-            }
-            if (Boolean.TRUE.equals(trace.isHasZero())) {
-                sb.append("0");
-            }
-            sb.append(" → ").append(trace.getDestination());
+            sb.append(trace.getDestination());
             sb.append(")");
             if (Boolean.FALSE.equals(trace.isConditionResult())) {
                 sb.append(" [cond: false]");

diff --git a/infra/schema/src/main/resources/xml/ns/public/common/common-core-3.xsd b/infra/schema/src/main/resources/xml/ns/public/common/common-core-3.xsd
@@ -22885,6 +22885,19 @@
                     </xsd:documentation>
                 </xsd:annotation>
             </xsd:element>
+            <xsd:element name="pushChanges" type="xsd:boolean" minOccurs="0">
+                <xsd:annotation>
+                    <xsd:documentation>
+                        "Pushes" changes even if they are phantom. For example, when disabling already disabled user with this
+                        option, all related administrativeStatus-sourced mappings are evaluated even if they would be normally
+                        skipped. This restores behavior of midPoint before 4.2.
+                    </xsd:documentation>
+                    <xsd:appinfo>
+                        <a:since>4.2</a:since>
+                        <a:experimental>true</a:experimental>
+                    </xsd:appinfo>
+                </xsd:annotation>
+            </xsd:element>
             <xsd:element name="raw" type="xsd:boolean" minOccurs="0">
                 <xsd:annotation>
                     <xsd:documentation>

diff --git a/infra/schema/src/main/resources/xml/ns/public/common/common-model-context-3.xsd b/infra/schema/src/main/resources/xml/ns/public/common/common-model-context-3.xsd
@@ -1428,6 +1428,13 @@
                     <xsd:element name="conditionResultNew" type="xsd:boolean" minOccurs="0"/>
                     <xsd:element name="implicitSourcePath" type="t:ItemPathType" minOccurs="0"/>
                     <xsd:element name="implicitTargetPath" type="t:ItemPathType" minOccurs="0"/>
+                    <xsd:element name="pushChangesRequested" type="xsd:boolean" minOccurs="0">
+                        <xsd:annotation>
+                            <xsd:appinfo>
+                                <a:since>4.2</a:since>
+                            </xsd:appinfo>
+                        </xsd:annotation>
+                    </xsd:element>
                     <xsd:element name="source" type="tns:MappingSourceEvaluationTraceType" minOccurs="0" maxOccurs="unbounded">
                         <xsd:annotation>
                             <xsd:appinfo>
@@ -1454,6 +1461,13 @@
                             </xsd:appinfo>
                         </xsd:annotation>
                     </xsd:element>
+                    <xsd:element name="pushChanges" type="xsd:boolean" minOccurs="0">
+                        <xsd:annotation>
+                            <xsd:appinfo>
+                                <a:since>4.2</a:since>
+                            </xsd:appinfo>
+                        </xsd:annotation>
+                    </xsd:element>
                     <xsd:element name="textTrace" type="xsd:string" minOccurs="0"/>
                 </xsd:sequence>
             </xsd:extension>
@@ -1757,33 +1771,13 @@
                             </xsd:documentation>
                         </xsd:annotation>
                     </xsd:element>
-                    <xsd:element name="hasPlus" type="xsd:boolean" minOccurs="0">
-                        <xsd:annotation>
-                            <xsd:documentation>
-                                Does this tuple contain a value that is present in plus set of its source?
-                                (This is an internal implementation information. So this item can change in later versions.)
-                            </xsd:documentation>
-                            <xsd:appinfo>
-                                <a:since>4.2</a:since>
-                            </xsd:appinfo>
-                        </xsd:annotation>
-                    </xsd:element>
-                    <xsd:element name="hasMinus" type="xsd:boolean" minOccurs="0">
-                        <xsd:annotation>
-                            <xsd:documentation>
-                                Does this tuple contain a value that is present in minus set of its source?
-                                (This is an internal implementation information. So this item can change in later versions.)
-                            </xsd:documentation>
-                            <xsd:appinfo>
-                                <a:since>4.2</a:since>
-                            </xsd:appinfo>
-                        </xsd:annotation>
-                    </xsd:element>
-                    <xsd:element name="hasZero" type="xsd:boolean" minOccurs="0">
+                    <xsd:element name="inputOrigin" type="xsd:string" minOccurs="0">
                         <xsd:annotation>
                             <xsd:documentation>
-                                Does this tuple contain a value that is present in zero set of its source?
-                                (This is an internal implementation information. So this item can change in later versions.)
+                                String representation of the input origin.
+                                P means plus, M means minus, Z means zero for each input in question.
+                                (This awkward form was chosen because multivalued properties in containers
+                                have no guaranteed value ordering.)
                             </xsd:documentation>
                             <xsd:appinfo>
                                 <a:since>4.2</a:since>

diff --git a/model/model-api/src/main/java/com/evolveum/midpoint/model/api/ModelExecuteOptions.java b/model/model-api/src/main/java/com/evolveum/midpoint/model/api/ModelExecuteOptions.java
@@ -128,6 +128,23 @@ public static boolean isForce(ModelExecuteOptions options) {
         return is(options, ModelExecuteOptionsType.F_FORCE);
     }
 
+    public Boolean getPushChanges() {
+        return content.isPushChanges();
+    }
+
+    public static boolean isPushChanges(ModelExecuteOptions options) {
+        return is(options, F_PUSH_CHANGES);
+    }
+
+    public ModelExecuteOptions pushChanges(Boolean value) {
+        content.setPushChanges(value);
+        return this;
+    }
+
+    public ModelExecuteOptions pushChanges() {
+        return pushChanges(true);
+    }
+
     public Boolean getRaw() {
         return content.isRaw();
     }
@@ -414,6 +431,9 @@ public static ModelExecuteOptions fromRestOptions(List<String> options, PrismCon
             if (ModelExecuteOptionsType.F_FORCE.getLocalPart().equals(option)) {
                 retVal.force(true);
             }
+            if (F_PUSH_CHANGES.getLocalPart().equals(option)) {
+                retVal.pushChanges(true);
+            }
             if (F_NO_CRYPT.getLocalPart().equals(option)) {
                 retVal.noCrypt(true);
             }

diff --git a/...um/midpoint/model/common/expression/evaluator/transformation/CombinatorialEvaluation.java b/...um/midpoint/model/common/expression/evaluator/transformation/CombinatorialEvaluation.java
@@ -145,7 +145,8 @@ private void transform(List<PlusMinusZero> sets, PlusMinusZero outputSet) {
         List<Collection<PrismValue>> domains = createDomainsForSets(sets);
         logDomainsForSets(domains, sets, outputSet);
         MiscUtil.carthesian(domains, valuesTuple -> {
-            try (ValueTupleTransformation<V> valueTupleTransformation = new ValueTupleTransformation<>(valuesTuple, outputSet, this, parentResult)) {
+            try (ValueTupleTransformation<V> valueTupleTransformation =
+                    new ValueTupleTransformation<>(sets, valuesTuple, outputSet, this, parentResult)) {
                 valueTupleTransformation.evaluate();
             }
         });

diff --git a/...m/midpoint/model/common/expression/evaluator/transformation/ValueTupleTransformation.java b/...m/midpoint/model/common/expression/evaluator/transformation/ValueTupleTransformation.java
@@ -12,6 +12,7 @@
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
+import java.util.stream.Collectors;
 
 import com.evolveum.midpoint.prism.ValueMetadata;
 
@@ -64,6 +65,11 @@ class ValueTupleTransformation<V extends PrismValue> implements AutoCloseable {
      */
     @NotNull private final List<SourceTriple<?, ?>> sourceTripleList;
 
+    /**
+     * Sets (plus/minus/zero) from which individual values were selected.
+     */
+    @NotNull private final List<PlusMinusZero> sets;
+
     /**
      * Currently transformed value tuple - one value from every source.
      */
@@ -89,21 +95,6 @@ class ValueTupleTransformation<V extends PrismValue> implements AutoCloseable {
      */
     private final int numberOfSources;
 
-//    /**
-//     * Does this tuple contain a value that is present in plus set of its source?
-//     */
-//    private boolean hasPlus;
-//
-//    /**
-//     * Does this tuple contain a value that is present in minus set of its source?
-//     */
-//    private boolean hasMinus;
-//
-//    /**
-//     * Does this tuple contain a value that is present in zero set of its source?
-//     */
-//    private boolean hasZero;
-
     /**
      * What state (old, new) should be input variables taken from?
      */
@@ -125,11 +116,12 @@ class ValueTupleTransformation<V extends PrismValue> implements AutoCloseable {
      */
     private Collection<V> transformationResult;
 
-    ValueTupleTransformation(List<PrismValue> valuesTuple, PlusMinusZero outputSet, CombinatorialEvaluation<V, ?, ?> combinatorialEvaluation,
-            OperationResult parentResult) {
+    ValueTupleTransformation(@NotNull List<PlusMinusZero> sets, List<PrismValue> valuesTuple, PlusMinusZero outputSet,
+            CombinatorialEvaluation<V, ?, ?> combinatorialEvaluation, OperationResult parentResult) {
         this.combinatorialEvaluation = combinatorialEvaluation;
         this.context = combinatorialEvaluation.context;
         this.sourceTripleList = combinatorialEvaluation.sourceTripleList;
+        this.sets = sets;
         this.valuesTuple = valuesTuple;
         this.outputSet = outputSet;
         this.inputVariableState = InputVariableState.forOutputSet(outputSet);
@@ -387,6 +379,23 @@ private void dumpValueCombinationToTrace() {
             SourceTriple<?, ?> sourceTriple = sourceTriplesIterator.next();
             trace.getInput().add(TraceUtil.toNamedValueType(pval, sourceTriple.getName(), combinatorialEvaluation.prismContext));
         }
+        trace.setInputOrigin(
+                sets.stream()
+                        .map(this::toChar)
+                        .collect(Collectors.joining()));
+    }
+
+    private String toChar(PlusMinusZero set) {
+        switch (set) {
+            case PLUS:
+                return "P";
+            case MINUS:
+                return "M";
+            case ZERO:
+                return "Z";
+            default:
+                throw new AssertionError(set);
+        }
     }
 
     private void recordBeforeTransformation() {
@@ -395,9 +404,6 @@ private void recordBeforeTransformation() {
                 context.isSkipEvaluationPlus(), context.isSkipEvaluationMinus(),
                 outputSet, inputVariableState);
         if (trace != null) {
-//            trace.setHasPlus(hasPlus);
-//            trace.setHasMinus(hasMinus);
-//            trace.setHasZero(hasZero);
             trace.setLocalContextDescription(context.getLocalContextDescription());
         }
     }

diff --git a/...-common/src/main/java/com/evolveum/midpoint/model/common/mapping/AbstractMappingImpl.java b/...-common/src/main/java/com/evolveum/midpoint/model/common/mapping/AbstractMappingImpl.java
@@ -284,6 +284,23 @@ public abstract class AbstractMappingImpl<V extends PrismValue, D extends ItemDe
      */
     private PrismValueDeltaSetTriple<V> outputTriple;
 
+    /**
+     * Whether we were requested to "push" (phantom) changes: source items that have a delta but their
+     * real value has not changed.
+     *
+     * TODO move to "configuration" options and provide via builder.
+     *  (Current way of determining via lens context is more a hack than real solution.)
+     */
+    @Experimental
+    private boolean pushChangesRequested;
+
+    /**
+     * Whether the conditions for pushing the changes at output were fulfilled,
+     * so we instruct the consolidator (or analogous component) to do that.
+     */
+    @Experimental
+    private boolean pushChanges;
+
     /**
      * Result of the condition evaluation in old vs. new state.
      */
@@ -758,8 +775,12 @@ private void recordSources() throws SchemaException {
     }
 
     private void recordOutput() {
-        if (trace != null && outputTriple != null) {
-            trace.setOutput(DeltaSetTripleType.fromDeltaSetTriple(outputTriple, beans.prismContext));
+        if (trace != null) {
+            if (outputTriple != null) {
+                trace.setOutput(DeltaSetTripleType.fromDeltaSetTriple(outputTriple, beans.prismContext));
+            }
+            trace.setPushChangesRequested(pushChangesRequested);
+            trace.setPushChanges(pushChanges);
         }
     }
 
@@ -1269,6 +1290,9 @@ private void evaluateExpression(OperationResult result)
 
         outputTriple = expression.evaluate(context, result);
 
+        pushChangesRequested = determinePushChangesRequested();
+        pushChanges = pushChangesRequested && sourcesChanged();
+
         if (outputTriple == null) {
 
             if (conditionResultNew) {
@@ -1307,6 +1331,8 @@ protected abstract TransformationValueMetadataComputer createValueMetadataComput
             ObjectNotFoundException, SchemaException, SecurityViolationException, ConfigurationException,
             ExpressionEvaluationException;
 
+    protected abstract boolean determinePushChangesRequested();
+
     @Override
     public PrismValueDeltaSetTriple<V> getOutputTriple() {
         if (outputTriple != null && InternalsConfig.consistencyChecks) {
@@ -1488,12 +1514,6 @@ void recordTimeTo(XMLGregorianCalendar timeTo) {
         }
     }
 
-//    // TEMPORARY
-//    List<MetadataMappingType> getMetadataMappings() {
-//        return mappingBean instanceof MappingType ?
-//                ((MappingType) mappingBean).getMetadataMapping() : null;
-//    }
-//
     @NotNull
     public ModelCommonBeans getBeans() {
         return beans;
@@ -1508,4 +1528,10 @@ public List<QName> getSourceNames() {
                 .map(Source::getName)
                 .collect(Collectors.toList());
     }
+
+    @Experimental
+    @Override
+    public boolean isPushChanges() {
+        return pushChanges;
+    }
 }
diff --git a/model/model-common/src/main/java/com/evolveum/midpoint/model/common/mapping/MappingImpl.java b/model/model-common/src/main/java/com/evolveum/midpoint/model/common/mapping/MappingImpl.java
@@ -7,6 +7,9 @@
 
 package com.evolveum.midpoint.model.common.mapping;
 
+import com.evolveum.midpoint.model.api.ModelExecuteOptions;
+import com.evolveum.midpoint.model.api.context.ModelContext;
+import com.evolveum.midpoint.model.common.expression.ModelExpressionThreadLocalHolder;
 import com.evolveum.midpoint.model.common.mapping.metadata.TransformationalMetadataComputation;
 import com.evolveum.midpoint.model.common.mapping.metadata.ItemValueMetadataProcessingSpec;
 import com.evolveum.midpoint.prism.ItemDefinition;
@@ -15,6 +18,7 @@
 import com.evolveum.midpoint.schema.result.OperationResult;
 import com.evolveum.midpoint.util.exception.*;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.MappingType;
+import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.ValueMetadataType;
 
 import org.jetbrains.annotations.NotNull;
@@ -85,4 +89,11 @@ private ItemValueMetadataProcessingSpec createProcessingSpec(OperationResult res
     public MappingImpl<V, D> clone() {
         return new MappingImpl<>(this);
     }
+
+    @Override
+    protected boolean determinePushChangesRequested() {
+        ModelContext<ObjectType> lensContext = ModelExpressionThreadLocalHolder.getLensContext();
+        ModelExecuteOptions options = lensContext != null ? lensContext.getOptions() : null;
+        return ModelExecuteOptions.isPushChanges(options);
+    }
 }
diff --git a/...ain/java/com/evolveum/midpoint/model/common/mapping/PrismValueDeltaSetTripleProducer.java b/...ain/java/com/evolveum/midpoint/model/common/mapping/PrismValueDeltaSetTripleProducer.java
@@ -56,4 +56,6 @@ default boolean isNormal() {
     default boolean isWeak() {
         return getStrength() == MappingStrengthType.WEAK;
     }
+
+    boolean isPushChanges();
 }
diff --git a/...rc/main/java/com/evolveum/midpoint/model/common/mapping/metadata/MetadataMappingImpl.java b/...rc/main/java/com/evolveum/midpoint/model/common/mapping/metadata/MetadataMappingImpl.java
@@ -33,6 +33,11 @@ protected TransformationValueMetadataComputer createValueMetadataComputer(Operat
         return null;
     }
 
+    @Override
+    protected boolean determinePushChangesRequested() {
+        return false;
+    }
+
     @Override
     public MetadataMappingImpl<V, D> clone() {
         return new MetadataMappingImpl<>(this);

diff --git a/...l/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/ItemValueWithOrigin.java b/...l/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/ItemValueWithOrigin.java
@@ -18,6 +18,7 @@
 import com.evolveum.midpoint.prism.delta.PrismValueDeltaSetTriple;
 import com.evolveum.midpoint.util.DebugDumpable;
 import com.evolveum.midpoint.util.DebugUtil;
+import com.evolveum.midpoint.util.annotation.Experimental;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectType;
 import org.jetbrains.annotations.NotNull;
 
@@ -142,4 +143,9 @@ public boolean isWeak() {
     boolean isSourceless() {
         return mapping != null && mapping.isSourceless();
     }
+
+    @Experimental
+    public boolean isPushChanges() {
+        return mapping != null && mapping.isPushChanges();
+    }
 }
diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/IvwoConsolidator.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/IvwoConsolidator.java
@@ -398,7 +398,7 @@ private Collection<I> findAddingOrigins() {
                 addingOrigins.addAll(equivalenceClass.zeroOrigins);
             } else if (addUnchangedValuesExceptForNormalMappings) {
                 for (I zeroIvwo : equivalenceClass.zeroOrigins) {
-                    if (zeroIvwo.isStrong() || zeroIvwo.isNormal() && zeroIvwo.isSourceless() || zeroIvwo.isWeak()) {
+                    if (zeroIvwo.isStrong() || zeroIvwo.isNormal() && (zeroIvwo.isSourceless() || zeroIvwo.isPushChanges()) || zeroIvwo.isWeak()) {
                         addingOrigins.add(zeroIvwo);
                     }
                 }