Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #59 from synopsys-arc-oss/Variables_handling_issues
[FIXED JENKINS-25635,JENKINS-25226] - Variables substitution issues
  • Loading branch information
rpetti committed Nov 1, 2014
2 parents 44c10b2 + 7b22e0b commit fe27171
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 30 deletions.
36 changes: 20 additions & 16 deletions src/main/java/hudson/plugins/perforce/utils/MacroStringHelper.java
Expand Up @@ -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("${");
}

/**
Expand All @@ -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 + "}");
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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;
}

Expand Down
Expand Up @@ -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;

/**
*
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
}
}
76 changes: 63 additions & 13 deletions 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;
Expand All @@ -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);

Expand All @@ -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");
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}
}

0 comments on commit fe27171

Please sign in to comment.