From 37f88338261b4bdc46f30867b369e556feaae382 Mon Sep 17 00:00:00 2001 From: Patryk Petrowski Date: Wed, 23 Nov 2016 14:02:14 +0100 Subject: [PATCH] Make NameAwareAttribute Iterable Allow faster iteration over NameAwareAttribute's values Fixes gh-429 --- .../ldap/core/DirContextAdapter.java | 108 ++++++++---------- .../ldap/core/NameAwareAttribute.java | 15 ++- .../ldap/core/NameAwareAttributes.java | 2 +- .../ldap/support/LdapUtils.java | 24 +++- 4 files changed, 78 insertions(+), 71 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 9ce525b7..b19c8276 100644 --- a/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java +++ b/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java @@ -458,8 +458,8 @@ public class DirContextAdapter implements DirContextOperations { */ private boolean isChanged(String name, Object[] values, boolean orderMatters) { - Attribute orig = originalAttrs.get(name); - Attribute prev = updatedAttrs.get(name); + NameAwareAttribute orig = originalAttrs.get(name); + NameAwareAttribute prev = updatedAttrs.get(name); // values == null and values.length == 0 is treated the same way boolean emptyNewValue = (values == null || values.length == 0); @@ -494,67 +494,43 @@ public class DirContextAdapter implements DirContextOperations { // Check contents of arrays // Order DOES matter, e.g. first names - try { - for (int i = 0; i < orig.size(); i++) { - Object obj = orig.get(i); - // TRUE if one value is not equal - if (!(obj instanceof String)) { - return true; - } - if (orderMatters) { - // check only the string with same index - if (!values[i].equals(obj)) { - return true; - } - } - else { - // check all strings - if (!ObjectUtils.containsElement(values, obj)) { - return true; - } - } - } - - } - catch (NamingException e) { - // TRUE if we can't access the value + if (isAttributeUpdated(values, orderMatters, orig)) return true; - } if (prev != null) { // Also check against updatedAttrs, since there might have been // a previous update - try { - for (int i = 0; i < prev.size(); i++) { - Object obj = prev.get(i); - // TRUE if one value is not equal - if (!(obj instanceof String)) { - return true; - } - if (orderMatters) { - // check only the string with same index - if (!values[i].equals(obj)) { - return true; - } - } - else { - // check all strings - if (!ObjectUtils.containsElement(values, obj)) { - return true; - } - } - } - - } - catch (NamingException e) { - // TRUE if we can't access the value + if (isAttributeUpdated(values, orderMatters, prev)) return true; - } } // FALSE since we have compared all values return false; } + private boolean isAttributeUpdated(Object[] values, boolean orderMatters, NameAwareAttribute orig) { + int i = 0; + for (Object obj : orig) { + // TRUE if one value is not equal + if (!(obj instanceof String)) { + return true; + } + if (orderMatters) { + // check only the string with same index + if (!values[i].equals(obj)) { + return true; + } + } + else { + // check all strings + if (!ObjectUtils.containsElement(values, obj)) { + return true; + } + } + i++; + } + return false; + } + /** * Checks if an entry has a specific attribute. * @@ -1413,7 +1389,7 @@ public class DirContextAdapter implements DirContextOperations { builder.append(" {"); try { - for (NamingEnumeration i = originalAttrs.getAll(); i.hasMore();) { + for (NamingEnumeration i = originalAttrs.getAll(); i.hasMore();) { Attribute attribute = i.next(); if (attribute.size() == 1) { builder.append(attribute.getID()); @@ -1421,15 +1397,10 @@ public class DirContextAdapter implements DirContextOperations { builder.append(attribute.get()); } else { - for (int j = 0; j < attribute.size(); j++) { - if (j > 0) { - builder.append(", "); - } - builder.append(attribute.getID()); - builder.append('['); - builder.append(j); - builder.append("]="); - builder.append(attribute.get(j)); + int j = 0; + for (Object value : (Iterable) attribute) { + appendAttributeValue(builder, attribute.getID(), value, j); + j++; } } @@ -1446,7 +1417,18 @@ public class DirContextAdapter implements DirContextOperations { return builder.toString(); } - /** + private void appendAttributeValue(StringBuilder builder, String attributeID, Object value, int index) throws NamingException { + if (index > 0) { + builder.append(", "); + } + builder.append(attributeID); + builder.append('['); + builder.append(index); + builder.append("]="); + builder.append(value); + } + + /** * {@inheritDoc} */ @Override diff --git a/core/src/main/java/org/springframework/ldap/core/NameAwareAttribute.java b/core/src/main/java/org/springframework/ldap/core/NameAwareAttribute.java index 5d00c0ac..6f9c0d76 100644 --- a/core/src/main/java/org/springframework/ldap/core/NameAwareAttribute.java +++ b/core/src/main/java/org/springframework/ldap/core/NameAwareAttribute.java @@ -40,7 +40,7 @@ import java.util.Set; * @author Mattias Hellborg Arthursson * @since 2.0 */ -public final class NameAwareAttribute implements Attribute { +public final class NameAwareAttribute implements Attribute, Iterable { private final String id; private final boolean orderMatters; @@ -219,6 +219,13 @@ public final class NameAwareAttribute implements Attribute { return orderMatters; } + /** + *

+ * Due to performance reasons it is not advised to iterate over the attribute's values using this method. + * Please use the {@link #iterator()} instead. + *

+ * {@inheritDoc} + */ @Override public Object get(int ix) throws NamingException { Iterator iterator = values.iterator(); @@ -348,4 +355,10 @@ public final class NameAwareAttribute implements Attribute { return String.format("NameAwareAttribute; id: %s; hasValuesAsNames: %s; orderMatters: %s; values: %s", id, hasValuesAsNames(), orderMatters, values); } + + @Override + public Iterator iterator() { + return values.iterator(); + } + } diff --git a/core/src/main/java/org/springframework/ldap/core/NameAwareAttributes.java b/core/src/main/java/org/springframework/ldap/core/NameAwareAttributes.java index 2c7acc6f..c2c8e574 100644 --- a/core/src/main/java/org/springframework/ldap/core/NameAwareAttributes.java +++ b/core/src/main/java/org/springframework/ldap/core/NameAwareAttributes.java @@ -69,7 +69,7 @@ public final class NameAwareAttributes implements Attributes { } @Override - public NamingEnumeration getAll() { + public NamingEnumeration getAll() { return new IterableNamingEnumeration(attributes.values()); } diff --git a/core/src/main/java/org/springframework/ldap/support/LdapUtils.java b/core/src/main/java/org/springframework/ldap/support/LdapUtils.java index 78339cb5..717db768 100644 --- a/core/src/main/java/org/springframework/ldap/support/LdapUtils.java +++ b/core/src/main/java/org/springframework/ldap/support/LdapUtils.java @@ -299,17 +299,29 @@ public final class LdapUtils { Assert.notNull(attribute, "Attribute must not be null"); Assert.notNull(callbackHandler, "callbackHandler must not be null"); - for (int i = 0; i < attribute.size(); i++) { - try { - callbackHandler.handleAttributeValue(attribute.getID(), attribute.get(i), i); + if (attribute instanceof Iterable) { + int i = 0; + for (Object obj : (Iterable) attribute) { + handleAttributeValue(attribute.getID(), obj, i, callbackHandler); + i++; } - catch (javax.naming.NamingException e) { - throw convertLdapException(e); + } + else { + for (int i = 0; i < attribute.size(); i++) { + try { + handleAttributeValue(attribute.getID(), attribute.get(i), i, callbackHandler); + } catch (javax.naming.NamingException e) { + throw convertLdapException(e); + } } } } - /** + private static void handleAttributeValue(String attributeID, Object value, int i, AttributeValueCallbackHandler callbackHandler) { + callbackHandler.handleAttributeValue(attributeID, value, i); + } + + /** * An {@link AttributeValueCallbackHandler} to collect values in a supplied * collection. *