Skip to content

Commit

Permalink
Fix failing TestFunctions
Browse files Browse the repository at this point in the history
  • Loading branch information
@mederly
mederly committed Apr 17, 2024
1 parent 8707a7b commit 4708741
Show file tree
Hide file tree
Showing 14 changed files with 168 additions and 131 deletions.
93 changes: 40 additions & 53 deletions ...volveum/midpoint/model/common/expression/evaluator/AbstractSearchExpressionEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import javax.xml.namespace.QName;

import com.evolveum.midpoint.model.common.expression.evaluator.caching.AssociationSearchQueryResult;
import com.evolveum.midpoint.model.common.expression.evaluator.transformation.ValueTransformationContext;
import com.evolveum.midpoint.schema.*;

import com.evolveum.midpoint.util.DOMUtil;
Expand Down Expand Up @@ -44,12 +45,10 @@
import com.evolveum.midpoint.prism.query.ObjectQuery;
import com.evolveum.midpoint.prism.util.CloneUtil;
import com.evolveum.midpoint.repo.common.ObjectResolver;
import com.evolveum.midpoint.repo.common.expression.ExpressionEvaluationContext;
import com.evolveum.midpoint.repo.common.expression.ExpressionUtil;
import com.evolveum.midpoint.schema.cache.CacheConfigurationManager;
import com.evolveum.midpoint.schema.cache.CacheType;
import com.evolveum.midpoint.schema.constants.ObjectTypes;
import com.evolveum.midpoint.schema.expression.VariablesMap;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.schema.util.MiscSchemaUtil;
import com.evolveum.midpoint.task.api.Task;
Expand Down Expand Up @@ -106,23 +105,16 @@ public abstract class AbstractSearchExpressionEvaluator<
this.objectResolver = objectResolver;
}

protected @NotNull List<V> transformSingleValue(
@NotNull VariablesMap variables,
boolean useNew,
@NotNull ExpressionEvaluationContext context,
@NotNull OperationResult result)
protected @NotNull List<V> transformSingleValue(@NotNull ValueTransformationContext vtCtx, @NotNull OperationResult result)
throws ExpressionEvaluationException, ObjectNotFoundException, SchemaException,
CommunicationException, ConfigurationException, SecurityViolationException {
return createEvaluation(variables, useNew, context, result)
return createEvaluation(vtCtx, result)
.execute();
}

/** Creates the instance of {@link Evaluation}, which is subclassed for each use of this (outer) class. */
abstract Evaluation createEvaluation(
@NotNull VariablesMap variables,
boolean useNew,
@NotNull ExpressionEvaluationContext context,
@NotNull OperationResult result) throws SchemaException;
abstract Evaluation createEvaluation(@NotNull ValueTransformationContext vtCtx, @NotNull OperationResult result)
throws SchemaException;

