Fix NullPointerException in LazyBuildMixIn on jenkins reload

Author: Dmytro Ukhlov

core/src/main/java/hudson/tasks/LogRotator.java

@@ -163,9 +163,12 @@ public void perform(Job<?, ?> job) throws IOException, InterruptedException {
         Run lsb = removeLastBuild ? null : job.getLastSuccessfulBuild();
         Run lstb = removeLastBuild ? null : job.getLastStableBuild();
 
+        Map<Integer, ? extends Run<?, ?>> runMap = job.getBuildsAsMap();
         if (numToKeep != -1) {
-            job.getBuildsAsMap().entrySet().stream().skip(numToKeep).map(Map.Entry::getValue)
-                    .filter(r -> !shouldKeepRun(r, lsb, lstb)).forEach(r -> {
+            // Iterate through keySet() instead of entrySet() or values() to avoid triggering lazy loading
+            // for the first `numToKeep` builds
+            runMap.keySet().stream().skip(numToKeep).map(runMap::get)
+                    .filter(r -> r != null && !shouldKeepRun(r, lsb, lstb)).forEach(r -> {
                 LOGGER.log(FINE, "{0} is to be removed", r);
                 try { r.delete(); }
                 catch (IOException ex) { exceptionMap.computeIfAbsent(r, key -> new HashSet<>()).add(ex); }
@@ -190,8 +193,10 @@ public void perform(Job<?, ?> job) throws IOException, InterruptedException {
         }
 
         if (artifactNumToKeep != null && artifactNumToKeep != -1) {
-            job.getBuildsAsMap().entrySet().stream().skip(artifactNumToKeep).map(Map.Entry::getValue)
-                    .filter(r -> !shouldKeepRun(r, lsb, lstb)).forEach(r -> {
+            // Iterate through keySet() instead of entrySet() or values() to avoid triggering lazy loading
+            // for the first `artifactNumToKeep` builds
+            runMap.keySet().stream().skip(artifactNumToKeep).map(runMap::get)
+                    .filter(r -> r != null && !shouldKeepRun(r, lsb, lstb)).forEach(r -> {
                 LOGGER.log(FINE, "{0} is to be purged of artifacts", r);
                 try { r.deleteArtifacts(); }
                 catch (IOException ex) { exceptionMap.computeIfAbsent(r, key -> new HashSet<>()).add(ex); }

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

@@ -169,12 +169,20 @@ public boolean remove(Object o) {
 
         @Override
         public Iterator<Integer> iterator() {
-            return new AdaptedIterator<>(BuildReferenceMapAdapter.this.entrySet().iterator()) {
-                @Override
-                protected Integer adapt(Map.Entry<Integer, R> e) {
-                    return e.getKey();
-                }
-            };
+            return Iterators.removeNull(Iterators.map(
+                    BuildReferenceMapAdapter.this.core.entrySet().iterator(), coreEntry -> {
+                        BuildReference<R> ref = coreEntry.getValue();
+                        if (!ref.isSet()) {
+                            R r = resolver.resolveBuildRef(ref);
+                            if (r == null) {
+                                return null;
+                            }
+                        }
+                        if (ref.isUnloadable()) {
+                            return null;
+                        }
+                        return ref.number;
+                    }));
         }
 
         @Override
@@ -273,19 +281,9 @@ public Iterator<Map.Entry<Integer, R>> iterator() {
                 private Map.Entry<Integer, R> current;
                 private final Iterator<Map.Entry<Integer, R>> it = Iterators.removeNull(Iterators.map(
                         BuildReferenceMapAdapter.this.core.entrySet().iterator(), coreEntry -> {
-                            BuildReference<R> ref = coreEntry.getValue();
-                            if (!ref.isSet()) {
-                                R r = resolver.resolveBuildRef(ref);
-                                // load not loaded or unloadable build
-                                if (r == null) {
-                                    return null;
-                                }
-                                return new EntryAdapter(coreEntry, r);
-                            }
-                            if (ref.isUnloadable()) {
-                                return null;
-                            }
-                            return new EntryAdapter(coreEntry);
+                            R r = resolver.resolveBuildRef(coreEntry.getValue());
+                            // if null - load not allowed or build is unloadable
+                            return r == null ? null : new AbstractMap.SimpleImmutableEntry<>(coreEntry.getKey(), r);
                         }));
 
                 @Override
@@ -314,59 +312,6 @@ public Spliterator<Map.Entry<Integer, R>> spliterator() {
         }
     }
 
-    private class EntryAdapter implements Entry<Integer, R> {
-        private final Map.Entry<Integer, BuildReference<R>> coreEntry;
-        private volatile R resolvedValue;
-
-        EntryAdapter(Map.Entry<Integer, BuildReference<R>> coreEntry) {
-            this(coreEntry, null);
-        }
-
-        EntryAdapter(Map.Entry<Integer, BuildReference<R>> coreEntry, R resolvedValue) {
-            this.coreEntry = coreEntry;
-            this.resolvedValue = resolvedValue;
-        }
-
-        private Map.Entry<Integer, R> getResolvedEntry() {
-            return new AbstractMap.SimpleEntry<>(getKey(), getValue());
-        }
-
-        @Override
-        public Integer getKey() {
-            return coreEntry.getKey();
-        }
-
-        @Override
-        public R getValue() {
-            R value = resolvedValue;
-            if (value != null) {
-                return value;
-            }
-            return resolvedValue = resolver.resolveBuildRef(coreEntry.getValue());
-        }
-
-        @Override
-        public R setValue(R value) {
-            // BuildReferenceAdapter is read only
-            throw new UnsupportedOperationException();
-        }
-
-        @Override
-        public String toString() {
-            return getResolvedEntry().toString();
-        }
-
-        @Override
-        public boolean equals(Object o) {
-            return (o instanceof Map.Entry<?, ?>) && getResolvedEntry().equals(o);
-        }
-
-        @Override
-        public int hashCode() {
-            return getResolvedEntry().hashCode();
-        }
-    }
-
     /**
      * An interface for resolving build references into actual build instances
      * and extracting basic metadata from them.