remove $n replacements, numerics to normal precedence

Author: BrandonArp

README.md

@@ -324,7 +324,7 @@ ex:
 tranformations is a list of TransformationSet objects.  Each TransformationSet has an inject (Map\<String, DimensionInjection\>),
  remove (List\<String\>) and a findAndReplace (Map\<String, List\<String\>\>).
 
- A DimensionInjection is just a value and a boolean of whether or not to overwrite existing values.
+ A DimensionInjection is just a value and a boolean of whether or not to overwrite existing values. Of omitted, overwrite defaults to true.
 
  The keys in findAndReplace are regular expressions used to match metrics.  If matched, the list of replacements is
  executed, allowing for a single input metric to be recorded multiple times with different names or dimensions.

src/main/java/com/arpnetworking/metrics/mad/sources/TransformingSource.java

@@ -48,7 +48,6 @@
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.util.stream.Collectors;
 
 /**
  * Implementation of {@link Source} which wraps another {@link Source}
@@ -143,10 +142,8 @@ public void notify(final Observable observable, final Object event) {
                                 final RegexAndMapReplacer.Replacement rep =
                                         RegexAndMapReplacer.replaceAll(metricPattern, metricName, replacement, variablesMap);
                                 final String replacedString = rep.getReplacement();
-                                final List<String> consumedDimensions = rep.getVariablesMatched().stream()
-                                        .filter(var -> var.startsWith("dimension:") || var.indexOf(':') == -1) // Only dimension vars
-                                        .map(var -> var.substring(var.indexOf(":") + 1)) // Strip the prefix
-                                        .collect(Collectors.toList());
+                                final List<String> consumedDimensions = rep.getVariablesMatched()
+                                        .getOrDefault("dimension", ImmutableList.of());
 
                                 final int tagsStart = replacedString.indexOf(';');
                                 if (tagsStart == -1) {

src/main/java/com/arpnetworking/utility/RegexAndMapReplacer.java

@@ -16,16 +16,18 @@
 package com.arpnetworking.utility;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 /**
  * A regex replacement utility that can also replace tokens not found in the regex.
  *
- * $n where n is a number in the replace string is replaced by the pattern's match group n
  * ${name} in the replace string is replaced by the pattern's named capture, or by the value of the variable
  * with that name from the variables map
  * \ is used as an escape character and may be used to escape '$', '{', and '}' characters so that they will
@@ -43,38 +45,44 @@ public final class RegexAndMapReplacer {
      * @param variables map of variables to include
      * @return a string with replacement tokens replaced
      */