// Override the default in this case. It makes more sense like this.
@Override
Expand All @@ -133,14 +125,8 @@ protected boolean isIncludeNullInputs() {
/** The actual evaluation process. It's a separate class to avoid moving all the parameters along. */
protected abstract class Evaluation {

/** Variables to be used when evaluating the expressions (in the query and in "populate" expressions). */
@NotNull private final VariablesMap variables;

/** Are we evaluating the "old" or "new" state of the world? */
private final boolean useNew;

/** The whole evaluation context. */
@NotNull final ExpressionEvaluationContext context;
/** The value transformation operation context. */
@NotNull final ValueTransformationContext vtCtx;

@NotNull private final OperationResult result;

Expand All @@ -151,13 +137,8 @@ protected abstract class Evaluation {
@NotNull final Class<O> targetTypeClass;

protected Evaluation(
@NotNull VariablesMap variables,
boolean useNew,
@NotNull ExpressionEvaluationContext context,
@NotNull OperationResult result) throws SchemaException {
this.variables = variables;
this.useNew = useNew;
this.context = context;
@NotNull ValueTransformationContext vtCtx, @NotNull OperationResult result) throws SchemaException {
this.vtCtx = vtCtx;
this.result = result;

this.targetTypeQName = determineTargetTypeQName();
Expand Down Expand Up @@ -203,7 +184,7 @@ protected QName getDefaultTargetType() {
return List.of(createResultValue(defaultTargetOid, targetTypeQName, null, newValueDeltas));
}

if (Boolean.TRUE.equals(expressionEvaluatorBean.isCreateOnDemand()) && useNew) {
if (Boolean.TRUE.equals(expressionEvaluatorBean.isCreateOnDemand()) && vtCtx.isEvaluateNew()) {
try {
PrismObject<O> createdObject = createOnDemand();
if (createdObject != null) {
Expand All @@ -228,7 +209,7 @@ protected QName getDefaultTargetType() {

private void log(String source, int values) {
LOGGER.trace("Search expression {} (useNew: {}) resolved via {}: returning {} values",
context, useNew, source, values);
vtCtx, vtCtx.isEvaluateNew(), source, values);
}

private @Nullable List<ItemDelta<V, D>> createNewValueDeltas()
Expand All @@ -238,9 +219,9 @@ private void log(String source, int values) {
if (valuePopulationDef != null) {
if (outputDefinition instanceof PrismContainerDefinition<?> pcd) {
return PopulatorUtil.computePopulateItemDeltas(
valuePopulationDef, pcd, variables, context, result);
valuePopulationDef, pcd, vtCtx.getVariablesMap(), vtCtx.getExpressionEvaluationContext(), result);
} else {
LOGGER.warn("Search expression {} applied to non-container target, ignoring populate definition", context);
LOGGER.warn("Search expression {} applied to non-container target, ignoring populate definition", vtCtx);
return null;
}
} else {
Expand All @@ -257,20 +238,21 @@ private void log(String source, int values) {

List<ObjectQuery> queries = new ArrayList<>();
for (var filterBean : filterBeans) {
ObjectQuery rawQuery = createRawQuery(filterBean, context);
ObjectQuery rawQuery = createRawQuery(filterBean);
LOGGER.trace("XML query converted to: {}", rawQuery.debugDumpLazily());

var eeCtx = vtCtx.getExpressionEvaluationContext();
ObjectQuery evaluatedQuery = ExpressionUtil.evaluateQueryExpressions(
rawQuery,
variables,
context.getExpressionProfile(),
context.getExpressionFactory(),
context.getContextDescription(),
vtCtx.getVariablesMap(),
eeCtx.getExpressionProfile(),
eeCtx.getExpressionFactory(),
vtCtx.getContextDescription(),
getTask(),
result);
LOGGER.trace("Query after expressions evaluation: {}", evaluatedQuery.debugDumpLazily());

ObjectQuery extendedQuery = extendQuery(evaluatedQuery, context);
ObjectQuery extendedQuery = extendQuery(evaluatedQuery);
LOGGER.trace("Query after extension: {}", extendedQuery.debugDumpLazily());

queries.add(extendedQuery);
Expand All @@ -279,11 +261,11 @@ private void log(String source, int values) {
return queries;
}

protected ObjectQuery createRawQuery(SearchFilterType filter, ExpressionEvaluationContext params) throws SchemaException, ExpressionEvaluationException {
protected ObjectQuery createRawQuery(SearchFilterType filter) throws SchemaException, ExpressionEvaluationException {
return prismContext.getQueryConverter().createObjectQuery(targetTypeClass, filter);
}

protected ObjectQuery extendQuery(ObjectQuery query, ExpressionEvaluationContext params)
protected ObjectQuery extendQuery(ObjectQuery query)
throws ExpressionEvaluationException {
return query;
}
Expand All @@ -300,6 +282,7 @@ private List<V> executeSearchUsingCache(

CacheInfo cacheInfo = getCacheInfo();
ObjectSearchStrategyType searchStrategy = getSearchStrategy();
var eeCtx = vtCtx.getExpressionEvaluationContext();

if (cacheInfo == null) {
return ObjectFound.unwrap(
Expand All @@ -319,7 +302,7 @@ private List<V> executeSearchUsingCache(
executeSearch(queries, searchStrategy, createOnDemandRetry, newValueDeltas));
}

List<V> cachedResult = cache.getSearchResult(targetTypeClass, queries, searchStrategy, context);
List<V> cachedResult = cache.getSearchResult(targetTypeClass, queries, searchStrategy, eeCtx);
if (cachedResult != null) {
cacheInfo.logHit(targetTypeClass, queries);
return CloneUtil.clone(cachedResult);
Expand All @@ -330,7 +313,7 @@ private List<V> executeSearchUsingCache(
if (!freshResult.isEmpty()) {
// we don't want to cache negative results (e.g. if used with focal objects it might mean that they would
// be attempted to create multiple times)
cache.putSearchResult(targetTypeClass, queries, searchStrategy, context, freshResult);
cache.putSearchResult(targetTypeClass, queries, searchStrategy, eeCtx, freshResult);
}
return ObjectFound.unwrap(freshResult);
}
Expand Down Expand Up @@ -407,18 +390,18 @@ private Collection<ObjectFound<O, V>> executeSearch(
try {
return executeSearch(query, options, additionalAttributeDeltas);
} catch (IllegalStateException e) { // this comes from checkConsistence methods
throw new IllegalStateException(e.getMessage() + " in " + context, e);
throw new IllegalStateException(e.getMessage() + " in " + vtCtx, e);
} catch (SchemaException e) {
throw new SchemaException(e.getMessage() + " in " + context, e);
throw new SchemaException(e.getMessage() + " in " + vtCtx, e);
} catch (SystemException e) {
throw new SystemException(e.getMessage() + " in " + context, e);
throw new SystemException(e.getMessage() + " in " + vtCtx, e);
} catch (CommunicationException | ConfigurationException | SecurityViolationException e) {
if (searchOnResource && tryAlsoRepository) {
var retryOptions = createNoFetchReadOnlyCollection();
try {
return executeSearch(query, retryOptions, additionalAttributeDeltas);
} catch (SchemaException e1) {
throw new SchemaException(e1.getMessage() + " in " + context, e1);
throw new SchemaException(e1.getMessage() + " in " + vtCtx, e1);
} catch (CommunicationException | ConfigurationException | SecurityViolationException e1) {
// TODO improve handling of exception.. we do not want to
// stop whole projection computation, but what to do if the
Expand Down Expand Up @@ -506,10 +489,11 @@ QName determineRelation(QName relation, ExpressionType relationExpression)
if (relationExpression != null) {
var relationDef = PrismContext.get().definitionFactory()
.newPropertyDefinition(ObjectReferenceType.F_RELATION, DOMUtil.XSD_QNAME);
// Variables should be non-relativistic here, so we can safely compute the "new" value even for the old state.
var eeCtx = vtCtx.getExpressionEvaluationContext();
// Variables are non-relativistic here, so the following call is adequate for both evaluateOld/evaluateNew cases.
PrismValue value = ExpressionUtil.evaluateExpression(
variables, relationDef, relationExpression, context.getExpressionProfile(),
context.getExpressionFactory(), context.getContextDescription(), getTask(), result);
vtCtx.getVariablesMap(), relationDef, relationExpression, eeCtx.getExpressionProfile(),
eeCtx.getExpressionFactory(), vtCtx.getContextDescription(), getTask(), result);
return value != null ? (QName) value.getRealValue() : null;
} else {
return relation;
Expand All @@ -520,6 +504,9 @@ private PrismObject<O> createOnDemand()
throws ExpressionEvaluationException, ObjectNotFoundException, SchemaException, CommunicationException,
ConfigurationException, SecurityViolationException, ObjectAlreadyExistsException {

var eeCtx = vtCtx.getExpressionEvaluationContext();
var variables = vtCtx.getVariablesMap();

LOGGER.trace("Going to create assignment targets on demand, variables:\n{}", lazy(variables::formatVariables));
PrismObjectDefinition<O> objectDefinition =
prismContext.getSchemaRegistry().findObjectDefinitionByCompileTimeClass(targetTypeClass);
Expand All @@ -529,15 +516,15 @@ private PrismObject<O> createOnDemand()

if (populateObjectConfig == null) {
LOGGER.warn("No populateObject in search expression in {}, object created on demand will be empty. "
+ "Subsequent operations will most likely fail", context);
+ "Subsequent operations will most likely fail", vtCtx);
} else {
List<ItemDelta<V, D>> populateDeltas =
PopulatorUtil.computePopulateItemDeltas(
populateObjectConfig, objectDefinition, variables, context, result);
populateObjectConfig, objectDefinition, variables, eeCtx, result);
ItemDeltaCollectionsUtil.applyTo(populateDeltas, newObject);
}

LOGGER.debug("Creating object on demand from {}: {}", context, newObject);
LOGGER.debug("Creating object on demand from {}: {}", vtCtx, newObject);
LOGGER.trace("Creating object on demand:\n{}", newObject.debugDumpLazily(1));

ObjectDelta<O> addDelta = newObject.createAddDelta();
Expand Down Expand Up @@ -570,7 +557,7 @@ protected boolean isCreateOnDemandSafe() {
}

private Task getTask() {
return context.getTask();
return vtCtx.getExpressionEvaluationContext().getTask();
}
}

Expand Down
14 changes: 5 additions & 9 deletions ...midpoint/model/common/expression/evaluator/AssignmentTargetSearchExpressionEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@
import javax.xml.namespace.QName;

import com.evolveum.midpoint.common.LocalizationService;
import com.evolveum.midpoint.model.common.expression.evaluator.transformation.ValueTransformationContext;
import com.evolveum.midpoint.prism.PrismContainerDefinition;
import com.evolveum.midpoint.prism.PrismContainerValue;
import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.prism.crypto.Protector;
import com.evolveum.midpoint.prism.delta.ItemDelta;
import com.evolveum.midpoint.prism.delta.ItemDeltaCollectionsUtil;
import com.evolveum.midpoint.repo.common.ObjectResolver;
import com.evolveum.midpoint.repo.common.expression.ExpressionEvaluationContext;
import com.evolveum.midpoint.schema.expression.VariablesMap;
import com.evolveum.midpoint.schema.internals.InternalsConfig;
import com.evolveum.midpoint.schema.result.OperationResult;
import com.evolveum.midpoint.util.exception.SchemaException;
Expand Down Expand Up @@ -57,12 +56,10 @@ class AssignmentTargetSearchExpressionEvaluator

@Override
Evaluation createEvaluation(
@NotNull VariablesMap variables,
boolean useNew,
@NotNull ExpressionEvaluationContext context,
@NotNull OperationResult result) throws SchemaException {
@NotNull ValueTransformationContext vtCtx, @NotNull OperationResult result)
throws SchemaException {

return new Evaluation(variables, useNew, context, result) {
return new Evaluation(vtCtx, result) {

protected @NotNull PrismContainerValue<AssignmentType> createResultValue(
String oid,
Expand Down Expand Up @@ -94,8 +91,7 @@ Evaluation createEvaluation(
}
prismContext.adopt(assignmentCVal, FocusType.COMPLEX_TYPE, FocusType.F_ASSIGNMENT);
if (InternalsConfig.consistencyChecks) {
assignmentCVal.assertDefinitions(
() -> "assignmentCVal in assignment expression in " + context.getContextDescription());
assignmentCVal.assertDefinitions(() -> "assignmentCVal in assignment expression in " + vtCtx);
}
return assignmentCVal;
}
Expand Down

0 comments on commit 4708741

Please sign in to comment.