From cfabe261e8887131cc109af2964adaebb917b809 Mon Sep 17 00:00:00 2001
From: Mattias Hellborg Arthursson <mattias@261consulting.com>
Date: Wed, 14 Aug 2013 11:17:23 +0200
Subject: [PATCH] LDAP-119: modifyAttributes produces invalid
 SchemaViolationException in a particular use case Now checking if all current
 values are deleted; if so issuing a REPLACE modification istead of individual
 DELETE and ADD.

---
 .../ldap/core/DirContextAdapter.java          | 58 ++++++++++---------
 .../ldap/LdapTemplateModifyITest.java         | 15 ++++-
 2 files changed, 46 insertions(+), 27 deletions(-)

diff --git a/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java b/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java
index 1995014a..34ffcccf 100644
--- a/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java
+++ b/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java
@@ -16,12 +16,14 @@
 
 package org.springframework.ldap.core;
 
-import java.util.ArrayList;
-import java.util.Hashtable;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.SortedSet;
-import java.util.TreeSet;
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.commons.lang.builder.EqualsBuilder;
+import org.apache.commons.lang.builder.HashCodeBuilder;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.springframework.ldap.NoSuchAttributeException;
+import org.springframework.ldap.support.LdapUtils;
+import org.springframework.util.StringUtils;
 
 import javax.naming.Context;
 import javax.naming.Name;
@@ -36,15 +38,12 @@ import javax.naming.directory.BasicAttributes;
 import javax.naming.directory.DirContext;
 import javax.naming.directory.ModificationItem;
 import javax.naming.directory.SearchControls;
-
-import org.apache.commons.lang.ArrayUtils;
-import org.apache.commons.lang.builder.EqualsBuilder;
-import org.apache.commons.lang.builder.HashCodeBuilder;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.springframework.ldap.NoSuchAttributeException;
-import org.springframework.ldap.support.LdapUtils;
-import org.springframework.util.StringUtils;
+import java.util.ArrayList;
+import java.util.Hashtable;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.SortedSet;
+import java.util.TreeSet;
 
 /**
  * Adapter that implements the interesting methods of the DirContext interface.
@@ -376,18 +375,25 @@ public class DirContextAdapter implements DirContextOperations {
 			}
 		}
 
-		// We have now traversed and removed all values from the original that
-		// were also present in the new values. The remaining values in the
-		// original must be the ones that were removed.
-		if (originalClone.size() > 0) {
-			modificationList.add(new ModificationItem(
-					DirContext.REMOVE_ATTRIBUTE, originalClone));
-		}
+        // We have now traversed and removed all values from the original that
+        // were also present in the new values. The remaining values in the
+        // original must be the ones that were removed.
+        if(originalClone.size() > 0 && originalClone.size() == originalAttr.size()) {
+            // This is actually a complete replacement of the attribute values.
+            // Fall back to REPLACE
+            modificationList.add(new ModificationItem(DirContext.REPLACE_ATTRIBUTE,
+                    addedValuesAttribute));
+        } else {
+            if (originalClone.size() > 0) {
+                modificationList.add(new ModificationItem(
+                        DirContext.REMOVE_ATTRIBUTE, originalClone));
+            }
 
-		if (addedValuesAttribute.size() > 0) {
-			modificationList.add(new ModificationItem(DirContext.ADD_ATTRIBUTE,
-					addedValuesAttribute));
-		}
+            if (addedValuesAttribute.size() > 0) {
+                modificationList.add(new ModificationItem(DirContext.ADD_ATTRIBUTE,
+                        addedValuesAttribute));
+            }
+        }
 	}
 
 	/**
diff --git a/test/integration-tests/src/test/java/org/springframework/ldap/LdapTemplateModifyITest.java b/test/integration-tests/src/test/java/org/springframework/ldap/LdapTemplateModifyITest.java
index df898afc..cc1c61da 100644
--- a/test/integration-tests/src/test/java/org/springframework/ldap/LdapTemplateModifyITest.java
+++ b/test/integration-tests/src/test/java/org/springframework/ldap/LdapTemplateModifyITest.java
@@ -275,7 +275,7 @@ public class LdapTemplateModifyITest extends AbstractLdapTemplateIntegrationTest
 	}
 
     @Test
-    public void verifyCompleteReplacementOfUniqueMemberAttribute_Ldap119() {
+    public void verifyCompleteReplacementOfUniqueMemberAttribute_Ldap119Workaround() {
         DirContextOperations ctx = tested.lookupContext("cn=ROLE_USER,ou=groups");
         ctx.setAttributeValues("uniqueMember",
                 new String[]{"cn=Some Person4,ou=company1,c=Sweden,dc=jayway,dc=se"},
@@ -285,6 +285,19 @@ public class LdapTemplateModifyITest extends AbstractLdapTemplateIntegrationTest
         tested.modifyAttributes(ctx);
     }
 
+    /**
+     * This test originally failed on ApacheDS complaining that the uniqueMember attribute
+     * was emptied.
+     */
+    @Test
+    public void verifyCompleteReplacementOfUniqueMemberAttribute_Ldap119() {
+        DirContextOperations ctx = tested.lookupContext("cn=ROLE_USER,ou=groups");
+        ctx.setAttributeValues("uniqueMember",
+                new String[]{"cn=Some Person4,ou=company1,c=Sweden,dc=jayway,dc=se"});
+        ctx.getModificationItems();
+
+        tested.modifyAttributes(ctx);
+    }
 	private Attributes setupAttributes() {
 		Attributes attributes = new BasicAttributes();
 		BasicAttribute ocattr = new BasicAttribute("objectclass");