BUG-4803: fix MutableOffsetMap's insertion order
[yangtools.git] / common / util / src / main / java / org / opendaylight / yangtools / util / MutableOffsetMap.java
index fe7aa69401b9de057afc9121ea79599684e64716..b49b51d493802f437b3cf7fd19140a2e737097ed 100644 (file)
@@ -10,16 +10,17 @@ 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.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Set;
@@ -40,53 +41,66 @@ 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.orderedOffsets(source.keySet()), source, new LinkedHashMap<K, V>());
+        }
+
+        Ordered(final Map<K, Integer> offsets, final V[] objects) {
+            super(offsets, objects, new LinkedHashMap<K, V>());
+        }
+
+        @Override
+        Object removedObject() {
+            return REMOVED;
+        }
+
+        @Override
+        UnmodifiableMapPhase<K, V> modifiedMap(final List<K> keys, final V[] objects) {
+            return new ImmutableOffsetMap.Ordered<>(OffsetMapCache.orderedOffsets(keys), objects);
+        }
+
+        @Override
+        UnmodifiableMapPhase<K, V> unmodifiedMap(final Map<K, Integer> offsets, final V[] objects) {
+            return new ImmutableOffsetMap.Ordered<>(offsets, objects);
+        }
+    }
+
     private static final Object[] EMPTY_ARRAY = new Object[0];
+    private static final Object REMOVED = new Object();
     private final Map<K, Integer> offsets;
-    private final Map<K, V> newKeys;
-    private V[] objects;
+    private HashMap<K, V> newKeys;
+    private Object[] 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,42 +108,63 @@ 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);
+        return new MutableOffsetMap.Ordered<>(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);
+    public static <K, V> MutableOffsetMap<K, V> of() {
+        return new MutableOffsetMap.Ordered<>();
     }
 
-    public static <K, V> MutableOffsetMap<K, V> forKeySet(final Collection<K> keySet) {
-        return forOffsets(OffsetMapCache.offsetsFor(keySet));
-    }
+    abstract Object removedObject();
+    abstract UnmodifiableMapPhase<K, V> modifiedMap(List<K> keys, V[] objects);
+    abstract UnmodifiableMapPhase<K, V> unmodifiedMap(Map<K, Integer> offsets, V[] objects);
 
     @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;
+        if (offset != null) {
+            final Object obj = objects[offset];
+            if (!REMOVED.equals(obj)) {
+                return obj != null;
+            }
+        }
+
+        return newKeys.containsKey(key);
     }
 
     @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];
+        if (offset != null) {
+            final Object obj = objects[offset];
+
+            /*
+             * This is a bit tricky:  Ordered will put REMOVED to removed objects to retain strict insertion order.
+             * Unordered will add null, indicating that the slot may be reused in future. Hence if we see a REMOVED
+             * marker, we need to fall back to checking with new keys.
+             */
+            if (!REMOVED.equals(obj)) {
+                @SuppressWarnings("unchecked")
+                final V ret = (V)obj;
+                return ret;
+            }
+        }
+
+        return newKeys.get(key);
     }
 
     private void cloneArray() {
@@ -142,76 +177,102 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
     }
 
     @Override
