BUG-4803: make MutableOffsetMap abstract

We will need two implementations, one Ordered and Unordered, have make
the base class abstract and implement the Ordered version.

Change-Id: I26826c30470b47aba88ddfb106fd6e7afcd7f288
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 1ff5a5c7cc7cd5cf8be864e04b853e9d448f5dc8..2dec7ad70949a1ad8cbe591c7e361bc9c26daf1f 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
@@ -46,7 +46,7 @@ public abstract class ImmutableOffsetMap<K, V> implements UnmodifiableMapPhase<K
 
         @Override
         public MutableOffsetMap<K, V> toModifiableMap() {
-            return new MutableOffsetMap<>(this);
+            return MutableOffsetMap.copyOf(this);
         }
     }
 

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 fe7aa69401b9de057afc9121ea79599684e64716..0bd0583a5084095988241d99d0a75918f0101308 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
@@ -10,14 +10,15 @@ package org.opendaylight.yangtools.util;
 import com.google.common.annotations.Beta;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableMap;
 import java.util.AbstractMap;
 import java.util.AbstractSet;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.ConcurrentModificationException;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.Map;
@@ -40,53 +41,50 @@ import java.util.Set;
  * @param <V> the type of mapped values
  */
 @Beta
-public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements Cloneable, ModifiableMapPhase<K, V> {
+public abstract class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements Cloneable, ModifiableMapPhase<K, V> {
+    static final class Ordered<K, V> extends MutableOffsetMap<K, V> {
+        Ordered() {
+            super(new LinkedHashMap<K, V>());
+        }
+
+        Ordered(final Map<K, V> source) {
+            super(OffsetMapCache.offsetsFor(source.keySet()), source, new LinkedHashMap<K, V>());
+        }
+
+        Ordered(final Map<K, Integer> offsets, final V[] objects) {
+            super(offsets, objects, new LinkedHashMap<K, V>());
+        }
+    }
+
     private static final Object[] EMPTY_ARRAY = new Object[0];
     private final Map<K, Integer> offsets;
-    private final Map<K, V> newKeys;
+    private HashMap<K, V> newKeys;
     private V[] objects;
     private int removed = 0;
     private transient volatile int modCount;
     private boolean needClone = true;
 
-    public MutableOffsetMap() {
-        this(Collections.<K>emptySet());
+    MutableOffsetMap(final Map<K, Integer> offsets, final V[] objects, final HashMap<K, V> newKeys) {
+        Verify.verify(newKeys.isEmpty());
+        this.offsets = Preconditions.checkNotNull(offsets);
+        this.objects = Preconditions.checkNotNull(objects);
+        this.newKeys = Preconditions.checkNotNull(newKeys);
     }
 
     @SuppressWarnings("unchecked")
-    protected MutableOffsetMap(final Collection<K> keySet) {
-        if (!keySet.isEmpty()) {
-            removed = keySet.size();
-            offsets = OffsetMapCache.offsetsFor(keySet);
-            objects = (V[])new Object[removed];
-        } else {
-            offsets = ImmutableMap.of();
-            objects = (V[])EMPTY_ARRAY;
-        }
-
-        this.newKeys = new LinkedHashMap<>();
-    }
-
-    protected MutableOffsetMap(final ImmutableOffsetMap<K, V> m) {
-        this(m.offsets(), m.objects());
+    MutableOffsetMap(final HashMap<K, V> newKeys) {
+        this(ImmutableMap.<K, Integer>of(), (V[]) EMPTY_ARRAY, newKeys);
     }
 
     @SuppressWarnings("unchecked")
-    protected MutableOffsetMap(final Map<K, V> m) {
-        this(OffsetMapCache.offsetsFor(m.keySet()), (V[])m.values().toArray());
-    }
+    MutableOffsetMap(final Map<K, Integer> offsets, final Map<K, V> source, final HashMap<K, V> newKeys) {
+        this(offsets, (V[]) new Object[offsets.size()], newKeys);
 
-    protected MutableOffsetMap(final MutableOffsetMap<K, V> m) {
-        this.offsets = m.offsets;
-        this.objects = m.objects;
-        this.newKeys = new LinkedHashMap<>(m.newKeys);
-        this.removed = m.removed;
-    }
+        for (Entry<K, V> e : source.entrySet()) {
+            objects[offsets.get(e.getKey())] = Preconditions.checkNotNull(e.getValue());
+        }
 
-    private MutableOffsetMap(final Map<K, Integer> offsets, final V[] objects) {
-        this.offsets = Preconditions.checkNotNull(offsets);
-        this.objects = Preconditions.checkNotNull(objects);
-        this.newKeys = new LinkedHashMap<>();
+        this.needClone = false;
     }
 
     public static <K, V> MutableOffsetMap<K, V> copyOf(final Map<K, V> m) {
@@ -94,40 +92,35 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
             return ((MutableOffsetMap<K, V>) m).clone();
         }
         if (m instanceof ImmutableOffsetMap) {
-            return ((ImmutableOffsetMap<K, V>) m).toModifiableMap();
+            final ImmutableOffsetMap<K, V> om = (ImmutableOffsetMap<K, V>) m;
+            return new MutableOffsetMap.Ordered<>(om.offsets(), om.objects());
         }
 
-        return new MutableOffsetMap<>(m);
-    }
-
-    public static <K, V> MutableOffsetMap<K, V> forOffsets(final Map<K, Integer> offsets) {
-        @SuppressWarnings("unchecked")
-        final V[] objects = (V[]) new Object[offsets.size()];
-        return new MutableOffsetMap<>(offsets, objects);
+        return new MutableOffsetMap.Ordered<>(m);
     }
 
-    public static <K, V> MutableOffsetMap<K, V> forKeySet(final Collection<K> keySet) {
-        return forOffsets(OffsetMapCache.offsetsFor(keySet));
+    public static <K, V> MutableOffsetMap<K, V> of() {
+        return new MutableOffsetMap.Ordered<>();
     }
 
     @Override
-    public int size() {
+    public final int size() {
         return offsets.size() + newKeys.size() - removed;
     }
 
     @Override
-    public boolean isEmpty() {
+    public final boolean isEmpty() {
         return size() == 0;
     }
 
     @Override
-    public boolean containsKey(final Object key) {
+    public final boolean containsKey(final Object key) {
         final Integer offset = offsets.get(key);
         return offset == null ? newKeys.containsKey(key) : objects[offset] != null;
     }
 
     @Override
-    public V get(final Object key) {
+    public final V get(final Object key) {
         final Integer offset = offsets.get(key);
         return offset == null ? newKeys.get(key) : objects[offset];
     }
@@ -197,7 +190,7 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
     }
 
     @Override
-    public Set<Entry<K, V>> entrySet() {
+    public final Set<Entry<K, V>> entrySet() {
         return new EntrySet();
     }
 
@@ -261,14 +254,24 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
         return new ImmutableOffsetMap.Ordered<>(OffsetMapCache.offsetsFor(keyset), values);
     }
 
+    @SuppressWarnings("unchecked")
     @Override
     public MutableOffsetMap<K, V> clone() {
-        // FIXME: super.clone()
-        return new MutableOffsetMap<K, V>(this);
+        final MutableOffsetMap<K, V> ret;
+
+        try {
+            ret = (MutableOffsetMap<K, V>) super.clone();
+        } catch (CloneNotSupportedException e) {
+           throw new IllegalStateException("Clone is expected to work", e);
+        }
+
+        ret.newKeys = (HashMap<K, V>) newKeys.clone();
+        ret.needClone = true;
+        return ret;
     }
 
     @Override
-    public int hashCode() {
+    public final int hashCode() {
         int result = 0;
 
         for (Entry<K, Integer> e : offsets.entrySet()) {
@@ -282,7 +285,7 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
     }
 
     @Override
-    public boolean equals(final Object o) {
+    public final boolean equals(final Object o) {
         if (o == this) {
             return true;
         }
@@ -337,22 +340,22 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
     }
 
     @Override
-    public Set<K> keySet() {
+    public final Set<K> keySet() {
         return new KeySet();
     }
 
     @VisibleForTesting
-    boolean needClone() {
+    final boolean needClone() {
         return needClone;
     }
 
     @VisibleForTesting
-    Object array() {
+    final Object array() {
         return objects;
     }
 
     @VisibleForTesting
-    Object newKeys() {
+    final Object newKeys() {
         return newKeys;
     }
 

diff --git a/common/util/src/main/java/org/opendaylight/yangtools/util/SharedSingletonMap.java b/common/util/src/main/java/org/opendaylight/yangtools/util/SharedSingletonMap.java
index 3af4c0a98864d8fdb973cea64adf6e8a84c6d38a..9e1776adfb2c7b69a23e19e816d5df5dd89e9c15 100644 (file)
--- a/common/util/src/main/java/org/opendaylight/yangtools/util/SharedSingletonMap.java
+++ b/common/util/src/main/java/org/opendaylight/yangtools/util/SharedSingletonMap.java
@@ -34,7 +34,7 @@ public abstract class SharedSingletonMap<K, V> implements Serializable, Unmodifi
 
         @Override
         public ModifiableMapPhase<K, V> toModifiableMap() {
-            return new MutableOffsetMap<K, V>(this);
+            return MutableOffsetMap.copyOf(this);
         }
     }
 

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 8f95699c83396cd773c723a3febbba2d81c6a6df..4c4f6bbf18ebe7ae9f5bc418c0e90662884d290c 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,7 +15,6 @@ 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;
@@ -333,7 +332,7 @@ public class OffsetMapTest {
 
     @Test
     public void testEmptyMutable() throws CloneNotSupportedException {
-        final MutableOffsetMap<String, String> map = new MutableOffsetMap<>();
+        final MutableOffsetMap<String, String> map = MutableOffsetMap.of();
         assertTrue(map.isEmpty());
 
         final Map<String, String> other = map.clone();
@@ -341,15 +340,6 @@ public class OffsetMapTest {
         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();