Skip to content
Permalink
Browse files

[JENKINS-43934] Flatten GString’s found in step arguments, even when …

…present in nested collections.
  • Loading branch information
jglick committed May 9, 2017
1 parent 66f2753 commit 549ae9bdcc83289c0af71db5cc9ed67878b6a6eb
@@ -58,6 +58,7 @@
import java.io.PrintStream;
import java.io.Serializable;
import java.lang.annotation.Annotation;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
@@ -70,6 +71,8 @@
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.model.Jenkins;
import org.codehaus.groovy.reflection.CachedClass;
import org.codehaus.groovy.reflection.ReflectionCache;
import org.jenkinsci.Symbol;

import static org.jenkinsci.plugins.workflow.cps.ThreadTaskResult.*;
@@ -353,22 +356,51 @@ private NamedArgsAndClosure(Map<?,?> namedArgs, Closure body) {

for (Map.Entry<?,?> entry : namedArgs.entrySet()) {
String k = entry.getKey().toString(); // coerces GString and more
Object v = entry.getValue();
// coerce GString, to save StepDescriptor.newInstance() from being made aware of that
// this isn't the only type coercion that Groovy does, so this is not very kosher, but
// doing a proper coercion like Groovy does require us to know the type that the receiver
// expects.
//
// For the reference, Groovy does:
// ReflectionCache.getCachedClass(types[i]).coerceArgument(a)
if (v instanceof GString) {
v = v.toString();
}
Object v = flattenGString(entry.getValue());
this.namedArgs.put(k, v);
}
}
}

/**
* Coerce {@link GString}, to save {@link StepDescriptor#newInstance(Map)} from being made aware of that.
* This is not the only type coercion that Groovy does, so this is not very kosher, but
* doing a proper coercion like Groovy does require us to know the type that the receiver
* expects.
* For reference, Groovy does {@linkplain ReflectionCache#getCachedClass ReflectionCache.getCachedClass(types[i]).}{@linkplain CachedClass#coerceArgument coerceArgument(a)}.
* Note that {@link DescribableModel#instantiate} would also handle {@link GString} in {@code coerce},
* but better to do it here in the Groovy-specific code so we do not need to rely on that.
* @return {@code v} or an equivalent with all {@link GString}s flattened, including in nested {@link List}s or {@link Map}s
*/
private static Object flattenGString(Object v) {
if (v instanceof GString) {
return v.toString();
} else if (v instanceof List) {
boolean mutated = false;
List<Object> r = new ArrayList<>();
for (Object o : ((List<?>) v)) {
Object o2 = flattenGString(o);
mutated |= o != o2;
r.add(o2);
}
return mutated ? r : v;
} else if (v instanceof Map) {
boolean mutated = false;
Map<Object,Object> r = new LinkedHashMap<>();
for (Map.Entry<?,?> e : ((Map<?, ?>) v).entrySet()) {
Object k = e.getKey();
Object k2 = flattenGString(k);
Object o = e.getValue();
Object o2 = flattenGString(o);
mutated |= k != k2 || o != o2;
r.put(k2, o2);
}
return mutated ? r : v;
} else {
return v;
}
}

static NamedArgsAndClosure parseArgs(Object arg, StepDescriptor d) {
boolean singleArgumentOnly = false;
try {
@@ -25,6 +25,10 @@
package org.jenkinsci.plugins.workflow;

import hudson.model.Result;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.inject.Inject;
import static org.hamcrest.Matchers.containsString;

@@ -34,6 +38,11 @@
import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl;
import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl;
import org.jenkinsci.plugins.workflow.steps.AbstractSynchronousStepExecution;
import org.jenkinsci.plugins.workflow.steps.Step;
import org.jenkinsci.plugins.workflow.steps.StepContext;
import org.jenkinsci.plugins.workflow.steps.StepDescriptor;
import org.jenkinsci.plugins.workflow.steps.StepExecution;
import org.jenkinsci.plugins.workflow.steps.SynchronousStepExecution;
import static org.junit.Assert.*;
import org.junit.ClassRule;
import org.junit.Rule;
@@ -62,11 +71,48 @@
r.assertLogContains("but this is still from a step", b2);
}

@Issue("JENKINS-43934")
@Test public void flattenGString() throws Exception {
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition("def x = 'the message'; echo \"What is ${x}?\""));
p.setDefinition(new CpsFlowDefinition("def message = myJoin(['the', /${'message'.toLowerCase(Locale.ENGLISH)}/]); echo(/What is $message?/)", true));
r.assertLogContains("What is the message?", r.assertBuildStatusSuccess(p.scheduleBuild2(0)));
}
public static class MyJoinStep extends Step {
public final String args;
@DataBoundConstructor public MyJoinStep(String args) {this.args = args;}
@Override public StepExecution start(StepContext context) throws Exception {
return new Exec(context, args);
}
private static class Exec extends SynchronousStepExecution<String> {
final String args;
Exec(StepContext context, String args) {
super(context);
this.args = args;
}
@Override protected String run() throws Exception {
return args;
}
}
@TestExtension("flattenGString") public static class DescriptorImpl extends StepDescriptor {
@Override public String getFunctionName() {
return "myJoin";
}
@Override public Set<? extends Class<?>> getRequiredContext() {
return Collections.emptySet();
}
@Override public Step newInstance(Map<String, Object> arguments) throws Exception {
List<?> args = (List<?>) arguments.get("args");
StringBuilder b = new StringBuilder();
for (Object arg : args) {
if (b.length() > 0) {
b.append(' ');
}
b.append((String) arg);
}
return new MyJoinStep(b.toString());
}
}
}

/**
* Tests the ability to execute meta-step with clean syntax

0 comments on commit 549ae9b

Please sign in to comment.
You can’t perform that action at this time.