Fix for LDAP-13 and LDAP-46: Now uses the original Attribute to verify equality between values.

This should enable the Attribute implementation to use schema-specific information when comparing Attribute values.
2007-04-20 09:03:07 +00:00
parent 65c02867d6
commit 96fa2e3afa
3 changed files with 87 additions and 100 deletions
--- a/spring-ldap/.classpath
+++ b/spring-ldap/.classpath
@@ -1,55 +1,54 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<classpath>
-	<classpathentry kind="src" path="src/main/java"/>
-	<classpathentry kind="src" path="target/gen-java-src"/>
-	<classpathentry kind="src" path="src/test/java"/>
-	<classpathentry kind="src" path="src/itest/java"/>
-	<classpathentry kind="src" path="src/itest-openldap/java"/>
-	<classpathentry kind="src" path="src/test/resources"/>
-	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
-	<classpathentry kind="lib" path="lib/global/commons-logging.jar" sourcepath="lib/source/commons-logging-src.zip"/>
-	<classpathentry kind="lib" path="lib/global/spring-beans.jar" sourcepath="lib/source/spring-src.zip"/>
-	<classpathentry kind="lib" path="lib/global/spring-context.jar" sourcepath="lib/source/spring-src.zip"/>
-	<classpathentry kind="lib" path="lib/global/spring-core.jar" sourcepath="lib/source/spring-src.zip"/>
-	<classpathentry kind="lib" path="lib/global/spring-dao.jar" sourcepath="lib/source/spring-src.zip"/>
-	<classpathentry kind="lib" path="lib/test/spring-mock.jar" sourcepath="lib/source/spring-src.zip"/>
-	<classpathentry kind="lib" path="lib/test/gsbase.jar"/>
-	<classpathentry kind="lib" path="lib/global/commons-lang.jar" sourcepath="lib/source/commons-lang-src.zip"/>
-	<classpathentry kind="lib" path="lib/global/commons-collections.jar" sourcepath="lib/source/commons-collections-src.zip"/>
-	<classpathentry kind="lib" path="lib/test/junit.jar" sourcepath="lib/source/junit-src.zip"/>
-	<classpathentry kind="lib" path="lib/test/easymock.jar" sourcepath="lib/source/easymock-src.zip"/>
-	<classpathentry kind="lib" path="lib/test/clover.jar"/>
-	<classpathentry kind="lib" path="lib/test/ehcache.jar"/>
-	<classpathentry kind="lib" path="lib/test/ehcache-constructs.jar"/>
-	<classpathentry kind="lib" path="lib/test/jxl.jar"/>
-	<classpathentry kind="lib" path="lib/test/cglib-nodep.jar"/>
-	<classpathentry kind="lib" path="lib/buildtime/acegi-security.jar" sourcepath="lib/source/acegi-security-src.zip"/>
-	<classpathentry kind="lib" path="lib/test/antlr.jar"/>
-	<classpathentry kind="lib" path="lib/test/apacheds-core.jar"/>
-	<classpathentry kind="lib" path="lib/test/apacheds-core-shared.jar"/>
-	<classpathentry kind="lib" path="lib/test/jdbm.jar"/>
-	<classpathentry kind="lib" path="lib/test/shared-asn1.jar"/>
-	<classpathentry kind="lib" path="lib/test/shared-ldap.jar"/>
-	<classpathentry kind="lib" path="lib/test/slf4j-log4j12.jar"/>
-	<classpathentry kind="lib" path="lib/test/log4j.jar"/>
-	<classpathentry kind="lib" path="lib/test/apacheds-server-jndi.jar"/>
-	<classpathentry kind="lib" path="lib/test/apacheds-protocol-shared.jar"/>
-	<classpathentry kind="lib" path="lib/test/apacheds-protocol-changepw.jar"/>
-	<classpathentry kind="lib" path="lib/test/apacheds-protocol-kerberos.jar"/>
-	<classpathentry kind="lib" path="lib/test/apacheds-kerberos-shared.jar"/>
-	<classpathentry kind="lib" path="lib/test/mina-core.jar"/>
-	<classpathentry kind="lib" path="lib/test/mina-filter-ssl.jar"/>
-	<classpathentry kind="lib" path="lib/test/apacheds-server-main.jar"/>
-	<classpathentry kind="lib" path="lib/test/apacheds-protocol-ldap.jar"/>
-	<classpathentry kind="lib" path="lib/test/junit-addons.jar"/>
-	<classpathentry kind="lib" path="lib/test/DDSteps.jar"/>
-	<classpathentry kind="lib" path="lib/global/ldapbp.jar"/>
-	<classpathentry kind="lib" path="lib/test/junitperf.jar"/>
-	<classpathentry kind="lib" path="lib/test/backport-util-concurrent.jar"/>
-	<classpathentry kind="lib" path="lib/test/shared-asn1-codec.jar"/>
-	<classpathentry kind="lib" path="lib/buildtime/spring-jdbc.jar" sourcepath="lib/source/spring-src.zip"/>
-	<classpathentry kind="lib" path="lib/test/aopalliance.jar"/>
-	<classpathentry kind="lib" path="lib/test/spring-aop.jar" sourcepath="lib/source/spring-src.zip"/>
-	<classpathentry kind="lib" path="lib/test/hsqldb.jar"/>
-	<classpathentry kind="output" path="bin"/>
-</classpath>
+<?xml version="1.0" encoding="UTF-8"?>
+<classpath>
+	<classpathentry kind="src" path="src/main/java"/>
+	<classpathentry kind="src" path="target/gen-java-src"/>
+	<classpathentry kind="src" path="src/test/java"/>
+	<classpathentry kind="src" path="src/itest/java"/>
+	<classpathentry kind="src" path="src/itest-openldap/java"/>
+	<classpathentry kind="src" path="src/test/resources"/>
+	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER"/>
+	<classpathentry kind="lib" path="lib/global/commons-logging.jar" sourcepath="lib/source/commons-logging-src.zip"/>
+	<classpathentry kind="lib" path="lib/global/spring-beans.jar" sourcepath="lib/source/spring-src.zip"/>
+	<classpathentry kind="lib" path="lib/global/spring-context.jar" sourcepath="lib/source/spring-src.zip"/>
+	<classpathentry kind="lib" path="lib/global/spring-core.jar" sourcepath="lib/source/spring-src.zip"/>
+	<classpathentry kind="lib" path="lib/global/spring-dao.jar" sourcepath="lib/source/spring-src.zip"/>
+	<classpathentry kind="lib" path="lib/test/spring-mock.jar" sourcepath="lib/source/spring-src.zip"/>
+	<classpathentry kind="lib" path="lib/test/gsbase.jar"/>
+	<classpathentry kind="lib" path="lib/global/commons-lang.jar" sourcepath="lib/source/commons-lang-src.zip"/>
+	<classpathentry kind="lib" path="lib/test/junit.jar" sourcepath="lib/source/junit-src.zip"/>
+	<classpathentry kind="lib" path="lib/test/easymock.jar" sourcepath="lib/source/easymock-src.zip"/>
+	<classpathentry kind="lib" path="lib/test/clover.jar"/>
+	<classpathentry kind="lib" path="lib/test/ehcache.jar"/>
+	<classpathentry kind="lib" path="lib/test/ehcache-constructs.jar"/>
+	<classpathentry kind="lib" path="lib/test/jxl.jar"/>
+	<classpathentry kind="lib" path="lib/test/cglib-nodep.jar"/>
+	<classpathentry kind="lib" path="lib/buildtime/acegi-security.jar" sourcepath="lib/source/acegi-security-src.zip"/>
+	<classpathentry kind="lib" path="lib/test/antlr.jar"/>
+	<classpathentry kind="lib" path="lib/test/apacheds-core.jar"/>
+	<classpathentry kind="lib" path="lib/test/apacheds-core-shared.jar"/>
+	<classpathentry kind="lib" path="lib/test/jdbm.jar"/>
+	<classpathentry kind="lib" path="lib/test/shared-asn1.jar"/>
+	<classpathentry kind="lib" path="lib/test/shared-ldap.jar"/>
+	<classpathentry kind="lib" path="lib/test/slf4j-log4j12.jar"/>
+	<classpathentry kind="lib" path="lib/test/log4j.jar"/>
+	<classpathentry kind="lib" path="lib/test/apacheds-server-jndi.jar"/>
+	<classpathentry kind="lib" path="lib/test/apacheds-protocol-shared.jar"/>
+	<classpathentry kind="lib" path="lib/test/apacheds-protocol-changepw.jar"/>
+	<classpathentry kind="lib" path="lib/test/apacheds-protocol-kerberos.jar"/>
+	<classpathentry kind="lib" path="lib/test/apacheds-kerberos-shared.jar"/>
+	<classpathentry kind="lib" path="lib/test/mina-core.jar"/>
+	<classpathentry kind="lib" path="lib/test/mina-filter-ssl.jar"/>
+	<classpathentry kind="lib" path="lib/test/apacheds-server-main.jar"/>
+	<classpathentry kind="lib" path="lib/test/apacheds-protocol-ldap.jar"/>
+	<classpathentry kind="lib" path="lib/test/junit-addons.jar"/>
+	<classpathentry kind="lib" path="lib/test/DDSteps.jar"/>
+	<classpathentry kind="lib" path="lib/global/ldapbp.jar"/>
+	<classpathentry kind="lib" path="lib/test/junitperf.jar"/>
+	<classpathentry kind="lib" path="lib/test/backport-util-concurrent.jar"/>
+	<classpathentry kind="lib" path="lib/test/shared-asn1-codec.jar"/>
+	<classpathentry kind="lib" path="lib/buildtime/spring-jdbc.jar" sourcepath="lib/source/spring-src.zip"/>
+	<classpathentry kind="lib" path="lib/test/aopalliance.jar"/>
+	<classpathentry kind="lib" path="lib/test/spring-aop.jar" sourcepath="lib/source/spring-src.zip"/>
+	<classpathentry kind="lib" path="lib/test/hsqldb.jar"/>
+	<classpathentry kind="output" path="bin"/>
+</classpath>
--- a/spring-ldap/ivy.xml
+++ b/spring-ldap/ivy.xml
@@ -21,7 +21,6 @@
 	<!--  global (core) dependencies -->
 	<dependency org="commons-logging" name="commons-logging" rev="1.0.4" />
 	<dependency org="commons-lang" name="commons-lang" rev="2.1" />
