Skip to content

Commit

Permalink
Fix assignment policy plus/minus/zero evaluation
Browse files Browse the repository at this point in the history 
Assignment policy rules were evaluated using membership in plus, minus,
and zero set of "evaluated assignment triple", e.g. assignment was
considered to be added when it was present in the plus set. This was
not quite correct if validity come into play.

So here we introduced more precise AssignmentOrigin information and
its isBeingAdded, isBeingDeleted and isBeingModified methods. They
are used mainly for AssignmentModificationConstraintEvaluator and
HasAssignmentConstraintEvaluator.

This fixes MID-4944 and MID-4216.
  • Loading branch information
@mederly
mederly committed Mar 3, 2020
1 parent 40bcca2 commit f32ea11
Show file tree
Hide file tree
Showing 32 changed files with 658 additions and 451 deletions.
17 changes: 17 additions & 0 deletions infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/AddDeleteReplace.java
View file
@@ -0,0 +1,17 @@
/*
* Copyright (c) 2014 Evolveum and contributors
*
* This work is dual-licensed under the Apache License 2.0
* and European Union Public License. See LICENSE file for details.
*/
package com.evolveum.midpoint.prism.delta;

/**
* Denotes ADD, DELETE, REPLACE sets in item deltas.
*
*/
public enum AddDeleteReplace {

ADD, DELETE, REPLACE

}
3 changes: 2 additions & 1 deletion ...rc/main/java/com/evolveum/midpoint/model/impl/controller/ModelInteractionServiceImpl.java
View file
Expand Up @@ -27,6 +27,7 @@
import com.evolveum.midpoint.TerminateSessionEvent;
import com.evolveum.midpoint.model.api.authentication.*;
import com.evolveum.midpoint.model.common.stringpolicy.*;
import com.evolveum.midpoint.model.impl.lens.projector.AssignmentOrigin;
import com.evolveum.midpoint.prism.*;
import com.evolveum.midpoint.schema.cache.CacheConfigurationManager;
import com.evolveum.midpoint.schema.util.*;
Expand Down Expand Up @@ -1462,7 +1463,7 @@ private boolean determineDeputyValidity(PrismObject<UserType> potentialDeputy, L
// TODO some special mode for verification of the validity - we don't need complete calculation here!
EvaluatedAssignment<UserType> assignment = assignmentEvaluator
.evaluate(assignmentIdi, PlusMinusZero.ZERO, false, potentialDeputy.asObjectable(),
potentialDeputy.toString(), false, task, result);
potentialDeputy.toString(), AssignmentOrigin.createInObject(), task, result);
if (!assignment.isValid()) {
continue;
}
Expand Down
43 changes: 24 additions & 19 deletions ...l/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/AssignmentCollector.java
View file
Expand Up @@ -10,6 +10,8 @@
import java.util.Collection;
import java.util.List;

import com.evolveum.midpoint.model.impl.lens.projector.AssignmentOrigin;
import com.evolveum.midpoint.prism.*;
import com.evolveum.midpoint.schema.cache.CacheConfigurationManager;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
Expand All @@ -22,10 +24,6 @@
import com.evolveum.midpoint.model.common.SystemObjectCache;
import com.evolveum.midpoint.model.common.mapping.MappingFactory;
import com.evolveum.midpoint.model.impl.lens.projector.mappings.MappingEvaluator;
import com.evolveum.midpoint.prism.PrismContainerDefinition;
import com.evolveum.midpoint.prism.PrismContainerValue;
import com.evolveum.midpoint.prism.PrismContext;
import com.evolveum.midpoint.prism.PrismObject;
import com.evolveum.midpoint.prism.delta.PlusMinusZero;
import com.evolveum.midpoint.prism.util.ItemDeltaItem;
import com.evolveum.midpoint.prism.util.ObjectDeltaObject;
Expand All @@ -48,7 +46,10 @@
import com.evolveum.midpoint.xml.ns._public.common.common_3.ArchetypePolicyType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentHolderType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentType;
import com.evolveum.midpoint.xml.ns._public.common.common_3.SystemConfigurationType;

import static com.evolveum.midpoint.util.MiscUtil.emptyIfNull;

