Merge pull request #198 from jenkinsci/substitute

Use Util.replaceMacro instead of our custom replacement logic

Author: carlossg

src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java

@@ -1,5 +1,6 @@
 package org.csanchez.jenkins.plugins.kubernetes;
 
+import static hudson.Util.*;
 import static java.util.stream.Collectors.*;
 import static org.csanchez.jenkins.plugins.kubernetes.ContainerTemplate.*;
 
@@ -10,8 +11,6 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
@@ -23,17 +22,13 @@
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 
+import hudson.Util;
 import hudson.model.Label;
 import hudson.model.Node;
 import hudson.tools.ToolLocationNodeProperty;
 
 public class PodTemplateUtils {
 
-    private static final String PLACEHOLDER_KEY = "key";
-    private static final String PLACEHOLDER_FORMAT = "\\$\\{%s\\}";
-    private static final String PLACEHOLDER_REGEX = String.format(PLACEHOLDER_FORMAT, "(?<" + PLACEHOLDER_KEY + ">[a-zA-Z0-9_]+)");
-    private static final Pattern PLACEHOLDER_PATTERN = Pattern.compile(PLACEHOLDER_REGEX);
-
     /**
      * Combines a {@link ContainerTemplate} with its parent.
      * @param parent        The parent container template (nullable).
@@ -221,52 +216,44 @@ public static PodTemplate getTemplateByName(@CheckForNull String name, Collectio
      * @return      The substituted value if found, or the input value otherwise.
      */
     public static String substituteEnv(String s) {
-        return substitute(s, System.getenv());
+        return replaceMacro(s, System.getenv());
     }
 
     /**
      * Substitutes a placeholder with a value found in the environment.
+     * @deprecated check if it is null or empty in the caller method, then use {@link #substituteEnv(String)}
      * @param s             The placeholder. Should be use the format: ${placeholder}.
      * @param defaultValue  The default value to return if no match is found.
      * @return              The substituted value if found, or the default value otherwise.
      */
+    @Deprecated
     public static String substituteEnv(String s, String defaultValue) {
         return substitute(s, System.getenv(), defaultValue);
     }
 
     /**
      * Substitutes a placeholder with a value found in the specified map.
+     * @deprecated use {@link Util#replaceMacro(String, Map)}
      * @param s             The placeholder. Should be use the format: ${placeholder}.
      * @param properties    The map with the key value pairs to use for substitution.
      * @return              The substituted value if found, or the input value otherwise.
      */
+    @Deprecated
     public static String substitute(String s, Map<String, String> properties) {
-        return substitute(s, properties, null);
+        return replaceMacro(s, properties);
     }
 
     /**
      * Substitutes a placeholder with a value found in the specified map.
+     * @deprecated check if it is null or empty in the caller method, then use {@link #substitute(String,Map)}
      * @param s             The placeholder. Should be use the format: ${placeholder}.
      * @param properties    The map with the key value pairs to use for substitution.
      * @param defaultValue  The default value to return if no match is found.
      * @return              The substituted value if found, or the default value otherwise.
      */
+    @Deprecated
     public static String substitute(String s, Map<String, String> properties, String defaultValue) {
-        if (Strings.isNullOrEmpty(s)) {
-            return defaultValue;
-        }
-
-        Matcher m = PLACEHOLDER_PATTERN.matcher(s);
-        while (m.find()) {
-            String key = m.group(PLACEHOLDER_KEY);
-            String val = properties.get(key);
-            if (val != null) {
-                s = s.replaceAll(String.format(PLACEHOLDER_FORMAT, key), Matcher.quoteReplacement(val));
-            } else if (defaultValue != null) {
-                s = s.replaceAll(String.format(PLACEHOLDER_FORMAT, key), defaultValue);
-            }
-        }
-        return s;
+        return Strings.isNullOrEmpty(s) ? defaultValue : replaceMacro(s, properties);
     }
 
     private static List<TemplateEnvVar> combineEnvVars(ContainerTemplate parent, ContainerTemplate template) {

src/main/java/org/csanchez/jenkins/plugins/kubernetes/ProvisioningCallback.java

@@ -428,8 +428,8 @@ private Pod getPodTemplate(KubernetesSlave slave, PodTemplate template) {
 
     private Map<String, Quantity> getResourcesMap(String memory, String cpu) {
         ImmutableMap.Builder<String, Quantity> builder = ImmutableMap.<String, Quantity> builder();
-        String actualMemory = substituteEnv(memory, null);
-        String actualCpu = substituteEnv(cpu, null);
+        String actualMemory = substituteEnv(memory);
+        String actualCpu = substituteEnv(cpu);
         if (StringUtils.isNotBlank(actualMemory)) {
             Quantity memoryQuantity = new Quantity(actualMemory);
             builder.put("memory", memoryQuantity);

src/test/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtilsTest.java

@@ -363,10 +363,10 @@ public void shouldSubstituteMultipleEnvVarsAndIgnoreMissing() {
     }
 
     @Test
-    public void shouldSubstituteMultipleEnvVarsAndUseDefaultsForMissing() {
+    public void shouldSubstituteMultipleEnvVarsAndNotUseDefaultsForMissing() {
         Map<String, String> properties = new HashMap<>();
         properties.put("key1", "value1");
         properties.put("key2", "value2");
-        assertEquals("value1 or value2 or defaultValue", substitute("${key1} or ${key2} or ${key3}", properties, "defaultValue"));
+        assertEquals("value1 or value2 or ${key3}", substitute("${key1} or ${key2} or ${key3}", properties, "defaultValue"));
     }
 }