-	<dependency org="commons-collections" name="commons-collections" rev="3.1" />
 	<dependency org="org.springframework" name="spring-beans" rev="2.0.4"/>
 	<dependency org="org.springframework" name="spring-core" rev="2.0.4"/>
 	<dependency org="org.springframework" name="spring-context" rev="2.0.4"/>
--- a/spring-ldap/src/main/java/org/springframework/ldap/core/DirContextAdapter.java
+++ b/spring-ldap/src/main/java/org/springframework/ldap/core/DirContextAdapter.java
@@ -17,9 +17,7 @@
 package org.springframework.ldap.core;

 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.Hashtable;
-import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.SortedSet;
@@ -39,7 +37,6 @@ import javax.naming.directory.DirContext;
 import javax.naming.directory.ModificationItem;
 import javax.naming.directory.SearchControls;

-import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.lang.ArrayUtils;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.builder.EqualsBuilder;
@@ -274,25 +271,16 @@ public class DirContextAdapter implements DirContextOperations {
            // Attribute has been removed.
            modificationList.add(new ModificationItem(
                    DirContext.REMOVE_ATTRIBUTE, changedAttr));
-        } else if (changedAttr.size() != 0) {
-            // Collect all modifications to attribute individually (this also
-            // covers additions to a previously non-existant attribute).
-            Collection oldValues = new LinkedList();
-            Collection newValues = new LinkedList();
-
-            collectAttributeValues(oldValues, currentAttribute);
-            collectAttributeValues(newValues, changedAttr);
-            Collection myModifications = new LinkedList();
-
-            Collection addedValues = CollectionUtils.subtract(newValues,
-                    oldValues);
-            Collection removedValues = CollectionUtils.subtract(oldValues,
-                    newValues);
-
-            collectModifications(DirContext.REMOVE_ATTRIBUTE, changedAttr,
-                    removedValues, myModifications);
-            collectModifications(DirContext.ADD_ATTRIBUTE, changedAttr,
-                    addedValues, myModifications);
+        } else if ((currentAttribute == null || currentAttribute.size() == 0)
+                && changedAttr.size() > 0) {
+            // Attribute has been added.
+            modificationList.add(new ModificationItem(DirContext.ADD_ATTRIBUTE,
+                    changedAttr));
+        } else if (changedAttr.size() > 0) {
+            // Change of multivalue Attribute. Collect additions and removals
+            // individually.
+            List myModifications = new LinkedList();
+            collectModifications(currentAttribute, changedAttr, myModifications);

            if (myModifications.isEmpty()) {
                // This means that the attributes are not equal, but the
@@ -306,31 +294,32 @@ public class DirContextAdapter implements DirContextOperations {
        }
    }

-    private void collectModifications(int modificationType, Attribute attr,
-            Collection values, Collection c) {
-        if (values.size() > 0) {
-            BasicAttribute modificationAttribute = new BasicAttribute(attr
-                    .getID());
-            for (Iterator iter = values.iterator(); iter.hasNext();) {
-                modificationAttribute.add(iter.next());
+    private void collectModifications(Attribute originalAttr,
+            Attribute changedAttr, List modificationList)
+            throws NamingException {
+
+        Attribute originalClone = (Attribute) originalAttr.clone();
+        Attribute addedValuesAttribute = new BasicAttribute(originalAttr
+                .getID());
+
+        for (int i = 0; i < changedAttr.size(); i++) {
+            Object attributeValue = changedAttr.get(i);
+            if (!originalClone.remove(attributeValue)) {
+                addedValuesAttribute.add(attributeValue);
            }
-            c
-                    .add(new ModificationItem(modificationType,
-                            modificationAttribute));
-        }
-    }
-
-    private void collectAttributeValues(Collection valueCollection,
-            Attribute attribute) throws NamingException {
-
-        if (attribute == null) {
-            return;
        }

-        NamingEnumeration attributeValues = attribute.getAll();
-        while (attributeValues.hasMoreElements()) {
-            Object value = (Object) attributeValues.nextElement();
-            valueCollection.add(value);
+        // 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));
+        }
+
+        if (addedValuesAttribute.size() > 0) {
+            modificationList.add(new ModificationItem(DirContext.ADD_ATTRIBUTE,
+                    addedValuesAttribute));
        }
    }