Avoid the need to clone outputs in scripts

PrismValueDeltaSetTriple cannot contain values that have parents. So, midPoint will now clone expression output values that are parts of larger structures (i.e., they have a parent).
Evolveum · Sep 9, 2022 · 2e7286a · 2e7286a
1 parent 1cbad10
commit 2e7286a
Show file tree

Hide file tree

Showing 11 changed files with 80 additions and 59 deletions.
diff --git a/...um/midpoint/model/common/expression/evaluator/transformation/CombinatorialEvaluation.java b/...um/midpoint/model/common/expression/evaluator/transformation/CombinatorialEvaluation.java
@@ -33,7 +33,8 @@
  * Each combinations of values from sources is evaluated separately and the resulting values
  * are sorted out into plus-minus-zero sets.
  */
-class CombinatorialEvaluation<V extends PrismValue, D extends ItemDefinition, E extends TransformExpressionEvaluatorType> extends TransformationalEvaluation<V, D, E>  {
+class CombinatorialEvaluation<V extends PrismValue, D extends ItemDefinition<?>, E extends TransformExpressionEvaluatorType>
+        extends TransformationalEvaluation<V, D, E>  {
 
     private static final Trace LOGGER = TraceManager.getTrace(CombinatorialEvaluation.class);
 
@@ -235,7 +236,7 @@ private List<SourceTriple<?,?>> createSourceTriplesList() throws SchemaException
         Collection<Source<?, ?>> sources = context.getSources();
         List<SourceTriple<?,?>> sourceTriples = new ArrayList<>(sources.size());
         for (Source<?,?> source: sources) {
-            //noinspection unchecked
+            //noinspection unchecked,rawtypes
             sourceTriples.add(
                     createSourceTriple((Source) source));
         }
@@ -253,7 +254,7 @@ private SourceTriple<V, D> createSourceTriple(Source<V, D> source) throws Schema
             }
         }
         if (evaluator.isIncludeNullInputs()) {
-            addFakeNullValues(sourceTriple, source);
+            addFakeNullValues(sourceTriple);
         }
         LOGGER.trace("Processed source {} triple\n{}", source.getName().getLocalPart(), sourceTriple.debugDumpLazily(1));
         return sourceTriple;
@@ -265,7 +266,7 @@ private SourceTriple<V, D> createSourceTriple(Source<V, D> source) throws Schema
      *
      * TODO Verify the correctness of this algorithm.
      */
