Fixed storing policy state.

infra/util/src/main/java/com/evolveum/midpoint/util/xml/DomAwareEqualsStrategy.java

@@ -15,7 +15,7 @@
  */
 package com.evolveum.midpoint.util.xml;
 
-import org.jvnet.jaxb2_commons.lang.EqualsStrategy;
+import com.evolveum.midpoint.util.PrettyPrinter;
 import org.jvnet.jaxb2_commons.lang.JAXBEqualsStrategy;
 import org.jvnet.jaxb2_commons.locator.ObjectLocator;
 import org.w3c.dom.Element;
@@ -31,23 +31,48 @@
  */
 public class DomAwareEqualsStrategy extends JAXBEqualsStrategy {
 
-	public static EqualsStrategy INSTANCE = new DomAwareEqualsStrategy();
+	public static DomAwareEqualsStrategy INSTANCE = new DomAwareEqualsStrategy();
+
+	private static final boolean ENABLE_BRUTAL_DEBUGGING = false;       // keep false to avoid unnecessary code execution
+	private boolean traceAll = false;
+	private boolean traceNotEqual = false;
+
+	public boolean isTraceAll() {
+		return traceAll;
+	}
+
+	public void setTraceAll(boolean traceAll) {
+		this.traceAll = traceAll;
+	}
+
+	public boolean isTraceNotEqual() {
+		return traceNotEqual;
+	}
+
+	public void setTraceNotEqual(boolean traceNotEqual) {
+		this.traceNotEqual = traceNotEqual;
+	}
 
 	@Override
 	protected boolean equalsInternal(ObjectLocator leftLocator,
 			ObjectLocator rightLocator, Object lhs, Object rhs) {
-//		System.out.println("DomAwareEqualsStrategy: "+PrettyPrinter.prettyPrint(lhs)+"<=>"+PrettyPrinter.prettyPrint(rhs));
+		if (ENABLE_BRUTAL_DEBUGGING && traceAll) {
+			System.out.println("DomAwareEqualsStrategy: "+ PrettyPrinter.prettyPrint(lhs)+"<=>"+PrettyPrinter.prettyPrint(rhs));
+		}
+		boolean result;
 		if (lhs instanceof String && rhs instanceof String) {
-			return DOMUtil.compareTextNodeValues((String)lhs, (String)rhs);
+			result = DOMUtil.compareTextNodeValues((String)lhs, (String)rhs);
 		} else if (lhs instanceof Element && rhs instanceof Element) {
 			final Element left = (Element) lhs;
 			final Element right = (Element) rhs;
-			boolean result = DOMUtil.compareElement(left, right, false);
-//			System.out.println("cmp: "+PrettyPrinter.prettyPrint(left)+"<=>"+PrettyPrinter.prettyPrint(right)+": "+result);
-			return result;
+			result = DOMUtil.compareElement(left, right, false);
 		} else {
-			return super.equalsInternal(leftLocator, rightLocator, lhs, rhs);
+			result = super.equalsInternal(leftLocator, rightLocator, lhs, rhs);
+		}
+		if (ENABLE_BRUTAL_DEBUGGING && (traceAll || traceNotEqual && !result)) {
+			System.out.println("cmp: "+PrettyPrinter.prettyPrint(lhs)+"<=>"+PrettyPrinter.prettyPrint(rhs)+": "+result);
 		}
+		return result;
 	}
 
 }

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/AssignmentSpec.java

