Fix Item.diff method seriously

Removed diffValues() that was introduced recently and changed diff()
to get its semantics - because no other semantics are suitable for this
method.

Also including explicit test for MID-6129.

(cherry picked from commit 2904e2f)

infra/prism-api/src/main/java/com/evolveum/midpoint/prism/Item.java

@@ -422,18 +422,18 @@ default Collection<Object> getRealValuesOrRawTypes(PrismContext prismContext) {
 
     /**
      * Computes a difference (delta) with the specified item using IGNORE_METADATA_CONSIDER_DIFFERENT_IDS equivalence strategy.
-     */
-    ItemDelta<V,D> diff(Item<V,D> other);
-
-    /**
-     * Computes a difference (delta) with the specified item using IGNORE_METADATA_CONSIDER_DIFFERENT_IDS equivalence strategy.
+     *
      * Compares item values only -- does NOT dive into lower levels.
      */
-    ItemDelta<V,D> diffValues(Item<V,D> other);
+    default ItemDelta<V,D> diff(Item<V,D> other) {
+        return diff(other, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DIFF);
+    }
 
     /**
      * Computes a difference (delta) with the specified item using given equivalence strategy.
      * Note this method cannot accept general EquivalenceStrategy here; it needs the parameterized strategy.
+     *
+     * Compares item values only -- does NOT dive into lower levels.
      */
     ItemDelta<V,D> diff(Item<V,D> other, @NotNull ParameterizedEquivalenceStrategy strategy);
 

infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/ItemImpl.java

@@ -542,34 +542,10 @@ public void merge(Item<V,D> otherItem) throws SchemaException {
         }
     }
 
-    public ItemDelta<V,D> diff(Item<V,D> other) {
-        return diff(other, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DIFF);
-    }
-
-    @Override
-    public ItemDelta<V,D> diffValues(Item<V,D> other) {
-        return diff(other, true, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DIFF);
-    }
-
     public ItemDelta<V,D> diff(Item<V,D> other, @NotNull ParameterizedEquivalenceStrategy strategy) {
-        return diff(other, false, strategy);
-    }
-
-    public ItemDelta<V,D> diff(Item<V,D> other, boolean rootValuesOnly, @NotNull ParameterizedEquivalenceStrategy strategy) {
-        List<? extends ItemDelta> itemDeltas = new ArrayList<>();
-        diffInternal(other, itemDeltas, rootValuesOnly, strategy);
-        if (itemDeltas.isEmpty()) {
-            return null;
-        }
-        if (itemDeltas.size() > 1) {
-            throw new UnsupportedOperationException("Item multi-delta diff is not supported yet");
-        }
-        return itemDeltas.get(0);
-    }
-
-    void diffInternal(Item<V, D> other, Collection<? extends ItemDelta> deltas,
-            ParameterizedEquivalenceStrategy strategy) {
-        diffInternal(other, deltas, false, strategy);
+        List<ItemDelta<V, D>> itemDeltas = new ArrayList<>();
+        diffInternal(other, itemDeltas, true, strategy);
+        return MiscUtil.extractSingleton(itemDeltas);
     }
 
     void diffInternal(Item<V, D> other, Collection<? extends ItemDelta> deltas, boolean rootValuesOnly,

infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerImpl.java

@@ -689,7 +689,7 @@ public List<? extends ItemDelta> diffModifications(PrismContainer<C> other) {
 
     public List<? extends ItemDelta> diffModifications(PrismContainer<C> other, ParameterizedEquivalenceStrategy strategy) {
         List<? extends ItemDelta> itemDeltas = new ArrayList<>();
-        diffInternal(other, itemDeltas, strategy);
+        diffInternal(other, itemDeltas, false, strategy);
         return itemDeltas;
     }
 

infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java

@@ -1052,7 +1052,7 @@ private void diffItems(PrismContainerValue<C> thisValue, PrismContainerValue<C>
                 }
             }
             // The "delete" delta will also result from the following diff
-            ((ItemImpl) thisItem).diffInternal(otherItem, deltas, strategy);
+            ((ItemImpl) thisItem).diffInternal(otherItem, deltas, false, strategy);
         }
 
         other.getItems();

infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismObjectImpl.java

