From 5d2954405cb34e310427a414f5322cbc96339aca Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 17 Dec 2015 17:18:47 +0100 Subject: [PATCH] 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 --- .../yangtools/util/ImmutableOffsetMap.java | 42 ++++++------- .../yangtools/util/MutableOffsetMap.java | 61 ++++++++++--------- 2 files changed, 52 insertions(+), 51 deletions(-) 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 1366856c77..df0479ed9f 100644 --- 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 implements UnmodifiableMapPhase 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 e : offsets.entrySet()) { - if (!objects[e.getValue()].equals(om.get(e.getKey()))) { - return false; - } + try { + // Ensure all objects are present + for (Entry 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 014a59f1c3..1263d85cf2 100644 --- 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 extends AbstractMap 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 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 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 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 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 -- 2.36.6