Refactor BuildReferenceMapAdapter

Author: Dmytro Ukhlov

core/src/main/java/hudson/model/RunMap.java

@@ -201,7 +201,7 @@ protected String getIdOf(R r) {
 
     /**
      * Add a <em>new</em> build to the map.
-     * Do not use when loading existing builds (use {@link #put(Integer, Object)}).
+     * Do not use when loading existing builds (use {@link #putAll(Map)}).
      */
     @Override
     public R put(R r) {
@@ -218,7 +218,7 @@ public R put(R r) {
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
-        return super._put(r);
+        return super.put(r);
     }
 
     @CheckForNull

core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java

@@ -35,9 +35,7 @@
 import hudson.util.CopyOnWriteMap;
 import java.io.File;
 import java.io.IOException;
-import java.util.AbstractCollection;
 import java.util.AbstractMap;
-import java.util.AbstractSet;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
@@ -46,10 +44,7 @@
 import java.util.NoSuchElementException;
 import java.util.Set;
 import java.util.SortedMap;
-import java.util.Spliterator;
-import java.util.Spliterators;
 import java.util.TreeMap;
-import java.util.function.IntConsumer;
 import java.util.function.IntPredicate;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -101,146 +96,23 @@
 public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R> implements SortedMap<Integer, R> {
     private final CopyOnWriteMap.Tree<Integer, BuildReference<R>> core = new CopyOnWriteMap.Tree<>(
             Collections.reverseOrder());
-
-    private LazyLoadRunMapEntrySet<R> entrySet = new LazyLoadRunMapEntrySet<>(this);
-
-    private transient volatile Set<Integer> keySet;
-    private transient volatile Collection<R> values;
+    private final BuildReferenceMapAdapter<R> adapter = new BuildReferenceMapAdapter<>(core, this::resolveBuildRef,
+            this::getNumberOf, this::removeValue);
 
     @Override
     public Set<Integer> keySet() {
-        Set<Integer> ks = keySet;
-        if (ks == null) {
-            ks = new AbstractSet<>() {
-                @Override
-                public Iterator<Integer> iterator() {
-                    return new Iterator() {
-                        private final Iterator<Entry<Integer, R>> it = entrySet().iterator();
-
-                        @Override
-                        public boolean hasNext() {
-                            return it.hasNext();
-                        }
-
-                        @Override
-                        public Integer next() {
-                            return it.next().getKey();
-                        }
-
-                        @Override
-                        public void remove() {
-                            it.remove();
-                        }
-                    };
-                }
-
-                @Override
-                public Spliterator<Integer> spliterator() {
-                    return new Spliterators.AbstractIntSpliterator(
-                            Long.MAX_VALUE,
-                            Spliterator.DISTINCT | Spliterator.ORDERED | Spliterator.SORTED) {
-                        private final Iterator<Integer> it = iterator();
-
-                        @Override
-                        public boolean tryAdvance(IntConsumer action) {
-                            if (action == null) {
-                                throw new NullPointerException();
-                            }
-                            if (it.hasNext()) {
-                                action.accept(it.next());
-                                return true;
-                            }
-                            return false;
-                        }
-
-                        @Override
-                        public Comparator<Integer> getComparator() {
-                            return Collections.reverseOrder();
-                        }
-                    };
-                }
-
-                @Override
-                public int size() {
-                    return AbstractLazyLoadRunMap.this.size();
-                }
-
-                @Override
-                public boolean isEmpty() {
-                    return AbstractLazyLoadRunMap.this.isEmpty();
-                }
-
-                @Override
-                public void clear() {
-                    AbstractLazyLoadRunMap.this.clear();
-                }
-
-                @Override
-                public boolean contains(Object k) {
-                    return AbstractLazyLoadRunMap.this.containsKey(k);
-                }
-            };
-            keySet = ks;
-        }
-        return ks;
+        return adapter.keySet();
     }
 
     @Override
     public Collection<R> values() {
-        Collection<R> vals = values;
-        if (vals == null) {
-            vals = new AbstractCollection<>() {
-                @Override
-                public Iterator<R> iterator() {
-                    return new Iterator<>() {
-                        private final Iterator<Entry<Integer, R>> it = entrySet().iterator();
-
-                        @Override
-                        public boolean hasNext() {
-                            return it.hasNext();
-                        }
-
-                        @Override
-                        public R next() {
-                            return it.next().getValue();
-                        }
-
-                        @Override
-                        public void remove() {
-                            it.remove();
-                        }
-                    };
-                }
-
-                @Override
-                public Spliterator<R> spliterator() {
-                    return Spliterators.spliteratorUnknownSize(
-                            iterator(), Spliterator.DISTINCT | Spliterator.ORDERED);
-                }
-
-                @Override
-                public int size() {
-                    return AbstractLazyLoadRunMap.this.size();
-                }
-
-                @Override
-                public boolean isEmpty() {
-                    return AbstractLazyLoadRunMap.this.isEmpty();
-                }
-
-                @Override
-                public void clear() {
-                    AbstractLazyLoadRunMap.this.clear();
-                }
+        return adapter.values();
+    }
 
-                @Override
-                public boolean contains(Object v) {
-                    return AbstractLazyLoadRunMap.this.containsValue(v);
-                }
-            };
-            values = vals;
-        }
-        return vals;
+    @Override
+    public Set<Entry<Integer, R>> entrySet() {
+        assert baseDirInitialized();
+        return adapter.entrySet();
     }
 
     /**
@@ -359,13 +231,17 @@ public Comparator<? super Integer> comparator() {
 
     @Override
     public boolean isEmpty() {
-        return search(Integer.MAX_VALUE, DESC) == null;
+        return adapter.isEmpty();
     }
 
     @Override
-    public Set<Entry<Integer, R>> entrySet() {
-        assert baseDirInitialized();
-        return entrySet;
+    public boolean containsKey(Object value) {
+        return adapter.containsKey(value);
+    }
+
+    @Override
+    public boolean containsValue(Object value) {
+        return adapter.containsValue(value);
     }
 
     /**
@@ -379,7 +255,7 @@ public SortedMap<Integer, R> getLoadedBuilds() {
                 res.put(entry.getKey(), buildRef);
             }
         }
-        return Collections.unmodifiableSortedMap(new BuildReferenceMapAdapter<>(this, res));
+        return Collections.unmodifiableSortedMap(new BuildReferenceMapAdapter<>(res, this::resolveBuildRef, this::getNumberOf));
     }
 
     /**
@@ -390,33 +266,17 @@ public SortedMap<Integer, R> getLoadedBuilds() {
      */
     @Override
     public SortedMap<Integer, R> subMap(Integer fromKey, Integer toKey) {
-        // TODO: if this method can produce a lazy map, that'd be wonderful
-        // because due to the lack of floor/ceil/higher/lower kind of methods
-        // to look up keys in SortedMap, various places of Jenkins rely on
-        // subMap+firstKey/lastKey combo.
-
-        R start = search(fromKey, DESC);
-        if (start == null)    return EMPTY_SORTED_MAP;
-
-        R end = search(toKey, ASC);
-        if (end == null)      return EMPTY_SORTED_MAP;
-
-        for (R i = start; i != end; ) {
-            i = search(getNumberOf(i) - 1, DESC);
-            assert i != null;
-        }
-
-        return Collections.unmodifiableSortedMap(new BuildReferenceMapAdapter<>(this, core.subMap(fromKey, toKey)));
+        return Collections.unmodifiableSortedMap(adapter.subMap(fromKey, toKey));
     }
 
     @Override
     public SortedMap<Integer, R> headMap(Integer toKey) {
-        return subMap(Integer.MAX_VALUE, toKey);
+        return Collections.unmodifiableSortedMap(adapter.headMap(toKey));
     }
 
     @Override
     public SortedMap<Integer, R> tailMap(Integer fromKey) {
-        return subMap(fromKey, Integer.MIN_VALUE);
+        return Collections.unmodifiableSortedMap(adapter.tailMap(fromKey));
     }
 
     @Override
@@ -463,7 +323,7 @@ public R get(int n) {
      * @since 2.14
      */
     public boolean runExists(int number) {
-        return this.core.containsKey(number);
+        return adapter.containsKey(number);
     }
 
     /**
@@ -552,7 +412,7 @@ private R resolveBuildRef(BuildReference<R> ref) {
     }
 
     public R getByNumber(int n) {
-        return resolveBuildRef(core.get(n));
+        return adapter.get(n);
     }
 
     /**
@@ -597,14 +457,13 @@ public synchronized void putAll(Map<? extends Integer, ? extends R> newData) {
         core.putAll(newWrapperData);
     }
 
-    /**
-     * Return underlining {@link BuildReference} core map.
-     *
-     * @return
-     *      full build reference map.
-     */
-    /*package*/ SortedMap<Integer, BuildReference<R>> all() {
-        return core;
+    @Override
+    public R remove(Object key) {
+        R val = get(key);
+        if (val == null) {
+            return null;
+        }
+        return removeValue(val) ? val : null;
     }
 
     /**
@@ -652,7 +511,6 @@ protected BuildReference<R> createReference(R r) {
         return new BuildReference<>(getIdOf(r), r);
     }
 
-
     /**
      * Parses {@code R} instance from data in the specified directory.
      *
@@ -694,7 +552,5 @@ public enum Direction {
         ASC, DESC, EXACT
     }
 
-    private static final SortedMap EMPTY_SORTED_MAP = Collections.unmodifiableSortedMap(new TreeMap());
-
     static final Logger LOGGER = Logger.getLogger(AbstractLazyLoadRunMap.class.getName());
 }