Avoid resolve variables in random order

jenkinsci · Feb 22, 2018 · d602b6b · d602b6b
1 parent 911aca9
commit d602b6b
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,19 @@
 # Generic Webhook Plugin Changelog
 Changelog of Generic Webhook Plugin.
+## Unreleased
+### No issue
+
+**Doc**
+
+
+[911aca914fcfb79](https://github.com/jenkinsci/generic-webhook-trigger-plugin/commit/911aca914fcfb79) Tomas Bjerre *2018-02-22 17:05:26*
+
+**Helpful node about underscores for HTTP Header vars**
+
+
+[52b9b5258c9a2e3](https://github.com/jenkinsci/generic-webhook-trigger-plugin/commit/52b9b5258c9a2e3) Gmanfunky *2018-02-17 02:22:25*
+
+
 ## 1.26 (2018-02-07 15:23:32)
 ### GitHub [#37](https://github.com/jenkinsci/generic-webhook-trigger-plugin/issues/37) NullPointerException at GenericTriggerResults (version 1.25)    *bug*  
 

diff --git a/src/main/java/org/jenkinsci/plugins/gwt/GenericTrigger.java b/src/main/java/org/jenkinsci/plugins/gwt/GenericTrigger.java
@@ -14,10 +14,14 @@
 import hudson.triggers.Trigger;
 import hudson.triggers.TriggerDescriptor;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.Enumeration;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 import java.util.logging.Logger;
 
 import jenkins.model.ParameterizedJobMixIn;
@@ -166,18 +170,40 @@ String renderText(String regexpFilterText, Map<String, String> resolvedVariables
     if (isNullOrEmpty(regexpFilterText)) {
       return "";
     }
-    for (final Entry<String, String> variableEntry : resolvedVariables.entrySet()) {
-      final String key = "\\$" + variableEntry.getKey();
-      final String resolvedVariable = variableEntry.getValue();
+    final List<String> variables = getVariablesInResolveOrder(resolvedVariables.keySet());
+    for (final String variable : variables) {
+      final String key = "\\$" + variable;
+      final String resolvedVariable = resolvedVariables.get(variable);
       try {
-        regexpFilterText = regexpFilterText.replaceAll(key, resolvedVariable);
+        regexpFilterText =
+            regexpFilterText //
+                .replaceAll(key, resolvedVariable);
       } catch (final IllegalArgumentException e) {
         throw new RuntimeException("Tried to replace " + key + " with " + resolvedVariable, e);
       }
     }
     return regexpFilterText;
   }
 
+  @VisibleForTesting
+  List<String> getVariablesInResolveOrder(Set<String> unsorted) {
+    final List<String> variables = new ArrayList<String>(unsorted);
+    Collections.sort(
+        variables,
+        new Comparator<String>() {
+          @Override
+          public int compare(String o1, String o2) {
+            if (o1.length() == o2.length()) {
+              return o1.compareTo(o2);
+            } else if (o1.length() > o2.length()) {
+              return -1;
+            }
+            return 1;
+          }
+        });
+    return variables;
+  }
+
   public List<GenericVariable> getGenericVariables() {
     return genericVariables;
   }

diff --git a/src/main/java/org/jenkinsci/plugins/gwt/resolvers/VariablesResolver.java b/src/main/java/org/jenkinsci/plugins/gwt/resolvers/VariablesResolver.java
@@ -1,12 +1,11 @@
 package org.jenkinsci.plugins.gwt.resolvers;
 
-import static com.google.common.collect.Maps.newHashMap;
-
 import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 
 import org.jenkinsci.plugins.gwt.GenericHeaderVariable;
 import org.jenkinsci.plugins.gwt.GenericRequestVariable;
@@ -50,7 +49,7 @@ private <T> T firstNotNull(T o1, T o2) {
   }
 
   public Map<String, String> getVariables() {
-    Map<String, String> resolvedVariables = newHashMap();
+    final Map<String, String> resolvedVariables = new TreeMap<>();
     resolvedVariables.putAll(
         requestHeaderResolver.getRequestHeaders(configuredGenericHeaderVariables, incomingHeaders));
     resolvedVariables.putAll(

diff --git a/src/test/java/org/jenkinsci/plugins/gwt/GenericTriggerTest.java b/src/test/java/org/jenkinsci/plugins/gwt/GenericTriggerTest.java
@@ -1,9 +1,12 @@
 package org.jenkinsci.plugins.gwt;
 
 import static com.google.common.collect.Maps.newHashMap;
+import static com.google.common.collect.Sets.newHashSet;
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.util.List;
 import java.util.Map;
+import java.util.TreeMap;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -42,6 +45,44 @@ public void testThatIsMatchingAcceptsIfMatching() {
         .isTrue();
   }
 
+  @Test
+  public void testThatVariablesAreResolvedWithLongestVariableFirst() {
+    resolvedVariables = new TreeMap<>();
+    resolvedVariables.put("key1", "resolved1");
+    resolvedVariables.put("key2", "resolved2only");
+    resolvedVariables.put("key2andthensome", "resolved2andmore");
+
+    final String text = "$key1 $key2 $key2andthensome $key2";
+    final String actual = sut.renderText(text, resolvedVariables);
+
+    assertThat(actual) //
+        .isEqualTo("resolved1 resolved2only resolved2andmore resolved2only");
+  }
+
+  @Test
+  public void testThatVariablesAreResolvedInOrder() {
+    final List<String> actual =
+        sut.getVariablesInResolveOrder(
+            newHashSet( //
+                "var1", //
+                "var2", //
+                "var2andmore", //
+                "var31", //
+                "var3a", //
+                "var3b" //
+                ));
+
+    assertThat(actual) //
+        .containsExactly( //
+            "var2andmore", //
+            "var31", //
+            "var3a", //
+            "var3b", //
+            "var1", //
+            "var2" //
+            );
+  }
+
   @Test
   public void testThatIsMatchingRejectsIfNotMatching() {