[FIXED JENKINS-29952] Offer PROP as a shorthand for env.PROP from wit…

…hin Groovy code.
jenkinsci · Sep 17, 2016 · 2d71de2 · 2d71de2
1 parent ac46350
commit 2d71de2
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 16 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScript.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsScript.java
@@ -77,19 +77,8 @@ public CpsScript() throws IOException {
 
     @SuppressWarnings("unchecked") // Binding
     final void $initialize() throws IOException {
+        // TODO JENKINS-33353 could make this a GlobalVariable instead
         getBinding().setVariable(STEPS_VAR, new DSL(execution.getOwner()));
-        Run<?,?> run = $build();
-        if (run != null) {
-            EnvVars paramEnv = new EnvVars();
-            ParametersAction a = run.getAction(ParametersAction.class);
-            if (a != null) {
-                for (ParameterValue v : a) {
-                    v.buildEnvironment(run, paramEnv);
-                }
-            }
-            EnvVars.resolve(paramEnv);
-            getBinding().getVariables().putAll(paramEnv);
-        }
     }
 
 
@@ -122,14 +111,26 @@ public final Object invokeMethod(String name, Object args) {
 
     @Override
     public Object getProperty(String property) {
-        GlobalVariable v = GlobalVariable.byName(property, $buildNoException());
+        // cf. CpsWhitelist.permitsMethod
+        Run<?,?> b = $buildNoException();
+        GlobalVariable v = GlobalVariable.byName(property, b);
         if (v != null) {
             try {
                 return v.getValue(this);
             } catch (Exception x) {
                 throw new InvokerInvocationException(x);
             }
         }
+        if (b != null) {
+            try {
+                String value = EnvActionImpl.forRun(b).getProperty(property);
+                if (value != null) {
+                    return value;
+                }
+            } catch (IOException x) {
+                LOGGER.log(Level.WARNING, null, x);
+            }
+        }
         return super.getProperty(property);
     }
 

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsWhitelist.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsWhitelist.java
@@ -1,5 +1,7 @@
 package org.jenkinsci.plugins.workflow.cps;
 
+import hudson.model.Run;
+import java.io.IOException;
 import org.codehaus.groovy.runtime.GStringImpl;
 import org.codehaus.groovy.runtime.ScriptBytecodeAdapter;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist;
@@ -10,6 +12,8 @@
 import java.lang.reflect.Method;
 import java.util.Map;
 import java.util.WeakHashMap;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import jenkins.model.Jenkins;
 
 /**
@@ -18,6 +22,9 @@
  * @author Kohsuke Kawaguchi
  */
 class CpsWhitelist extends AbstractWhitelist {
+
+    private static final Logger LOGGER = Logger.getLogger(CpsWhitelist.class.getName());
+
     private CpsWhitelist() {}
 
     @Override
@@ -38,9 +45,22 @@ public boolean permitsMethod(Method method, Object receiver, Object[] args) {
                 return true;
             }
             if (name.equals("getProperty") && args.length == 1 && args[0] instanceof String) {
-                if (GlobalVariable.byName((String) args[0], ((CpsScript) receiver).$buildNoException()) != null) {
+                String property = (String) args[0];
+                CpsScript script = (CpsScript) receiver;
+                Run<?,?> b = script.$buildNoException();
+                if (GlobalVariable.byName(property, b) != null) {
                     return true;
                 }
+                if (b != null) {
+                    try {
+                        String value = EnvActionImpl.forRun(b).getProperty(property);
+                        if (value != null) {
+                            return true;
+                        }
+                    } catch (IOException x) {
+                        LOGGER.log(Level.WARNING, null, x);
+                    }
+                }
             }
         }
         if (receiver instanceof DSL && method.getName().equals("invokeMethod")) {

diff --git a/src/main/java/org/jenkinsci/plugins/workflow/cps/EnvActionImpl.java b/src/main/java/org/jenkinsci/plugins/workflow/cps/EnvActionImpl.java
@@ -77,7 +77,7 @@ private EnvActionImpl() {
         return Collections.unmodifiableMap(env);
     }
 
-    @Override public Object getProperty(String propertyName) {
+    @Override public String getProperty(String propertyName) {
         try {
             CpsThread t = CpsThread.current();
             return EnvironmentExpander.getEffectiveEnvironment(getEnvironment(), t.getContextVariable(EnvVars.class), t.getContextVariable(EnvironmentExpander.class)).get(propertyName);

diff --git a/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java b/src/test/java/org/jenkinsci/plugins/workflow/WorkflowTest.java
@@ -261,6 +261,7 @@ public static final class Execution extends AbstractSynchronousNonBlockingStepEx
         }
     }
 
+    @Issue("JENKINS-29952")
     @Test public void env() {
         story.addStep(new Statement() {
             @Override public void evaluate() throws Throwable {
@@ -279,7 +280,8 @@ public static final class Execution extends AbstractSynchronousNonBlockingStepEx
                         + "  sh 'echo tag3=$BUILD_TAG stuff=$STUFF'\n"
                         + "  env.PATH=\"/opt/stuff/bin:${env.PATH}\"\n"
                         + "  sh 'echo shell PATH=$PATH'\n"
-                        + "  echo \"groovy PATH=${env.PATH}\""
+                        + "  echo \"groovy PATH=${env.PATH}\"\n"
+                        + "  echo \"simplified groovy PATH=${PATH}\"\n"
                         + "}", true));
                 startBuilding();
                 SemaphoreStep.waitForStart("env/1", b);
@@ -297,6 +299,7 @@ public static final class Execution extends AbstractSynchronousNonBlockingStepEx
                 story.j.assertLogContains("tag3=custom2 stuff=more", b);
                 story.j.assertLogContains("shell PATH=/opt/stuff/bin:", b);
                 story.j.assertLogContains("groovy PATH=/opt/stuff/bin:", b);
+                story.j.assertLogContains("simplified groovy PATH=/opt/stuff/bin:", b);
                 EnvironmentAction a = b.getAction(EnvironmentAction.class);
                 assertNotNull(a);
                 assertEquals("custom2", a.getEnvironment().get("BUILD_TAG"));