-    public V put(final K key, final V value) {
+    public final V put(final K key, final V value) {
         Preconditions.checkNotNull(value);
         final Integer offset = offsets.get(Preconditions.checkNotNull(key));
-        if (offset == null) {
-            final V ret = newKeys.put(key, value);
-            if (ret == null) {
-                modCount++;
+        if (offset != null) {
+            final Object obj = objects[offset];
+
+            /*
+             * Put which can potentially replace something in objects. Replacing an object does not cause iterators
+             * to be invalidated and does follow insertion order (since it is not a fresh insert). If the object has
+             * been removed, we fall back to newKeys.
+             */
+            if (!REMOVED.equals(obj)) {
+                @SuppressWarnings("unchecked")
+                final V ret = (V)obj;
+
+                cloneArray();
+                objects[offset] = value;
+                if (ret == null) {
+                    modCount++;
+                    removed--;
+                }
+
+                return ret;
             }
-            return ret;
         }
 
-        cloneArray();
-        final V ret = objects[offset];
-        objects[offset] = value;
+        final V ret = newKeys.put(key, value);
         if (ret == null) {
             modCount++;
-            removed--;
         }
-
         return ret;
     }
 
     @Override
-    public V remove(final Object key) {
+    public final V remove(final Object key) {
         final Integer offset = offsets.get(key);
-        if (offset == null) {
-            final V ret = newKeys.remove(key);
-            if (ret != null) {
-                modCount++;
+        if (offset != null) {
+            final Object obj = objects[offset];
+
+            /*
+             * A previous remove() may have indicated that the objects slot cannot be reused. In that case we need
+             * to fall back to checking with newKeys.
+             */
+            if (!REMOVED.equals(obj)) {
+                cloneArray();
+
+                @SuppressWarnings("unchecked")
+                final V ret = (V)obj;
+                objects[offset] = removedObject();
+                if (ret != null) {
+                    modCount++;
+                    removed++;
+                }
+                return ret;
             }
-            return ret;
         }
 
-        cloneArray();
-        final V ret = objects[offset];
-        objects[offset] = null;
+        final V ret = newKeys.remove(key);
         if (ret != null) {
             modCount++;
-            removed++;
         }
         return ret;
     }
 
     @Override
-    public void clear() {
+    public final void clear() {
         if (size() != 0) {
             newKeys.clear();
             cloneArray();
-            Arrays.fill(objects, null);
+            Arrays.fill(objects, removedObject());
             removed = objects.length;
             modCount++;
         }
     }
 
     @Override
-    public Set<Entry<K, V>> entrySet() {
+    public final Set<Entry<K, V>> entrySet() {
         return new EntrySet();
     }
 
     @Override
     public Map<K, V> toUnmodifiableMap() {
-        if (newKeys.isEmpty() && removed == 0) {
+        if (removed == 0 && newKeys.isEmpty()) {
             // Make sure next modification clones the array, as we leak it to the map we return.
             needClone = true;
+
+            // We have ended up with no removed objects, hence this cast is safe
+            @SuppressWarnings("unchecked")
+            final V[] values = (V[])objects;
+
             /*
              * TODO: we could track the ImmutableOffsetMap from which this one was instantiated and if we do not
              *       perform any modifications, just return the original instance. The trade-off is increased complexity
              *       and an additional field in this class.
              */
-            return new ImmutableOffsetMap.Ordered<>(offsets, objects);
+            return unmodifiedMap(offsets, values);
         }
 
         final int s = size();
@@ -219,15 +280,16 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
             return ImmutableMap.of();
         }
         if (s == 1) {
-            return ImmutableMap.copyOf(this);
+            return SharedSingletonMap.copyOf(this);
         }
 
         // Construct the set of keys
-        final Collection<K> keyset = new ArrayList<>(s);
+        final List<K> keyset = new ArrayList<>(s);
         if (removed != 0) {
             if (removed != offsets.size()) {
                 for (Entry<K, Integer> e : offsets.entrySet()) {
-                    if (objects[e.getValue()] != null) {
+                    final Object o = objects[e.getValue()];
+                    if (o != null && !REMOVED.equals(o)) {
                         keyset.add(e.getKey());
                     }
                 }
@@ -244,9 +306,11 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
         if (removed != 0) {
             if (removed != offsets.size()) {
                 for (Entry<K, Integer> e : offsets.entrySet()) {
-                    final V o = objects[e.getValue()];
-                    if (o != null) {
-                        values[i++] = o;
+                    final Object o = objects[e.getValue()];
+                    if (o != null && !REMOVED.equals(o)) {
+                        @SuppressWarnings("unchecked")
+                        final V v = (V) o;
+                        values[i++] = v;
                     }
                 }
             }
@@ -258,17 +322,27 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
             values[i++] = v;
         }
 
-        return new ImmutableOffsetMap.Ordered<>(OffsetMapCache.offsetsFor(keyset), values);
+        return modifiedMap(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 +356,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;
         }
@@ -323,8 +397,8 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
 
             // 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()))) {
+                final Object obj = objects[e.getValue()];
+                if (obj != null && !REMOVED.equals(obj) && !obj.equals(other.get(e.getKey()))) {
                     return false;
                 }
             }
@@ -337,22 +411,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;
     }
 
@@ -451,7 +525,8 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
         private void updateNextKey() {
             while (oldIterator.hasNext()) {
                 final Entry<K, Integer> e = oldIterator.next();
-                if (objects[e.getValue()] != null) {
+                final Object obj = objects[e.getValue()];
+                if (obj != null && !REMOVED.equals(obj)) {
                     nextKey = e.getKey();
                     return;
                 }
@@ -480,7 +555,7 @@ public final class MutableOffsetMap<K, V> extends AbstractMap<K, V> implements C
             final Integer offset = offsets.get(currentKey);
             if (offset != null) {
                 cloneArray();
-                objects[offset] = null;
+                objects[offset] = removedObject();
                 removed++;
             } else {
                 newIterator.remove();