[FIXED JENKINS-29711] Handle monomorphic single-parameters to steps.

jenkinsci · Aug 24, 2016 · 7857e23 · 7857e23
1 parent 6719c9f
commit 7857e23
Show file tree

Hide file tree

Showing 5 changed files with 123 additions and 6 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/DSL.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/DSL.java
@@ -231,8 +231,15 @@ protected Object invokeDescribable(String symbol, Object _args) {
         List<StepDescriptor> metaSteps = StepDescriptor.metaStepsOf(symbol);
         StepDescriptor metaStep = metaSteps.size()==1 ? metaSteps.get(0) : null;
 
+        boolean singleArgumentOnly = false;
+        if (metaStep != null) {
+            DescribableModel<?> metaModel = new DescribableModel(metaStep.clazz);
+            singleArgumentOnly = metaModel.hasSingleRequiredParameter() && metaModel.getParameters().size() == 1;
+        }
+
         // The only time a closure is valid is when the resulting Describable is immediately executed via a meta-step
-        NamedArgsAndClosure args = parseArgs(_args, metaStep!=null && metaStep.takesImplicitBlockArgument(), UninstantiatedDescribable.ANONYMOUS_KEY);
+        NamedArgsAndClosure args = parseArgs(_args, metaStep!=null && metaStep.takesImplicitBlockArgument(),
+                UninstantiatedDescribable.ANONYMOUS_KEY, singleArgumentOnly);
         UninstantiatedDescribable ud = new UninstantiatedDescribable(symbol, null, args.namedArgs);
 
         if (metaStep==null) {
@@ -353,7 +360,14 @@ private NamedArgsAndClosure(Map<?,?> namedArgs, Closure body) {
     }
 
     static NamedArgsAndClosure parseArgs(Object arg, StepDescriptor d) {
-        return parseArgs(arg,d.takesImplicitBlockArgument(), loadSoleArgumentKey(d));
+        boolean singleArgumentOnly = false;
+        try {
+            DescribableModel<?> stepModel = new DescribableModel<>(d.clazz);
+            singleArgumentOnly = stepModel.hasSingleRequiredParameter() && stepModel.getParameters().size() == 1;
+        } catch (NoStaplerConstructorException e) {
+            // Ignore steps without databound constructors and treat them as normal.
+        }
+        return parseArgs(arg,d.takesImplicitBlockArgument(), loadSoleArgumentKey(d), singleArgumentOnly);
     }
 
     /**
@@ -379,7 +393,7 @@ static NamedArgsAndClosure parseArgs(Object arg, StepDescriptor d) {
      *      If the context in which this method call happens allow implicit sole default argument, specify its name.
      *      If null, the call must be with names arguments.
      */
-    static NamedArgsAndClosure parseArgs(Object arg, boolean expectsBlock, String soleArgumentKey) {
+    static NamedArgsAndClosure parseArgs(Object arg, boolean expectsBlock, String soleArgumentKey, boolean singleRequiredArg) {
         if (arg instanceof NamedArgsAndClosure)
             return (NamedArgsAndClosure) arg;
         if (arg instanceof Map) // TODO is this clause actually used?
@@ -400,9 +414,9 @@ static NamedArgsAndClosure parseArgs(Object arg, boolean expectsBlock, String so
                 a = a.subList(0,a.size()-1);
             }
 
-            if (a.size()==1 && a.get(0) instanceof Map && !((Map) a.get(0)).containsKey("$class")) {
+            if (a.size()==1 && a.get(0) instanceof Map && !((Map) a.get(0)).containsKey("$class") && !singleRequiredArg) {
                 // this is how Groovy passes in Map
-                return new NamedArgsAndClosure((Map)a.get(0),c);
+                return new NamedArgsAndClosure((Map) a.get(0), c);
             }
 
             switch (a.size()) {

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/DSLTest.java b/src/test/java/org/jenkinsci/plugins/workflow/DSLTest.java
@@ -139,4 +139,15 @@ public void nonexistentFunctions() throws Exception {
         r.assertLogContains("constructible with compass and straightedge", b);
     }
 
+    /**
+     * Tests the ability to execute a step with an unnamed monomorphic describable argument.
+     */
+    @Issue("JENKINS-29711")
+    @Test
+    public void monomorphic() throws Exception {
+        WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "mon");
+        p.setDefinition(new CpsFlowDefinition("monomorphStep firstArg:'one', secondArg:'two'"));
+        r.assertLogContains("First arg: one, second arg: two", r.assertBuildStatusSuccess(p.scheduleBuild2(0)));
+    }
+
 }
diff --git a/src/test/java/org/jenkinsci/plugins/workflow/cps/SnippetizerTest.java b/src/test/java/org/jenkinsci/plugins/workflow/cps/SnippetizerTest.java
@@ -46,6 +46,8 @@
 import org.jenkinsci.plugins.workflow.testMetaStep.Colorado;
 import org.jenkinsci.plugins.workflow.testMetaStep.Hawaii;
 import org.jenkinsci.plugins.workflow.testMetaStep.Island;
+import org.jenkinsci.plugins.workflow.testMetaStep.MonomorphicData;
+import org.jenkinsci.plugins.workflow.testMetaStep.MonomorphicStep;
 import org.jenkinsci.plugins.workflow.testMetaStep.Oregon;
 import org.jenkinsci.plugins.workflow.testMetaStep.StateMetaStep;
 import org.jenkinsci.plugins.workflow.testMetaStep.chemical.CarbonMonoxide;
@@ -58,7 +60,6 @@
 import org.jvnet.hudson.test.MockFolder;
 
 import java.util.Arrays;
-import java.util.Collection;
 import java.util.logging.Level;
 
 import static org.hamcrest.CoreMatchers.*;
@@ -262,4 +263,12 @@ public class SnippetizerTest {
         GroovyShell shell = new GroovyShell(r.jenkins.getPluginManager().uberClassLoader);
         shell.parse(dsld);
     }
+
+    @Issue("JENKINS-29711")
+    @Test
+    public void monomorphic() throws Exception {
+        MonomorphicStep monomorphicStep = new MonomorphicStep(new MonomorphicData("one", "two"));
+        st.assertRoundTrip(monomorphicStep, "monomorphStep([firstArg: 'one', secondArg: 'two'])");
+    }
+
 }
diff --git a/src/test/java/org/jenkinsci/plugins/workflow/testMetaStep/MonomorphicData.java b/src/test/java/org/jenkinsci/plugins/workflow/testMetaStep/MonomorphicData.java
@@ -0,0 +1,26 @@
+package org.jenkinsci.plugins.workflow.testMetaStep;
+
+import hudson.Extension;
+import hudson.model.AbstractDescribableImpl;
+import hudson.model.Descriptor;
+import org.kohsuke.stapler.DataBoundConstructor;
+
+public class MonomorphicData extends AbstractDescribableImpl<MonomorphicData> {
+    public final String firstArg;
+
+    public final String secondArg;
+
+    @DataBoundConstructor
+    public MonomorphicData(String firstArg, String secondArg) {
+        this.firstArg = firstArg;
+        this.secondArg = secondArg;
+    }
+
+    public String getArgs() {
+        return "First arg: " + firstArg + ", second arg: " + secondArg;
+    }
+
+    @Extension
+    public static class DescriptorImpl extends Descriptor<MonomorphicData> {
+    }
+}
diff --git a/src/test/java/org/jenkinsci/plugins/workflow/testMetaStep/MonomorphicStep.java b/src/test/java/org/jenkinsci/plugins/workflow/testMetaStep/MonomorphicStep.java
@@ -0,0 +1,57 @@
+package org.jenkinsci.plugins.workflow.testMetaStep;
+
+import com.google.inject.Inject;
+import hudson.Extension;
+import hudson.model.TaskListener;
+import org.jenkinsci.plugins.workflow.DSLTest;
+import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl;
+import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl;
+import org.jenkinsci.plugins.workflow.steps.AbstractSynchronousNonBlockingStepExecution;
+import org.jenkinsci.plugins.workflow.steps.StepContextParameter;
+import org.kohsuke.stapler.DataBoundConstructor;
+
+/**
+ * @author Andrew Bayer
+ * @see DSLTest
+ */
+public class MonomorphicStep extends AbstractStepImpl {
+
+    public final MonomorphicData data;
+
+    @DataBoundConstructor
+    public MonomorphicStep(MonomorphicData data) {
+        this.data = data;
+    }
+
+    private static final class Execution extends AbstractSynchronousNonBlockingStepExecution<Void> {
+        @Inject
+        private transient MonomorphicStep step;
+        @StepContextParameter
+        private transient TaskListener listener;
+
+        @Override protected Void run() throws Exception {
+            listener.getLogger().println(step.data.getArgs());
+            return null;
+        }
+
+        private static final long serialVersionUID = 1L;
+
+    }
+
+    @Extension
+    public static final class DescriptorImpl extends AbstractStepDescriptorImpl {
+        public DescriptorImpl() {
+            super(Execution.class);
+        }
+
+        @Override public String getFunctionName() {
+            return "monomorphStep";
+        }
+
+        @Override public String getDisplayName() {
+            return "Testing monomorphic single parameter.";
+        }
+    }
+
+
+}