Merge pull request #59 from synopsys-arc-oss/Variables_handling_issues

[FIXED JENKINS-25635,JENKINS-25226] - Variables substitution issues

src/main/java/hudson/plugins/perforce/utils/MacroStringHelper.java

@@ -193,7 +193,7 @@ private static String substituteParametersNoCheck(
      * @return true if the string 
      */
     public static boolean containsMacro(@CheckForNull String str) {
-        return str != null && str.matches(".*\\$\\{.*\\}.*");
+        return str != null && str.contains("${");
     }
 
     /**
@@ -203,7 +203,7 @@ public static boolean containsMacro(@CheckForNull String str) {
      * @return true if the string contains the specified variable
      */
     public static boolean containsVariable(@CheckForNull String str, @Nonnull String variableName) {
-        return str != null && str.matches(".*\\$\\{" + variableName + "\\}.*");
+        return str != null && str.contains("${" + variableName + "}");
     }
 
      /**
@@ -250,7 +250,8 @@ private static String substituteParametersNoCheck (
     }
 
     /**
-     * Substitute parameters and validate contents of the resulting string
+     * Substitute parameters and validate contents of the resulting string.
+     * Environment variables have the highest priority.
      * @param inputString Input string
      * @param instance Instance of {@link PerforceSCM}
      * @param build Related build
@@ -266,20 +267,9 @@ private static String substituteParametersNoCheck(
         if (!containsMacro(inputString)) {
             return inputString;
         }
-
-        String string = substituteParametersNoCheck(inputString, instance, 
-                build.getProject(), build.getBuiltOn(), env);
-
-        // Substitute default build variables
-        Map<String, String> substitutions = new HashMap<String, String>();
-        getDefaultBuildSubstitutions(build, substitutions);    
-        String result = MacroStringHelper.substituteParametersNoCheck(string, substitutions);
-        result = MacroStringHelper.substituteParametersNoCheck(result, build.getBuildVariables());
-        if (!containsMacro(string)) {
-            return string;
-        }
+        String result = inputString;
 
-         // The last attempts: Try to build the full environment
+        // The last attempts: Try to build the full environment
         Map<String, String> environmentVarsFromExtensions = new TreeMap<String, String>();
         boolean useEnvironment = true;
         for (StackTraceElement ste : (new Throwable()).getStackTrace()) { // Inspect the stacktrace to avoid the infinite recursion
@@ -298,7 +288,21 @@ private static String substituteParametersNoCheck(
             }
         }
         result = MacroStringHelper.substituteParametersNoCheck(result, environmentVarsFromExtensions);
+
+        // Intermediate
+        if (!containsMacro(result)) {
+            return result;
+        }
 
+        // Substitute static variables
+        result = substituteParametersNoCheck(result, instance, build.getProject(), build.getBuiltOn(), env);
+
+        // Substitute default build variables
+        Map<String, String> substitutions = new HashMap<String, String>();
+        getDefaultBuildSubstitutions(build, substitutions);    
+        result = MacroStringHelper.substituteParametersNoCheck(result, substitutions);
+        result = MacroStringHelper.substituteParametersNoCheck(result, build.getBuildVariables());
+
         return result;
     }
 

src/test/java/com/synopsys/arc/jenkinsci/plugins/perforce/MacroStringHelperTest.java

@@ -25,8 +25,10 @@
 
 import hudson.plugins.perforce.utils.MacroStringHelper;
 import hudson.plugins.perforce.utils.ParameterSubstitutionException;
+import javax.annotation.Nonnull;
 import junit.framework.Assert;
 import org.junit.Test;
+import org.jvnet.hudson.test.Bug;
 
 /**
  *
@@ -59,6 +61,28 @@ public void Sanity_CheckNull() {
         checkCorrectString(null);
     }
 
+    @Bug(25365)
+    public @Test void checkMultiLineString() {
+        checkStringForMacros("${param1}/...\n${param2}/...", true);
+        checkStringForMacros("$vdvdvd/...\n\t${param2}/...", true);
+        checkStringForMacros("${param1}/...\nJust a stub string", true);
+
+        // no macros
+        checkStringForMacros("//depot1/path1/... //placeholder/path1/...\r\n\t//depot1/path2/... //placeholder/path2/...", false);
+        checkStringForMacros("//depot1/path1/... //placeholder/path1/...\n//depot1/path2/... //placeholder/path2/...", false);
+    }
+
+    public static void checkStringForMacros(@Nonnull String string, boolean expectMacro) {
+        boolean isMacro = MacroStringHelper.containsMacro(string);
+        if (isMacro != expectMacro) {
+            if (expectMacro) {
+                Assert.fail("Macros have not been found in '" + string + "'");
+            } else {
+                Assert.fail("Wrong macro discodvered in '" + string + "'");
+            }
+        }
+    }
+
     public static void checkTest(String string, boolean expectError) {
         try {
             MacroStringHelper.checkString(string);
@@ -84,5 +108,5 @@ public static void checkCorrectString(String str) {
     public static void checkInorrectString(String str) {
         System.out.println("Checking incorrect string '"+str+"' ...");
         checkTest(str, true);
-    }
+    }  
 }

src/test/java/hudson/plugins/perforce/PerforceSCMTest.java

@@ -1,19 +1,32 @@
 package hudson.plugins.perforce;
 
+import hudson.model.FreeStyleBuild;
 import hudson.plugins.perforce.config.DepotType;
 import hudson.model.FreeStyleProject;
 import hudson.model.Hudson;
+import hudson.model.ParametersAction;
+import hudson.model.ParametersDefinitionProperty;
+import hudson.model.Result;
+import hudson.model.StringParameterDefinition;
+import hudson.model.StringParameterValue;
 import hudson.plugins.perforce.PerforceToolInstallation.DescriptorImpl;
 import hudson.plugins.perforce.browsers.P4Web;
 import hudson.plugins.perforce.config.CleanTypeConfig;
 import hudson.plugins.perforce.config.MaskViewConfig;
 import hudson.plugins.perforce.config.WorkspaceCleanupConfig;
+import hudson.plugins.perforce.utils.MacroStringHelper;
 import hudson.tools.ToolProperty;
+import java.net.MalformedURLException;
 
 import java.net.URL;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.LinkedList;
 import java.util.List;
+import java.util.concurrent.Future;
+import javax.annotation.Nonnull;
+import static junit.framework.Assert.assertNotNull;
+import org.jvnet.hudson.test.Bug;
 
 import org.jvnet.hudson.test.HudsonTestCase;
 import org.jvnet.hudson.test.recipes.LocalData;
@@ -35,11 +48,7 @@ public class PerforceSCMTest extends HudsonTestCase {
      */
     public void testConfigRoundtrip() throws Exception {
         FreeStyleProject project = createFreeStyleProject();
-        P4Web browser = new P4Web(new URL("http://localhost/"));
-        PerforceSCM scm = new PerforceSCM(
-        		"user", "pass", "client", "port", "", "exe", "sysRoot",
-        		"sysDrive", "label", "counter", "upstreamProject", "shared", "charset", "charset2", "user", false, true, true, true, true, true, false,
-                        false, true, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true, TEST_DEPOT, TEST_WORKSPACE_CLEANUP, TEST_MASKVIEW);
+        PerforceSCM scm = createPerforceSCMStub();
         scm.setProjectPath("path");
         project.setScm(scm);
 
@@ -58,9 +67,9 @@ public void testConfigRoundtripWithNoSystemRoot() throws Exception {
 	FreeStyleProject project = createFreeStyleProject();
         P4Web browser = new P4Web(new URL("http://localhost/"));
         PerforceSCM scm = new PerforceSCM(
-                        "user", "pass", "client", "port", "", "exe", "",
-                        "", "label", "counter", "upstreamProject", "shared", "charset", "charset2", "user", false, true, true, true, true, true, false,
-                        false, true, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true, EMPTY_DEPOT, EMPTY_WORKSPACE_CLEANUP, EMPTY_MASKVIEW);
+            "user", "pass", "client", "port", "", "exe", "",
+            "", "label", "counter", "upstreamProject", "shared", "charset", "charset2", "user", false, true, true, true, true, true, false,
+            false, true, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true, EMPTY_DEPOT, EMPTY_WORKSPACE_CLEANUP, EMPTY_MASKVIEW);
         assertEquals("", scm.getP4SysDrive());
         assertEquals("", scm.getP4SysRoot());
         scm.setProjectPath("path");
@@ -76,11 +85,7 @@ public void testConfigRoundtripWithNoSystemRoot() throws Exception {
 
     public void testConfigRoundtripWithStream() throws Exception {
         FreeStyleProject project = createFreeStyleProject();
-        P4Web browser = new P4Web(new URL("http://localhost/"));
-        PerforceSCM scm = new PerforceSCM(
-        		"user", "pass", "client", "port", "", "exe", "sysRoot",
-        		"sysDrive", "label", "counter", "upstreamProject", "shared", "charset", "charset2", "user", false, true, true, true, true, true, false,
-                        false, true, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true, EMPTY_DEPOT, EMPTY_WORKSPACE_CLEANUP, EMPTY_MASKVIEW);
+        PerforceSCM scm = createPerforceSCMStub();
         scm.setP4Stream("stream");
         scm.setUseStreamDepot(true);
         project.setScm(scm);
@@ -400,4 +405,49 @@ public void testP4UpstreamProjectRenaming() throws Exception {
         scm = (PerforceSCM) downstreamProject.getScm();
         assertEquals(scm.p4UpstreamProject, newName);
     }
+
+    /**
+     * Checks that the variables substitution works properly for build parameters.
+     * Actually, it's a test for {@link MacroStringHelper}, but it requires a {@link HudsonTestCase} environment.
+     */
+    @Bug(25226)
+    public void testCheckParamSubstitutionOrder() throws Exception {
+        final String projectPath_format = "//depot1/%s/... //client/path1/...";
+
+        final FreeStyleProject prj = createFreeStyleProject();
+        prj.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("PARAM1", "defaultValue")));
+
+        PerforceToolInstallation stubInstallation = new PerforceToolInstallation("p4_stub", "echo", new LinkedList<ToolProperty<?>>());
+        PerforceToolInstallation.DescriptorImpl descriptor = (PerforceToolInstallation.DescriptorImpl) Hudson.getInstance().getDescriptor(PerforceToolInstallation.class);
+        descriptor.setInstallations(new PerforceToolInstallation[] { stubInstallation });
+        descriptor.save();
+
+        final PerforceSCM scm = PerforceSCMTest.createPerforceSCMStub();
+        scm.setProjectPath(String.format(projectPath_format, "${PARAM1}"));
+        scm.setP4Tool("p4_stub");
+        prj.setScm(scm);
+
+        // Run without params
+        Future<FreeStyleBuild> fBuild = prj.scheduleBuild2(0);
+        assertNotNull(fBuild);
+        FreeStyleBuild build = fBuild.get();
+        assertLogContains(String.format(projectPath_format, "defaultValue"), build);
+
+        // Run with params
+        fBuild = prj.scheduleBuild2(0, null, new ParametersAction(new StringParameterValue("PARAM1", "value")));
+        assertNotNull(fBuild);
+        build = fBuild.get();
+        assertLogContains(String.format(projectPath_format, "value"), build);
+    }    
+
+    /**
+     * Creates {@link PerforceSCM} with default fields.
+     */
+    public static @Nonnull PerforceSCM createPerforceSCMStub() throws MalformedURLException {
+        P4Web browser = new P4Web(new URL("http://localhost/")); 
+        return new PerforceSCM(
+        		"user", "pass", "client", "port", "", "exe", "sysRoot",
+        		"sysDrive", "label", "counter", "upstreamProject", "shared", "charset", "charset2", "user", false, true, true, true, true, true, false,
+                        false, true, false, false, false, "${basename}", 0, -1, browser, "exclude_user", "exclude_file", true, TEST_DEPOT, TEST_WORKSPACE_CLEANUP, TEST_MASKVIEW);
+    }
 }