[JENKINS-75986] Refactor BuildReferenceMapAdapter (#10946)

Co-authored-by: Jesse Glick <jglick@cloudbees.com>
Co-authored-by: Dmytro Ukhlov <dukhlov@csod.com>

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

@@ -95,7 +95,7 @@ public final class RunMap<R extends Run<?, R>> extends AbstractLazyLoadRunMap<R>
      *      Used to create new instance of {@link Run}.
      * @since 2.451
      */
-    public RunMap(@NonNull Job<?, ?> job, Constructor cons) {
+    public RunMap(@NonNull Job<?, ?> job, Constructor<R> cons) {
         this.job = Objects.requireNonNull(job);
         this.cons = cons;
         initBaseDir(job.getBuildDir());
@@ -105,7 +105,7 @@ public final class RunMap<R extends Run<?, R>> extends AbstractLazyLoadRunMap<R>
      * @deprecated Use {@link #RunMap(Job, Constructor)}.
      */
     @Deprecated
-    public RunMap(File baseDir, Constructor cons) {
+    public RunMap(File baseDir, Constructor<R> cons) {
         job = null;
         this.cons = cons;
         initBaseDir(baseDir);
@@ -187,6 +187,8 @@ public final class RunMap<R extends Run<?, R>> extends AbstractLazyLoadRunMap<R>
      */
     public interface Constructor<R extends Run<?, R>> {
         R create(File dir) throws IOException;
+
+        Class<R> getBuildClass();
     }
 
     @Override
@@ -201,7 +203,7 @@ public final class RunMap<R extends Run<?, R>> extends AbstractLazyLoadRunMap<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) {
@@ -298,6 +300,11 @@ public final class RunMap<R extends Run<?, R>> extends AbstractLazyLoadRunMap<R>
         return null;
     }
 
+    @Override
+    protected Class<R> getBuildClass() {
+        return cons.getBuildClass();
+    }
+
     /**
      * Backward compatibility method that notifies {@link RunMap} of who the owner is.
      *

core/src/main/java/hudson/util/Iterators.java

@@ -35,6 +35,7 @@ import java.util.ListIterator;
 import java.util.NoSuchElementException;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Function;
 import org.kohsuke.accmod.Restricted;
 import org.kohsuke.accmod.restrictions.NoExternalUse;
 
@@ -326,6 +327,18 @@ public class Iterators {
         return com.google.common.collect.Iterators.filter(itr, Objects::nonNull);
     }
 
+    /**
+     * Wraps another iterator and map iterable objects.
+     */
+    public static <T, U> Iterator<U> map(final Iterator<T> itr, Function<T, U> mapper) {
+        return new AdaptedIterator<>(itr) {
+            @Override
+            protected U adapt(T item) {
+                return mapper.apply(item);
+            }
+        };
+    }
+
     /**
      * Returns an {@link Iterable} that iterates over all the given {@link Iterable}s.
      *

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

@@ -35,9 +35,7 @@ import hudson.model.listeners.RunListener;
 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.Map;
 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,28 @@ import org.kohsuke.accmod.restrictions.NoExternalUse;
 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::getBuildClass) {
+        @Override
+        protected boolean removeValue(R value) {
+            return AbstractLazyLoadRunMap.this.removeValue(value);
+        }
+    };
 
     @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();
+        return adapter.values();
+    }
 
-                        @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();
-                }
-
-                @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();
     }
 
     /**
@@ -354,18 +231,22 @@ public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R>
 
     @Override
     public Comparator<? super Integer> comparator() {
-        return Collections.reverseOrder();
+        return core.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 +260,7 @@ public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R>
                 res.put(entry.getKey(), buildRef);
             }
         }
-        return Collections.unmodifiableSortedMap(new BuildReferenceMapAdapter<>(this, res));
+        return new BuildReferenceMapAdapter<>(res, this::resolveBuildRef, this::getNumberOf, this::getBuildClass);
     }
 
     /**
@@ -390,33 +271,17 @@ public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R>
      */
     @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 adapter.subMap(fromKey, toKey);
     }
 
     @Override
     public SortedMap<Integer, R> headMap(Integer toKey) {
-        return subMap(Integer.MAX_VALUE, toKey);
+        return adapter.headMap(toKey);
     }
 
     @Override
     public SortedMap<Integer, R> tailMap(Integer fromKey) {
-        return subMap(fromKey, Integer.MIN_VALUE);
+        return adapter.tailMap(fromKey);
     }
 
     @Override
@@ -443,11 +308,7 @@ public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R>
 
     @Override
     public R get(Object key) {
-        if (key instanceof Integer) {
-            int n = (Integer) key;
-            return get(n);
-        }
-        return super.get(key);
+        return adapter.get(key);
     }
 
     public R get(int n) {
@@ -552,7 +413,7 @@ public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R>
     }
 
     public R getByNumber(int n) {
-        return resolveBuildRef(core.get(n));
+        return adapter.get(n);
     }
 
     /**
@@ -597,14 +458,9 @@ public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R>
         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) {
+        return adapter.remove(key);
     }
 
     /**
@@ -652,7 +508,6 @@ public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R>
         return new BuildReference<>(getIdOf(r), r);
     }
 
-
     /**
      * Parses {@code R} instance from data in the specified directory.
      *
@@ -664,6 +519,8 @@ public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R>
      */
     protected abstract R retrieve(File dir) throws IOException;
 
+    protected abstract Class<R> getBuildClass();
+
     public synchronized boolean removeValue(R run) {
         return core.remove(getNumberOf(run)) != null;
     }
@@ -694,7 +551,5 @@ public abstract class AbstractLazyLoadRunMap<R> extends AbstractMap<Integer, R>
         ASC, DESC, EXACT
     }
 
-    private static final SortedMap EMPTY_SORTED_MAP = Collections.unmodifiableSortedMap(new TreeMap());
-
     static final Logger LOGGER = Logger.getLogger(AbstractLazyLoadRunMap.class.getName());
 }

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

@@ -1,54 +1,69 @@
 package jenkins.model.lazy;
 
-import edu.umd.cs.findbugs.annotations.Nullable;
 import hudson.util.AdaptedIterator;
 import hudson.util.Iterators;
+import java.util.AbstractCollection;
 import java.util.AbstractMap;
-import java.util.ArrayList;
+import java.util.AbstractSet;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.SortedMap;
+import java.util.Spliterator;
+import java.util.Spliterators;
+import java.util.function.Function;
+import java.util.function.IntConsumer;
+import java.util.function.Supplier;
 
 /**
  * Take {@code SortedMap<Integer,BuildReference<R>>} and make it look like {@code SortedMap<Integer,R>}.
  *
- * When {@link BuildReference} lost the build object, we'll use {@link AbstractLazyLoadRunMap#getById(String)}
- * to obtain one.
+ * <p>
+ * When {@link BuildReference} lost the build object, we'll use {@link #buildRefResolver} to obtain one.
+ * </p>
+ *
+ * <p>
+ * By default, this adapter provides a read-only view of the underlying {@link #core} map,
+ * which may be modified externally to change the adapter's state.
+ * Support for removal operations through {@link #entrySet()}, {@link #values()},
+ * {@link #keySet()}, and their iterators can be enabled by overriding
+ * {@link #removeValue(Object)}. This method is invoked by the internal collection views
+ * to perform the actual removal logic (such as updating {@link #core} or performing other actions).
+ * </p>
+ *
+ * <p>
+ * Some operations are weakly implemented (for example, {@link #size()} may be approximate).
+ * This adapter implements {@link SortedMap}, which does not allow {@code null} keys; however,
+ * methods such as {@code get(null)} or {@code containsKey(null)} do not throw a {@link NullPointerException}
+ * and instead return {@code null} or {@code false}, respectively, indicating that the key was not found.
+ * </p>
  *
  * @author Kohsuke Kawaguchi
  */
-class BuildReferenceMapAdapter<R> implements SortedMap<Integer, R> {
-    private final AbstractLazyLoadRunMap<R> loader;
+class BuildReferenceMapAdapter<R> extends AbstractMap<Integer, R> implements SortedMap<Integer, R> {
+    private final Function<BuildReference<R>, R> buildRefResolver;
+    private final Function<R, Integer> buildNumberProvider;
+    private final Supplier<Class<R>> buildClassProvider;
 
     private final SortedMap<Integer, BuildReference<R>> core;
 
-    BuildReferenceMapAdapter(AbstractLazyLoadRunMap<R> loader, SortedMap<Integer, BuildReference<R>> core) {
-        this.loader = loader;
+    private final Set<Integer> keySet = new KeySetAdapter();
+    private final Collection<R> values = new ValuesAdapter();
+    private final Set<Map.Entry<Integer, R>> entrySet = new EntrySetAdapter();
+
+    BuildReferenceMapAdapter(SortedMap<Integer, BuildReference<R>> core,
+                             Function<BuildReference<R>, R> buildRefResolver,
+                             Function<R, Integer> buildNumberProvider,
+                             Supplier<Class<R>> buildClassProvider) {
+        this.buildRefResolver = buildRefResolver;
+        this.buildNumberProvider = buildNumberProvider;
         this.core = core;
+        this.buildClassProvider = buildClassProvider;
     }
 
-    private R unwrap(@Nullable BuildReference<R> ref) {
-        if (ref == null)  return null;
-
-        R v = ref.get();
-        if (v == null)
-            v = loader.getById(ref.id);
-        return v;
-    }
-
-    private BuildReference<R> wrap(@Nullable R value) {
-        if (value == null)    return null;
-        return loader.createReference(value);
-    }
-
-
-
-
     @Override
     public Comparator<? super Integer> comparator() {
         return core.comparator();
@@ -56,17 +71,20 @@ class BuildReferenceMapAdapter<R> implements SortedMap<Integer, R> {
 
     @Override
     public SortedMap<Integer, R> subMap(Integer fromKey, Integer toKey) {
-        return new BuildReferenceMapAdapter<>(loader, core.subMap(fromKey, toKey));
+        return new BuildReferenceMapAdapter<>(core.subMap(fromKey, toKey),
+                buildRefResolver, buildNumberProvider, buildClassProvider);
     }
 
     @Override
     public SortedMap<Integer, R> headMap(Integer toKey) {
-        return new BuildReferenceMapAdapter<>(loader, core.headMap(toKey));
+        return new BuildReferenceMapAdapter<>(core.headMap(toKey),
+                buildRefResolver, buildNumberProvider, buildClassProvider);
     }
 
     @Override
     public SortedMap<Integer, R> tailMap(Integer fromKey) {
-        return new BuildReferenceMapAdapter<>(loader, core.tailMap(fromKey));
+        return new BuildReferenceMapAdapter<>(core.tailMap(fromKey),
+                buildRefResolver, buildNumberProvider, buildClassProvider);
     }
 
     @Override
@@ -81,305 +99,218 @@ class BuildReferenceMapAdapter<R> implements SortedMap<Integer, R> {
 
     @Override
     public Set<Integer> keySet() {
-        return core.keySet();
+        return keySet;
     }
 
     @Override
     public Collection<R> values() {
-        return new CollectionAdapter(core.values());
+        return values;
     }
 
     @Override
     public Set<Entry<Integer, R>> entrySet() {
-        return new SetAdapter(core.entrySet());
-    }
-
-    @Override
-    public int size() {
-        return core.size();
+        return entrySet;
     }
 
     @Override
     public boolean isEmpty() {
-        return core.isEmpty();
+        return entrySet().isEmpty();
     }
 
     @Override
     public boolean containsKey(Object key) {
-        return core.containsKey(key);
+        BuildReference<R> ref = key instanceof Integer ? core.get(key) : null;
+        if (ref == null) {
+            return false;
+        }
+        // if found, check if value is loadable
+        if (!ref.isSet()) {
+            buildRefResolver.apply(ref);
+        }
+        return !ref.isUnloadable();
     }
 
     @Override
     public boolean containsValue(Object value) {
-        return core.containsValue(value); // TODO should this be core.containsValue(wrap(value))?
+        if (!buildClassProvider.get().isInstance(value)) {
+            return false;
+        }
+        R val = buildClassProvider.get().cast(value);
+        return val.equals(get(buildNumberProvider.apply(val)));
     }
 
     @Override
     public R get(Object key) {
-        return unwrap(core.get(key));
-    }
-
-    @Override
-    public R put(Integer key, R value) {
-        return unwrap(core.put(key, wrap(value)));
+        return key instanceof Integer ? buildRefResolver.apply(core.get(key)) : null;
     }
 
     @Override
     public R remove(Object key) {
-        return unwrap(core.remove(key));
-    }
-
-    @Override
-    public void putAll(Map<? extends Integer, ? extends R> m) {
-        for (Entry<? extends Integer, ? extends R> e : m.entrySet())
-            put(e.getKey(), e.getValue());
-    }
-
-    @Override
-    public void clear() {
-        core.clear();
-    }
-
-    @Override
-    public boolean equals(Object o) {
-        return core.equals(o); // TODO this is wrong
-    }
-
-    @Override
-    public int hashCode() {
-        return core.hashCode();
-    }
-
-    @Override public String toString() {
-        return new LinkedHashMap<>(this).toString();
-    }
-
-    private class CollectionAdapter implements Collection<R> {
-        private final Collection<BuildReference<R>> core;
-
-        private CollectionAdapter(Collection<BuildReference<R>> core) {
-            this.core = core;
+        R val = get(key);
+        if (val == null) {
+            return null;
         }
+        return removeValue(val) ? val : null;
+    }
 
+    protected boolean removeValue(R value) {
+        throw new UnsupportedOperationException();
+    }
+
+    private class KeySetAdapter extends AbstractSet<Integer> {
         @Override
         public int size() {
-            return core.size();
+            return BuildReferenceMapAdapter.this.size();
         }
 
         @Override
         public boolean isEmpty() {
-            return core.isEmpty();
+            return BuildReferenceMapAdapter.this.isEmpty();
         }
 
         @Override
-        public boolean contains(Object o) {
-            // TODO: to properly pass this onto core, we need to wrap o into BuildReference but also needs to figure out ID.
-            throw new UnsupportedOperationException();
+        public boolean contains(Object k) {
+            return BuildReferenceMapAdapter.this.containsKey(k);
+        }
+
+        @Override
+        public boolean remove(Object o) {
+            return BuildReferenceMapAdapter.this.remove(o) != null;
+        }
+
+        @Override
+        public Iterator<Integer> iterator() {
+            return new AdaptedIterator<>(BuildReferenceMapAdapter.this.entrySet().iterator()) {
+                @Override
+                protected Integer adapt(Entry<Integer, R> e) {
+                    return e.getKey();
+                }
+            };
+        }
+
+        @Override
+        public Spliterator<Integer> spliterator() {
+            return new Spliterators.AbstractIntSpliterator(Long.MAX_VALUE,
+                    Spliterator.DISTINCT | Spliterator.ORDERED | Spliterator.SORTED) {
+                private final Iterator<Integer> it = KeySetAdapter.this.iterator();
+
+                @Override
+                public boolean tryAdvance(IntConsumer action) {
+                    Objects.requireNonNull(action);
+                    if (it.hasNext()) {
+                        action.accept(it.next());
+                        return true;
+                    }
+                    return false;
+                }
+
+                @Override
+                public Comparator<? super Integer> getComparator() {
+                    return BuildReferenceMapAdapter.this.comparator();
+                }
+            };
+        }
+    }
+
+    private class ValuesAdapter extends AbstractCollection<R> {
+        @Override
+        public int size() {
+            return BuildReferenceMapAdapter.this.size();
+        }
+
+        @Override
+        public boolean isEmpty() {
+            return BuildReferenceMapAdapter.this.isEmpty();
+        }
+
+        @Override
+        public boolean contains(Object v) {
+            return BuildReferenceMapAdapter.this.containsValue(v);
+        }
+
+        @Override
+        public boolean remove(Object o) {
+            return buildClassProvider.get().isInstance(o) &&
+                    BuildReferenceMapAdapter.this.removeValue(buildClassProvider.get().cast(o));
         }
 
         @Override
         public Iterator<R> iterator() {
-            // silently drop null, as if we didn't have them in this collection in the first place
-            // this shouldn't be indistinguishable from concurrent modifications to the collection
-            return Iterators.removeNull(new AdaptedIterator<>(core.iterator()) {
+            return new AdaptedIterator<>(BuildReferenceMapAdapter.this.entrySet().iterator()) {
                 @Override
-                protected R adapt(BuildReference<R> ref) {
-                    return unwrap(ref);
+                protected R adapt(Entry<Integer, R> e) {
+                    return e.getValue();
                 }
-            });
+            };
         }
 
         @Override
-        public Object[] toArray() {
-            List<Object> list = new ArrayList<>(size());
-            for (var e : this) {
-                list.add(e);
-            }
-            return list.toArray();
-        }
-
-        @Override
-        public <T> T[] toArray(T[] a) {
-            return new ArrayList<>(this).toArray(a);
-        }
-
-        @Override
-        public boolean add(R value) {
-            return core.add(wrap(value));
-        }
-
-        @Override
-        public boolean remove(Object o) {
-//            return core.remove(o);
-            // TODO: to properly pass this onto core, we need to wrap o into BuildReference but also needs to figure out ID.
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public boolean containsAll(Collection<?> c) {
-            for (Object o : c) {
-                if (!contains(o))
-                    return false;
-            }
-            return true;
-        }
-
-        @Override
-        public boolean addAll(Collection<? extends R> c) {
-            boolean b = false;
-            for (R r : c) {
-                b |= add(r);
-            }
-            return b;
-        }
-
-        @Override
-        public boolean removeAll(Collection<?> c) {
-            boolean b = false;
-            for (Object o : c) {
-                b |= remove(o);
-            }
-            return b;
-        }
-
-        @Override
-        public boolean retainAll(Collection<?> c) {
-            // TODO: to properly pass this onto core, we need to wrap o into BuildReference but also needs to figure out ID.
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public void clear() {
-            core.clear();
-        }
-
-        @Override
-        public boolean equals(Object o) {
-            return core.equals(o);
-        }
-
-        @Override
-        public int hashCode() {
-            return core.hashCode();
+        public Spliterator<R> spliterator() {
+            return Spliterators.spliteratorUnknownSize(iterator(), Spliterator.DISTINCT | Spliterator.ORDERED);
         }
     }
 
-    private class SetAdapter implements Set<Entry<Integer, R>> {
-        private final Set<Entry<Integer, BuildReference<R>>> core;
-
-        private SetAdapter(Set<Entry<Integer, BuildReference<R>>> core) {
-            this.core = core;
-        }
-
+    private class EntrySetAdapter extends AbstractSet<Entry<Integer, R>> {
         @Override
         public int size() {
-            return core.size();
+            return BuildReferenceMapAdapter.this.core.size();
         }
 
         @Override
         public boolean isEmpty() {
-            return core.isEmpty();
+            return this.stream().findFirst().isEmpty();
         }
 
         @Override
         public boolean contains(Object o) {
-            // TODO: to properly pass this onto core, we need to wrap o into BuildReference but also needs to figure out ID.
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public Iterator<Entry<Integer, R>> iterator() {
-            return Iterators.removeNull(new AdaptedIterator<>(core.iterator()) {
-                @Override
-                protected Entry<Integer, R> adapt(Entry<Integer, BuildReference<R>> e) {
-                    return _unwrap(e);
-                }
-            });
-        }
-
-        @Override
-        public Object[] toArray() {
-            List<Object> list = new ArrayList<>(size());
-            for (var e : this) {
-                list.add(e);
+            if (o instanceof Map.Entry<?, ?> e && e.getKey() instanceof Integer key) {
+                return e.getValue() != null && e.getValue().equals(BuildReferenceMapAdapter.this.get(key));
             }
-            return list.toArray();
-        }
-
-        @Override
-        public <T> T[] toArray(T[] a) {
-            return new ArrayList<>(this).toArray(a);
-        }
-
-        @Override
-        public boolean add(Entry<Integer, R> value) {
-            return core.add(_wrap(value));
+            return false;
         }
 
         @Override
         public boolean remove(Object o) {
-//            return core.remove(o);
-            // TODO: to properly pass this onto core, we need to wrap o into BuildReference but also needs to figure out ID.
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public boolean containsAll(Collection<?> c) {
-            for (Object o : c) {
-                if (!contains(o))
-                    return false;
+            if (o instanceof Map.Entry<?, ?> e) {
+                return buildClassProvider.get().isInstance(e.getValue()) &&
+                        BuildReferenceMapAdapter.this.removeValue(buildClassProvider.get().cast(e.getValue()));
             }
-            return true;
+            return false;
         }
 
         @Override
-        public boolean addAll(Collection<? extends Entry<Integer, R>> c) {
-            boolean b = false;
-            for (Entry<Integer, R> r : c) {
-                b |= add(r);
-            }
-            return b;
+        public Iterator<Entry<Integer, R>> iterator() {
+            return new Iterator<>() {
+                private Entry<Integer, R> current;
+                private final Iterator<Entry<Integer, R>> it = Iterators.removeNull(Iterators.map(
+                        BuildReferenceMapAdapter.this.core.entrySet().iterator(), coreEntry -> {
+                            R v = BuildReferenceMapAdapter.this.buildRefResolver.apply(coreEntry.getValue());
+                            return v == null ? null : new AbstractMap.SimpleEntry<>(coreEntry.getKey(), v);
+                        }));
+
+                @Override
+                public boolean hasNext() {
+                    return it.hasNext();
+                }
+
+                @Override
+                public Entry<Integer, R> next() {
+                    return current = it.next();
+                }
+
+                @Override
+                public void remove() {
+                    if (current == null) {
+                        throw new IllegalStateException();
+                    }
+                    BuildReferenceMapAdapter.this.removeValue(current.getValue());
+                }
+            };
         }
 
         @Override
-        public boolean removeAll(Collection<?> c) {
-            boolean b = false;
-            for (Object o : c) {
-                b |= remove(o);
-            }
-            return b;
-        }
-
-        @Override
-        public boolean retainAll(Collection<?> c) {
-            // TODO: to properly pass this onto core, we need to wrap o into BuildReference but also needs to figure out ID.
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public void clear() {
-            core.clear();
-        }
-
-        @Override
-        public boolean equals(Object o) {
-            return core.equals(o);
-        }
-
-        @Override
-        public int hashCode() {
-            return core.hashCode();
-        }
-
-        private Entry<Integer, BuildReference<R>> _wrap(Entry<Integer, R> e) {
-            return new AbstractMap.SimpleEntry<>(e.getKey(), wrap(e.getValue()));
-        }
-
-        private Entry<Integer, R> _unwrap(Entry<Integer, BuildReference<R>> e) {
-            R v = unwrap(e.getValue());
-            if (v == null)
-                return null;
-            return new AbstractMap.SimpleEntry<>(e.getKey(), v);
+        public Spliterator<Map.Entry<Integer, R>> spliterator() {
+            return Spliterators.spliteratorUnknownSize(iterator(), Spliterator.DISTINCT | Spliterator.ORDERED);
         }
     }
 }

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

@@ -148,6 +148,11 @@ public abstract class LazyBuildMixIn<JobT extends Job<JobT, RunT> & Queue.Task &
             public RunT create(File dir) throws IOException {
                 return loadBuild(dir);
             }
+
+            @Override
+            public Class<RunT> getBuildClass() {
+                return LazyBuildMixIn.this.getBuildClass();
+            }
         });
         return r;
     }

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

@@ -1,116 +0,0 @@
-package jenkins.model.lazy;
-
-import java.util.AbstractMap;
-import java.util.AbstractSet;
-import java.util.Iterator;
-import java.util.Map;
-import java.util.NoSuchElementException;
-import java.util.Set;
-import java.util.Spliterator;
-import java.util.Spliterators;
-import jenkins.model.lazy.AbstractLazyLoadRunMap.Direction;
-
-/**
- * Set that backs {@link AbstractLazyLoadRunMap#entrySet()}.
- *
- * @author Kohsuke Kawaguchi
- */
-class LazyLoadRunMapEntrySet<R> extends AbstractSet<Map.Entry<Integer, R>> {
-    private final AbstractLazyLoadRunMap<R> owner;
-    private final BuildReferenceMapAdapter<R> adapter;
-
-    LazyLoadRunMapEntrySet(AbstractLazyLoadRunMap<R> owner) {
-        this.owner = owner;
-        this.adapter = new BuildReferenceMapAdapter<>(owner, owner.all());
-    }
-
-    private synchronized Set<Map.Entry<Integer, R>> all() {
-        return this.adapter.entrySet();
-    }
-
-    @Override
-    public int size() {
-        return all().size();
-    }
-
-    @Override
-    public boolean isEmpty() {
-        return owner.newestBuild() == null;
-    }
-
-    @Override
-    public boolean contains(Object o) {
-        if (o instanceof Map.Entry) {
-            Map.Entry<?, ?> e = (Map.Entry<?, ?>) o;
-            Object k = e.getKey();
-            if (k instanceof Integer) {
-                return owner.getByNumber((Integer) k).equals(e.getValue());
-            }
-        }
-        return false;
-    }
-
-    @Override
-    public Iterator<Map.Entry<Integer, R>> iterator() {
-        return new Iterator<>() {
-            R last = null;
-            R next = owner.newestBuild();
-
-            @Override
-            public boolean hasNext() {
-                return next != null;
-            }
-
-            @Override
-            public Map.Entry<Integer, R> next() {
-                last = next;
-                if (last != null) {
-                    next = owner.search(owner.getNumberOf(last) - 1, Direction.DESC);
-                } else
-                    throw new NoSuchElementException();
-                return entryOf(last);
-            }
-
-            private Map.Entry<Integer, R> entryOf(R r) {
-                return new AbstractMap.SimpleImmutableEntry<>(owner.getNumberOf(r), r);
-            }
-
-            @Override
-            public void remove() {
-                if (last == null)
-                    throw new UnsupportedOperationException();
-                owner.removeValue(last);
-            }
-        };
-    }
-
-    @Override
-    public Spliterator<Map.Entry<Integer, R>> spliterator() {
-        return Spliterators.spliteratorUnknownSize(
-                iterator(), Spliterator.DISTINCT | Spliterator.ORDERED);
-    }
-
-    @Override
-    public Object[] toArray() {
-        return all().toArray();
-    }
-
-    @Override
-    public <T> T[] toArray(T[] a) {
-        return all().toArray(a);
-    }
-
-    @Override
-    public boolean add(Map.Entry<Integer, R> integerREntry) {
-        throw new UnsupportedOperationException();
-    }
-
-    @Override
-    public boolean remove(Object o) {
-        if (o instanceof Map.Entry) {
-            Map.Entry e = (Map.Entry) o;
-            return owner.removeValue((R) e.getValue());
-        }
-        return false;
-    }
-}

core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java

@@ -306,21 +306,21 @@ class AbstractLazyLoadRunMapTest {
         a.purgeCache();
         assertEquals("[]", a.getLoadedBuilds().keySet().toString());
         Iterator<Map.Entry<Integer, Build>> iterator = entries.iterator();
-        assertEquals("[5]", a.getLoadedBuilds().keySet().toString());
+        assertEquals("[]", a.getLoadedBuilds().keySet().toString());
         assertTrue(iterator.hasNext());
         assertEquals("[5]", a.getLoadedBuilds().keySet().toString());
         Map.Entry<Integer, Build> entry = iterator.next();
-        assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
+        assertEquals("[5]", a.getLoadedBuilds().keySet().toString());
         assertEquals(5, entry.getKey().intValue());
-        assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
+        assertEquals("[5]", a.getLoadedBuilds().keySet().toString());
         assertEquals(5, entry.getValue().n);
-        assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
+        assertEquals("[5]", a.getLoadedBuilds().keySet().toString());
         assertTrue(iterator.hasNext());
         entry = iterator.next();
         assertEquals(3, entry.getKey().intValue());
-        assertEquals("[5, 3, 1]", a.getLoadedBuilds().keySet().toString(), ".next() precomputes the one after that too");
+        assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
         assertEquals(3, entry.getValue().n);
-        assertEquals("[5, 3, 1]", a.getLoadedBuilds().keySet().toString());
+        assertEquals("[5, 3]", a.getLoadedBuilds().keySet().toString());
         assertTrue(iterator.hasNext());
         entry = iterator.next();
         assertEquals(1, entry.getKey().intValue());

core/src/test/java/jenkins/model/lazy/FakeMap.java

@@ -48,4 +48,9 @@ public class FakeMap extends AbstractLazyLoadRunMap<Build> {
         //new Exception("loading #" + n).printStackTrace();
         return new Build(Integer.parseInt(n));
     }
+
+    @Override
+    protected Class<Build> getBuildClass() {
+        return Build.class;
+    }
 }

test/src/test/java/hudson/model/ViewJobTest.java

@@ -65,7 +65,17 @@ class ViewJobTest {
         }
 
         @Override protected void reload() {
-            runs.load(this, d -> new R(J.this, d));
+            runs.load(this, new RunMap.Constructor<>() {
+                @Override
+                public R create(File dir) throws IOException {
+                    return new R(J.this, dir);
+                }
+
+                @Override
+                public Class<R> getBuildClass() {
+                    return R.class;
+                }
+            });
         }
 
         @Override public TopLevelItemDescriptor getDescriptor() {

test/src/test/java/jenkins/widgets/BuildTimeTrendTest.java

@@ -207,7 +207,17 @@ class BuildTimeTrendTest {
         @Override
         protected SortedMap<Integer, NonAbstractBuild> _getRuns() {
             if (runMap == null) {
-                runMap = new RunMap<>(this.getBuildDir(), this::createBuildFromDir);
+                runMap = new RunMap<>(this.getBuildDir(), new RunMap.Constructor<NonAbstractBuild>() {
+                    @Override
+                    public NonAbstractBuild create(File dir) throws IOException {
+                        return createBuildFromDir(dir);
+                    }
+
+                    @Override
+                    public Class<NonAbstractBuild> getBuildClass() {
+                        return NonAbstractBuild.class;
+                    }
+                });
             }
             return runMap;
         }