import static org.apache.commons.lang3.ObjectUtils.defaultIfNull;

/**
* @author katka
Expand All @@ -73,23 +74,24 @@ public class AssignmentCollector {
@Autowired private Clock clock;
@Autowired private CacheConfigurationManager cacheConfigurationManager;

public <AH extends AssignmentHolderType> Collection<EvaluatedAssignment<AH>> collect(PrismObject<AH> assignmentHolder, PrismObject<SystemConfigurationType> systemConfiguration, boolean loginMode, Task task, OperationResult result) throws SchemaException {
public <AH extends AssignmentHolderType> Collection<EvaluatedAssignment<AH>> collect(PrismObject<AH> assignmentHolder,
boolean loginMode, Task task, OperationResult result) throws SchemaException {

LensContext<AH> lensContext = createAuthenticationLensContext(assignmentHolder, result);

AH assignmentHolderType = assignmentHolder.asObjectable();
Collection<AssignmentType> forcedAssignments = null;
Collection<AssignmentType> forcedAssignments;
try {
forcedAssignments = LensUtil.getForcedAssignments(lensContext.getFocusContext().getLifecycleModel(),
assignmentHolderType.getLifecycleState(), objectResolver, prismContext, task, result);
} catch (ObjectNotFoundException | CommunicationException | ConfigurationException | SecurityViolationException
| ExpressionEvaluationException e1) {
LOGGER.error("Forced assignments defined for lifecycle {} won't be evaluated", assignmentHolderType.getLifecycleState(), e1);
forcedAssignments = null;
}
Collection<EvaluatedAssignment<AH>> evaluatedAssignments = new ArrayList<>();

if (!assignmentHolderType.getAssignment().isEmpty() || forcedAssignments != null) {

AssignmentEvaluator.Builder<AH> builder =
new AssignmentEvaluator.Builder<AH>()
.repository(repositoryService)
Expand All @@ -114,30 +116,33 @@ public <AH extends AssignmentHolderType> Collection<EvaluatedAssignment<AH>> col

AssignmentEvaluator<AH> assignmentEvaluator = builder.build();

evaluatedAssignments.addAll(evaluateAssignments(assignmentHolderType, assignmentHolderType.getAssignment(), false, assignmentEvaluator,task, result));
evaluatedAssignments.addAll(evaluateAssignments(assignmentHolderType, assignmentHolderType.getAssignment(),
AssignmentOrigin.createInObject(), assignmentEvaluator,task, result));

evaluatedAssignments.addAll(evaluateAssignments(assignmentHolderType, forcedAssignments, true, assignmentEvaluator, task, result));
evaluatedAssignments.addAll(evaluateAssignments(assignmentHolderType, forcedAssignments,
AssignmentOrigin.createVirtual(), assignmentEvaluator, task, result));
}

return evaluatedAssignments;

}

private <AH extends AssignmentHolderType> Collection<EvaluatedAssignment<AH>> evaluateAssignments(AH assignmentHolder, Collection<AssignmentType> assignments, boolean virtual, AssignmentEvaluator<AH> assignmentEvaluator, Task task, OperationResult result) {
private <AH extends AssignmentHolderType> Collection<EvaluatedAssignment<AH>> evaluateAssignments(AH assignmentHolder,
Collection<AssignmentType> assignments, AssignmentOrigin origin, AssignmentEvaluator<AH> assignmentEvaluator, Task task, OperationResult result) {

List<EvaluatedAssignment<AH>> evaluatedAssignments = new ArrayList<>();
RepositoryCache.enter(cacheConfigurationManager);
try {
for (AssignmentType assignmentType: assignments) {
PrismContainerDefinition<AssignmentType> standardAssignmentDefinition = prismContext.getSchemaRegistry()
.findObjectDefinitionByCompileTimeClass(AssignmentHolderType.class)
.findContainerDefinition(AssignmentHolderType.F_ASSIGNMENT);
for (AssignmentType assignmentType: emptyIfNull(assignments)) {
try {
PrismContainerDefinition definition = assignmentType.asPrismContainerValue().getDefinition();
if (definition == null) {
// TODO: optimize
definition = prismContext.getSchemaRegistry().findObjectDefinitionByCompileTimeClass(AssignmentHolderType.class).findContainerDefinition(AssignmentHolderType.F_ASSIGNMENT);
}
//noinspection unchecked
PrismContainerDefinition<AssignmentType> definition = defaultIfNull(
assignmentType.asPrismContainerValue().getDefinition(), standardAssignmentDefinition);
ItemDeltaItem<PrismContainerValue<AssignmentType>,PrismContainerDefinition<AssignmentType>> assignmentIdi =
new ItemDeltaItem<>(LensUtil.createAssignmentSingleValueContainer(assignmentType), definition);
EvaluatedAssignment<AH> assignment = assignmentEvaluator.evaluate(assignmentIdi, PlusMinusZero.ZERO, false, assignmentHolder, assignmentHolder.toString(), virtual, task, result);
EvaluatedAssignment<AH> assignment = assignmentEvaluator.evaluate(assignmentIdi, PlusMinusZero.ZERO, false, assignmentHolder, assignmentHolder.toString(), origin, task, result);
evaluatedAssignments.add(assignment);
} catch (SchemaException | ObjectNotFoundException | ExpressionEvaluationException | PolicyViolationException | SecurityViolationException | ConfigurationException | CommunicationException e) {
LOGGER.error("Error while processing assignment of {}: {}; assignment: {}",
Expand Down
9 changes: 5 additions & 4 deletions ...l/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/AssignmentEvaluator.java
View file
Expand Up @@ -14,6 +14,7 @@

import com.evolveum.midpoint.common.ActivationComputer;
import com.evolveum.midpoint.model.api.ModelExecuteOptions;
import com.evolveum.midpoint.model.impl.lens.projector.AssignmentOrigin;
import com.evolveum.midpoint.model.impl.lens.projector.mappings.AssignedFocusMappingEvaluationRequest;
import com.evolveum.midpoint.prism.delta.DeltaSetTriple;
import com.evolveum.midpoint.prism.polystring.PolyString;
Expand Down Expand Up @@ -221,15 +222,16 @@ private EvaluationContext(@NotNull EvaluatedAssignmentImpl<AH> evalAssignment,
*/
public EvaluatedAssignmentImpl<AH> evaluate(
ItemDeltaItem<PrismContainerValue<AssignmentType>,PrismContainerDefinition<AssignmentType>> assignmentIdi,
PlusMinusZero primaryAssignmentMode, boolean evaluateOld, ObjectType source, String sourceDescription, boolean forcedAssignment, Task task, OperationResult parentResult)
PlusMinusZero primaryAssignmentMode, boolean evaluateOld, ObjectType source, String sourceDescription,
AssignmentOrigin origin, Task task, OperationResult parentResult)
throws SchemaException, ObjectNotFoundException, ExpressionEvaluationException, PolicyViolationException, SecurityViolationException, ConfigurationException, CommunicationException {
OperationResult result = parentResult.subresult(OP_EVALUATE)
.setMinor()
.addArbitraryObjectAsParam("primaryAssignmentMode", primaryAssignmentMode)
.addParam("evaluateOld", evaluateOld)
.addArbitraryObjectAsParam("source", source)
.addParam("sourceDescription", sourceDescription)
.addParam("forcedAssignment", forcedAssignment)
.addArbitraryObjectAsParam("origin", origin)
.build();
AssignmentEvaluationTraceType trace;
if (result.isTracingNormal(AssignmentEvaluationTraceType.class)) {
Expand All @@ -247,8 +249,7 @@ public EvaluatedAssignmentImpl<AH> evaluate(
try {
assertSourceNotNull(source, assignmentIdi);

EvaluatedAssignmentImpl<AH> evalAssignmentImpl = new EvaluatedAssignmentImpl<>(assignmentIdi, evaluateOld, prismContext);
evalAssignmentImpl.setVirtual(forcedAssignment);
EvaluatedAssignmentImpl<AH> evalAssignmentImpl = new EvaluatedAssignmentImpl<>(assignmentIdi, evaluateOld, origin, prismContext);

EvaluationContext ctx = new EvaluationContext(
evalAssignmentImpl,
Expand Down
25 changes: 11 additions & 14 deletions ...del-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/EvaluatedAssignmentImpl.java
View file
Expand Up @@ -13,6 +13,7 @@

import javax.xml.namespace.QName;

import com.evolveum.midpoint.model.impl.lens.projector.AssignmentOrigin;
import com.evolveum.midpoint.model.impl.lens.projector.mappings.AssignedFocusMappingEvaluationRequest;
import com.evolveum.midpoint.prism.*;
import com.evolveum.midpoint.model.api.context.*;
Expand Down Expand Up @@ -95,21 +96,21 @@ public class EvaluatedAssignmentImpl<AH extends AssignmentHolderType> implements
private boolean isValid;
private boolean wasValid;
private boolean forceRecon; // used also to force recomputation of parentOrgRefs
private boolean presentInCurrentObject;
private boolean presentInOldObject;
@NotNull private final AssignmentOrigin origin;
private Collection<String> policySituations = new HashSet<>();

private PrismContext prismContext;

public EvaluatedAssignmentImpl(
@NotNull ItemDeltaItem<PrismContainerValue<AssignmentType>, PrismContainerDefinition<AssignmentType>> assignmentIdi,
boolean evaluatedOld, PrismContext prismContext) {
boolean evaluatedOld, @NotNull AssignmentOrigin origin, PrismContext prismContext) {
this.assignmentIdi = assignmentIdi;
this.evaluatedOld = evaluatedOld;
this.constructionTriple = prismContext.deltaFactory().createDeltaSetTriple();
this.personaConstructionTriple = prismContext.deltaFactory().createDeltaSetTriple();
this.roles = prismContext.deltaFactory().createDeltaSetTriple();
this.prismContext = prismContext;
this.origin = origin;
}

@NotNull
Expand Down Expand Up @@ -368,22 +369,19 @@ void evaluateConstructions(ObjectDeltaObject<AH> focusOdo, Task task, OperationR
evaluateConstructions(focusOdo, null, null, task, result);
}

public void setPresentInCurrentObject(boolean presentInCurrentObject) {
this.presentInCurrentObject = presentInCurrentObject;
}

public void setPresentInOldObject(boolean presentInOldObject) {
this.presentInOldObject = presentInOldObject;
@NotNull
public AssignmentOrigin getOrigin() {
return origin;
}

@Override
public boolean isPresentInCurrentObject() {
return presentInCurrentObject;
return origin.isCurrent();
}

@Override
public boolean isPresentInOldObject() {
return presentInOldObject;
return origin.isOld();
}

@NotNull
Expand Down Expand Up @@ -526,8 +524,7 @@ public String debugDump(int indent) {
DebugUtil.debugDumpWithLabelLn(sb, "focusPolicyRules " + ruleCountInfo(focusPolicyRules), focusPolicyRules, indent+1);
DebugUtil.debugDumpWithLabelLn(sb, "thisTargetPolicyRules " + ruleCountInfo(thisTargetPolicyRules), thisTargetPolicyRules, indent+1);
DebugUtil.debugDumpWithLabelLn(sb, "otherTargetsPolicyRules " + ruleCountInfo(otherTargetsPolicyRules), otherTargetsPolicyRules, indent+1);
DebugUtil.debugDumpWithLabelLn(sb, "Present in old object", isPresentInOldObject(), indent+1);
DebugUtil.debugDumpWithLabel(sb, "Present in current object", isPresentInCurrentObject(), indent+1);
DebugUtil.debugDumpWithLabelLn(sb, "origin", origin.toString(), indent+1);
return sb.toString();
}

Expand Down Expand Up @@ -605,7 +602,7 @@ public List<EvaluatedAssignmentTargetImpl> getNonNegativeTargets() {
public PlusMinusZero getMode() {
if (assignmentIdi.getItemNew() == null || assignmentIdi.getItemNew().isEmpty()) {
return MINUS;
} else if (presentInCurrentObject) {
} else if (origin.isCurrent()) {
return ZERO;
} else {
return PLUS;
Expand Down

0 comments on commit f32ea11

Please sign in to comment.