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 <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 5dcac60cc6977bf3f7371835baa5d11c3b827341..3876c73ee4c761b52b8e2a191dc488f7ff5f8277 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
@@ -38,7 +38,7 @@ import javax.annotation.Nonnull;
  * @param <V> the type of mapped values
  */
 @Beta
-public class ImmutableOffsetMap<K, V> extends AbstractLazyValueMap<K, V> implements Cloneable, UnmodifiableMapPhase<K, V>, Serializable {
+public class ImmutableOffsetMap<K, V> extends AbstractLazyValueMap<K, V> implements UnmodifiableMapPhase<K, V>, Serializable {
     private static final long serialVersionUID = 1L;
 
     private final Map<K, Integer> offsets;
@@ -220,11 +220,6 @@ public class ImmutableOffsetMap<K, V> extends AbstractLazyValueMap<K, V> impleme
         return new MutableOffsetMap<>(this);
     }
 
-    @Override
-    public ImmutableOffsetMap<K, V> clone() throws CloneNotSupportedException {
-        return new ImmutableOffsetMap<>(this);
-    }
-
     Map<K, Integer> 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 6f5ed87487f442296dc886b416816a85b2aaf39a..d5df1ef80a753cb0042810f10eb2cccb6ba63402 100644 (file)
--- 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<String, String>) ImmutableOffsetMap.copyOf(twoEntryMap);
     }
 
+    @Test(expected=IllegalArgumentException.class)
+    public void testWrongImmutableConstruction() {
+        new ImmutableOffsetMap<String, String>(Collections.<String, Integer>emptyMap(), new Object[1]);
+    }
+
     @Test
     public void testCopyEmptyMap() {
         final Map<String, String> source = Collections.emptyMap();
@@ -59,19 +65,43 @@ public class OffsetMapTest {
 
     @Test
     public void testCopyMap() {
-        final Map<String, String> result = createMap();
-
-        assertTrue(result instanceof ImmutableOffsetMap);
+        final ImmutableOffsetMap<String, String> 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<String, String> mutable = map.toModifiableMap();
+        final Map<String, String> copy = ImmutableOffsetMap.copyOf(mutable);
+
+        assertEquals(mutable, copy);
+        assertEquals(map, copy);
+        assertNotSame(mutable, copy);
+        assertNotSame(map, copy);
+    }
+
+    @Test
+    public void testImmutableSimpleEquals() {
+        final Map<String, String> map = createMap();
+
+        assertTrue(map.equals(map));
+        assertFalse(map.equals(null));
+        assertFalse(map.equals("string"));
+    }
+
+    @Test
+    public void testImmutableCopyConstructor() {
+        final ImmutableOffsetMap<String, String> source = createMap();
+        final ImmutableOffsetMap<String, String> 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<String, String> 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<String, String> map = new MutableOffsetMap<>();
+        assertTrue(map.isEmpty());
+
+        final Map<String, String> other = map.clone();
+        assertEquals(other, map);
+        assertNotSame(other, map);
+    }
+
+    @Test
+    public void testMutableWithKeyset() {
+        final MutableOffsetMap<String, String> 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<String, String> mutable = createMap().toModifiableMap();
@@ -320,6 +380,19 @@ public class OffsetMapTest {
         assertEquals(ImmutableMap.of("k2", "v2"), result);
     }
 
+    @Test
+    public void testMutableToNewSingleton() {
+        final MutableOffsetMap<String, String> mutable = createMap().toModifiableMap();
+
+        mutable.remove("k1");
+        mutable.put("k3", "v3");
+
+        final Map<String, String> result = mutable.toUnmodifiableMap();
+
+        assertTrue(result instanceof ImmutableOffsetMap);
+        assertEquals(ImmutableMap.of("k2", "v2", "k3", "v3"), result);
+    }
+
     @Test
     public void testMutableSize() {
         final MutableOffsetMap<String, String> mutable = createMap().toModifiableMap();
@@ -440,6 +513,17 @@ public class OffsetMapTest {
         }
     }
 
+    @Test
+    public void testMutableSimpleEquals() {
+        final ImmutableOffsetMap<String, String> source = createMap();
+        final Map<String, String> 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<String, String> map = createMap().toModifiableMap();