From 95e6b17ca85bdc847ca8a015f3af2c1fda9dfbf6 Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Mon, 18 Nov 2013 16:46:45 +0000
Subject: [PATCH] Fix OPENDJ-1203: MemoryBackend does not raise an error when trying to delete non-existent attribute

---
 opendj-sdk/opendj3/opendj-core/src/main/java/org/forgerock/opendj/ldap/Entries.java               |   59 ++++++++++++++++-------------
 opendj-sdk/opendj3/opendj-core/src/test/java/org/forgerock/opendj/ldap/MemoryBackendTestCase.java |    6 +++
 2 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/opendj-sdk/opendj3/opendj-core/src/main/java/org/forgerock/opendj/ldap/Entries.java b/opendj-sdk/opendj3/opendj-core/src/main/java/org/forgerock/opendj/ldap/Entries.java
index 98b776c..2398706 100644
--- a/opendj-sdk/opendj3/opendj-core/src/main/java/org/forgerock/opendj/ldap/Entries.java
+++ b/opendj-sdk/opendj3/opendj-core/src/main/java/org/forgerock/opendj/ldap/Entries.java
@@ -32,7 +32,7 @@
 import static com.forgerock.opendj.ldap.CoreMessages.ERR_ENTRY_INCREMENT_CANNOT_PARSE_AS_INT;
 import static com.forgerock.opendj.ldap.CoreMessages.ERR_ENTRY_INCREMENT_INVALID_VALUE_COUNT;
 import static com.forgerock.opendj.ldap.CoreMessages.ERR_ENTRY_INCREMENT_NO_SUCH_ATTRIBUTE;
-import static com.forgerock.opendj.ldap.CoreMessages.ERR_ENTRY_NO_SUCH_VALUE;
+import static com.forgerock.opendj.ldap.CoreMessages.*;
 import static com.forgerock.opendj.ldap.CoreMessages.ERR_ENTRY_UNKNOWN_MODIFICATION_TYPE;
 import static org.forgerock.opendj.ldap.ErrorResultException.newErrorResult;
 
@@ -628,20 +628,7 @@
      */
     public static Entry modifyEntry(final Entry entry, final Modification change,
             final Collection<? super ByteString> conflictingValues) throws ErrorResultException {
-        final ModificationType modType = change.getModificationType();
-        if (modType.equals(ModificationType.ADD)) {
-            entry.addAttribute(change.getAttribute(), conflictingValues);
-        } else if (modType.equals(ModificationType.DELETE)) {
-            entry.removeAttribute(change.getAttribute(), conflictingValues);
-        } else if (modType.equals(ModificationType.REPLACE)) {
-            entry.replaceAttribute(change.getAttribute());
-        } else if (modType.equals(ModificationType.INCREMENT)) {
-            incrementAttribute(entry, change.getAttribute());
-        } else {
-            throw newErrorResult(ResultCode.UNWILLING_TO_PERFORM,
-                    ERR_ENTRY_UNKNOWN_MODIFICATION_TYPE.get(String.valueOf(modType)).toString());
-        }
-        return entry;
+        return modifyEntry0(entry, change, conflictingValues, true);
     }
 
     /**
@@ -768,20 +755,38 @@
         final Collection<ByteString> conflictingValues =
                 isPermissive ? null : new ArrayList<ByteString>(0);
         for (final Modification change : changes) {
-            modifyEntry(entry, change, conflictingValues);
+            modifyEntry0(entry, change, conflictingValues, isPermissive);
+        }
+        return entry;
+    }
+
+    private static Entry modifyEntry0(final Entry entry, final Modification change,
+            final Collection<? super ByteString> conflictingValues, final boolean isPermissive)
+            throws ErrorResultException {
+        final ModificationType modType = change.getModificationType();
+        if (modType.equals(ModificationType.ADD)) {
+            entry.addAttribute(change.getAttribute(), conflictingValues);
             if (!isPermissive && !conflictingValues.isEmpty()) {
-                if (change.getModificationType().equals(ModificationType.ADD)) {
-                    // Duplicate values.
-                    throw newErrorResult(ResultCode.ATTRIBUTE_OR_VALUE_EXISTS,
-                            ERR_ENTRY_DUPLICATE_VALUES.get(
-                                    change.getAttribute().getAttributeDescriptionAsString())
-                                    .toString());
-                } else {
-                    // Missing values.
-                    throw newErrorResult(ResultCode.NO_SUCH_ATTRIBUTE, ERR_ENTRY_NO_SUCH_VALUE.get(
-                            change.getAttribute().getAttributeDescriptionAsString()).toString());
-                }
+                // Duplicate values.
+                throw newErrorResult(ResultCode.ATTRIBUTE_OR_VALUE_EXISTS,
+                        ERR_ENTRY_DUPLICATE_VALUES.get(
+                                change.getAttribute().getAttributeDescriptionAsString()).toString());
             }
+        } else if (modType.equals(ModificationType.DELETE)) {
+            final boolean hasChanged =
+                    entry.removeAttribute(change.getAttribute(), conflictingValues);
+            if (!isPermissive && (!hasChanged || !conflictingValues.isEmpty())) {
+                // Missing attribute or values.
+                throw newErrorResult(ResultCode.NO_SUCH_ATTRIBUTE, ERR_ENTRY_NO_SUCH_VALUE.get(
+                        change.getAttribute().getAttributeDescriptionAsString()).toString());
+            }
+        } else if (modType.equals(ModificationType.REPLACE)) {
+            entry.replaceAttribute(change.getAttribute());
+        } else if (modType.equals(ModificationType.INCREMENT)) {
+            incrementAttribute(entry, change.getAttribute());
+        } else {
+            throw newErrorResult(ResultCode.UNWILLING_TO_PERFORM,
+                    ERR_ENTRY_UNKNOWN_MODIFICATION_TYPE.get(String.valueOf(modType)).toString());
         }
         return entry;
     }
diff --git a/opendj-sdk/opendj3/opendj-core/src/test/java/org/forgerock/opendj/ldap/MemoryBackendTestCase.java b/opendj-sdk/opendj3/opendj-core/src/test/java/org/forgerock/opendj/ldap/MemoryBackendTestCase.java
index e7cb053..6b729c1 100644
--- a/opendj-sdk/opendj3/opendj-core/src/test/java/org/forgerock/opendj/ldap/MemoryBackendTestCase.java
+++ b/opendj-sdk/opendj3/opendj-core/src/test/java/org/forgerock/opendj/ldap/MemoryBackendTestCase.java
@@ -336,6 +336,12 @@
         connection.modify("dn: dc=example,dc=com", "changetype: modify", "delete: dc", "dc: xxx");
     }
 
+    @Test(expectedExceptions = ConstraintViolationException.class)
+    public void testModifyStrictWithMissingAttribute() throws Exception {
+        final Connection connection = getConnection();
+        connection.modify("dn: dc=example,dc=com", "changetype: modify", "delete: cn");
+    }
+
     @Test
     public void testSearchAttributesOperational() throws Exception {
         final Connection connection = getConnection();

--
Gitblit v1.10.0