From af417ec7dcb18d3ca93992f585803b5bd8d79c10 Mon Sep 17 00:00:00 2001 From: Dmitriy Ukhlov Date: Wed, 20 Aug 2025 01:01:41 -0700 Subject: [PATCH] [JENKINS-75986] Refactor `BuildReferenceMapAdapter` (#10946) Co-authored-by: Jesse Glick Co-authored-by: Dmytro Ukhlov --- core/src/main/java/hudson/model/RunMap.java | 13 +- core/src/main/java/hudson/util/Iterators.java | 13 + .../model/lazy/AbstractLazyLoadRunMap.java | 213 ++------- .../model/lazy/BuildReferenceMapAdapter.java | 443 ++++++++---------- .../jenkins/model/lazy/LazyBuildMixIn.java | 5 + .../model/lazy/LazyLoadRunMapEntrySet.java | 116 ----- .../lazy/AbstractLazyLoadRunMapTest.java | 12 +- .../test/java/jenkins/model/lazy/FakeMap.java | 5 + .../test/java/hudson/model/ViewJobTest.java | 12 +- .../jenkins/widgets/BuildTimeTrendTest.java | 12 +- 10 files changed, 282 insertions(+), 562 deletions(-) delete mode 100644 core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java diff --git a/core/src/main/java/hudson/model/RunMap.java b/core/src/main/java/hudson/model/RunMap.java index 8f0f9da822..2204ad4f97 100644 --- a/core/src/main/java/hudson/model/RunMap.java +++ b/core/src/main/java/hudson/model/RunMap.java @@ -95,7 +95,7 @@ public final class RunMap> extends AbstractLazyLoadRunMap * Used to create new instance of {@link Run}. * @since 2.451 */ - public RunMap(@NonNull Job job, Constructor cons) { + public RunMap(@NonNull Job job, Constructor cons) { this.job = Objects.requireNonNull(job); this.cons = cons; initBaseDir(job.getBuildDir()); @@ -105,7 +105,7 @@ public final class RunMap> extends AbstractLazyLoadRunMap * @deprecated Use {@link #RunMap(Job, Constructor)}. */ @Deprecated - public RunMap(File baseDir, Constructor cons) { + public RunMap(File baseDir, Constructor cons) { job = null; this.cons = cons; initBaseDir(baseDir); @@ -187,6 +187,8 @@ public final class RunMap> extends AbstractLazyLoadRunMap */ public interface Constructor> { R create(File dir) throws IOException; + + Class getBuildClass(); } @Override @@ -201,7 +203,7 @@ public final class RunMap> extends AbstractLazyLoadRunMap /** * Add a new 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> extends AbstractLazyLoadRunMap return null; } + @Override + protected Class getBuildClass() { + return cons.getBuildClass(); + } + /** * Backward compatibility method that notifies {@link RunMap} of who the owner is. * diff --git a/core/src/main/java/hudson/util/Iterators.java b/core/src/main/java/hudson/util/Iterators.java index f091404ee8..809c92011a 100644 --- a/core/src/main/java/hudson/util/Iterators.java +++ b/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 Iterator map(final Iterator itr, Function 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. * diff --git a/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java b/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java index a438da3251..345a6bd173 100644 --- a/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java +++ b/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 extends AbstractMap implements SortedMap { private final CopyOnWriteMap.Tree> core = new CopyOnWriteMap.Tree<>( Collections.reverseOrder()); - - private LazyLoadRunMapEntrySet entrySet = new LazyLoadRunMapEntrySet<>(this); - - private transient volatile Set keySet; - private transient volatile Collection values; + private final BuildReferenceMapAdapter adapter = new BuildReferenceMapAdapter<>(core, + this::resolveBuildRef, this::getNumberOf, this::getBuildClass) { + @Override + protected boolean removeValue(R value) { + return AbstractLazyLoadRunMap.this.removeValue(value); + } + }; @Override public Set keySet() { - Set ks = keySet; - if (ks == null) { - ks = new AbstractSet<>() { - @Override - public Iterator iterator() { - return new Iterator() { - private final Iterator> 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 spliterator() { - return new Spliterators.AbstractIntSpliterator( - Long.MAX_VALUE, - Spliterator.DISTINCT | Spliterator.ORDERED | Spliterator.SORTED) { - private final Iterator 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 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 values() { - Collection vals = values; - if (vals == null) { - vals = new AbstractCollection<>() { - @Override - public Iterator iterator() { - return new Iterator<>() { - private final Iterator> 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 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> entrySet() { + assert baseDirInitialized(); + return adapter.entrySet(); } /** @@ -354,18 +231,22 @@ public abstract class AbstractLazyLoadRunMap extends AbstractMap @Override public Comparator comparator() { - return Collections.reverseOrder(); + return core.comparator(); } @Override public boolean isEmpty() { - return search(Integer.MAX_VALUE, DESC) == null; + return adapter.isEmpty(); } @Override - public Set> 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 extends AbstractMap 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 extends AbstractMap */ @Override public SortedMap 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 headMap(Integer toKey) { - return subMap(Integer.MAX_VALUE, toKey); + return adapter.headMap(toKey); } @Override public SortedMap tailMap(Integer fromKey) { - return subMap(fromKey, Integer.MIN_VALUE); + return adapter.tailMap(fromKey); } @Override @@ -443,11 +308,7 @@ public abstract class AbstractLazyLoadRunMap extends AbstractMap @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 extends AbstractMap } public R getByNumber(int n) { - return resolveBuildRef(core.get(n)); + return adapter.get(n); } /** @@ -597,14 +458,9 @@ public abstract class AbstractLazyLoadRunMap extends AbstractMap core.putAll(newWrapperData); } - /** - * Return underlining {@link BuildReference} core map. - * - * @return - * full build reference map. - */ - /*package*/ SortedMap> all() { - return core; + @Override + public R remove(Object key) { + return adapter.remove(key); } /** @@ -652,7 +508,6 @@ public abstract class AbstractLazyLoadRunMap extends AbstractMap return new BuildReference<>(getIdOf(r), r); } - /** * Parses {@code R} instance from data in the specified directory. * @@ -664,6 +519,8 @@ public abstract class AbstractLazyLoadRunMap extends AbstractMap */ protected abstract R retrieve(File dir) throws IOException; + protected abstract Class getBuildClass(); + public synchronized boolean removeValue(R run) { return core.remove(getNumberOf(run)) != null; } @@ -694,7 +551,5 @@ public abstract class AbstractLazyLoadRunMap extends AbstractMap ASC, DESC, EXACT } - private static final SortedMap EMPTY_SORTED_MAP = Collections.unmodifiableSortedMap(new TreeMap()); - static final Logger LOGGER = Logger.getLogger(AbstractLazyLoadRunMap.class.getName()); } diff --git a/core/src/main/java/jenkins/model/lazy/BuildReferenceMapAdapter.java b/core/src/main/java/jenkins/model/lazy/BuildReferenceMapAdapter.java index d9771de365..a7196da107 100644 --- a/core/src/main/java/jenkins/model/lazy/BuildReferenceMapAdapter.java +++ b/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>} and make it look like {@code SortedMap}. * - * When {@link BuildReference} lost the build object, we'll use {@link AbstractLazyLoadRunMap#getById(String)} - * to obtain one. + *

+ * When {@link BuildReference} lost the build object, we'll use {@link #buildRefResolver} to obtain one. + *

+ * + *

+ * 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). + *

