[JENKINS-19418] Fix random HTTP 404 when viewing build details

Added a unit test (and supporting localExpiredBuilder) to expose a defect in the search() method of AbstractLazyLoadRunMap whereby the index contained an entry for the requested build number, but the BuildReference's referent was null. Fixed the search() method to handle this rare[ish] corner case. (cherry picked from commit 7c9aab3)
jenkinsci · Sep 30, 2013 · b6dfb7a · b6dfb7a
1 parent b358d71
commit b6dfb7a
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 0 deletions.
diff --git a/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java b/core/src/main/java/jenkins/model/lazy/AbstractLazyLoadRunMap.java
@@ -495,6 +495,15 @@ public R search(final int n, final Direction d) {
             }
             return getById(idOnDisk.get(lo-1));
         case EXACT:
+            if (hi<=0)                 return null;
+            R r = load(idOnDisk.get(hi-1), null);
+            if (r==null) {
+                return null;
+            }
+
+            int found = getNumberOf(r);
+            if (found==n)
+                return r;   // exact match
             return null;
         default:
             throw new AssertionError();

diff --git a/core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java b/core/src/test/java/jenkins/model/lazy/AbstractLazyLoadRunMapTest.java
@@ -54,6 +54,21 @@ public class AbstractLazyLoadRunMapTest extends Assert {
     @Rule
     public FakeMapBuilder localBuilder = new FakeMapBuilder();
 
+    @Rule
+    public FakeMapBuilder localExpiredBuilder = new FakeMapBuilder() {
+        @Override
+        public FakeMap make() {
+            assert getDir()!=null;
+            return new FakeMap(getDir()){
+                @Override
+                protected BuildReference<Build> createReference(Build r) {
+                    return new BuildReference<Build>(getIdOf(r), /* pretend referent expired */ null);
+                }
+            };
+        }
+    };
+
+
     @BeforeClass
     public static void setUpClass() {
         AbstractLazyLoadRunMap.LOGGER.setLevel(Level.OFF);
@@ -131,6 +146,18 @@ public void search() {
         assertNull(a.search(-99, Direction.DESC));
     }
 
+    @Test
+    public void searchExactWhenIndexedButSoftReferenceExpired() throws IOException {
+        final FakeMap m = localExpiredBuilder.add(1, "A").add(2, "B").make();
+
+        // force index creation
+        m.entrySet();
+
+        m.search(1, Direction.EXACT).asserts(1, "A");
+        assertNull(m.search(3, Direction.EXACT));
+        assertNull(m.search(0, Direction.EXACT));
+    }
+
     /**
      * If load fails, search needs to gracefully handle it
      */

diff --git a/core/src/test/java/jenkins/model/lazy/FakeMapBuilder.java b/core/src/test/java/jenkins/model/lazy/FakeMapBuilder.java
@@ -39,6 +39,10 @@
 public class FakeMapBuilder implements TestRule {
     private File dir;
 
+    protected File getDir() {
+        return dir;
+    }
+
     public FakeMapBuilder() {
     }