-        public static Replacement replaceAll(
+    public static Replacement replaceAll(
             final Pattern pattern,
             final String input,
             final String replace,
             final LinkedHashMap<String, Map<String, String>> variables) {
         final Matcher matcher = pattern.matcher(input);
         boolean found = matcher.find();
         if (found) {
-            final ImmutableList.Builder<String> variablesUsedBuilder = ImmutableList.builder();
+            final Map<String, ImmutableList.Builder<String>> variablesUsed = Maps.newHashMap();
             final StringBuilder builder = new StringBuilder();
             int lastMatchedIndex = 0;
             do {
                 builder.append(input.substring(lastMatchedIndex, matcher.start()));
                 lastMatchedIndex = matcher.end();
-                appendReplacement(matcher, replace, builder, variables, variablesUsedBuilder);
+                appendReplacement(matcher, replace, builder, variables, variablesUsed);
                 found = matcher.find();
             } while (found);
             // Append left-over string after the matches
             if (lastMatchedIndex < input.length() - 1) {
                 builder.append(input.substring(lastMatchedIndex, input.length()));
             }
-            return new Replacement(builder.toString(), variablesUsedBuilder.build());
+            final ImmutableMap<String, ImmutableList<String>> immutableVars = variablesUsed.entrySet()
+                    .stream()
+                    .collect(
+                            Collectors.collectingAndThen(
+                                    Collectors.toMap(ImmutableMap.Entry::getKey, entry -> entry.getValue().build()),
+                                    ImmutableMap::copyOf));
+            return new Replacement(builder.toString(), immutableVars);
         }
-        return new Replacement(input, ImmutableList.of());
+        return new Replacement(input, ImmutableMap.of());
     }
 
     private static void appendReplacement(
             final Matcher matcher,
             final String replacement,
             final StringBuilder replacementBuilder,
             final LinkedHashMap<String, Map<String, String>> variables,
-            final ImmutableList.Builder<String> variablesUsedBuilder) {
+            final Map<String, ImmutableList.Builder<String>> variablesUsed) {
         final StringBuilder tokenBuilder = new StringBuilder();
         int x = -1;
         while (x < replacement.length() - 1) {
@@ -85,7 +93,7 @@ private static void appendReplacement(
                 processEscapedCharacter(replacement, x, replacementBuilder);
             } else {
                 if (c == '$') {
-                    x += writeReplacementToken(replacement, x, replacementBuilder, matcher, variables, tokenBuilder, variablesUsedBuilder);
+                    x += writeReplacementToken(replacement, x, replacementBuilder, matcher, variables, tokenBuilder, variablesUsed);
                 } else {
                     replacementBuilder.append(c);
                 }
@@ -114,7 +122,7 @@ private static int writeReplacementToken(
             final Matcher matcher,
             final LinkedHashMap<String, Map<String, String>> variables,
             final StringBuilder tokenBuilder,
-            final ImmutableList.Builder<String> variablesUsedBuilder) {
+            final Map<String, ImmutableList.Builder<String>> variablesUsed) {
         boolean inReplaceBrackets = false;
         boolean tokenNumeric = true;
         tokenBuilder.setLength(0);  // reset the shared builder
@@ -147,85 +155,68 @@ private static int writeReplacementToken(
                 throw new IllegalArgumentException("Invalid replacement token, expected '}' at col " + x + ": " + replacement);
             }
             x++; // Consume the }
-            output.append(getReplacement(matcher, tokenBuilder.toString(), tokenNumeric, variables, variablesUsedBuilder));
+            output.append(getReplacement(matcher, tokenBuilder.toString(), variables, variablesUsed));
         } else {
-            // Consume until we hit a non-digit character
-            while (x < replacement.length()) {
-                c = replacement.charAt(x);
-                if (Character.isDigit(c)) {
-                    tokenBuilder.append(c);
-                } else {
-                    break;
-                }
-                x++;
-            }
-            if (tokenBuilder.length() == 0) {
                 throw new IllegalArgumentException(
                         String.format(
-                                "Invalid replacement token, non-numeric tokens must be surrounded by { } at col %d: %s",
+                                "Invalid replacement token, tokens must be surrounded by { } at col %d: %s",
                                 x,
                                 replacement));
-            }
-            output.append(getReplacement(matcher, tokenBuilder.toString(), true, variables, variablesUsedBuilder));
         }
         return x - offset - 1;
     }
 
     private static String getReplacement(
             final Matcher matcher,
             final String replaceToken,
-            final boolean numeric,
             final LinkedHashMap<String, Map<String, String>> variables,
-            final ImmutableList.Builder<String> variablesUsedBuilder) {
-        if (numeric) {
-            final int replaceGroup = Integer.parseInt(replaceToken);
-            return matcher.group(replaceGroup);
+            final Map<String, ImmutableList.Builder<String>> variablesUsed) {
+        final int prefixSeparatorIndex = replaceToken.indexOf(':');
+        final String prefix;
+        final String variableName;
+        if (prefixSeparatorIndex == replaceToken.length() - 1) {
+            throw new IllegalArgumentException(
+                    String.format("found prefix in variable replacement, but no variable name found: '%s'",
+                    replaceToken));
+        }
+        if (prefixSeparatorIndex != -1) {
+            prefix = replaceToken.substring(0, prefixSeparatorIndex);
+            variableName = replaceToken.substring(prefixSeparatorIndex + 1);
         } else {
-            final int prefixSeparatorIndex = replaceToken.indexOf(':');
-            final String prefix;
-            final String variableName;
-            if (prefixSeparatorIndex == replaceToken.length() - 1) {
-                throw new IllegalArgumentException(
-                        String.format("found prefix in variable replacement, but no variable name found: '%s'",
-                        replaceToken));
-            }
-            if (prefixSeparatorIndex != -1) {
-                prefix = replaceToken.substring(0, prefixSeparatorIndex);
-                variableName = replaceToken.substring(prefixSeparatorIndex + 1);
-            } else {
-                prefix = "";
-                variableName = replaceToken;
-            }
-            if (prefix.isEmpty()) {
-                return replaceNonPrefix(matcher, variables, variablesUsedBuilder, variableName);
+            prefix = "";
+            variableName = replaceToken;
+        }
+        if (prefix.isEmpty()) {
+            return replaceNonPrefix(matcher, variables, variablesUsed, variableName);
 
-            } else {
-                return replacePrefix(matcher, variables, variablesUsedBuilder, prefix, variableName);
-            }
+        } else {
+            return replacePrefix(matcher, variables, variablesUsed, prefix, variableName);
         }
     }
 
     private static String replacePrefix(
             final Matcher matcher,
             final LinkedHashMap<String, Map<String, String>> variables,
-            final ImmutableList.Builder<String> variablesUsedBuilder,
+            final Map<String, ImmutableList.Builder<String>> variablesUsed,
             final String prefix,
             final String variableName) {
         if (prefix.equals("capture")) {
             if (isNumeric(variableName)) {
                 final Integer groupIndex = Integer.valueOf(variableName);
-                return matcher.group(groupIndex);
-            } else {
-                try {
-                    return matcher.group(variableName);
-                } catch (final IllegalArgumentException ignored2) { // No group with this name
-                    return "";
+                if (groupIndex >= 0 && groupIndex <= matcher.groupCount()) {
+                    return matcher.group(groupIndex);
                 }
             }
+
+            try {
+                return matcher.group(variableName);
+            } catch (final IllegalArgumentException ignored) { // No group with this name
+                return "";
+            }
         }
 
         // Only record variables that are not captures
-        variablesUsedBuilder.add(String.format("%s:%s", prefix, variableName));
+        variablesUsed.computeIfAbsent(prefix, key -> ImmutableList.builder()).add(variableName);
 
         final Map<String, String> variableMap = variables.get(prefix);
         if (variableMap == null) {
@@ -237,9 +228,16 @@ private static String replacePrefix(
     private static String replaceNonPrefix(
             final Matcher matcher,
             final LinkedHashMap<String, Map<String, String>> variables,
-            final ImmutableList.Builder<String> variablesUsedBuilder,
+            final Map<String, ImmutableList.Builder<String>> variablesUsed,
             final String variableName) {
-        // First try the capture group with the name
+        // First try to check against the capture group number
+        if (isNumeric(variableName)) {
+            final int replaceGroup = Integer.parseInt(variableName);
+            if (replaceGroup >= 0 && replaceGroup <= matcher.groupCount()) {
+                return matcher.group(replaceGroup);
+            }
+        }
+        // Then try the capture group with the name
         try {
             return matcher.group(variableName);
         } catch (final IllegalArgumentException e) { // No group with this name
@@ -248,7 +246,7 @@ private static String replaceNonPrefix(
                 final Map<String, String> variableMap = entry.getValue();
                 final String replacement = variableMap.get(variableName);
                 if (replacement != null) {
-                    variablesUsedBuilder.add(String.format("%s:%s", entry.getKey(), variableName));
+                    variablesUsed.computeIfAbsent(entry.getKey(), key -> ImmutableList.builder()).add(variableName);
                     return replacement;
                 }
             }
@@ -280,17 +278,17 @@ public String getReplacement() {
             return _replacement;
         }
 
-        public ImmutableList<String> getVariablesMatched() {
+        public ImmutableMap<String, ImmutableList<String>> getVariablesMatched() {
             return _variablesMatched;
         }
 
-        private Replacement(final String replacement, final ImmutableList<String> variablesMatched) {
+        private Replacement(final String replacement, final ImmutableMap<String, ImmutableList<String>> variablesMatched) {
 
             _replacement = replacement;
             _variablesMatched = variablesMatched;
         }
 
         private final String _replacement;
-        private final ImmutableList<String> _variablesMatched;
+        private final ImmutableMap<String, ImmutableList<String>> _variablesMatched;
     }
 }

src/test/java/com/arpnetworking/metrics/mad/sources/TransformingSourceTest.java

@@ -51,8 +51,8 @@ public void setUp() {
         _transformSetBuilder = new TransformingSource.TransformationSet.Builder()
                 .setFindAndReplace(ImmutableMap.of(
                         "foo/([^/]*)/bar", ImmutableList.of("foo/bar"),
-                        "cat/([^/]*)/dog", ImmutableList.of("cat/dog", "cat/dog/$1"),
-                        "tagged/([^/]*)/dog", ImmutableList.of("tagged/dog;animal=$1"),
+                        "cat/([^/]*)/dog", ImmutableList.of("cat/dog", "cat/dog/${1}"),
+                        "tagged/([^/]*)/dog", ImmutableList.of("tagged/dog;animal=${1}"),
                         "named/(?<animal>[^/]*)", ImmutableList.of("named/extracted_animal;extracted=${animal}"),
                         "tagged/([^/]*)/animal", ImmutableList.of("tagged/${animal}/animal")));
         _transformingSourceBuilder = new TransformingSource.Builder()

src/test/java/com/arpnetworking/utility/RegexAndMapReplacerTest.java

@@ -71,28 +71,27 @@ public void testInvalidNoVariableName() {
         final String result = RegexAndMapReplacer.replaceAll(pattern, input, replace, new LinkedHashMap<>()).getReplacement();
     }
 
-    @Test
-    public void testNumericWithClosingCurly() {
+    @Test(expected = IllegalArgumentException.class)
+    public void testInvalidReplacementTokenMissingOpen() {
         final Pattern pattern = Pattern.compile("test");
         final String input = "test";
-        final String replace = "$0}";
-        final String expected = "test}";
-        testExpression(pattern, input, replace, expected, ImmutableMap.of());
+        final String replace = "$variable"; // replacement variable has no {
+        final String result = RegexAndMapReplacer.replaceAll(pattern, input, replace, new LinkedHashMap<>()).getReplacement();
     }
 
     @Test(expected = IllegalArgumentException.class)
-    public void testInvalidReplacementTokenMissingOpen() {
+    public void testInvalidReplacementTokenNumeric() {
         final Pattern pattern = Pattern.compile("test");
         final String input = "test";
-        final String replace = "$variable"; // replacement variable has no {
+        final String replace = "$0"; // replacement variable has no {
         final String result = RegexAndMapReplacer.replaceAll(pattern, input, replace, new LinkedHashMap<>()).getReplacement();
     }
 
     @Test
     public void testGroup0Replace() {
         final Pattern pattern = Pattern.compile("test");
         final String input = "test";
-        final String replace = "$0";
+        final String replace = "${0}";
         final String expected = "test";
         testExpression(pattern, input, replace, expected, ImmutableMap.of());
     }
@@ -128,7 +127,7 @@ public void testSingleMatchPartialStaticReplacePrefix() {
     public void testSingleMatchPartialMultipleGroupNumberReplace() {
         final Pattern pattern = Pattern.compile("(test)/pattern/(foo)");
         final String input = "test/pattern/foo";
-        final String replace = "this is a $1 pattern called $2";
+        final String replace = "this is a ${1} pattern called ${2}";
         final String expected = "this is a test pattern called foo";
         testExpression(pattern, input, replace, expected, ImmutableMap.of());
     }
@@ -142,6 +141,26 @@ public void testSingleMatchPartialMultipleGroupNameReplace() {
         testExpression(pattern, input, replace, expected, ImmutableMap.of());
     }
 
+    @Test
+    public void testSingleVariableReplaceAsNumber() {
+        final Pattern pattern = Pattern.compile("test/pattern/foo");
+        final String input = "test/pattern/foo";
+        final String replace = "this is a ${1} pattern";
+        final String expected = "this is a test pattern";
+        testExpression(pattern, input, replace, expected, ImmutableMap.of("1", "test"));
+    }
+
+    @Test
+    public void testSingleVariableReplaceAsNumberPrefixed() {
+        final Pattern pattern = Pattern.compile("test/pattern/foo");
+        final String input = "test/pattern/foo";
+        final String replace = "this is a ${prefix1:1} pattern";
+        final String expected = "this is a test pattern";
+        final LinkedHashMap<String, Map<String, String>> variables = new LinkedHashMap<>();
+        variables.put("prefix1", ImmutableMap.of("1", "test"));
+        testExpression(pattern, input, replace, expected, variables);
+    }
+
     @Test
     public void testSingleMatchPartialMultipleVariableReplace() {
         final Pattern pattern = Pattern.compile("test/pattern/foo");
@@ -188,7 +207,9 @@ public void testMatchPrefix() {
         variables.put("prefix1", ImmutableMap.of("var", "var1"));
         variables.put("prefix2", ImmutableMap.of("var", "var2"));
         final RegexAndMapReplacer.Replacement replacement = testExpression(pattern, input, replace, expected, variables);
-        Assert.assertEquals(ImmutableList.of("prefix1:var", "prefix2:var"), replacement.getVariablesMatched());
+        Assert.assertEquals(
+                ImmutableMap.of("prefix1", ImmutableList.of("var"), "prefix2", ImmutableList.of("var")),
+                replacement.getVariablesMatched());
     }
 
     @Test
@@ -201,7 +222,9 @@ public void testMatchPrecedence() {
         variables.put("prefix1", ImmutableMap.of("var", "1-var", "var2", "1-var2"));
         variables.put("prefix2", ImmutableMap.of("var", "2-var", "var2", "2-var2", "var3", "2-var3"));
         final RegexAndMapReplacer.Replacement replacement = testExpression(pattern, input, replace, expected, variables);
-        Assert.assertEquals(ImmutableList.of("prefix1:var2", "prefix2:var3"), replacement.getVariablesMatched());
+        Assert.assertEquals(
+                ImmutableMap.of("prefix1", ImmutableList.of("var2"), "prefix2", ImmutableList.of("var3")),
+                replacement.getVariablesMatched());
     }
 
     @Test