+ * + *

+ * 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. + *

* * @author Kohsuke Kawaguchi */ -class BuildReferenceMapAdapter implements SortedMap { - private final AbstractLazyLoadRunMap loader; +class BuildReferenceMapAdapter extends AbstractMap implements SortedMap { + private final Function, R> buildRefResolver; + private final Function buildNumberProvider; + private final Supplier> buildClassProvider; private final SortedMap> core; - BuildReferenceMapAdapter(AbstractLazyLoadRunMap loader, SortedMap> core) { - this.loader = loader; + private final Set keySet = new KeySetAdapter(); + private final Collection values = new ValuesAdapter(); + private final Set> entrySet = new EntrySetAdapter(); + + BuildReferenceMapAdapter(SortedMap> core, + Function, R> buildRefResolver, + Function buildNumberProvider, + Supplier> buildClassProvider) { + this.buildRefResolver = buildRefResolver; + this.buildNumberProvider = buildNumberProvider; this.core = core; + this.buildClassProvider = buildClassProvider; } - private R unwrap(@Nullable BuildReference ref) { - if (ref == null) return null; - - R v = ref.get(); - if (v == null) - v = loader.getById(ref.id); - return v; - } - - private BuildReference wrap(@Nullable R value) { - if (value == null) return null; - return loader.createReference(value); - } - - - - @Override public Comparator comparator() { return core.comparator(); @@ -56,17 +71,20 @@ class BuildReferenceMapAdapter implements SortedMap { @Override public SortedMap 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 headMap(Integer toKey) { - return new BuildReferenceMapAdapter<>(loader, core.headMap(toKey)); + return new BuildReferenceMapAdapter<>(core.headMap(toKey), + buildRefResolver, buildNumberProvider, buildClassProvider); } @Override public SortedMap 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 implements SortedMap { @Override public Set keySet() { - return core.keySet(); + return keySet; } @Override public Collection values() { - return new CollectionAdapter(core.values()); + return values; } @Override public Set> 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 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 m) { - for (Entry 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 { - private final Collection> core; - - private CollectionAdapter(Collection> 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 { @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 iterator() { + return new AdaptedIterator<>(BuildReferenceMapAdapter.this.entrySet().iterator()) { + @Override + protected Integer adapt(Entry e) { + return e.getKey(); + } + }; + } + + @Override + public Spliterator spliterator() { + return new Spliterators.AbstractIntSpliterator(Long.MAX_VALUE, + Spliterator.DISTINCT | Spliterator.ORDERED | Spliterator.SORTED) { + private final Iterator 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 getComparator() { + return BuildReferenceMapAdapter.this.comparator(); + } + }; + } + } + + private class ValuesAdapter extends AbstractCollection { + @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 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 ref) { - return unwrap(ref); + protected R adapt(Entry e) { + return e.getValue(); } - }); + }; } @Override - public Object[] toArray() { - List list = new ArrayList<>(size()); - for (var e : this) { - list.add(e); - } - return list.toArray(); - } - - @Override - public 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 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 spliterator() { + return Spliterators.spliteratorUnknownSize(iterator(), Spliterator.DISTINCT | Spliterator.ORDERED); } } - private class SetAdapter implements Set> { - private final Set>> core; - - private SetAdapter(Set>> core) { - this.core = core; - } - + private class EntrySetAdapter extends AbstractSet> { @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> iterator() { - return Iterators.removeNull(new AdaptedIterator<>(core.iterator()) { - @Override - protected Entry adapt(Entry> e) { - return _unwrap(e); - } - }); - } - - @Override - public Object[] toArray() { - List 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[] toArray(T[] a) { - return new ArrayList<>(this).toArray(a); - } - - @Override - public boolean add(Entry 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> c) { - boolean b = false; - for (Entry r : c) { - b |= add(r); - } - return b; + public Iterator> iterator() { + return new Iterator<>() { + private Entry current; + private final Iterator> 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 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> _wrap(Entry e) { - return new AbstractMap.SimpleEntry<>(e.getKey(), wrap(e.getValue())); - } - - private Entry _unwrap(Entry> e) { - R v = unwrap(e.getValue()); - if (v == null) - return null; - return new AbstractMap.SimpleEntry<>(e.getKey(), v); + public Spliterator> spliterator() { + return Spliterators.spliteratorUnknownSize(iterator(), Spliterator.DISTINCT | Spliterator.ORDERED); } } } diff --git a/core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java b/core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java index 0c58c90a79..28e056080c 100644 --- a/core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java +++ b/core/src/main/java/jenkins/model/lazy/LazyBuildMixIn.java @@ -148,6 +148,11 @@ public abstract class LazyBuildMixIn & Queue.Task & public RunT create(File dir) throws IOException { return loadBuild(dir); } + + @Override + public Class getBuildClass() { + return LazyBuildMixIn.this.getBuildClass(); + } }); return r; } diff --git a/core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java b/core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java deleted file mode 100644 index 5f241e67eb..0000000000 --- a/core/src/main/java/jenkins/model/lazy/LazyLoadRunMapEntrySet.java +++ /dev/null @@ -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 extends AbstractSet> { - private final AbstractLazyLoadRunMap owner; - private final BuildReferenceMapAdapter adapter; - - LazyLoadRunMapEntrySet(AbstractLazyLoadRunMap owner) { - this.owner = owner; - this.adapter = new BuildReferenceMapAdapter<>(owner, owner.all()); - } - - private synchronized Set> 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> iterator() { - return new Iterator<>() { - R last = null; - R next = owner.newestBuild(); - - @Override - public boolean hasNext() { - return next != null; - } - - @Override - public Map.Entry 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 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> spliterator() { - return Spliterators.spliteratorUnknownSize( - iterator(), Spliterator.DISTINCT | Spliterator.ORDERED); - } - - @Override - public Object[] toArray() { - return all().toArray(); - } - - @Override - public T[] toArray(T[] a) { - return all().toArray(a); - } - - @Override - public boolean add(Map.Entry 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; - } -} diff --git a/core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java b/core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java index 8b0131e52c..ce1f66c572 100644 --- a/core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java +++ b/core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java @@ -306,21 +306,21 @@ class AbstractLazyLoadRunMapTest { a.purgeCache(); assertEquals("[]", a.getLoadedBuilds().keySet().toString()); Iterator> 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 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()); diff --git a/core/src/test/java/jenkins/model/lazy/FakeMap.java b/core/src/test/java/jenkins/model/lazy/FakeMap.java index 4dbe93dcc4..37abe6b139 100644 --- a/core/src/test/java/jenkins/model/lazy/FakeMap.java +++ b/core/src/test/java/jenkins/model/lazy/FakeMap.java @@ -48,4 +48,9 @@ public class FakeMap extends AbstractLazyLoadRunMap { //new Exception("loading #" + n).printStackTrace(); return new Build(Integer.parseInt(n)); } + + @Override + protected Class getBuildClass() { + return Build.class; + } } diff --git a/test/src/test/java/hudson/model/ViewJobTest.java b/test/src/test/java/hudson/model/ViewJobTest.java index 10c622d710..b39d46863b 100644 --- a/test/src/test/java/hudson/model/ViewJobTest.java +++ b/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 getBuildClass() { + return R.class; + } + }); } @Override public TopLevelItemDescriptor getDescriptor() { diff --git a/test/src/test/java/jenkins/widgets/BuildTimeTrendTest.java b/test/src/test/java/jenkins/widgets/BuildTimeTrendTest.java index 54749b5fa8..ea1438210b 100644 --- a/test/src/test/java/jenkins/widgets/BuildTimeTrendTest.java +++ b/test/src/test/java/jenkins/widgets/BuildTimeTrendTest.java @@ -207,7 +207,17 @@ class BuildTimeTrendTest { @Override protected SortedMap _getRuns() { if (runMap == null) { - runMap = new RunMap<>(this.getBuildDir(), this::createBuildFromDir); + runMap = new RunMap<>(this.getBuildDir(), new RunMap.Constructor() { + @Override + public NonAbstractBuild create(File dir) throws IOException { + return createBuildFromDir(dir); + } + + @Override + public Class getBuildClass() { + return NonAbstractBuild.class; + } + }); } return runMap; }