From e1404672e50c8f6b3c80b522a1988c9827c251e8 Mon Sep 17 00:00:00 2001
From: Jean-Noël Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Tue, 22 Sep 2015 11:50:17 +0000
Subject: [PATCH] Improve AttributeDescription: now implements Comparable, align with SDK

---
 opendj-server-legacy/src/main/java/org/opends/server/types/Attribute.java                             |   72 ++++----
 opendj-server-legacy/src/test/java/org/opends/server/types/AttributeBuilderTest.java                  |    9 
 opendj-server-legacy/src/main/java/org/opends/server/types/AbstractAttribute.java                     |   62 +------
 opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/EntryHistorical.java          |   32 +--
 opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/HistoricalAttributeValue.java |   22 ++
 opendj-server-legacy/src/main/java/org/opends/server/types/AttributeDescription.java                  |  259 ++++++++++++++++++++++++++++++--
 6 files changed, 319 insertions(+), 137 deletions(-)

diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/EntryHistorical.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/EntryHistorical.java
index 7adbf56..49a0de2 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/EntryHistorical.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/EntryHistorical.java
@@ -61,6 +61,8 @@
  */
 public class EntryHistorical
 {
+  private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
+
   /** Name of the attribute used to store historical information. */
   public static final String HISTORICAL_ATTRIBUTE_NAME = "ds-sync-hist";
   /**
@@ -72,8 +74,6 @@
   /** Name of the entryuuid attribute. */
   public static final String ENTRYUUID_ATTRIBUTE_NAME = "entryuuid";
 
-  private static final LocalizedLogger logger = LocalizedLogger.getLoggerForThisClass();
-
   /**
    * The delay to purge the historical information.
    * <p>
@@ -303,7 +303,7 @@
 
     // Read from this entryHistorical,
     // Create one empty if none was existing in this entryHistorical.
-    AttributeDescription attrDesc = new AttributeDescription(modAttr);
+    AttributeDescription attrDesc = AttributeDescription.create(modAttr);
     AttrHistorical attrHist = attributesHistorical.get(attrDesc);
     if (attrHist == null)
     {
@@ -350,8 +350,7 @@
     for (Map.Entry<AttributeDescription, AttrHistorical> mapEntry : attributesHistorical.entrySet())
     {
       AttributeDescription attrDesc = mapEntry.getKey();
-      AttributeType type = attrDesc.attributeType;
-      String optionsString = attrDesc.toOptionsString();
+      String options = attrDesc.toString();
       AttrHistorical attrHist = mapEntry.getValue();
 
       CSN deleteTime = attrHist.getDeleteTime();
@@ -370,7 +369,7 @@
             // this hist must be purged now, so skip its encoding
             continue;
           }
-          String strValue = encode(DEL, type, optionsString, attrValHist.getValueDeleteTime(), value);
+          String strValue = encode(DEL, options, attrValHist.getValueDeleteTime(), value);
           builder.add(strValue);
         }
         else if (attrValHist.getValueUpdateTime() != null)
@@ -387,18 +386,18 @@
           // unit tests do not like changing it
           if (attrDel && updateTime == deleteTime && value != null)
           {
-            strValue = encode(REPL, type, optionsString, updateTime, value);
+            strValue = encode(REPL, options, updateTime, value);
             attrDel = false;
           }
           else if (value != null)
           {
-            strValue = encode(ADD, type, optionsString, updateTime, value);
+            strValue = encode(ADD, options, updateTime, value);
           }
           else
           {
             // "add" without any value is suspicious. Tests never go there.
             // Is this used to encode "add" with an empty string?
-            strValue = encode(ADD, type, optionsString, updateTime);
+            strValue = encode(ADD, options, updateTime);
           }
 
           builder.add(strValue);
@@ -412,8 +411,7 @@
           // this hist must be purged now, so skip its encoding
           continue;
         }
-        String strValue = encode(ATTRDEL, type, optionsString, deleteTime);
-        builder.add(strValue);
+        builder.add(encode(ATTRDEL, options, deleteTime));
       }
     }
 
@@ -445,16 +443,14 @@
     return needsPurge;
   }
 
-  private String encode(HistAttrModificationKey modKey, AttributeType type,
-      String optionsString, CSN changeTime)
+  private String encode(HistAttrModificationKey modKey, String options, CSN changeTime)
   {
-    return type.getNormalizedPrimaryName() + optionsString + ":" + changeTime + ":" + modKey;
+    return options + ":" + changeTime + ":" + modKey;
   }
 
-  private String encode(HistAttrModificationKey modKey, AttributeType type,
-      String optionsString, CSN changeTime, ByteString value)
+  private String encode(HistAttrModificationKey modKey, String options, CSN changeTime, ByteString value)
   {
-    return type.getNormalizedPrimaryName() + optionsString + ":" + changeTime + ":" + modKey + ":" + value;
+    return options + ":" + changeTime + ":" + modKey + ":" + value;
   }
 
   /**
@@ -580,7 +576,7 @@
             AttrHistorical attrInfo = newHistorical.attributesHistorical.get(attrDesc);
             if (attrInfo == null)
             {
-              attrInfo = AttrHistorical.createAttributeHistorical(attrDesc.attributeType);
+              attrInfo = AttrHistorical.createAttributeHistorical(attrDesc.getAttributeType());
               newHistorical.attributesHistorical.put(attrDesc, attrInfo);
             }
             attrInfo.assign(histVal.getHistKey(), histVal.getAttributeValue(), csn);
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/HistoricalAttributeValue.java b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/HistoricalAttributeValue.java
index 72c56ee..5e583d4 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/HistoricalAttributeValue.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/HistoricalAttributeValue.java
@@ -135,7 +135,7 @@
         isModDN = true;
       }
     }
-    this.attrDesc = attrType != null ? new AttributeDescription(attrType, options) : null;
+    this.attrDesc = attrType != null ? AttributeDescription.create(attrType, options) : null;
 
     csn = new CSN(token[1]);
     histKey = HistAttrModificationKey.decodeKey(token[2]);
@@ -159,6 +159,16 @@
     }
   }
 
+  private AttributeType getAttributeType()
+  {
+    return attrDesc != null ? attrDesc.getAttributeType() : null;
+  }
+
+  private Set<String> getOptions()
+  {
+    return attrDesc != null ? attrDesc.getOptions() : Collections.<String> emptySet();
+  }
+
   /**
    * Get the String form of the attribute type.
    *
@@ -216,8 +226,8 @@
    */
   public Modification generateMod()
   {
-    AttributeBuilder builder = new AttributeBuilder(attrDesc.attributeType, attrString);
-    builder.setOptions(attrDesc.options);
+    AttributeBuilder builder = new AttributeBuilder(getAttributeType(), attrString);
+    builder.setOptions(getOptions());
 
     if (histKey != ATTRDEL)
     {
@@ -247,7 +257,7 @@
    */
   public boolean isADDOperation()
   {
-    return attrDesc.attributeType == null && !isModDN;
+    return getAttributeType() == null && !isModDN;
   }
 
   /**
@@ -258,7 +268,7 @@
    */
   public boolean isMODDNOperation()
   {
-    return attrDesc.attributeType == null && isModDN;
+    return getAttributeType() == null && isModDN;
   }
 
   @Override
@@ -266,7 +276,7 @@
   {
     final StringBuilder sb = new StringBuilder();
     sb.append(attrString);
-    for (String option : attrDesc.options)
+    for (String option : getOptions())
     {
       sb.append(";").append(option);
     }
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/types/AbstractAttribute.java b/opendj-server-legacy/src/main/java/org/opends/server/types/AbstractAttribute.java
index b84cbe6..7324666 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/types/AbstractAttribute.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/types/AbstractAttribute.java
@@ -33,8 +33,6 @@
 import org.forgerock.opendj.ldap.DecodeException;
 import org.forgerock.opendj.ldap.schema.MatchingRule;
 
-import static org.opends.server.util.StaticUtils.*;
-
 /** An abstract base class for implementing new types of {@link Attribute}. */
 @org.opends.server.types.PublicAPI(
     stability = org.opends.server.types.StabilityLevel.UNCOMMITTED,
@@ -146,27 +144,16 @@
   /**
    * {@inheritDoc}
    * <p>
-   * This implementation returns <code>true</code> if the provided
-   * collection of options is <code>null</code> or empty. If the
+   * This implementation returns {@code true} if the provided
+   * collection of options is {@code null} or empty. If the
    * collection is non-empty and this attribute does not have any
-   * options then it returns <code>false</code>. Otherwise, each
-   * option in the provided collection is checked using
-   * {@link #hasOption(String)} and <code>true</code> is
+   * options then it returns {@code false}. Otherwise, {@code true} is
    * returned if all the provided options are present.
    */
   @Override
   public boolean hasAllOptions(Collection<String> options)
   {
-    if (options == null || options.isEmpty())
-    {
-      return true;
-    }
-
-    if (hasOptions())
-    {
-      return hasAllOptions0(options);
-    }
-    return false;
+    return AttributeDescription.containsAllOptions(getOptions(), options);
   }
 
   @Override
@@ -194,24 +181,12 @@
    * This implementation calls {@link #getOptions()} to
    * retrieve this attribute's set of options and then compares them
    * one at a time against the provided option. All comparisons are
-   * case insensitive (this is why we iterate through the set of
-   * options, rather than doing a simpler set membership test).
+   * case insensitive.
    */
   @Override
   public boolean hasOption(String option)
   {
-    String noption = toLowerCase(option);
-
-    // Cannot use Set.contains() because the options are not normalized.
-    for (String o : getOptions())
-    {
-      String no = toLowerCase(o);
-      if (no.equals(noption))
-      {
-        return true;
-      }
-    }
-    return false;
+    return AttributeDescription.containsOption(getOptions(), option);
   }
 
   /**
@@ -259,29 +234,12 @@
   @Override
   public boolean optionsEqual(Set<String> options)
   {
-    if (options == null)
+    if (options != null)
     {
-      return !hasOptions();
+      return getOptions().size() == options.size()
+          && hasAllOptions(options);
     }
-
-    if (getOptions().size() == options.size())
-    {
-      return hasAllOptions0(options);
-    }
-    return false;
-  }
-
-  /** Cannot use Set.containsAll() because the set of options are not normalized. */
-  private boolean hasAllOptions0(Collection<String> options)
-  {
-    for (String option : options)
-    {
-      if (!hasOption(option))
-      {
-        return false;
-      }
-    }
-    return true;
+    return !hasOptions();
   }
 
   @Override
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/types/Attribute.java b/opendj-server-legacy/src/main/java/org/opends/server/types/Attribute.java
index 7366e4f..b0b0917 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/types/Attribute.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/types/Attribute.java
@@ -61,10 +61,10 @@
    *
    * @param assertionValue
    *          The assertion value for which to make the determination.
-   * @return <CODE>UNDEFINED</CODE> if this attribute does not have
-   *         an approximate matching rule, <CODE>TRUE</CODE> if at
+   * @return {@link ConditionResult#UNDEFINED} if this attribute does not have
+   *         an approximate matching rule, {@link ConditionResult#TRUE} if at
    *         least one value is approximately equal to the provided
-   *         value, or <CODE>false</CODE> otherwise.
+   *         value, or {@link ConditionResult#FALSE} otherwise.
    */
   ConditionResult approximatelyEqualTo(ByteString assertionValue);
 
@@ -73,8 +73,8 @@
    *
    * @param value
    *          The value for which to make the determination.
-   * @return <CODE>true</CODE> if this attribute has the specified
-   *         value, or <CODE>false</CODE> if not.
+   * @return {@code true} if this attribute has the specified
+   *         value, or {@code false} if not.
    */
   boolean contains(ByteString value);
 
@@ -84,8 +84,8 @@
    *
    * @param values
    *          The set of values for which to make the determination.
-   * @return <CODE>true</CODE> if this attribute contains all the
-   *         values in the provided collection, or <CODE>false</CODE>
+   * @return {@code true} if this attribute contains all the
+   *         values in the provided collection, or {@code false}
    *         if it does not contain at least one of them.
    */
   boolean containsAll(Collection<ByteString> values);
@@ -95,8 +95,8 @@
    *
    * @param assertionValue
    *          The assertion value for which to make the determination.
-   * @return <CODE>true</CODE> if this attribute matches the specified assertion
-   *         value, or <CODE>false</CODE> if not.
+   * @return {@code true} if this attribute matches the specified assertion
+   *         value, or {@code false} if not.
    */
   ConditionResult matchesEqualityAssertion(ByteString assertionValue);
 
@@ -107,9 +107,9 @@
    *
    * @param o
    *          The object for which to make the determination.
-   * @return <CODE>true</CODE> if the provided object is an
+   * @return {@code true} if the provided object is an
    *         attribute that is equal to this attribute, or
-   *         <CODE>false</CODE> if not.
+   *         {@code false} if not.
    */
   @Override
   boolean equals(Object o);
@@ -152,30 +152,26 @@
    *
    * @param assertionValue
    *          The assertion value for which to make the determination.
-   * @return <CODE>UNDEFINED</CODE> if this attribute does not have
-   *         an ordering matching rule, <CODE>TRUE</CODE> if at
+   * @return {@link ConditionResult#UNDEFINED} if this attribute does not have
+   *         an ordering matching rule, {@link ConditionResult#TRUE} if at
    *         least one value is greater than or equal to the provided
-   *         assertion value, or <CODE>false</CODE> otherwise.
+   *         assertion value, or {@link ConditionResult#FALSE} otherwise.
    */
   ConditionResult greaterThanOrEqualTo(ByteString assertionValue);
 
   /**
-   * Indicates whether this attribute has all of the options in the
-   * provided collection.
+   * Indicates whether this attribute has all of the options in the provided collection.
    *
    * @param options
-   *          The collection of options for which to make the
-   *          determination (may be <code>null</code>).
-   * @return <CODE>true</CODE> if this attribute has all of the
-   *         specified options, or <CODE>false</CODE> if it does not
-   *         have at least one of them.
+   *          The collection of options for which to make the determination (may be {@code null}).
+   * @return {@code true} if this attribute has all of the specified options,
+   *         or {@code false} if it does not have at least one of them.
    */
   boolean hasAllOptions(Collection<String> options);
 
   /**
    * Retrieves the hash code for this attribute. It will be calculated
-   * as the sum of the hash code for the attribute type and all
-   * values.
+   * as the sum of the hash code for the attribute type and all values.
    *
    * @return The hash code for this attribute.
    */
@@ -187,24 +183,24 @@
    *
    * @param option
    *          The option for which to make the determination.
-   * @return <CODE>true</CODE> if this attribute has the specified
-   *         option, or <CODE>false</CODE> if not.
+   * @return {@code true} if this attribute has the specified option,
+   *         or {@code false} if not.
    */
   boolean hasOption(String option);
 
   /**
    * Indicates whether this attribute has any options at all.
    *
-   * @return <CODE>true</CODE> if this attribute has at least one
-   *         option, or <CODE>false</CODE> if not.
+   * @return {@code true} if this attribute has at least one
+   *         option, or {@code false} if not.
    */
   boolean hasOptions();
 
   /**
-   * Returns <code>true</code> if this attribute contains no
+   * Returns {@code true} if this attribute contains no
    * attribute values.
    *
-   * @return <CODE>true</CODE> if this attribute contains no
+   * @return {@code true} if this attribute contains no
    *         attribute values.
    */
   boolean isEmpty();
@@ -229,7 +225,7 @@
    * Returns an iterator over the attribute values in this attribute.
    * The attribute values are returned in the order in which they were
    * added this attribute. The returned iterator does not support
-   * attribute value removals via its <code>remove</code> method.
+   * attribute value removals via {@link Iterator#remove()}.
    *
    * @return An iterator over the attribute values in this attribute.
    */
@@ -242,10 +238,10 @@
    *
    * @param assertionValue
    *          The assertion value for which to make the determination.
-   * @return <CODE>UNDEFINED</CODE> if this attribute does not have
-   *         an ordering matching rule, <CODE>TRUE</CODE> if at
+   * @return {@link ConditionResult#UNDEFINED} if this attribute does not have
+   *         an ordering matching rule, {@link ConditionResult#TRUE} if at
    *         least one value is less than or equal to the provided
-   *         assertion value, or <CODE>false</CODE> otherwise.
+   *         assertion value, or {@link ConditionResult#FALSE} otherwise.
    */
   ConditionResult lessThanOrEqualTo(ByteString assertionValue);
 
@@ -259,10 +255,10 @@
    *          The subAny components to use in the determination.
    * @param subFinal
    *          The subFinal component to use in the determination.
-   * @return <CODE>UNDEFINED</CODE> if this attribute does not have
-   *         a substring matching rule, <CODE>TRUE</CODE> if at
+   * @return {@link ConditionResult#UNDEFINED} if this attribute does not have
+   *         a substring matching rule, {@link ConditionResult#TRUE} if at
    *         least one value matches the provided substring, or
-   *         <CODE>FALSE</CODE> otherwise.
+   *         {@link ConditionResult#FALSE} otherwise.
    */
   ConditionResult matchesSubstring(ByteString subInitial,
       List<ByteString> subAny, ByteString subFinal);
@@ -273,8 +269,8 @@
    *
    * @param options
    *          The set of options for which to make the determination
-   *          (may be <code>null</code>).
-   * @return <CODE>true</CODE> if this attribute has exactly the
+   *          (may be {@code null}).
+   * @return {@code true} if this attribute has exactly the
    *         specified set of options.
    */
   boolean optionsEqual(Set<String> options);
diff --git a/opendj-server-legacy/src/main/java/org/opends/server/types/AttributeDescription.java b/opendj-server-legacy/src/main/java/org/opends/server/types/AttributeDescription.java
index 2df2879..77ecc28 100644
--- a/opendj-server-legacy/src/main/java/org/opends/server/types/AttributeDescription.java
+++ b/opendj-server-legacy/src/main/java/org/opends/server/types/AttributeDescription.java
@@ -24,37 +24,251 @@
  */
 package org.opends.server.types;
 
+import static org.opends.server.util.StaticUtils.*;
+
+import java.util.Collection;
+import java.util.Iterator;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+
+import org.forgerock.util.Reject;
 
 /** Temporary class until we move to {@link org.forgerock.opendj.ldap.AttributeDescription}. */
-public final class AttributeDescription
+public final class AttributeDescription implements Comparable<AttributeDescription>
 {
-  final AttributeType attributeType;
-  final Set<String> options;
+  private final AttributeType attributeType;
+  private final Set<String> options;
 
-  AttributeDescription(Attribute attr)
+  private AttributeDescription(AttributeType attributeType, Set<String> options)
   {
-    this(attr.getAttributeType(), attr.getOptions());
-  }
-
-  AttributeDescription(AttributeType attributeType, Set<String> options)
-  {
+    Reject.ifNull(attributeType);
+    Reject.ifNull(options);
     this.attributeType = attributeType;
     this.options = options;
   }
 
-  String toOptionsString()
+  /**
+   * Creates an attribute description with the attribute type and options of the provided
+   * {@link Attribute}.
+   *
+   * @param attr
+   *          The attribute.
+   * @return The attribute description.
+   * @throws NullPointerException
+   *           If {@code attributeType} or {@code options} was {@code null}.
+   */
+  public static AttributeDescription create(Attribute attr)
   {
-    if (options != null)
+    return create(attr.getAttributeType(), attr.getOptions());
+  }
+
+  /**
+   * Creates an attribute description having the provided attribute type and options.
+   *
+   * @param attributeType
+   *          The attribute type.
+   * @param options
+   *          The attribute options.
+   * @return The attribute description.
+   * @throws NullPointerException
+   *           If {@code attributeType} or {@code options} was {@code null}.
+   */
+  public static AttributeDescription create(AttributeType attributeType, Set<String> options)
+  {
+    return new AttributeDescription(attributeType, options);
+  }
+
+  /**
+   * Returns the attribute type associated with this attribute description.
+   *
+   * @return The attribute type associated with this attribute description.
+   */
+  public AttributeType getAttributeType()
+  {
+    return attributeType;
+  }
+
+  /**
+   * Returns the set of not normalized options contained in this attribute description.
+   *
+   * @return A set containing the not normalized options.
+   */
+  public Set<String> getOptions()
+  {
+    return options;
+  }
+
+  /**
+   * Indicates whether the provided set of not normalized options contains the provided option.
+   *
+   * @param options
+   *          The set of not normalized options where to do the search.
+   * @param optionToFind
+   *          The option for which to make the determination.
+   * @return {@code true} if the provided set of options has the provided option, or {@code false}
+   *         if not.
+   */
+  public static boolean containsOption(Set<String> options, String optionToFind)
+  {
+    String normToFind = toLowerCase(optionToFind);
+
+    // Cannot use Set.contains() because the options are not normalized.
+    for (String o : options)
     {
-      StringBuilder optionsBuilder = new StringBuilder();
-      for (String s : options)
+      String norm = toLowerCase(o);
+      if (norm.equals(normToFind))
       {
-        optionsBuilder.append(';').append(s);
+        return true;
       }
-      return optionsBuilder.toString();
     }
-    return "";
+    return false;
+  }
+
+  /**
+   * Indicates whether the provided first set of not normalized options contains all values from the
+   * second set of not normalized options.
+   *
+   * @param options1
+   *          The first set of not normalized options where to do the search.
+   * @param options2
+   *          The second set of not normalized options that must all be found.
+   * @return {@code true} if the first provided set of options has all the options from the second
+   *         provided set of options.
+   */
+  public static boolean containsAllOptions(Collection<String> options1, Collection<String> options2)
+  {
+    if (options1 == options2)
+    {
+      return true;
+    }
+    else if (isEmpty(options2))
+    {
+      return true;
+    }
+    else if (isEmpty(options1))
+    {
+      return false;
+    }
+    // normalize all options before calling containsAll()
+    Set<String> set1 = toLowercaseSet(options1);
+    Set<String> set2 = toLowercaseSet(options2);
+    return set1.size() >= set2.size() && set1.containsAll(set2);
+  }
+
+  /**
+   * Indicates whether the provided first set of not normalized options equals the second set of not
+   * normalized options.
+   *
+   * @param options1
+   *          The first set of not normalized options.
+   * @param options2
+   *          The second set of not normalized options.
+   * @return {@code true} if the first provided set of options equals the second provided set of
+   *         options.
+   */
+  public static boolean optionsEqual(Set<String> options1, Set<String> options2)
+  {
+    if (options1 == options2)
+    {
+      return true;
+    }
+    else if (isEmpty(options2))
+    {
+      return isEmpty(options1);
+    }
+    else if (isEmpty(options1))
+    {
+      return false;
+    }
+    // normalize all options before calling containsAll()
+    Set<String> set1 = toLowercaseSet(options1);
+    Set<String> set2 = toLowercaseSet(options2);
+    return set1.equals(set2);
+  }
+
+  private static boolean isEmpty(Collection<String> col)
+  {
+    return col == null || col.isEmpty();
+  }
+
+  private static SortedSet<String> toLowercaseSet(Collection<String> strings)
+  {
+    final SortedSet<String> results = new TreeSet<>();
+    for (String s : strings)
+    {
+      results.add(toLowerCase(s));
+    }
+    return results;
+  }
+
+  @Override
+  public int compareTo(AttributeDescription other)
+  {
+    if (this == other)
+    {
+      return 0;
+    }
+    return compare(attributeType, options, other.attributeType, other.options);
+  }
+
+  /**
+   * Compares the first attribute type and options to the second attribute type and options, as if
+   * they were both instances of {@link AttributeDescription}.
+   * <p>
+   * The attribute types are compared first and then, if equal, the options are normalized, sorted,
+   * and compared.
+   *
+   * @param attrType1
+   *          The first attribute type to be compared.
+   * @param options1
+   *          The first options to be compared.
+   * @param attrType2
+   *          The second attribute type to be compared.
+   * @param options2
+   *          The second options to be compared.
+   * @return A negative integer, zero, or a positive integer as this attribute description is less
+   *         than, equal to, or greater than the specified attribute description.
+   * @throws NullPointerException
+   *           If {@code name} was {@code null}.
+   * @see AttributeDescription#compareTo(AttributeDescription)
+   */
+  public static int compare(AttributeType attrType1, Set<String> options1,
+      AttributeType attrType2, Set<String> options2)
+  {
+    int cmp = attrType1.compareTo(attrType2);
+    if (cmp != 0)
+    {
+      return cmp;
+    }
+    if (options1 == options2)
+    {
+      return 0;
+    }
+    return compare(toLowercaseSet(options1), toLowercaseSet(options2));
+  }
+
+  private static int compare(SortedSet<String> options1, SortedSet<String> options2)
+  {
+    Iterator<String> it1 = options1.iterator();
+    Iterator<String> it2 = options2.iterator();
+    while (it1.hasNext() && it2.hasNext())
+    {
+      int cmp = it1.next().compareTo(it2.next());
+      if (cmp != 0)
+      {
+        return cmp;
+      }
+    }
+    if (it1.hasNext())
+    {
+      return 1;
+    }
+    else if (it2.hasNext())
+    {
+      return -1;
+    }
+    return 0;
   }
 
   @Override
@@ -69,7 +283,7 @@
       return false;
     }
     final AttributeDescription other = (AttributeDescription) obj;
-    return attributeType.equals(other.attributeType) && options.equals(other.options);
+    return attributeType.equals(other.attributeType) && optionsEqual(options, other.options);
   }
 
   @Override
@@ -85,6 +299,13 @@
   @Override
   public String toString()
   {
-    return getClass().getSimpleName() + "(" + "attributeType=" + attributeType + ", options=" + options + ")";
+    final StringBuilder buffer = new StringBuilder();
+    buffer.append(attributeType.getNormalizedPrimaryName());
+    for (String option : options)
+    {
+      buffer.append(';');
+      buffer.append(option);
+    }
+    return buffer.toString();
   }
-}
\ No newline at end of file
+}
diff --git a/opendj-server-legacy/src/test/java/org/opends/server/types/AttributeBuilderTest.java b/opendj-server-legacy/src/test/java/org/opends/server/types/AttributeBuilderTest.java
index fbd98d4..64ca536 100644
--- a/opendj-server-legacy/src/test/java/org/opends/server/types/AttributeBuilderTest.java
+++ b/opendj-server-legacy/src/test/java/org/opends/server/types/AttributeBuilderTest.java
@@ -1396,7 +1396,7 @@
 
 
   /**
-   * Tests {@link Attribute#hasAllOptions(java.util.Collection)}.
+   * Tests {@link Attribute#hasAllOptions(Collection)}.
    *
    * @param testCase
    *          Test case index (useful for debugging).
@@ -1416,20 +1416,21 @@
       AttributeType type, String name, String[] options, String[] values)
       throws Exception
   {
-    // Check hasAllOptions().
+    Assert.assertTrue(a.hasAllOptions(null));
     Assert.assertTrue(a.hasAllOptions(Collections.<String> emptySet()));
     Assert.assertTrue(a.hasAllOptions(Arrays.asList(options)));
 
     if (options.length > 1)
     {
-      Assert.assertTrue(a.hasAllOptions(Arrays.asList(options).subList(1,
-          options.length)));
+      Assert.assertTrue(a.hasAllOptions(Arrays.asList(options).subList(1, options.length)));
     }
 
     List<String> tmp = newArrayList(options);
     tmp.add("xxxx");
     Assert.assertFalse(a.hasAllOptions(tmp));
 
+    Assert.assertFalse(a.hasAllOptions(newHashSet("xxxx")));
+
     tmp.clear();
     for (String option : options)
     {

--
Gitblit v1.10.0