BUG-4803: fix equals() method

The implementation required the map's ordering to be the same, which is
a violation of the Map contract.

Change-Id: I8f3308b9cabd5c011e7c2dadfe520407ad2c4672
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/common/util/src/main/java/org/opendaylight/yangtools/util/ImmutableOffsetMap.java b/common/util/src/main/java/org/opendaylight/yangtools/util/ImmutableOffsetMap.java
index 1366856c773093046c0e4cd8e30ea9436213896e..df0479ed9f72dc30d342fbb118115d42b57ba08f 100644 (file)
--- a/common/util/src/main/java/org/opendaylight/yangtools/util/ImmutableOffsetMap.java
+++ b/common/util/src/main/java/org/opendaylight/yangtools/util/ImmutableOffsetMap.java
@@ -139,42 +139,42 @@ public final class ImmutableOffsetMap<K, V> implements UnmodifiableMapPhase<K, V
         if (o == this) {
             return true;
         }
-        if (o == null) {
+        if (!(o instanceof Map)) {
             return false;
         }
 
         if (o instanceof ImmutableOffsetMap) {
             final ImmutableOffsetMap<?, ?> om = (ImmutableOffsetMap<?, ?>) o;
-            if (offsets.equals(om.offsets) && Arrays.deepEquals(objects, om.objects)) {
-                return true;
+
+            // If the offset match, the arrays have to match, too
+            if (offsets.equals(om.offsets)) {
+                return Arrays.deepEquals(objects, om.objects);
             }
         } else if (o instanceof MutableOffsetMap) {
             // Let MutableOffsetMap do the actual work.
             return o.equals(this);
-        } else if (o instanceof Map) {
-            final Map<?, ?> om = (Map<?, ?>)o;
+        }
 
-            // Size and key sets have to match
-            if (size() != om.size() || !keySet().equals(om.keySet())) {
-                return false;
-            }
+        final Map<?, ?> other = (Map<?, ?>)o;
+
+        // Size and key sets have to match
+        if (size() != other.size() || !keySet().equals(other.keySet())) {
+            return false;
+        }
 
-            try {
-                // Ensure all objects are present
-                for (Entry<K, Integer> e : offsets.entrySet()) {
-                    if (!objects[e.getValue()].equals(om.get(e.getKey()))) {
-                        return false;
-                    }
+        try {
+            // Ensure all objects are present
+            for (Entry<K, Integer> e : offsets.entrySet()) {
+                if (!objects[e.getValue()].equals(other.get(e.getKey()))) {
+                    return false;
                 }
-            } catch (ClassCastException e) {
-                // Can be thrown by om.get() indicating we have incompatible key types
-                return false;
             }
-
-            return true;
+        } catch (ClassCastException e) {
+            // Can be thrown by other.get() indicating we have incompatible key types
+            return false;
         }
 
-        return false;
+        return true;
     }
 
     @Override

diff --git a/common/util/src/main/java/org/opendaylight/yangtools/util/MutableOffsetMap.java b/common/util/src/main/java/org/opendaylight/yangtools/util/MutableOffsetMap.java
index 014a59f1c3f2172a2936bab41659676ed76bf27e..1263d85cf29d1f281783772444481fedc18d98c5 100644 (file)
--- a/common/util/src/main/java/org/opendaylight/yangtools/util/MutableOffsetMap.java
+++ b/common/util/src/main/java/org/opendaylight/yangtools/util/MutableOffsetMap.java
@@ -286,53 +286,54 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
         if (o == this) {
             return true;
         }
-        if (o == null) {
+        if (!(o instanceof Map)) {
             return false;
         }
 
         if (o instanceof ImmutableOffsetMap) {
             final ImmutableOffsetMap<?, ?> om = (ImmutableOffsetMap<?, ?>) o;
-            if (newKeys.isEmpty() && offsets == om.offsets() && Arrays.deepEquals(objects, om.objects())) {
-                return true;
+
+            if (newKeys.isEmpty() && offsets.equals(om.offsets())) {
+                return Arrays.deepEquals(objects, om.objects());
             }
         } else if (o instanceof MutableOffsetMap) {
             final MutableOffsetMap<?, ?> om = (MutableOffsetMap<?, ?>) o;
-            if (offsets == om.offsets && Arrays.deepEquals(objects, om.objects) && newKeys.equals(om.newKeys)) {
-                return true;
-            }
-        } else if (o instanceof Map) {
-            final Map<?, ?> om = (Map<?, ?>)o;
 
-            // Size and key sets have to match
-            if (size() != om.size() || !keySet().equals(om.keySet())) {
-                return false;
+            if (offsets.equals(om.offsets)) {
+                return Arrays.deepEquals(objects, om.objects) && newKeys.equals(om.newKeys);
             }
+        }
 
-            try {
-                // Ensure all newKeys are present. Note newKeys is guaranteed to
-                // not contain null value.
-                for (Entry<K, V> e : newKeys.entrySet()) {
-                    if (!e.getValue().equals(om.get(e.getKey()))) {
-                        return false;
-                    }
-                }
+        // Fall back to brute map compare
+        final Map<?, ?> other = (Map<?, ?>)o;
 
-                // Ensure all objects are present
-                for (Entry<K, Integer> e : offsets.entrySet()) {
-                    final V v = objects[e.getValue()];
-                    if (v != null && !v.equals(om.get(e.getKey()))) {
-                        return false;
-                    }
+        // Size and key sets have to match
+        if (size() != other.size() || !keySet().equals(other.keySet())) {
+            return false;
+        }
+
+        try {
+            // Ensure all newKeys are present. Note newKeys is guaranteed to
+            // not contain null value.
+            for (Entry<K, V> e : newKeys.entrySet()) {
+                if (!e.getValue().equals(other.get(e.getKey()))) {
+                    return false;
                 }
-            } catch (ClassCastException e) {
-                // Can be thrown by om.get() and indicate we have incompatible key types
-                return false;
             }
 
-            return true;
+            // Ensure all objects are present
+            for (Entry<K, Integer> e : offsets.entrySet()) {
+                final V v = objects[e.getValue()];
+                if (v != null && !v.equals(other.get(e.getKey()))) {
+                    return false;
+                }
+            }
+        } catch (ClassCastException e) {
+            // Can be thrown by other.get() and indicate we have incompatible key types
+            return false;
         }
 
-        return false;
+        return true;
     }
 
     @Override