AMBARI-2926 - PropertyHelper.getPropertyCategory() does not account for replacement tokens in property id

ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java

@@ -59,11 +59,6 @@ public abstract class BaseProvider {
    */
   private final Map<String, Pattern> patterns;
 
-  /**
-   * Regular expression to check for replacement arguments (e.g. $1) in a property id.
-   */
-  private static final Pattern CHECK_FOR_METRIC_ARGUMENTS_REGEX = Pattern.compile(".*\\$\\d+.*");
-
   /**
    * The logger.
    */
@@ -228,8 +223,7 @@ public abstract class BaseProvider {
    * @return true if the given property id contains any replacement arguments
    */
   protected boolean containsArguments(String propertyId) {
-    Matcher matcher = CHECK_FOR_METRIC_ARGUMENTS_REGEX.matcher(propertyId);
-    return matcher.find();
+    return PropertyHelper.containsArguments(propertyId);
   }
 
   /**

ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java

@@ -32,6 +32,8 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * Utility class that provides Property helper methods.
@@ -53,6 +55,11 @@ public class PropertyHelper {
   private static final Map<Resource.Type, Map<String, Map<String, PropertyInfo>>> GANGLIA_PROPERTY_IDS_2 = readPropertyProviderIds(GANGLIA_PROPERTIES_FILE_2);
   private static final Map<Resource.Type, Map<Resource.Type, String>> KEY_PROPERTY_IDS = readKeyPropertyIds(KEY_PROPERTIES_FILE);
 
+  /**
+   * Regular expression to check for replacement arguments (e.g. $1) in a property id.
+   */
+  private static final Pattern CHECK_FOR_METRIC_ARGUMENTS_REGEX = Pattern.compile(".*\\$\\d+.*");
+
   /**
    * Metrics versions.
    */
@@ -139,7 +146,21 @@ public class PropertyHelper {
    * @return the property category; null if there is no category
    */
   public static String getPropertyCategory(String absProperty) {
-    int lastPathSep = absProperty.lastIndexOf(EXTERNAL_PATH_SEP);
+
+    int lastPathSep;
+    if (containsArguments(absProperty)) {
+      boolean inString = false;
+      for (lastPathSep = absProperty.length() - 1; lastPathSep > 0; --lastPathSep) {
+        char c = absProperty.charAt(lastPathSep);
+        if (c == '"') {
+          inString = !inString;
+        } else if (c == EXTERNAL_PATH_SEP && !inString) {
+          break;
+        }
+      }
+    } else {
+      lastPathSep = absProperty.lastIndexOf(EXTERNAL_PATH_SEP);
+    }
     return lastPathSep == -1 ? null : absProperty.substring(0, lastPathSep);
   }
 
@@ -162,6 +183,19 @@ public class PropertyHelper {
     return categories;
   }
 
+  /**
+   * Check to see if the given property id contains replacement arguments (e.g. $1)
+   *
+   * @param propertyId  the property id to check
+   *
+   * @return true if the given property id contains any replacement arguments
+   */
+  public static boolean containsArguments(String propertyId) {
+    Matcher matcher = CHECK_FOR_METRIC_ARGUMENTS_REGEX.matcher(propertyId);
+    return matcher.find();
+  }
+
+
   /**
    * Get all of the property ids associated with the given request.
    *

ambari-server/src/test/java/org/apache/ambari/server/controller/utilities/PropertyHelperTest.java

@@ -22,7 +22,10 @@ import org.apache.ambari.server.controller.spi.Resource;
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 
 
 /**
@@ -68,5 +71,72 @@ public class PropertyHelperTest {
     Assert.assertNotNull(info);
     Assert.assertEquals("Hadoop:service=NameNode,name=JvmMetrics.MemHeapUsedM", info.getPropertyId());
   }
+
+  @Test
+  public void testGetPropertyCategory() {
+    String propertyId = "metrics/yarn/Queue/$1.replaceAll(\",q(\\d+)=\",\"/\").substring(1)/AppsRunning";
+
+    String category = PropertyHelper.getPropertyCategory(propertyId);
+
+    Assert.assertEquals("metrics/yarn/Queue/$1.replaceAll(\",q(\\d+)=\",\"/\").substring(1)", category);
+
+    category = PropertyHelper.getPropertyCategory(category);
+
+    Assert.assertEquals("metrics/yarn/Queue", category);
+
+    category = PropertyHelper.getPropertyCategory(category);
+
+    Assert.assertEquals("metrics/yarn", category);
+
+    category = PropertyHelper.getPropertyCategory(category);
+
+    Assert.assertEquals("metrics", category);
+
+    category = PropertyHelper.getPropertyCategory(category);
+
+    Assert.assertNull(category);
+  }
+
+  @Test
+  public void testGetCategories() {
+    String propertyId = "metrics/yarn/Queue/$1.replaceAll(\",q(\\d+)=\",\"/\").substring(1)/AppsRunning";
+
+    Set<String> categories = PropertyHelper.getCategories(Collections.singleton(propertyId));
+
+    Assert.assertTrue(categories.contains("metrics/yarn/Queue/$1.replaceAll(\",q(\\d+)=\",\"/\").substring(1)"));
+    Assert.assertTrue(categories.contains("metrics/yarn/Queue"));
+    Assert.assertTrue(categories.contains("metrics/yarn"));
+    Assert.assertTrue(categories.contains("metrics"));
+
+    String propertyId2 = "foo/bar/baz";
+    Set<String> propertyIds = new HashSet<String>();
+    propertyIds.add(propertyId);
+    propertyIds.add(propertyId2);
+
+    categories = PropertyHelper.getCategories(propertyIds);
+
+    Assert.assertTrue(categories.contains("metrics/yarn/Queue/$1.replaceAll(\",q(\\d+)=\",\"/\").substring(1)"));
+    Assert.assertTrue(categories.contains("metrics/yarn/Queue"));
+    Assert.assertTrue(categories.contains("metrics/yarn"));
+    Assert.assertTrue(categories.contains("metrics"));
+    Assert.assertTrue(categories.contains("foo/bar"));
+    Assert.assertTrue(categories.contains("foo"));
+  }
+
+  @Test
+  public void testContainsArguments() {
+    Assert.assertFalse(PropertyHelper.containsArguments("foo"));
+    Assert.assertFalse(PropertyHelper.containsArguments("foo/bar"));
+    Assert.assertFalse(PropertyHelper.containsArguments("foo/bar/baz"));
+
+    Assert.assertTrue(PropertyHelper.containsArguments("foo/bar/$1/baz"));
+    Assert.assertTrue(PropertyHelper.containsArguments("foo/bar/$1/baz/$2"));
+    Assert.assertTrue(PropertyHelper.containsArguments("$1/foo/bar/$2/baz"));
+    Assert.assertTrue(PropertyHelper.containsArguments("$1/foo/bar/$2/baz/$3"));
+
+    Assert.assertTrue(PropertyHelper.containsArguments("metrics/yarn/Queue/$1.replaceAll(\",q(\\d+)=\",\"/\").substring(1)"));
+
+    Assert.assertFalse(PropertyHelper.containsArguments("$X/foo/bar/$Y/baz/$Z"));
+  }
 }