@@ -282,7 +282,7 @@ public ObjectDelta<O> diff(PrismObject<O> other, ParameterizedEquivalenceStrateg
         objectDelta.setOid(getOid());
 
         Collection<? extends ItemDelta> itemDeltas = new ArrayList<>();
-        diffInternal(other, itemDeltas, strategy);
+        diffInternal(other, itemDeltas, false, strategy);
         objectDelta.addModifications(itemDeltas);
 
         return objectDelta;

infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/item/DummyContainerImpl.java

@@ -487,17 +487,6 @@ public boolean valuesEqual(Collection<PrismContainerValue<C>> matchValues,
         return realContainer.valuesEqual(matchValues, comparator);
     }
 
-    public ItemDelta<PrismContainerValue<C>, PrismContainerDefinition<C>> diff(
-            Item<PrismContainerValue<C>, PrismContainerDefinition<C>> other) {
-        return realContainer.diff(other);
-    }
-
-    @Override
-    public ItemDelta<PrismContainerValue<C>, PrismContainerDefinition<C>> diffValues(
-            Item<PrismContainerValue<C>, PrismContainerDefinition<C>> other) {
-        return realContainer.diffValues(other);
-    }
-
     public ItemDelta<PrismContainerValue<C>, PrismContainerDefinition<C>> diff(
             Item<PrismContainerValue<C>, PrismContainerDefinition<C>> other,
             @NotNull ParameterizedEquivalenceStrategy strategy) {

infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/item/DummyPropertyImpl.java

@@ -377,17 +377,6 @@ public boolean valuesEqual(Collection<PrismPropertyValue<T>> matchValues,
         return realProperty.valuesEqual(matchValues, comparator);
     }
 
-    public ItemDelta<PrismPropertyValue<T>, PrismPropertyDefinition<T>> diff(
-            Item<PrismPropertyValue<T>, PrismPropertyDefinition<T>> other) {
-        return realProperty.diff(other);
-    }
-
-    @Override
-    public ItemDelta<PrismPropertyValue<T>, PrismPropertyDefinition<T>> diffValues(
-            Item<PrismPropertyValue<T>, PrismPropertyDefinition<T>> other) {
-        return realProperty.diffValues(other);
-    }
-
     public ItemDelta<PrismPropertyValue<T>, PrismPropertyDefinition<T>> diff(
             Item<PrismPropertyValue<T>, PrismPropertyDefinition<T>> other,
             @NotNull ParameterizedEquivalenceStrategy strategy) {

infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/item/DummyReferenceImpl.java

@@ -305,17 +305,6 @@ public boolean valuesEqual(Collection<PrismReferenceValue> matchValues,
         return realReference.valuesEqual(matchValues, comparator);
     }
 
-    public ItemDelta<PrismReferenceValue, PrismReferenceDefinition> diff(
-            Item<PrismReferenceValue, PrismReferenceDefinition> other) {
-        return realReference.diff(other);
-    }
-
-    @Override
-    public ItemDelta<PrismReferenceValue, PrismReferenceDefinition> diffValues(
-            Item<PrismReferenceValue, PrismReferenceDefinition> other) {
-        return realReference.diffValues(other);
-    }
-
     public ItemDelta<PrismReferenceValue, PrismReferenceDefinition> diff(
             Item<PrismReferenceValue, PrismReferenceDefinition> other,
             @NotNull ParameterizedEquivalenceStrategy strategy) {

infra/schema/src/test/java/com/evolveum/midpoint/schema/TestParseDiffPatch.java

@@ -6,50 +6,47 @@
  */
 package com.evolveum.midpoint.schema;
 
-import static com.evolveum.midpoint.prism.util.PrismTestUtil.display;
-import static com.evolveum.midpoint.prism.util.PrismTestUtil.getPrismContext;
-import static org.testng.AssertJUnit.*;
-
-import java.io.File;
-import java.io.IOException;
-import java.util.Collection;
-import java.util.List;
-
-import javax.xml.bind.JAXBException;
-import javax.xml.namespace.QName;
-
-import com.evolveum.midpoint.prism.*;
+import com.evolveum.midpoint.prism.PrismContainer;
+import com.evolveum.midpoint.prism.PrismContext;
+import com.evolveum.midpoint.prism.PrismObject;
 import com.evolveum.midpoint.prism.delta.*;
 import com.evolveum.midpoint.prism.equivalence.EquivalenceStrategy;
-import com.evolveum.midpoint.prism.path.ItemPath;
 import com.evolveum.midpoint.prism.path.ItemName;
+import com.evolveum.midpoint.prism.path.ItemPath;
 import com.evolveum.midpoint.prism.polystring.PolyString;
+import com.evolveum.midpoint.prism.util.PrismAsserts;
+import com.evolveum.midpoint.prism.util.PrismTestUtil;
 import com.evolveum.midpoint.prism.xml.XmlTypeConverter;
 import com.evolveum.midpoint.prism.xnode.RootXNode;
 import com.evolveum.midpoint.prism.xnode.XNode;
-import com.evolveum.midpoint.util.QNameUtil;
-import com.evolveum.midpoint.xml.ns._public.common.api_types_3.ObjectModificationType;
-import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
-
-import com.evolveum.prism.xml.ns._public.types_3.ObjectDeltaType;
-import org.testng.AssertJUnit;
-import org.testng.annotations.BeforeSuite;
-import org.testng.annotations.Test;
-import org.w3c.dom.Element;
-import org.xml.sax.SAXException;
-
-import com.evolveum.midpoint.prism.util.PrismAsserts;
-import com.evolveum.midpoint.prism.util.PrismTestUtil;
 import com.evolveum.midpoint.schema.constants.MidPointConstants;
 import com.evolveum.midpoint.schema.constants.SchemaConstants;
 import com.evolveum.midpoint.schema.util.SchemaTestConstants;
 import com.evolveum.midpoint.util.MiscUtil;
 import com.evolveum.midpoint.util.PrettyPrinter;
+import com.evolveum.midpoint.util.QNameUtil;
 import com.evolveum.midpoint.util.exception.SchemaException;
+import com.evolveum.midpoint.xml.ns._public.common.api_types_3.ObjectModificationType;
+import com.evolveum.midpoint.xml.ns._public.common.common_3.*;
 import com.evolveum.prism.xml.ns._public.types_3.ItemDeltaType;
 import com.evolveum.prism.xml.ns._public.types_3.ModificationTypeType;
 import com.evolveum.prism.xml.ns._public.types_3.PolyStringType;
 import com.evolveum.prism.xml.ns._public.types_3.RawType;
+import org.testng.AssertJUnit;
+import org.testng.annotations.BeforeSuite;
+import org.testng.annotations.Test;
+import org.w3c.dom.Element;
+import org.xml.sax.SAXException;
+
+import javax.xml.bind.JAXBException;
+import javax.xml.namespace.QName;
+import java.io.File;
+import java.io.IOException;
+import java.util.Collection;
+import java.util.List;
+
+import static com.evolveum.midpoint.prism.util.PrismTestUtil.getPrismContext;
+import static org.testng.AssertJUnit.*;
 
 /**
  * @author semancik
@@ -672,7 +669,7 @@ public void testResourceNsFixUndeclaredPrefixes() throws SchemaException, SAXExc
             resourceFixed.checkConsistence();
 
             // WHEN
-            String xmlBroken = getPrismContext().serializeObjectToString(resourceBroken, PrismContext.LANG_XML);
+            String xmlBroken = getPrismContext().xmlSerializer().serialize(resourceBroken);
             ObjectDelta<ResourceType> resourceDelta = resourceBroken.diff(resourceFixed, EquivalenceStrategy.LITERAL_IGNORE_METADATA);
 
             // THEN
@@ -694,7 +691,7 @@ public void testResourceNsFixUndeclaredPrefixes() throws SchemaException, SAXExc
             PrismObject<ResourceType> resourceUpdated = resourceBroken.clone();
             resourceDelta.applyTo(resourceUpdated);
 
-            String xmlUpdated = getPrismContext().serializeObjectToString(resourceUpdated, PrismContext.LANG_XML);
+            String xmlUpdated = getPrismContext().xmlSerializer().serialize(resourceUpdated);
             System.out.println("UPDATED RESOURCE:");
             System.out.println(xmlUpdated);
             assertFalse("__UNDECLARED__ flag in updated resource", xmlUpdated.contains("__UNDECLARED__"));
@@ -738,6 +735,7 @@ private void assertModificationPolyStringValue(RawType value, PolyStringType...
         for (PolyStringType expectedValue: expectedValues) {
             if (expectedValue.getOrig().equals(valueAsPoly.getOrig()) && expectedValue.getNorm().equals(valueAsPoly.getNorm())) {
                 found = true;
+                break;
             }
         }
         assertTrue(found);
@@ -813,13 +811,13 @@ public void testCampaign() throws SchemaException, SAXException, IOException, JA
 
     @Test(enabled = false)
     public void testReplaceModelOperationContext() throws Exception {
-        PrismObject prismObject = PrismTestUtil.parseObject(new File(TEST_DIR, "task-modelOperationContext-before.xml"));
+        PrismObject<TaskType> prismObject = PrismTestUtil.parseObject(new File(TEST_DIR, "task-modelOperationContext-before.xml"));
 
-        ObjectDelta delta = getPrismContext().deltaFactory().object().createEmptyModifyDelta(TaskType.class, prismObject.getOid()
-        );
+        ObjectDelta<TaskType> delta = getPrismContext().deltaFactory().object().createEmptyModifyDelta(TaskType.class, prismObject.getOid());
+        //noinspection unchecked
         delta.addModificationReplaceContainer(TaskType.F_MODEL_OPERATION_CONTEXT);
 
-        PrismObject changed = prismObject.clone();
+        PrismObject<TaskType> changed = prismObject.clone();
         ItemDeltaCollectionsUtil.applyTo(delta.getModifications(), changed);
         Collection<? extends ItemDelta> processedModifications = prismObject.diffModifications(changed, EquivalenceStrategy.LITERAL_IGNORE_METADATA);
 
@@ -846,7 +844,7 @@ public void testDiffSameValues() throws Exception {
     }
 
     @Test
-    public void testDiffContainerValues() throws Exception {
+    public void testDiffContainerValues() {
         UserType user1 = new UserType(getPrismContext())
                 .beginAssignment()
                     .id(1L)
@@ -864,30 +862,22 @@ public void testDiffContainerValues() throws Exception {
                 .beginAssignment()
                     .targetRef("oid-c", RoleType.COMPLEX_TYPE)
                 .end();
-        PrismContainer<Containerable> assignment1 = user1.asPrismObject().findContainer(UserType.F_ASSIGNMENT);
-        PrismContainer<Containerable> assignment2 = user2.asPrismObject().findContainer(UserType.F_ASSIGNMENT);
-        ContainerDelta<Containerable> diff = assignment1.diff(assignment2);
-        ItemDelta<?,?> diffValues = assignment1.diffValues(assignment2);
+        PrismContainer<AssignmentType> assignment1 = user1.asPrismObject().findContainer(UserType.F_ASSIGNMENT);
+        PrismContainer<AssignmentType> assignment2 = user2.asPrismObject().findContainer(UserType.F_ASSIGNMENT);
+        ContainerDelta<AssignmentType> diff = assignment1.diff(assignment2);
 
-        display("assignment1", assignment1);
-        display("assignment2", assignment2);
-        display("diff", diff);
+        PrismTestUtil.display("assignment1", assignment1);
+        PrismTestUtil.display("assignment2", assignment2);
+        PrismTestUtil.display("diff", diff);
 
         assertEquals("Wrong values to add", assignment2.getValues(), diff.getValuesToAdd());
         assertEquals("Wrong values to delete", assignment1.getValues(), diff.getValuesToDelete());
         //noinspection SimplifiedTestNGAssertion
         assertEquals("Wrong values to replace", null, diff.getValuesToReplace());
-
-        display("diffValues", diffValues);
-
-        assertEquals("Wrong values to add", assignment2.getValues(), diffValues.getValuesToAdd());
-        assertEquals("Wrong values to delete", assignment1.getValues(), diffValues.getValuesToDelete());
-        //noinspection SimplifiedTestNGAssertion
-        assertEquals("Wrong values to replace", null, diffValues.getValuesToReplace());
     }
 
     @Test
-    public void testDiffSingleContainerValues() throws Exception {
+    public void testDiffSingleContainerValues() {
         UserType user1 = new UserType(getPrismContext())
                 .beginActivation()
                     .validFrom("2020-03-20T15:11:40.936+01:00")
@@ -901,12 +891,11 @@ public void testDiffSingleContainerValues() throws Exception {
         PrismContainer<ActivationType> activation1 = user1.asPrismObject().findContainer(UserType.F_ACTIVATION);
         PrismContainer<ActivationType> activation2 = user2.asPrismObject().findContainer(UserType.F_ACTIVATION);
 
-        // Note that .diff() is not supported in this context
-        ItemDelta<?, ?> diff = activation1.diffValues(activation2);
+        ItemDelta<?, ?> diff = activation1.diff(activation2);
 
-        display("activation1", activation1);
-        display("activation2", activation2);
-        display("diff", diff);
+        PrismTestUtil.display("activation1", activation1);
+        PrismTestUtil.display("activation2", activation2);
+        PrismTestUtil.display("diff", diff);
 
         assertEquals("Wrong values to add", activation2.getValues(), diff.getValuesToAdd());
         assertEquals("Wrong values to delete", activation1.getValues(), diff.getValuesToDelete());

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/focus/InboundProcessor.java

@@ -1092,7 +1092,7 @@ private <V extends PrismValue, D extends ItemDefinition, F extends FocusType> It
         if (targetFocusItem != null) {
             LOGGER.trace("Comparing focus item:\n{}\nto should be item:\n{}",
                     DebugUtil.debugDumpLazily(targetFocusItem, 1), DebugUtil.debugDumpLazily(shouldBeItem, 1));
-            ItemDelta diffDelta = targetFocusItem.diffValues(shouldBeItem);
+            ItemDelta diffDelta = targetFocusItem.diff(shouldBeItem);
             LOGGER.trace("The difference is:\n{}", DebugUtil.debugDumpLazily(diffDelta, 1));
 
             if (diffDelta != null) {