ChildNodeEntries improvements

jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ChildNodeEntries.java

@@ -16,15 +16,15 @@
  */
 package org.apache.jackrabbit.core.state;
 
-import org.apache.commons.collections.map.LinkedMap;
 import org.apache.jackrabbit.core.id.NodeId;
-import org.apache.jackrabbit.core.util.EmptyLinkedMap;
+import org.apache.jackrabbit.core.util.EmptyLinkedHashMap;
 import org.apache.jackrabbit.spi.Name;
 
 import java.util.List;
 import java.util.HashMap;
 import java.util.Collections;
 import java.util.ArrayList;
+import java.util.LinkedHashMap;
 import java.util.Map;
 
 /**
@@ -34,11 +34,21 @@
  */
 class ChildNodeEntries implements Cloneable {
 
+    @SuppressWarnings("unchecked")
+    private static final LinkedHashMap<NodeId, ChildNodeEntry> EMPTY_ENTRIES = EmptyLinkedHashMap.INSTANCE;
+
+    // The JavaDoc for Collections.emptyMap() states:
+    // "Implementations of this method need not create a separate Map object
+    // for each call."
+    // https://docs.oracle.com/javase/8/docs/api/java/util/Collections.html#emptyMap--
+    // This indicates that an implementation *might* return a separate Map object.
+    private static final Map<Name, Object> EMPTY_NAME_MAP = Collections.emptyMap();
+
     /**
      * Insertion-ordered map of entries
      * (key=NodeId, value=entry)
      */
-    private LinkedMap entries;
+    private LinkedHashMap<NodeId, ChildNodeEntry> entries;
 
     /**
      * Map used for lookup by name
@@ -57,7 +67,7 @@ class ChildNodeEntries implements Cloneable {
     }
 
     ChildNodeEntry get(NodeId id) {
-        return (ChildNodeEntry) entries.get(id);
+        return entries.get(id);
     }
 
     @SuppressWarnings("unchecked")
@@ -193,7 +203,7 @@ public ChildNodeEntry remove(Name nodeName, int index) {
         }
 
         // clean up name lookup map if necessary
-        if (siblings.size() == 0) {
+        if (siblings.isEmpty()) {
             // no more entries with that name left:
             // remove from name lookup map as well
             nameMap.remove(nodeName);
@@ -208,13 +218,13 @@ public ChildNodeEntry remove(Name nodeName, int index) {
     }
 
     /**
-     * Removes the child node entry refering to the node with the given id.
+     * Removes the child node entry referring to the node with the given id.
      *
      * @param id id of node whose entry is to be removed.
      * @return the removed entry or <code>null</code> if there is no such entry.
      */
     ChildNodeEntry remove(NodeId id) {
-        ChildNodeEntry entry = (ChildNodeEntry) entries.get(id);
+        ChildNodeEntry entry = entries.get(id);
         if (entry != null) {
             return remove(entry.getName(), entry.getIndex());
         }
@@ -260,8 +270,7 @@ List<ChildNodeEntry> removeAll(ChildNodeEntries other) {
         }
 
         List<ChildNodeEntry> result = new ArrayList<ChildNodeEntry>();
-        for (Object e : entries.values()) {
-            ChildNodeEntry entry = (ChildNodeEntry) e;
+        for (ChildNodeEntry entry : entries.values()) {
             ChildNodeEntry otherEntry = other.get(entry.getId());
             if (entry == otherEntry) {
                 continue;
@@ -294,8 +303,7 @@ List<ChildNodeEntry> retainAll(ChildNodeEntries other) {
         }
 
         List<ChildNodeEntry> result = new ArrayList<ChildNodeEntry>();
-        for (Object e : entries.values()) {
-            ChildNodeEntry entry = (ChildNodeEntry) e;
+        for (ChildNodeEntry entry : entries.values()) {
             ChildNodeEntry otherEntry = other.get(entry.getId());
             if (entry == otherEntry) {
                 result.add(entry);
@@ -313,15 +321,13 @@ public boolean isEmpty() {
         return entries.isEmpty();
     }
 
-    @SuppressWarnings("unchecked")
     public List<ChildNodeEntry> list() {
         return new ArrayList<ChildNodeEntry>(entries.values());
     }
 
     public List<ChildNodeEntry> getRenamedEntries(ChildNodeEntries that) {
         List<ChildNodeEntry> renamed = Collections.emptyList();
-        for (Object e : entries.values()) {
-            ChildNodeEntry entry = (ChildNodeEntry) e;
+        for (ChildNodeEntry entry : entries.values()) {
             ChildNodeEntry other = that.get(entry.getId());
             if (other != null && !entry.getName().equals(other.getName())) {
                 // child node entry with same id but different name exists in
@@ -375,7 +381,7 @@ public int hashCode() {
     protected Object clone() {
         try {
             ChildNodeEntries clone = (ChildNodeEntries) super.clone();
-            if (nameMap != Collections.EMPTY_MAP) {
+            if (nameMap != EMPTY_NAME_MAP) {
                 clone.shared = true;
                 shared = true;
             }
@@ -392,8 +398,8 @@ protected Object clone() {
      * Initializes the name and entries map with unmodifiable empty instances.
      */
     private void init() {
-        nameMap = Collections.emptyMap();
-        entries = EmptyLinkedMap.INSTANCE;
+        nameMap = EMPTY_NAME_MAP;
+        entries = EMPTY_ENTRIES;
         shared = false;
     }
 
@@ -403,11 +409,11 @@ private void init() {
      */
     @SuppressWarnings("unchecked")
     private void ensureModifiable() {
-        if (nameMap == Collections.EMPTY_MAP) {
+        if (nameMap == EMPTY_NAME_MAP) {
             nameMap = new HashMap<Name, Object>();
-            entries = new LinkedMap();
+            entries = new LinkedHashMap<NodeId, ChildNodeEntry>();
         } else if (shared) {
-            entries = (LinkedMap) entries.clone();
+            entries = (LinkedHashMap<NodeId, ChildNodeEntry>) entries.clone();
             nameMap = new HashMap<Name, Object>(nameMap);
             for (Map.Entry<Name, Object> entry : nameMap.entrySet()) {
                 Object value = entry.getValue();

jackrabbit-core/src/main/java/org/apache/jackrabbit/core/util/EmptyLinkedHashMap.java

@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.util;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Set;
+
+public class EmptyLinkedHashMap<K, V> extends LinkedHashMap<K, V> {
+
+    public static final EmptyLinkedHashMap INSTANCE = new EmptyLinkedHashMap();
+
+    private EmptyLinkedHashMap() { }
+
+    @Override
+    public V put(K key, V value) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public void putAll(Map<? extends K, ? extends V> m) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public V remove(Object key) {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public void clear() {
+        throw new UnsupportedOperationException();
+    }
+
+    @Override
+    public Set<K> keySet() {
+        return Collections.emptySet();
+    }
+
+    @Override
+    public Set<Map.Entry<K,V>> entrySet() {
+        return Collections.emptySet();
+    }
+
+    @Override
+    public Collection<V> values() {
+        return Collections.emptySet();
+    }
+
+    @Override
+    public Object clone() {
+        return this;
+    }
+}