-    private void addFakeNullValues(SourceTriple<V, D> sourceTriple, Source<V, D> source) {
+    private void addFakeNullValues(SourceTriple<V, D> sourceTriple) {
         if (sourceTriple.hasZeroSet()) {
             // This is good. Because we have zero set, it will be applied both for non-positive
             // and non-negative computations. Nothing has to be done.

diff --git a/...lveum/midpoint/model/common/expression/evaluator/transformation/SingleShotEvaluation.java b/...lveum/midpoint/model/common/expression/evaluator/transformation/SingleShotEvaluation.java
@@ -7,6 +7,12 @@
 
 package com.evolveum.midpoint.model.common.expression.evaluator.transformation;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+
+import org.jetbrains.annotations.NotNull;
+
 import com.evolveum.midpoint.prism.Item;
 import com.evolveum.midpoint.prism.ItemDefinition;
 import com.evolveum.midpoint.prism.PrismPropertyValue;
@@ -16,31 +22,22 @@
 import com.evolveum.midpoint.prism.path.ItemPath;
 import com.evolveum.midpoint.prism.polystring.PolyString;
 import com.evolveum.midpoint.repo.common.expression.ExpressionEvaluationContext;
-import com.evolveum.midpoint.repo.common.expression.ExpressionSyntaxException;
-import com.evolveum.midpoint.schema.expression.VariablesMap;
 import com.evolveum.midpoint.repo.common.expression.Source;
-import com.evolveum.midpoint.schema.constants.ExpressionConstants;
+import com.evolveum.midpoint.schema.expression.VariablesMap;
 import com.evolveum.midpoint.schema.result.OperationResult;
 import com.evolveum.midpoint.util.exception.*;
 import com.evolveum.midpoint.util.logging.Trace;
 import com.evolveum.midpoint.util.logging.TraceManager;
-import com.evolveum.midpoint.xml.ns._public.common.common_3.ScriptExpressionEvaluatorType;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.TransformExpressionEvaluatorType;
-
 import com.evolveum.midpoint.xml.ns._public.common.common_3.ValueTransformationEvaluationModeType;
 
-import org.jetbrains.annotations.NotNull;
-
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.List;
-
 /**
  * Application of expression evaluator in "single shot" ("absolute") mode:
  * (1) If there's no delta, the expression is evaluated once. All the results go into the zero set.
  * (2) If there's a delta, expression is evaluated twice (for old and for new values). The result is computed as the difference of the results.
  */
-class SingleShotEvaluation<V extends PrismValue, D extends ItemDefinition, E extends TransformExpressionEvaluatorType> extends TransformationalEvaluation<V, D, E> {
+class SingleShotEvaluation<V extends PrismValue, D extends ItemDefinition<?>, E extends TransformExpressionEvaluatorType>
+        extends TransformationalEvaluation<V, D, E> {
 
     private static final Trace LOGGER = TraceManager.getTrace(SingleShotEvaluation.class);
 
@@ -136,17 +133,6 @@ private void addSourcesToStaticVariables(VariablesMap scriptVariables, boolean u
         }
     }
 
-    private String getSourceName(Source<?, ?> source) throws ExpressionSyntaxException {
-        String explicitName = source.getName().getLocalPart();
-        if (explicitName != null) {
-            return explicitName;
-        } else if (context.getSources().size() == 1) {
-            return ExpressionConstants.VAR_INPUT;
-        } else {
-            throw new ExpressionSyntaxException("No name definition for source in "+context.getContextDescription());
-        }
-    }
-
     private Object getRealContent(Item<?,?> item, ItemPath residualPath) {
         if (residualPath == null || residualPath.isEmpty()) {
             return item;

diff --git a/...midpoint/model/common/expression/evaluator/transformation/TransformationalEvaluation.java b/...midpoint/model/common/expression/evaluator/transformation/TransformationalEvaluation.java
@@ -25,9 +25,6 @@
 
 import javax.xml.bind.JAXBElement;
 
-/**
- *
- */
 class TransformationalEvaluation<V extends PrismValue, D extends ItemDefinition, E extends TransformExpressionEvaluatorType> {
 
     @NotNull final ExpressionEvaluationContext context;

diff --git a/...m/midpoint/model/common/expression/evaluator/transformation/ValueTupleTransformation.java b/...m/midpoint/model/common/expression/evaluator/transformation/ValueTupleTransformation.java
@@ -133,7 +133,7 @@ class ValueTupleTransformation<V extends PrismValue> implements AutoCloseable {
                 .build();
         result.addParam("context", context.getContextDescription());
         if (result.isTracingNormal(ValueTransformationTraceType.class)) {
-            trace = new ValueTransformationTraceType(combinatorialEvaluation.prismContext);
+            trace = new ValueTransformationTraceType();
             result.getTraces().add(trace);
             dumpValueCombinationToTrace();
         } else {

diff --git a/.../com/evolveum/midpoint/model/common/expression/script/AbstractCachingScriptEvaluator.java b/.../com/evolveum/midpoint/model/common/expression/script/AbstractCachingScriptEvaluator.java
@@ -108,17 +108,23 @@ public <T, V extends PrismValue> List<V> evaluate(ScriptExpressionEvaluationCont
         // PrismReference? Shouldn't they be processed in the same way as
         // PrismProperty?
         if (evalRawResult instanceof Collection) {
+            //noinspection rawtypes
             for (Object evalRawResultElement : (Collection) evalRawResult) {
                 T evalResult = convertScalarResult(javaReturnType, evalRawResultElement, context);
-                values.add(ExpressionUtil.convertToPrismValue(
-                        evalResult, context.getOutputDefinition(), context.getContextDescription(), getPrismContext()));
+                values.add(
+                        ExpressionUtil.convertToPrismValue(
+                                evalResult, context.getOutputDefinition(), context.getContextDescription()));
             }
         } else if (evalRawResult instanceof PrismProperty<?>) {
-            values.addAll((Collection<? extends V>) PrismValueCollectionsUtil.cloneCollection(((PrismProperty<T>) evalRawResult).getValues()));
+            //noinspection unchecked
+            values.addAll(
+                    (Collection<? extends V>) PrismValueCollectionsUtil.cloneCollection(
+                            ((PrismProperty<T>) evalRawResult).getValues()));
         } else {
             T evalResult = convertScalarResult(javaReturnType, evalRawResult, context);
-            values.add(ExpressionUtil.convertToPrismValue(
-                    evalResult, context.getOutputDefinition(), context.getContextDescription(), getPrismContext()));
+            values.add(
+                    ExpressionUtil.convertToPrismValue(
+                            evalResult, context.getOutputDefinition(), context.getContextDescription()));
         }
 
         return values;

diff --git a/.../src/main/java/com/evolveum/midpoint/model/common/expression/script/ScriptExpression.java b/.../src/main/java/com/evolveum/midpoint/model/common/expression/script/ScriptExpression.java
@@ -161,7 +161,8 @@ public <V extends PrismValue> List<V> evaluate(ScriptExpressionEvaluationContext
 
             List<V> expressionResult = evaluator.evaluate(context);
             if (context.getTrace() != null) {
-                context.getTrace().getResult().addAll(TraceUtil.toAnyValueTypeList(expressionResult, prismContext));
+                context.getTrace().getResult().addAll(
+                        TraceUtil.toAnyValueTypeList(expressionResult, prismContext));
             }
 
             traceExpressionSuccess(context, expressionResult);

diff --git a/...om/evolveum/midpoint/model/common/expression/script/velocity/VelocityScriptEvaluator.java b/...om/evolveum/midpoint/model/common/expression/script/velocity/VelocityScriptEvaluator.java
@@ -89,8 +89,9 @@ public <T, V extends PrismValue> List<V> evaluate(ScriptExpressionEvaluationCont
         }
 
         List<V> values = new ArrayList<>();
-        values.add(ExpressionUtil.convertToPrismValue(
-                evalResult, context.getOutputDefinition(), context.getContextDescription(), getPrismContext()));
+        values.add(
+                ExpressionUtil.convertToPrismValue(
+                        evalResult, context.getOutputDefinition(), context.getContextDescription()));
         return values;
     }
 

diff --git a/model/model-impl/src/test/resources/expr/function-library.xml b/model/model-impl/src/test/resources/expr/function-library.xml
@@ -51,8 +51,6 @@
         <returnType>xsd:anyType</returnType> <!-- Returns PrismContainerValue -->
         <script>
             <code>
-                import com.evolveum.prism.xml.ns._public.types_3.ItemType
-
                 log.info('container = {} ({})', container, container?.class)
                 log.info('containerValue = {} ({})', containerValue, containerValue?.class)
                 container?.get(0)

diff --git a/model/model-intest/src/test/resources/identities/object-template-person.xml b/model/model-intest/src/test/resources/identities/object-template-person.xml
@@ -97,10 +97,9 @@
                                 .findAll { it.resourceRef?.oid == TestIdentities.RESOURCE_MULTI.oid }
 
                         if (single != null) {
-                            single.clone()
+                            single
                         } else if (!multi.isEmpty()) {
                             multi.min { it.tag }
-                                .clone()
                         } else {
                             null
                         }

diff --git a/repo/repo-common/src/main/java/com/evolveum/midpoint/repo/common/expression/Expression.java b/repo/repo-common/src/main/java/com/evolveum/midpoint/repo/common/expression/Expression.java
@@ -205,7 +205,8 @@ private PrismValueDeltaSetTriple<V> evaluateExpressionEvaluators(ExpressionEvalu
             processEvaluatorProfile(contextWithProcessedVariables, evaluator);
 
             //noinspection unchecked
-            PrismValueDeltaSetTriple<V> outputTriple = (PrismValueDeltaSetTriple<V>) evaluator.evaluate(contextWithProcessedVariables, result);
+            PrismValueDeltaSetTriple<V> outputTriple =
+                    (PrismValueDeltaSetTriple<V>) evaluator.evaluate(contextWithProcessedVariables, result);
 
             if (outputTriple != null) {
                 boolean allowEmptyRealValues = false;

diff --git a/...epo-common/src/main/java/com/evolveum/midpoint/repo/common/expression/ExpressionUtil.java b/...epo-common/src/main/java/com/evolveum/midpoint/repo/common/expression/ExpressionUtil.java
@@ -1161,40 +1161,71 @@ public static <T> boolean isEmpty(T val) {
         return false;
     }
 
-    public static <T, V extends PrismValue> V convertToPrismValue(T value, ItemDefinition definition, String contextDescription, PrismContext prismContext) throws ExpressionEvaluationException {
-        if (value == null) {
+    /**
+     * Guaranteed to return _detached_ (i.e., parent-less) prism value.
+     */
+    public static <T, V extends PrismValue> V convertToPrismValue(
+            T realValue, ItemDefinition<?> definition, String contextDescription)
+            throws ExpressionEvaluationException {
+
+        PrismValue prismValue = convertToPrismValueInternal(realValue, definition, contextDescription);
+        if (prismValue != null && prismValue.getParent() != null) {
+            //noinspection unchecked
+            return (V) prismValue.clone();
+        } else {
+            //noinspection unchecked
+            return (V) prismValue;
+        }
+    }
+
+    private static PrismValue convertToPrismValueInternal(
+            Object object, ItemDefinition<?> definition, String contextDescription)
+            throws ExpressionEvaluationException {
+        if (object == null) {
             return null;
         }
 
         if (definition instanceof PrismReferenceDefinition) {
-            return (V) ((ObjectReferenceType) value).asReferenceValue();
+
+            if (object instanceof Referencable) {
+                return ((Referencable) object).asReferenceValue();
+            } else if (object instanceof PrismReferenceValue) {
+                return (PrismReferenceValue) object;
+            } else {
+                throw new ExpressionEvaluationException(
+                        "Expected Referencable or PrismReferenceValue as expression output, got " + object.getClass());
+            }
 
         } else if (definition instanceof PrismContainerDefinition) {
 
-            if (value instanceof Containerable) {
+            if (object instanceof Containerable) {
                 try {
-                    prismContext.adopt((Containerable) value);
-                    ((Containerable) value).asPrismContainerValue().applyDefinition(definition);
+                    PrismContext.get().adopt((Containerable) object);
+                    ((Containerable) object).asPrismContainerValue().applyDefinition(definition);
                 } catch (SchemaException e) {
                     throw new ExpressionEvaluationException(e.getMessage() + " " + contextDescription, e);
                 }
-                return (V) ((Containerable) value).asPrismContainerValue();
-
-            } else if (value instanceof PrismContainerValue<?>) {
+                return ((Containerable) object).asPrismContainerValue();
+            } else if (object instanceof PrismContainerValue<?>) {
                 try {
-                    prismContext.adopt((PrismContainerValue<Containerable>) value);
-                    ((PrismContainerValue) value).applyDefinition(definition);
+                    PrismContext.get().adopt((PrismContainerValue<?>) object);
+                    ((PrismContainerValue<?>) object).applyDefinition(definition);
                 } catch (SchemaException e) {
                     throw new ExpressionEvaluationException(e.getMessage() + " " + contextDescription, e);
                 }
-                return (V) value;
-
+                return (PrismContainerValue<?>) object;
             } else {
-                throw new ExpressionEvaluationException("Expected Containerable or PrismContainerValue as expression output, got " + value.getClass());
+                throw new ExpressionEvaluationException(
+                        "Expected Containerable or PrismContainerValue as expression output, got " + object.getClass());
             }
 
         } else {
-            return (V) prismContext.itemFactory().createPropertyValue(value);
+
+            // This is really ugly hack. In ideal world, we should check if the object is not a PrismValue, and return
+            // it without wrapping in PPV. However, this would break some cases when we have no definition (see the
+            // implementation of MID-6775 in bd580662a15772e3a7addc17fe49f3479e5c3589).
+            return PrismContext.get().itemFactory().createPropertyValue(object);
+
         }
     }