diff --git a/changelog.txt b/changelog.txt index a6d976b1..d65e644f 100644 --- a/changelog.txt +++ b/changelog.txt @@ -21,6 +21,10 @@ Changes in version 1.3.0 (Nov 2008) * Re-used the same fix for LDAP-109 and LDAP-50 that was used in DefaultDirObjectFactory to secure the DirContextAdapter constructors from invalid CompositeNames. +* Made sure DirContextOperations#addAttributeValue(String, Object) does not add any duplicate + values per default; added alternate method: DirContextOperations#addAttributeValue(String, Object, boolean) + to enable behavior other than the default (i.e. allowing duplicates). + Changes in version 1.3.0.RC1 (Oct 2008) ------------------------------------------- * TLS connections are now supported using the DefaultTlsDirContextAuthenticationStrategy 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 4358c2c3..e3b8e098 100644 --- a/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java +++ b/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java @@ -77,6 +77,8 @@ import org.springframework.util.StringUtils; */ public class DirContextAdapter implements DirContextOperations { + private static final boolean DONT_ADD_IF_DUPLICATE_EXISTS = false; + private static final String EMPTY_STRING = ""; private static final boolean ORDER_DOESNT_MATTER = false; @@ -141,7 +143,8 @@ public class DirContextAdapter implements DirContextOperations { * @param referralUrl the referral url (if this instance results from a * referral). */ - public DirContextAdapter(Attributes attrs, Name dn, Name base, String referralUrl) { + public DirContextAdapter(Attributes attrs, Name dn, Name base, + String referralUrl) { if (attrs != null) { this.originalAttrs = attrs; } @@ -219,7 +222,8 @@ public class DirContextAdapter implements DirContextOperations { try { while (attributesEnumeration.hasMore()) { - Attribute oneAttribute = (Attribute) attributesEnumeration.next(); + Attribute oneAttribute = (Attribute) attributesEnumeration + .next(); tmpList.add(oneAttribute.getID()); } } @@ -276,7 +280,8 @@ public class DirContextAdapter implements DirContextOperations { log.debug("Number of modifications:" + tmpList.size()); } - return (ModificationItem[]) tmpList.toArray(new ModificationItem[tmpList.size()]); + return (ModificationItem[]) tmpList + .toArray(new ModificationItem[tmpList.size()]); } /** @@ -293,30 +298,37 @@ public class DirContextAdapter implements DirContextOperations { * @param modificationList the list in which to add the modifications. * @throws NamingException if thrown by called Attribute methods. */ - private void collectModifications(Attribute changedAttr, List modificationList) throws NamingException { + private void collectModifications(Attribute changedAttr, + List modificationList) throws NamingException { Attribute currentAttribute = originalAttrs.get(changedAttr.getID()); if (changedAttr.equals(currentAttribute)) { // No changes return; } - else if (currentAttribute != null && currentAttribute.size() == 1 && changedAttr.size() == 1) { + else if (currentAttribute != null && currentAttribute.size() == 1 + && changedAttr.size() == 1) { // Replace single-vale attribute. - modificationList.add(new ModificationItem(DirContext.REPLACE_ATTRIBUTE, changedAttr)); + modificationList.add(new ModificationItem( + DirContext.REPLACE_ATTRIBUTE, changedAttr)); } else if (changedAttr.size() == 0 && currentAttribute != null) { // Attribute has been removed. - modificationList.add(new ModificationItem(DirContext.REMOVE_ATTRIBUTE, changedAttr)); + modificationList.add(new ModificationItem( + DirContext.REMOVE_ATTRIBUTE, changedAttr)); } - else if ((currentAttribute == null || currentAttribute.size() == 0) && changedAttr.size() > 0) { + else if ((currentAttribute == null || currentAttribute.size() == 0) + && changedAttr.size() > 0) { // Attribute has been added. - modificationList.add(new ModificationItem(DirContext.ADD_ATTRIBUTE, changedAttr)); + modificationList.add(new ModificationItem(DirContext.ADD_ATTRIBUTE, + changedAttr)); } else if (changedAttr.size() > 0 && changedAttr.isOrdered()) { // This is a multivalue attribute and it is ordered - the original // value should be replaced with the new values so that the ordering // is preserved. - modificationList.add(new ModificationItem(DirContext.REPLACE_ATTRIBUTE, changedAttr)); + modificationList.add(new ModificationItem( + DirContext.REPLACE_ATTRIBUTE, changedAttr)); } else if (changedAttr.size() > 0) { // Change of multivalue Attribute. Collect additions and removals @@ -328,18 +340,21 @@ public class DirContextAdapter implements DirContextOperations { // This means that the attributes are not equal, but the // actual values are the same - thus the order must have // changed. This should result in a REPLACE_ATTRIBUTE operation. - myModifications.add(new ModificationItem(DirContext.REPLACE_ATTRIBUTE, changedAttr)); + myModifications.add(new ModificationItem( + DirContext.REPLACE_ATTRIBUTE, changedAttr)); } modificationList.addAll(myModifications); } } - private void collectModifications(Attribute originalAttr, Attribute changedAttr, List modificationList) + private void collectModifications(Attribute originalAttr, + Attribute changedAttr, List modificationList) throws NamingException { Attribute originalClone = (Attribute) originalAttr.clone(); - Attribute addedValuesAttribute = new BasicAttribute(originalAttr.getID()); + Attribute addedValuesAttribute = new BasicAttribute(originalAttr + .getID()); for (int i = 0; i < changedAttr.size(); i++) { Object attributeValue = changedAttr.get(i); @@ -352,11 +367,13 @@ public class DirContextAdapter implements DirContextOperations { // 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)); + modificationList.add(new ModificationItem( + DirContext.REMOVE_ATTRIBUTE, originalClone)); } if (addedValuesAttribute.size() > 0) { - modificationList.add(new ModificationItem(DirContext.ADD_ATTRIBUTE, addedValuesAttribute)); + modificationList.add(new ModificationItem(DirContext.ADD_ATTRIBUTE, + addedValuesAttribute)); } } @@ -572,6 +589,11 @@ public class DirContextAdapter implements DirContextOperations { * java.lang.String, java.lang.Object) */ public void addAttributeValue(String name, Object value) { + addAttributeValue(name, value, DONT_ADD_IF_DUPLICATE_EXISTS); + } + + public void addAttributeValue(String name, Object value, + boolean addIfDuplicateExists) { if (!updateMode && value != null) { Attribute attr = originalAttrs.get(name); if (attr == null) { @@ -593,7 +615,9 @@ public class DirContextAdapter implements DirContextOperations { // The attribute exists in the original attributes - clone // that and add the new entry to it attr = (Attribute) originalAttrs.get(name).clone(); - attr.add(value); + if (addIfDuplicateExists || !attr.contains(value)) { + attr.add(value); + } updatedAttrs.put(attr); } } @@ -601,8 +625,6 @@ public class DirContextAdapter implements DirContextOperations { attr.add(value); } } - - // Null values will not be added } /* @@ -651,7 +673,8 @@ public class DirContextAdapter implements DirContextOperations { * org.springframework.ldap.support.DirContextOperations#setAttributeValues * (java.lang.String, java.lang.Object[], boolean) */ - public void setAttributeValues(String name, Object[] values, boolean orderMatters) { + public void setAttributeValues(String name, Object[] values, + boolean orderMatters) { Attribute a = new BasicAttribute(name, orderMatters); for (int i = 0; values != null && i < values.length; i++) { @@ -711,7 +734,8 @@ public class DirContextAdapter implements DirContextOperations { */ public String[] getStringAttributes(String name) { try { - return (String[]) collectAttributeValuesAsList(name).toArray(new String[0]); + return (String[]) collectAttributeValuesAsList(name).toArray( + new String[0]); } catch (NoSuchAttributeException e) { // The attribute does not exist - contract says to return null. @@ -801,14 +825,16 @@ public class DirContextAdapter implements DirContextOperations { /** * @see javax.naming.directory.DirContext#getAttributes(Name, String[]) */ - public Attributes getAttributes(Name name, String[] attrIds) throws NamingException { + public Attributes getAttributes(Name name, String[] attrIds) + throws NamingException { return getAttributes(name.toString(), attrIds); } /** * @see javax.naming.directory.DirContext#getAttributes(String, String[]) */ - public Attributes getAttributes(String name, String[] attrIds) throws NamingException { + public Attributes getAttributes(String name, String[] attrIds) + throws NamingException { if (StringUtils.hasLength(name)) { throw new NameNotFoundException(); } @@ -829,7 +855,8 @@ public class DirContextAdapter implements DirContextOperations { * @see javax.naming.directory.DirContext#modifyAttributes(javax.naming.Name, * int, javax.naming.directory.Attributes) */ - public void modifyAttributes(Name name, int modOp, Attributes attrs) throws NamingException { + public void modifyAttributes(Name name, int modOp, Attributes attrs) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -837,7 +864,8 @@ public class DirContextAdapter implements DirContextOperations { * @see javax.naming.directory.DirContext#modifyAttributes(String, int, * Attributes) */ - public void modifyAttributes(String name, int modOp, Attributes attrs) throws NamingException { + public void modifyAttributes(String name, int modOp, Attributes attrs) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -845,7 +873,8 @@ public class DirContextAdapter implements DirContextOperations { * @see javax.naming.directory.DirContext#modifyAttributes(Name, * ModificationItem[]) */ - public void modifyAttributes(Name name, ModificationItem[] mods) throws NamingException { + public void modifyAttributes(Name name, ModificationItem[] mods) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -853,42 +882,48 @@ public class DirContextAdapter implements DirContextOperations { * @see javax.naming.directory.DirContext#modifyAttributes(String, * ModificationItem[]) */ - public void modifyAttributes(String name, ModificationItem[] mods) throws NamingException { + public void modifyAttributes(String name, ModificationItem[] mods) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } /** * @see javax.naming.directory.DirContext#bind(Name, Object, Attributes) */ - public void bind(Name name, Object obj, Attributes attrs) throws NamingException { + public void bind(Name name, Object obj, Attributes attrs) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } /** * @see javax.naming.directory.DirContext#bind(String, Object, Attributes) */ - public void bind(String name, Object obj, Attributes attrs) throws NamingException { + public void bind(String name, Object obj, Attributes attrs) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } /** * @see javax.naming.directory.DirContext#rebind(Name, Object, Attributes) */ - public void rebind(Name name, Object obj, Attributes attrs) throws NamingException { + public void rebind(Name name, Object obj, Attributes attrs) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } /** * @see javax.naming.directory.DirContext#rebind(String, Object, Attributes) */ - public void rebind(String name, Object obj, Attributes attrs) throws NamingException { + public void rebind(String name, Object obj, Attributes attrs) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } /** * @see javax.naming.directory.DirContext#createSubcontext(Name, Attributes) */ - public DirContext createSubcontext(Name name, Attributes attrs) throws NamingException { + public DirContext createSubcontext(Name name, Attributes attrs) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -896,7 +931,8 @@ public class DirContextAdapter implements DirContextOperations { * @see javax.naming.directory.DirContext#createSubcontext(String, * Attributes) */ - public DirContext createSubcontext(String name, Attributes attrs) throws NamingException { + public DirContext createSubcontext(String name, Attributes attrs) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -917,22 +953,24 @@ public class DirContextAdapter implements DirContextOperations { /** * @see javax.naming.directory.DirContext#getSchemaClassDefinition(Name) */ - public DirContext getSchemaClassDefinition(Name name) throws NamingException { + public DirContext getSchemaClassDefinition(Name name) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } /** * @see javax.naming.directory.DirContext#getSchemaClassDefinition(String) */ - public DirContext getSchemaClassDefinition(String name) throws NamingException { + public DirContext getSchemaClassDefinition(String name) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } /** * @see javax.naming.directory.DirContext#search(Name, Attributes, String[]) */ - public NamingEnumeration search(Name name, Attributes matchingAttributes, String[] attributesToReturn) - throws NamingException { + public NamingEnumeration search(Name name, Attributes matchingAttributes, + String[] attributesToReturn) throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -940,22 +978,24 @@ public class DirContextAdapter implements DirContextOperations { * @see javax.naming.directory.DirContext#search(String, Attributes, * String[]) */ - public NamingEnumeration search(String name, Attributes matchingAttributes, String[] attributesToReturn) - throws NamingException { + public NamingEnumeration search(String name, Attributes matchingAttributes, + String[] attributesToReturn) throws NamingException { throw new UnsupportedOperationException("Not implemented."); } /** * @see javax.naming.directory.DirContext#search(Name, Attributes) */ - public NamingEnumeration search(Name name, Attributes matchingAttributes) throws NamingException { + public NamingEnumeration search(Name name, Attributes matchingAttributes) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } /** * @see javax.naming.directory.DirContext#search(String, Attributes) */ - public NamingEnumeration search(String name, Attributes matchingAttributes) throws NamingException { + public NamingEnumeration search(String name, Attributes matchingAttributes) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -963,7 +1003,8 @@ public class DirContextAdapter implements DirContextOperations { * @see javax.naming.directory.DirContext#search(Name, String, * SearchControls) */ - public NamingEnumeration search(Name name, String filter, SearchControls cons) throws NamingException { + public NamingEnumeration search(Name name, String filter, + SearchControls cons) throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -971,7 +1012,8 @@ public class DirContextAdapter implements DirContextOperations { * @see javax.naming.directory.DirContext#search(String, String, * SearchControls) */ - public NamingEnumeration search(String name, String filter, SearchControls cons) throws NamingException { + public NamingEnumeration search(String name, String filter, + SearchControls cons) throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -979,8 +1021,8 @@ public class DirContextAdapter implements DirContextOperations { * @see javax.naming.directory.DirContext#search(Name, String, Object[], * SearchControls) */ - public NamingEnumeration search(Name name, String filterExpr, Object[] filterArgs, SearchControls cons) - throws NamingException { + public NamingEnumeration search(Name name, String filterExpr, + Object[] filterArgs, SearchControls cons) throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -988,8 +1030,8 @@ public class DirContextAdapter implements DirContextOperations { * @see javax.naming.directory.DirContext#search(String, String, Object[], * SearchControls) */ - public NamingEnumeration search(String name, String filterExpr, Object[] filterArgs, SearchControls cons) - throws NamingException { + public NamingEnumeration search(String name, String filterExpr, + Object[] filterArgs, SearchControls cons) throws NamingException { throw new UnsupportedOperationException("Not implemented."); } @@ -1157,14 +1199,16 @@ public class DirContextAdapter implements DirContextOperations { /** * @see javax.naming.Context#composeName(String, String) */ - public String composeName(String name, String prefix) throws NamingException { + public String composeName(String name, String prefix) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } /** * @see javax.naming.Context#addToEnvironment(String, Object) */ - public Object addToEnvironment(String propName, Object propVal) throws NamingException { + public Object addToEnvironment(String propName, Object propVal) + throws NamingException { throw new UnsupportedOperationException("Not implemented."); } diff --git a/core/src/main/java/org/springframework/ldap/core/DirContextOperations.java b/core/src/main/java/org/springframework/ldap/core/DirContextOperations.java index a240194c..c748f7d8 100644 --- a/core/src/main/java/org/springframework/ldap/core/DirContextOperations.java +++ b/core/src/main/java/org/springframework/ldap/core/DirContextOperations.java @@ -28,7 +28,8 @@ import javax.naming.directory.DirContext; * @author Mattias Hellborg Arthursson * @see DirContextAdapter */ -public interface DirContextOperations extends DirContext, AttributeModificationsAware { +public interface DirContextOperations extends DirContext, + AttributeModificationsAware { /** * Gets the update mode. An entry in update mode will keep track of its @@ -114,7 +115,9 @@ public interface DirContextOperations extends DirContext, AttributeModifications /** * Add a value to the Attribute with the specified name. If the Attribute - * doesn't exist it will be created. + * doesn't exist it will be created. This method makes sure that the there + * will be no duplicates of an added value - it the value exists it will not + * be added again. * * @param name the name of the Attribute to which the specified value should * be added. @@ -122,6 +125,24 @@ public interface DirContextOperations extends DirContext, AttributeModifications */ void addAttributeValue(String name, Object value); + /** + * Add a value to the Attribute with the specified name. If the Attribute + * doesn't exist it will be created. The addIfDuplicateExists + * parameter controls the handling of duplicates. It false, + * this method makes sure that the there will be no duplicates of an added + * value - it the value exists it will not be added again. + * + * @param name the name of the Attribute to which the specified value should + * be added. + * @param value the Attribute value to add. + * @param addIfDuplicateExists true will add the value + * regardless of whether there is an identical value already, allowing for + * duplicate attribute values; false will not add the value if + * it already exists. + */ + void addAttributeValue(String name, Object value, + boolean addIfDuplicateExists); + /** * Remove a value from the Attribute with the specified name. If the * Attribute doesn't exist, do nothing. diff --git a/core/src/test/java/org/springframework/ldap/core/DirContextAdapterTest.java b/core/src/test/java/org/springframework/ldap/core/DirContextAdapterTest.java index 61816a9d..ce4b925a 100644 --- a/core/src/test/java/org/springframework/ldap/core/DirContextAdapterTest.java +++ b/core/src/test/java/org/springframework/ldap/core/DirContextAdapterTest.java @@ -207,7 +207,7 @@ public class DirContextAdapterTest extends TestCase { public void testAddAttributeValueAttributeWithSameValueExists() throws NamingException { tested.setAttribute(new BasicAttribute("abc", "123")); - + // Perform test tested.addAttributeValue("abc", "123"); @@ -232,7 +232,7 @@ public class DirContextAdapterTest extends TestCase { assertEquals("123", attribute.get()); } - public void testAddAttributeValueInUpdateModeAttributeWithOtherValueExistsInOrigAttrs() throws NamingException { + public void testAddAttributeValueInUpdateModeAttributeWhenOtherValueExistsInOrigAttrs() throws NamingException { tested.setAttribute(new BasicAttribute("abc", "321")); tested.setUpdateMode(true); @@ -251,7 +251,7 @@ public class DirContextAdapterTest extends TestCase { assertEquals("123", attribute.get()); } - public void testAddAttributeValueInUpdateModeAttributeWithSameValueExistsInOrigAttrs() throws NamingException { + public void testGetModificationItemsOnAddAttributeValueInUpdateModeAttributeWhenSameValueExistsInOrigAttrs() throws NamingException { tested.setAttribute(new BasicAttribute("abc", "123")); tested.setUpdateMode(true); 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 24d123c8..477927b3 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 @@ -17,6 +17,7 @@ package org.springframework.ldap; import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertTrue; import static junit.framework.Assert.fail; import javax.naming.directory.Attributes; @@ -30,6 +31,7 @@ import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.ldap.core.DirContextAdapter; +import org.springframework.ldap.core.DirContextOperations; import org.springframework.ldap.core.DistinguishedName; import org.springframework.ldap.core.LdapTemplate; import org.springframework.test.context.ContextConfiguration; @@ -134,6 +136,14 @@ public class LdapTemplateModifyITest extends AbstractLdapTemplateIntegrationTest assertEquals("Another description", attributes[2]); } + @Test + public void testModifyAttributes_AddAttributeValueWithExistingValue() { + DirContextOperations ctx = tested.lookupContext("cn=ROLE_USER,ou=groups"); + ctx.addAttributeValue("uniqueMember", "cn=Some Person,ou=company1,c=Sweden,dc=jayway,dc=se"); + tested.modifyAttributes(ctx); + assertTrue(true); + } + @Test public void testModifyAttributes_MultiValueAddDuplicateToUnordered() { BasicAttribute attr = new BasicAttribute("description", "Some description");