Test and fix for JENKINS-44406 where StepDescriptorCache has NPE with…

… removed StepDescriptor
jenkinsci · May 22, 2017 · 3890d3e · 3890d3e
1 parent 204ac01
commit 3890d3e
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 15 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/nodes/StepDescriptorCache.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/nodes/StepDescriptorCache.java
@@ -28,7 +28,7 @@
 import hudson.ExtensionPoint;
 import hudson.init.InitMilestone;
 import hudson.init.Initializer;
-import hudson.util.Memoizer;
+import hudson.model.Descriptor;
 import jenkins.model.Jenkins;
 import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
 import org.kohsuke.accmod.Restricted;
@@ -56,26 +56,30 @@ public static void invalidateGlobalCache() {
     }
 
     public void invalidateAll() {
-        descriptorCache.clear();
+        store.clear();
     }
 
-    private final Memoizer<String, StepDescriptor> descriptorCache = new Memoizer<String, StepDescriptor>() {
-
-        public StepDescriptor compute(String descriptorId) {
-            Jenkins j = Jenkins.getInstance();
-            if (descriptorId != null && j != null) {
-                return (StepDescriptor) j.getDescriptor(descriptorId);
-            }
-            return null;
-        }
-    };
+    private final ConcurrentHashMap<String, StepDescriptor> store = new ConcurrentHashMap<String, StepDescriptor>();
 
     @CheckForNull
     public StepDescriptor getDescriptor(String descriptorId) {
-        if (descriptorId != null) {
-            return descriptorCache.get(descriptorId);
+        if (descriptorId == null) {
+            return null;
         }
-        return null;
 
+        StepDescriptor v = store.get(descriptorId);
+        if(v!=null) return v;
+
+        synchronized (this) {
+            v = store.get(descriptorId);
+            if (v != null) return v;
+            Jenkins j = Jenkins.getActiveInstance();
+            Descriptor d = j.getDescriptor(descriptorId);
+            if (d instanceof StepDescriptor) {
+                store.put(descriptorId, (StepDescriptor)d);
+                return (StepDescriptor)d;
+            }
+            return null;
+        }
     }
 }
diff --git a/src/test/java/org/jenkinsci/plugins/workflow/cps/nodes/StepDescriptorCacheTest.java b/src/test/java/org/jenkinsci/plugins/workflow/cps/nodes/StepDescriptorCacheTest.java
@@ -0,0 +1,49 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2017, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.jenkinsci.plugins.workflow.cps.nodes;
+
+import org.jenkinsci.plugins.workflow.cps.steps.LoadStep;
+import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+
+public class StepDescriptorCacheTest {
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+    @Test
+    public void testStepDescriptorCache() {
+        StepDescriptorCache cache = StepDescriptorCache.getPublicCache();
+        StepDescriptor expected = cache.getDescriptor("org.jenkinsci.plugins.workflow.cps.steps.LoadStep");
+        Assert.assertEquals(LoadStep.DescriptorImpl.class, expected.getClass());
+
+        StepDescriptor nullDescriptor = cache.getDescriptor("nonexistent");
+        Assert.assertNull(nullDescriptor);
+
+        nullDescriptor = cache.getDescriptor(null);
+        Assert.assertNull(nullDescriptor);
+    }
+}