Make single-valued properties update more robust

When trying to put multiple values to a single-value repo property
we issue a warning only. If the values are not equivalent, prism
delta application will throw a standardized exception for us.

repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/ModifyTest.java

@@ -394,6 +394,58 @@ public void test050ModifyUserEmployeeNumber() throws Exception {
         assertUserEmployeeNumber(user.getOid(), "new");
     }
 
+    @Test   // MID-4801
+    public void test055DeleteUserEmployeeNumberWrong() throws Exception {
+        final String TEST_NAME = "test055DeleteUserEmployeeNumberWrong";
+        TestUtil.displayTestTitle(TEST_NAME);
+
+        OperationResult result = new OperationResult(TEST_NAME);
+        UserType user = new UserType(prismContext)
+                .oid("055")
+                .name("user055")
+                .employeeNumber("old");
+
+        repositoryService.addObject(user.asPrismObject(), null, result);
+        assertUserEmployeeNumber(user.getOid(), "old");
+
+        List<ItemDelta<?, ?>> modifications = prismContext.deltaFor(UserType.class)
+                .item(UserType.F_EMPLOYEE_NUMBER)
+                    .delete("oldWrong")
+                .asItemDeltas();
+        repositoryService.modifyObject(UserType.class, user.getOid(), modifications, getModifyOptions(), result);
+
+        PrismObject<UserType> userAfter = repositoryService.getObject(UserType.class, user.getOid(), null, result);
+        display("user after", userAfter);
+        assertEquals("Wrong employeeNumber after operation", "old", userAfter.asObjectable().getEmployeeNumber());
+
+        assertUserEmployeeNumber(user.getOid(), "old");
+    }
+
+    @Test   // MID-4801
+    public void test056EmptyUserEmployeeNumberDelta() throws Exception {
+        final String TEST_NAME = "test056EmptyUserEmployeeNumberDelta";
+        TestUtil.displayTestTitle(TEST_NAME);
+
+        OperationResult result = new OperationResult(TEST_NAME);
+        UserType user = new UserType(prismContext)
+                .oid("056")
+                .name("user056")
+                .employeeNumber("old");
+
+        repositoryService.addObject(user.asPrismObject(), null, result);
+        assertUserEmployeeNumber(user.getOid(), "old");
+
+        List<ItemDelta<?, ?>> modifications = new ArrayList<>();
+        modifications.add(prismContext.deltaFactory().property().createDelta(UserType.F_EMPLOYEE_NUMBER, UserType.class));
+        repositoryService.modifyObject(UserType.class, user.getOid(), modifications, getModifyOptions(), result);
+
+        PrismObject<UserType> userAfter = repositoryService.getObject(UserType.class, user.getOid(), null, result);
+        display("user after", userAfter);
+        assertEquals("Wrong employeeNumber after operation", "old", userAfter.asObjectable().getEmployeeNumber());
+
+        assertUserEmployeeNumber(user.getOid(), "old");
+    }
+
     private void assertTaskOwner(String taskOid, String expectedOwnerOid) {
         Session session = open();
         //noinspection unchecked

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectDeltaUpdater.java

@@ -951,30 +951,21 @@ private void handleBasicOrEmbedded(Object bean, ItemDelta<?,?> delta, Attribute
 
         PrismValue prismValue;
         if (delta.isAdd()) {
-            Set<PrismValue> uniqueValuesToAdd = new HashSet<>(delta.getValuesToAdd());  // but what about matching rules?
-            assert !uniqueValuesToAdd.isEmpty();
-            if (uniqueValuesToAdd.size() > 1) {
-                // Maybe WARN or ERROR would be better?
-                throw new IllegalStateException("More than one value to be put into single-valued repository item. Values = "
-                        + uniqueValuesToAdd + ", attribute = " + attribute.getName());
-            }
-            prismValue = uniqueValuesToAdd.iterator().next();
-        } else if (delta.isDelete()) {
-            assert !delta.isReplace();
-            prismValue = null;
+            assert !delta.getValuesToAdd().isEmpty();
+            prismValue = getSingleValue(attribute, delta.getValuesToAdd());
         } else if (delta.isReplace()) {
-            Set<PrismValue> uniqueValuesToReplace = new HashSet<>(delta.getValuesToReplace());  // but what about matching rules?
-            if (uniqueValuesToReplace.size() > 1) {
-                // Maybe WARN or ERROR would be better?
-                throw new IllegalStateException("More than one value to be put into single-valued repository item. Values = "
-                        + uniqueValuesToReplace + ", attribute = " + attribute.getName());
-            } else if (uniqueValuesToReplace.size() == 1) {
-                prismValue = uniqueValuesToReplace.iterator().next();
+            if (!delta.getValuesToReplace().isEmpty()) {
+                prismValue = getSingleValue(attribute, delta.getValuesToReplace());
             } else {
                 prismValue = null;
             }
+        } else if (delta.isDelete()) {
+            // No values to add nor to replace, only deletions. Because we narrowed the delta before, we know that this delete
+            // delta is relevant - i.e. after its application, the (single) value of property will be removed.
+            prismValue = null;
         } else {
-            LOGGER.warn("Empty delta {} for attribute {} of {}", delta, attribute.getName(), bean.getClass().getName());
+            // This should not occur. The narrowing process eliminates empty deltas.
+            LOGGER.warn("Empty delta {} for attribute {} of {} -- skipping it", delta, attribute.getName(), bean.getClass().getName());
             return;
         }
 
@@ -990,6 +981,21 @@ private void handleBasicOrEmbedded(Object bean, ItemDelta<?,?> delta, Attribute
         }
     }
 
+    private PrismValue getSingleValue(Attribute<?,?> attribute, Collection<? extends PrismValue> valuesToSet) {
+        Set<PrismValue> uniqueValues = new HashSet<>(valuesToSet);
+        // This uniqueness check might be too strict: the values can be different from the point of default equality,
+        // yet the final verdict (when applying the delta to prism structure) can be that they are equal.
+        // Therefore we issue only a warning here.
+        // TODO We should either use the same "equals" algorithm as used for delta application, or we should apply the delta
+        //  first and here only take the result. But this is really a nitpicking now. ;)
+        if (uniqueValues.size() > 1) {
+            LOGGER.warn("More than one value to be put into single-valued repository item. This operation will probably "
+                    + "fail later because of delta execution in prism. If not, please report an error to the developers. "
+                    + "Values = {}, attribute = {}", uniqueValues, attribute);
+        }
+        return uniqueValues.iterator().next();
+    }
+
     private void handleElementCollection(Collection collection, ItemDelta delta, Attribute attribute, Object bean, PrismObject prismObject, PrismIdentifierGenerator idGenerator) {
         handleOneToMany(collection, delta, attribute, bean, prismObject, idGenerator);
     }