@@ -31,7 +31,7 @@
 public class AssignmentSpec implements Serializable {
 
 	@NotNull public final AssignmentType assignment;
-	@NotNull public final PlusMinusZero mode;
+	@NotNull public final PlusMinusZero mode;               // regarding the current object state (not the old one)
 
 	public AssignmentSpec(@NotNull AssignmentType assignment, @NotNull PlusMinusZero mode) {
 		this.assignment = assignment;

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/ChangeExecutor.java

@@ -148,7 +148,7 @@ public <O extends ObjectType> boolean executeChanges(LensContext<O> context, Tas
 			ObjectDelta<O> focusDelta = focusContext.getWaveExecutableDelta(context.getExecutionWave());
 
 			focusDelta = applyPendingObjectPolicyStateModifications(focusContext, focusDelta);
-			applyPendingAssignmentPolicyStateModifications(focusContext, focusDelta);
+			focusDelta = applyPendingAssignmentPolicyStateModifications(focusContext, focusDelta);
 
 			if (focusDelta != null) {
 
@@ -375,57 +375,57 @@ private <O extends ObjectType> ObjectDelta<O> applyPendingObjectPolicyStateModif
 		return focusDelta;
 	}
 
-	private <O extends ObjectType> void applyPendingAssignmentPolicyStateModifications(LensFocusContext<O> focusContext, ObjectDelta<O> focusDelta)
+	private <O extends ObjectType> ObjectDelta<O> applyPendingAssignmentPolicyStateModifications(LensFocusContext<O> focusContext, ObjectDelta<O> focusDelta)
 			throws SchemaException {
-		if (focusDelta != null) {
-			for (Map.Entry<AssignmentSpec, List<ItemDelta<?, ?>>> entry : focusContext
-					.getPendingAssignmentPolicyStateModifications().entrySet()) {
-				PlusMinusZero mode = entry.getKey().mode;
-				if (mode == PlusMinusZero.MINUS) {
-					continue;       // this assignment is being thrown out anyway, so let's ignore it
+		for (Map.Entry<AssignmentSpec, List<ItemDelta<?, ?>>> entry : focusContext
+				.getPendingAssignmentPolicyStateModifications().entrySet()) {
+			PlusMinusZero mode = entry.getKey().mode;
+			if (mode == PlusMinusZero.MINUS) {
+				continue;       // this assignment is being thrown out anyway, so let's ignore it
+			}
+			AssignmentType assignmentToFind = entry.getKey().assignment;
+			List<ItemDelta<?, ?>> modifications = entry.getValue();
+			if (modifications.isEmpty()) {
+				continue;
+			}
+			LOGGER.trace("Applying policy state modifications for {} ({}):\n{}", assignmentToFind, mode,
+					DebugUtil.debugDumpLazily(modifications));
+			if (mode == PlusMinusZero.ZERO) {
+				if (assignmentToFind.getId() == null) {
+					throw new IllegalStateException("Existing assignment with null id: " + assignmentToFind);
 				}
-				AssignmentType assignmentToFind = entry.getKey().assignment;
-				List<ItemDelta<?, ?>> modifications = entry.getValue();
-				if (modifications.isEmpty()) {
-					continue;
+				for (ItemDelta<?, ?> modification : modifications) {
+					focusDelta = focusContext.swallowToDelta(focusDelta, modification);
 				}
-				LOGGER.trace("Applying policy state modifications for {} ({}):\n{}", assignmentToFind, mode,
-						DebugUtil.debugDumpLazily(modifications));
-				if (mode == PlusMinusZero.ZERO) {
-					if (assignmentToFind.getId() == null) {
-						throw new IllegalStateException("Existing assignment with null id: " + assignmentToFind);
-					}
-					for (ItemDelta<?, ?> modification : modifications) {
-						focusContext.swallowToDelta(focusDelta, modification);
-					}
+			} else {
+				assert mode == PlusMinusZero.PLUS;
+				if (focusDelta != null && focusDelta.isAdd()) {
+					swallowIntoValues(((FocusType) focusDelta.getObjectToAdd().asObjectable()).getAssignment(),
+							assignmentToFind, modifications);
 				} else {
-					assert mode == PlusMinusZero.PLUS;
-					if (focusDelta.isAdd()) {
-						swallowIntoValues(((FocusType) focusDelta.getObjectToAdd().asObjectable()).getAssignment(),
-								assignmentToFind, modifications);
+					ContainerDelta<AssignmentType> assignmentDelta = focusDelta != null ?
+							focusDelta.findContainerDelta(FocusType.F_ASSIGNMENT) : null;
+					if (assignmentDelta == null) {
+						throw new IllegalStateException(
+								"We have 'plus' assignment to modify but there's no assignment delta. Assignment="
+										+ assignmentToFind + ", objectDelta=" + focusDelta);
+					}
+					if (assignmentDelta.isReplace()) {
+						swallowIntoValues(asContainerables(assignmentDelta.getValuesToReplace()), assignmentToFind,
+								modifications);
+					} else if (assignmentDelta.isAdd()) {
+						swallowIntoValues(asContainerables(assignmentDelta.getValuesToAdd()), assignmentToFind,
+								modifications);
 					} else {
-						ContainerDelta<AssignmentType> assignmentDelta = focusDelta.findContainerDelta(FocusType.F_ASSIGNMENT);
-						if (assignmentDelta == null) {
-							throw new IllegalStateException(
-									"We have 'plus' assignment to modify but there's no assignment delta. Assignment="
-											+ assignmentToFind + ", objectDelta=" + focusDelta);
-						}
-						if (assignmentDelta.isReplace()) {
-							swallowIntoValues(asContainerables(assignmentDelta.getValuesToReplace()), assignmentToFind,
-									modifications);
-						} else if (assignmentDelta.isAdd()) {
-							swallowIntoValues(asContainerables(assignmentDelta.getValuesToAdd()), assignmentToFind,
-									modifications);
-						} else {
-							throw new IllegalStateException(
-									"We have 'plus' assignment to modify but there're no values to add or replace in assignment delta. Assignment="
-											+ assignmentToFind + ", objectDelta=" + focusDelta);
-						}
+						throw new IllegalStateException(
+								"We have 'plus' assignment to modify but there're no values to add or replace in assignment delta. Assignment="
+										+ assignmentToFind + ", objectDelta=" + focusDelta);
 					}
 				}
 			}
 		}
 		focusContext.clearPendingAssignmentPolicyStateModifications();
+		return focusDelta;
 	}
 
 	private void swallowIntoValues(Collection<AssignmentType> assignments, AssignmentType assignmentToFind, List<ItemDelta<?, ?>> modifications)

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/EvaluatedAssignmentImpl.java

@@ -44,6 +44,9 @@
 import org.jetbrains.annotations.NotNull;
 
 import static com.evolveum.midpoint.prism.PrismContainerValue.asContainerable;
+import static com.evolveum.midpoint.prism.delta.PlusMinusZero.MINUS;
+import static com.evolveum.midpoint.prism.delta.PlusMinusZero.PLUS;
+import static com.evolveum.midpoint.prism.delta.PlusMinusZero.ZERO;
 
 /**
  * Evaluated assignment that contains all constructions and authorizations from the assignment
@@ -528,14 +531,17 @@ public List<EvaluatedAssignmentTargetImpl> getNonNegativeTargets() {
 		return rv;
 	}
 
+	/**
+	 * @return mode (adding, deleting, keeping) with respect to the *current* object (not the old one)
+	 */
 	@NotNull
 	public PlusMinusZero getMode() {
-		if (assignmentIdi.getItemOld() == null) {
-			return PlusMinusZero.PLUS;
-		} else if (assignmentIdi.getItemNew() == null) {
-			return PlusMinusZero.MINUS;
+		if (assignmentIdi.getItemNew() == null) {
+			return MINUS;
+		} else if (presentInCurrentObject) {
+			return ZERO;
 		} else {
-			return PlusMinusZero.ZERO;
+			return PLUS;
 		}
 	}
 }

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/LensElementContext.java

@@ -73,15 +73,23 @@ public abstract class LensElementContext<O extends ObjectType> implements ModelE
     private String iterationToken;
 
 	/**
-	 * These are policy state modifications that should be applied regardless of how the clockwork is exited
-	 * (e.g. in primary state, in final state, with an exception).
+	 * These are policy state modifications that should be applied.
+	 * Currently we apply them in ChangeExecutor.executeChanges only.
 	 *
-	 * Currently implemented only for focus, not for projections.
+	 * In the future we plan to be able to apply some state modifications even
+	 * if the clockwork is exited in non-standard way (e.g. in primary state or with an exception).
+	 * But we must be sure what policy state to store, because some constraints might be triggered
+	 * because of expectation of future state (like conflicting assignment is added etc.)
+	 * ---
+	 * Although placed in LensElementContext, support for this data is currently implemented only for focus, not for projections.
 	 */
 	@NotNull private transient final List<ItemDelta<?,?>> pendingObjectPolicyStateModifications = new ArrayList<>();
 
 	/**
 	 * Policy state modifications for assignments.
+	 *
+	 * Although we put here also deltas for assignments that are to be deleted, we do not execute these
+	 * (because we implement execution only for the standard exit-path from the clockwork).
 	 */
 	@NotNull private transient final Map<AssignmentSpec, List<ItemDelta<?,?>>> pendingAssignmentPolicyStateModifications = new HashMap<>();
 

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/policy/PolicyStateUpdater.java

@@ -24,6 +24,7 @@
 import com.evolveum.midpoint.prism.delta.PlusMinusZero;
 import com.evolveum.midpoint.prism.delta.builder.DeltaBuilder;
 import com.evolveum.midpoint.prism.path.IdItemPathSegment;
+import com.evolveum.midpoint.util.MiscUtil;
 import com.evolveum.midpoint.util.exception.SchemaException;
 import com.evolveum.midpoint.util.logging.Trace;
 import com.evolveum.midpoint.util.logging.TraceManager;
@@ -116,7 +117,9 @@ private ComputationResult compute(@NotNull List<EvaluatedPolicyRule> rulesToReco
 		cr.oldPolicySituations.addAll(existingPolicySituation);
 		cr.oldTriggeredRules.addAll(existingTriggeredPolicyRule);
 		cr.situationsNeedUpdate = !Objects.equals(cr.oldPolicySituations, cr.newPolicySituations);
-		cr.rulesNeedUpdate = !Objects.equals(cr.oldTriggeredRules, cr.newTriggeredRules);
+		// we do not use this, because it uses hashCode, that is (for some reason) wrongly computed
+		//cr.rulesNeedUpdate = !Objects.equals(cr.oldTriggeredRules, cr.newTriggeredRules);
+		cr.rulesNeedUpdate = !MiscUtil.unorderedCollectionEquals(cr.oldTriggeredRules, cr.newTriggeredRules);
 		return cr;
 	}
 

model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/lens/AbstractLensTest.java

@@ -92,14 +92,14 @@ public abstract class AbstractLensTest extends AbstractInternalModelIntegrationT
             "user-barbossa-modify-delete-assignment-account-dummy-attr.xml");
 
 	protected static final File ROLE_PIRATE_FILE = new File(TEST_DIR, "role-pirate.xml");
-	protected static final File ROLE_PIRATE_SITUATION_ONLY_FILE = new File(TEST_DIR, "role-pirate-situation-only.xml");
+	protected static final File ROLE_PIRATE_RECORD_ONLY_FILE = new File(TEST_DIR, "role-pirate-record-only.xml");
 	protected static final String ROLE_PIRATE_OID = "12345678-d34d-b33f-f00d-555555556666";
 
 	protected static final File ROLE_MUTINIER_FILE = new File(TEST_DIR, "role-mutinier.xml");
 	protected static final String ROLE_MUTINIER_OID = "12345678-d34d-b33f-f00d-555555556668";
 
 	protected static final File ROLE_JUDGE_FILE = new File(TEST_DIR, "role-judge.xml");
-	protected static final File ROLE_JUDGE_SITUATION_ONLY_FILE = new File(TEST_DIR, "role-judge-situation-only.xml");
+	protected static final File ROLE_JUDGE_RECORD_ONLY_FILE = new File(TEST_DIR, "role-judge-record-only.xml");
 	protected static final String ROLE_JUDGE_OID = "12345111-1111-2222-1111-121212111111";
 
 	protected static final File ROLE_CONSTABLE_FILE = new File(TEST_DIR, "role-constable.xml");

model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/lens/TestPolicyStateRecording.java

@@ -77,8 +77,8 @@ public void initSystem(Task initTask, OperationResult initResult) throws Excepti
 		super.initSystem(initTask, initResult);
 		setDefaultUserTemplate(USER_TEMPLATE_OID);
 
-		addObject(ROLE_PIRATE_SITUATION_ONLY_FILE);
-		addObject(ROLE_JUDGE_SITUATION_ONLY_FILE);
+		addObject(ROLE_PIRATE_RECORD_ONLY_FILE);
+		addObject(ROLE_JUDGE_RECORD_ONLY_FILE);
 
 		assumeAssignmentPolicy(AssignmentPolicyEnforcementType.FULL);