From d6a3c53995b12a7d46f08034d985188566e45000 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sat, 29 Aug 2015 11:02:52 +0200 Subject: [PATCH] BUG-4158: ImmutableOffsetMap should not be Cloneable Removes Cloneable from ImmutableOffsetMap, as it serves no real purpose. Also improve test coverage. Change-Id: I381fd5a796f132fa3620559e7159bd25bf6447b5 Signed-off-by: Robert Varga --- .../yangtools/util/ImmutableOffsetMap.java | 7 +- .../yangtools/util/OffsetMapTest.java | 98 +++++++++++++++++-- 2 files changed, 92 insertions(+), 13 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 5dcac60cc6..3876c73ee4 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 @@ -38,7 +38,7 @@ import javax.annotation.Nonnull; * @param the type of mapped values */ @Beta -public class ImmutableOffsetMap extends AbstractLazyValueMap implements Cloneable, UnmodifiableMapPhase, Serializable { +public class ImmutableOffsetMap extends AbstractLazyValueMap implements UnmodifiableMapPhase, Serializable { private static final long serialVersionUID = 1L; private final Map offsets; @@ -220,11 +220,6 @@ public class ImmutableOffsetMap extends AbstractLazyValueMap impleme return new MutableOffsetMap<>(this); } - @Override - public ImmutableOffsetMap clone() throws CloneNotSupportedException { - return new ImmutableOffsetMap<>(this); - } - Map offsets() { return offsets; } diff --git a/common/util/src/test/java/org/opendaylight/yangtools/util/OffsetMapTest.java b/common/util/src/test/java/org/opendaylight/yangtools/util/OffsetMapTest.java index 6f5ed87487..d5df1ef80a 100644 --- a/common/util/src/test/java/org/opendaylight/yangtools/util/OffsetMapTest.java +++ b/common/util/src/test/java/org/opendaylight/yangtools/util/OffsetMapTest.java @@ -15,6 +15,7 @@ import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterators; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -39,6 +40,11 @@ public class OffsetMapTest { return (ImmutableOffsetMap) ImmutableOffsetMap.copyOf(twoEntryMap); } + @Test(expected=IllegalArgumentException.class) + public void testWrongImmutableConstruction() { + new ImmutableOffsetMap(Collections.emptyMap(), new Object[1]); + } + @Test public void testCopyEmptyMap() { final Map source = Collections.emptyMap(); @@ -59,19 +65,43 @@ public class OffsetMapTest { @Test public void testCopyMap() { - final Map result = createMap(); - - assertTrue(result instanceof ImmutableOffsetMap); + final ImmutableOffsetMap map = createMap(); // Equality in both directions - assertEquals(twoEntryMap, result); - assertEquals(result, twoEntryMap); + assertEquals(twoEntryMap, map); + assertEquals(map, twoEntryMap); // Iterator order needs to be preserved - assertTrue(Iterators.elementsEqual(twoEntryMap.entrySet().iterator(), result.entrySet().iterator())); + assertTrue(Iterators.elementsEqual(twoEntryMap.entrySet().iterator(), map.entrySet().iterator())); // Should result in the same object - assertSame(result, ImmutableOffsetMap.copyOf(result)); + assertSame(map, ImmutableOffsetMap.copyOf(map)); + + final Map mutable = map.toModifiableMap(); + final Map copy = ImmutableOffsetMap.copyOf(mutable); + + assertEquals(mutable, copy); + assertEquals(map, copy); + assertNotSame(mutable, copy); + assertNotSame(map, copy); + } + + @Test + public void testImmutableSimpleEquals() { + final Map map = createMap(); + + assertTrue(map.equals(map)); + assertFalse(map.equals(null)); + assertFalse(map.equals("string")); + } + + @Test + public void testImmutableCopyConstructor() { + final ImmutableOffsetMap source = createMap(); + final ImmutableOffsetMap result = new ImmutableOffsetMap<>(source); + + assertSame(source.offsets(), result.offsets()); + assertSame(source.objects(), result.objects()); } @Test @@ -222,6 +252,17 @@ public class OffsetMapTest { assertTrue(map.containsKey("k2")); assertFalse(map.containsKey("non-existent")); assertFalse(map.containsKey(null)); + assertTrue(map.containsValue("v1")); + assertFalse(map.containsValue("non-existent")); + } + + @Test + public void testImmutableEquals() { + final Map map = createMap(); + + assertFalse(map.equals(threeEntryMap)); + assertFalse(map.equals(ImmutableMap.of("k1", "v1", "k3", "v3"))); + assertFalse(map.equals(ImmutableMap.of("k1", "v1", "k2", "different-value"))); } @Test @@ -296,6 +337,25 @@ public class OffsetMapTest { assertTrue(Iterators.elementsEqual(source.entrySet().iterator(), result.entrySet().iterator())); } + @Test + public void testEmptyMutable() throws CloneNotSupportedException { + final MutableOffsetMap map = new MutableOffsetMap<>(); + assertTrue(map.isEmpty()); + + final Map other = map.clone(); + assertEquals(other, map); + assertNotSame(other, map); + } + + @Test + public void testMutableWithKeyset() { + final MutableOffsetMap map = new MutableOffsetMap<>(ImmutableSet.of("k1", "k2")); + assertTrue(map.isEmpty()); + assertTrue(map.keySet().isEmpty()); + assertNull(map.get("k1")); + assertNull(map.remove("k2")); + } + @Test public void testMutableToEmpty() { final MutableOffsetMap mutable = createMap().toModifiableMap(); @@ -320,6 +380,19 @@ public class OffsetMapTest { assertEquals(ImmutableMap.of("k2", "v2"), result); } + @Test + public void testMutableToNewSingleton() { + final MutableOffsetMap mutable = createMap().toModifiableMap(); + + mutable.remove("k1"); + mutable.put("k3", "v3"); + + final Map result = mutable.toUnmodifiableMap(); + + assertTrue(result instanceof ImmutableOffsetMap); + assertEquals(ImmutableMap.of("k2", "v2", "k3", "v3"), result); + } + @Test public void testMutableSize() { final MutableOffsetMap mutable = createMap().toModifiableMap(); @@ -440,6 +513,17 @@ public class OffsetMapTest { } } + @Test + public void testMutableSimpleEquals() { + final ImmutableOffsetMap source = createMap(); + final Map map = source.toModifiableMap(); + + assertTrue(map.equals(map)); + assertFalse(map.equals(null)); + assertFalse(map.equals("string")); + assertTrue(map.equals(source)); + } + @Test public void testMutableIteratorBasics() { final MutableOffsetMap map = createMap().toModifiableMap(); -- 2.36.6