AMBARI-6780 - SOURCE and SINK metrics not working on host-component

ambari-server/src/main/java/org/apache/ambari/server/api/resources/ServiceConfigVersionResourceDefinition.java

@@ -18,13 +18,12 @@
 
 package org.apache.ambari.server.api.resources;
 
+import java.util.List;
+
 import org.apache.ambari.server.api.services.Request;
 import org.apache.ambari.server.api.util.TreeNode;
 import org.apache.ambari.server.controller.spi.Resource;
 
-import java.util.Collections;
-import java.util.List;
-
 public class ServiceConfigVersionResourceDefinition extends BaseResourceDefinition {
   /**
    * Constructor.
@@ -68,7 +67,9 @@ public class ServiceConfigVersionResourceDefinition extends BaseResourceDefiniti
 
         String serviceName = (String) resultNode.getObject().getPropertyValue("service_name");
         Long version = (Long) resultNode.getObject().getPropertyValue("serviceconfigversion");
-        href = href.substring(0, idx) + "cenfigurations/serviceconfigversions?service_name=" + serviceName + "&serviceconfigversion=" + version;
+        href = href.substring(0, idx)
+            + "configurations/serviceconfigversions?service_name="
+            + serviceName + "&serviceconfigversion=" + version;
 
         resultNode.setProperty("href", href);
       } else {

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

@@ -18,9 +18,6 @@
 
 package org.apache.ambari.server.controller.internal;
 
-import org.apache.ambari.server.controller.spi.PropertyProvider;
-import org.apache.ambari.server.controller.utilities.PropertyHelper;
-
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.HashMap;
@@ -31,6 +28,9 @@ import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.ambari.server.controller.spi.PropertyProvider;
+import org.apache.ambari.server.controller.utilities.PropertyHelper;
+
 /**
  *  Abstract property provider implementation.
  */
@@ -123,12 +123,13 @@ public abstract class AbstractPropertyProvider extends BaseProvider implements P
       return;
     }
 
-    String regExpKey = getRegExpKey(propertyId);
+    Map.Entry<String, Pattern> regexEntry = getRegexEntry(propertyId);
 
-    if (regExpKey != null) {
-      propertyInfo = componentMetricMap.get(regExpKey);
+    if (regexEntry != null) {
+      String regexKey = regexEntry.getKey();
+      propertyInfo = componentMetricMap.get(regexKey);
       if (propertyInfo != null) {
-        propertyInfoMap.put(regExpKey, propertyInfo);
+        propertyInfoMap.put(regexKey, propertyInfo);
         return;
       }
     }
@@ -144,14 +145,14 @@ public abstract class AbstractPropertyProvider extends BaseProvider implements P
       }
     }
 
-    if (regExpKey != null) {
-      if (!regExpKey.endsWith("/")){
-        regExpKey += "/";
-      }
+    if (regexEntry != null) {
+      // in the event that a category is being requested, the pattern must
+      // match all child properties; append \S* for this
+      String regexPattern = regexEntry.getValue().pattern();
+      regexPattern += "(\\S*)";
 
-      
       for (Map.Entry<String, PropertyInfo> entry : componentMetricMap.entrySet()) {
-        if (entry.getKey().startsWith(regExpKey)) {
+        if (entry.getKey().matches(regexPattern)) {
           propertyInfoMap.put(entry.getKey(), entry.getValue());
         }
       }
@@ -278,15 +279,18 @@ public abstract class AbstractPropertyProvider extends BaseProvider implements P
   protected void updateComponentMetricMap(
     Map<String, PropertyInfo> componentMetricMap, String propertyId) {
 
-    String regExpKey = getRegExpKey(propertyId);
-
+    String regexKey = null;
+    Map.Entry<String, Pattern> regexEntry = getRegexEntry(propertyId);
+    if (null != regexEntry) {
+      regexKey = regexEntry.getKey();
+    }
 
-    if (!componentMetricMap.containsKey(propertyId) && regExpKey != null &&
-      !regExpKey.equals(propertyId)) {
+    if (!componentMetricMap.containsKey(propertyId) && regexKey != null
+        && !regexKey.equals(propertyId)) {
 
-      PropertyInfo propertyInfo = componentMetricMap.get(regExpKey);
+      PropertyInfo propertyInfo = componentMetricMap.get(regexKey);
       if (propertyInfo != null) {
-        List<String> regexGroups = getRegexGroups(regExpKey, propertyId);
+        List<String> regexGroups = getRegexGroups(regexKey, propertyId);
         String key = propertyInfo.getPropertyId();
         for (String regexGroup : regexGroups) {
           regexGroup = regexGroup.replace("/", ".");

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

@@ -18,6 +18,16 @@
 
 package org.apache.ambari.server.controller.internal;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 import org.apache.ambari.server.controller.spi.Predicate;
 import org.apache.ambari.server.controller.spi.Request;
 import org.apache.ambari.server.controller.spi.Resource;
@@ -26,10 +36,6 @@ import org.apache.ambari.server.controller.utilities.PropertyHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.*;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
 /**
  * Base provider implementation for both property and resource providers.
  */
@@ -61,6 +67,19 @@ public abstract class BaseProvider {
   protected final static Logger LOG =
       LoggerFactory.getLogger(BaseProvider.class);
 
+  /**
+   * The regex pattern that will match on all $1, $2.method(0), and
+   * $3.method(\"/\",\"///\") and properly substitute. For example, it can turn
+   * <p/>
+   * {@code
+   * metrics/yarn/Queue/$1.replaceAll(\"([.])\",\"/\")/AppsCompleted/$2.substring(0)/foo/$3/bar/$4
+   * }
+   * <p/>
+   * into
+   * <p/>
+   * {@code metrics/yarn/Queue/-/AppsCompleted/-/foo/-/bar/-}
+   */
+  private static final Pattern METRIC_ARGUMENT_METHOD_REPLACEMENT = Pattern.compile("\\$\\d+(\\.\\w+(\\(\\\\\"\\S+\\\\\"\\)|\\(\\d+\\)))*");
 
   // ----- Constructors ------------------------------------------------------
 
@@ -71,13 +90,17 @@ public abstract class BaseProvider {
    */
   public BaseProvider(Set<String> propertyIds) {
     this.propertyIds = new HashSet<String>(propertyIds);
-    this.categoryIds = PropertyHelper.getCategories(propertyIds);
-    this.combinedIds = new HashSet<String>(propertyIds);
-    this.combinedIds.addAll(this.categoryIds);
-    this.patterns = new HashMap<String, Pattern>();
-    for (String id : this.combinedIds) {
+    categoryIds = PropertyHelper.getCategories(propertyIds);
+    combinedIds = new HashSet<String>(propertyIds);
+    combinedIds.addAll(categoryIds);
+    patterns = new HashMap<String, Pattern>();
+
+    // convert the argumented metric as it's defined in the JSON file to regex
+    for (String id : combinedIds) {
       if (containsArguments(id)) {
-        String pattern = id.replaceAll("\\$\\d+(\\.\\S+\\(\\S+\\))*", "(\\\\S+)");
+        String pattern = METRIC_ARGUMENT_METHOD_REPLACEMENT.matcher(id).replaceAll(
+            "(\\\\S*)");
+
         patterns.put(id, Pattern.compile(pattern));
       }
     }
@@ -93,15 +116,17 @@ public abstract class BaseProvider {
    */
   protected Set<String> checkConfigPropertyIds(Set<String> base, String configCategory) {
 
-    if (0 == base.size())
+    if (0 == base.size()) {
       return base;
+    }
 
     Set<String> unsupported = new HashSet<String>();
 
     for (String propertyId : base)
     {
-      if (!propertyId.startsWith(configCategory + "/desired_config"))
+      if (!propertyId.startsWith(configCategory + "/desired_config")) {
         unsupported.add(propertyId);
+      }
     }
 
     return unsupported;
@@ -110,7 +135,7 @@ public abstract class BaseProvider {
   public Set<String> checkPropertyIds(Set<String> propertyIds) {
     if (!this.propertyIds.containsAll(propertyIds)) {
       Set<String> unsupportedPropertyIds = new HashSet<String>(propertyIds);
-      unsupportedPropertyIds.removeAll(this.combinedIds);
+      unsupportedPropertyIds.removeAll(combinedIds);
 
       // If the property id is not in the set of known property ids we may still allow it if
       // its parent category is a known property. This allows for Map type properties where
@@ -150,17 +175,17 @@ public abstract class BaseProvider {
       propertyIds.addAll(PredicateHelper.getPropertyIds(predicate));
     }
 
-    if (!this.combinedIds.containsAll(propertyIds)) {
+    if (!combinedIds.containsAll(propertyIds)) {
       Set<String> keepers = new HashSet<String>();
       Set<String> unsupportedPropertyIds = new HashSet<String>(propertyIds);
-      unsupportedPropertyIds.removeAll(this.combinedIds);
+      unsupportedPropertyIds.removeAll(combinedIds);
 
       for (String unsupportedPropertyId : unsupportedPropertyIds) {
         if (checkCategory(unsupportedPropertyId) || checkRegExp(unsupportedPropertyId)) {
           keepers.add(unsupportedPropertyId);
         }
       }
-      propertyIds.retainAll(this.combinedIds);
+      propertyIds.retainAll(combinedIds);
       propertyIds.addAll(keepers);
     }
     return propertyIds;
@@ -173,7 +198,7 @@ public abstract class BaseProvider {
   protected boolean checkCategory(String unsupportedPropertyId) {
     String category = PropertyHelper.getPropertyCategory(unsupportedPropertyId);
     while (category != null) {
-      if(this.propertyIds.contains(category)) {
+      if(propertyIds.contains(category)) {
         return true;
       }
       category = PropertyHelper.getPropertyCategory(category);
@@ -191,20 +216,28 @@ public abstract class BaseProvider {
     return false;
   }
 
-  protected String getRegExpKey(String id) {
-    String regExpKey = null;
+  /**
+   * Gets the key/value mapping between a metric string in the JSON files and
+   * the Java regular expression pattern that matches it.
+   *
+   * @param id
+   * @return the entry, or {@code null} if none match the ID.
+   */
+  protected Map.Entry<String, Pattern> getRegexEntry(String id) {
+    Map.Entry<String, Pattern> regexEntry = null;
+
     for (Map.Entry<String, Pattern> entry : patterns.entrySet()) {
       Pattern pattern = entry.getValue();
       Matcher matcher = pattern.matcher(id);
 
       if (matcher.matches()) {
         String key = entry.getKey();
-        if (regExpKey == null || key.startsWith(regExpKey)) {
-          regExpKey = entry.getKey();
+        if (regexEntry == null || key.startsWith(regexEntry.getKey())) {
+          regexEntry = entry;
         }
       }
     }
-    return regExpKey;
+    return regexEntry;
   }
 
   /**

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

@@ -17,14 +17,6 @@
  */
 package org.apache.ambari.server.controller.utilities;
 
-import org.apache.ambari.server.controller.internal.PropertyInfo;
-import org.apache.ambari.server.controller.internal.RequestImpl;
-import org.apache.ambari.server.controller.spi.Request;
-import org.apache.ambari.server.controller.spi.Resource;
-import org.apache.ambari.server.controller.spi.TemporalInfo;
-import org.codehaus.jackson.map.ObjectMapper;
-import org.codehaus.jackson.type.TypeReference;
-
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
@@ -35,6 +27,14 @@ import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.ambari.server.controller.internal.PropertyInfo;
+import org.apache.ambari.server.controller.internal.RequestImpl;
+import org.apache.ambari.server.controller.spi.Request;
+import org.apache.ambari.server.controller.spi.Resource;
+import org.apache.ambari.server.controller.spi.TemporalInfo;
+import org.codehaus.jackson.map.ObjectMapper;
+import org.codehaus.jackson.type.TypeReference;
+
 /**
  * Utility class that provides Property helper methods.
  */
@@ -56,6 +56,26 @@ public class PropertyHelper {
    */
   private static final Pattern CHECK_FOR_METRIC_ARGUMENTS_REGEX = Pattern.compile(".*\\$\\d+.*");
 
+  /**
+   * This regular expression will match on every {@code /} in a given string
+   * that is not inside of quotes. The following string would be tokenized like
+   * so:
+   * <p/>
+   * {@code foo/$1.substring(5)/bar/$2.replaceAll(\"/a/b//c///\")/baz}
+   * <ul>
+   * <li>foo</li>
+   * <li>$1.substring(5)</li>
+   * <li>bar</li>
+   * <li>$2.replaceAll(\"/a/b//c///\")</li>
+   * <li>baz</li>
+   * </ul>
+   *
+   * This is necessary to be able to properly tokenize a property with {@code /}
+   * while also ensuring we don't match on {@code /} that appears inside of
+   * quotes.
+   */
+  private static final Pattern METRIC_CATEGORY_TOKENIZE_REGEX = Pattern.compile("/+(?=([^\"\\\\\\\\]*(\\\\\\\\.|\"([^\"\\\\\\\\]*\\\\\\\\.)*[^\"\\\\\\\\]*\"))*[^\"]*$)");
+
   public static String getPropertyId(String category, String name) {
     String propertyId =  (category == null || category.isEmpty())? name :
         (name == null || name.isEmpty()) ? category : category + EXTERNAL_PATH_SEP + name;
@@ -114,29 +134,82 @@ public class PropertyHelper {
   }
 
   /**
-   * Helper to get a property category from a string.
+   * Gets the parent category from a given property string. This method is used
+   * in many places by many different consumers. In general, it will check to
+   * see if the property contains arguments. If not, then a simple
+   * {@link String#substring(int, int)} is used along with
+   * {@link #EXTERNAL_PATH_SEP}.
+   * <p/>
+   * In the event that a property contains a $d parameter, it will attempt to
+   * strip out any embedded methods in the various tokens. For example, if a
+   * property of {@code foo/$1.substring(5)/bar/$2.substring(1)/baz} is given,
+   * the expected recursive categories would be:
+   * <ul>
+   * <li>foo</li>
+   * <li>foo/$1</li>
+   * <li>foo/$1/bar</li>
+   * <li>foo/$1/bar</li>
+   * <li>foo/$1/bar/$2</li>
+   * </ul>
    *
-   * @param absProperty  the fully qualified property
+   * {@code foo/$1.substring(5)/bar} is incorrect as a category.
+   *
+   * @param property
+   *          the fully qualified property
    *
    * @return the property category; null if there is no category
    */
-  public static String getPropertyCategory(String absProperty) {
-
-    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;
+  public static String getPropertyCategory(String property) {
+    int lastPathSep = -1;
+
+    if( !containsArguments(property) ){
+      lastPathSep = property.lastIndexOf(EXTERNAL_PATH_SEP);
+      return lastPathSep == -1 ? null : property.substring(0, lastPathSep);
+    }
+
+    // attempt to split the property into its parts
+    String[] tokens = METRIC_CATEGORY_TOKENIZE_REGEX.split(property);
+    if (null == tokens || tokens.length == 0) {
+      return null;
+    }
+
+    StringBuilder categoryBuilder = new StringBuilder();
+    for (int i = 0; i < tokens.length - 1; i++) {
+      String token = tokens[i];
+
+      // if the token contains arguments, turn $1.method() into $1,
+      if (containsArguments(token)) {
+        int methodIndex = token.indexOf('.');
+
+        if (methodIndex != -1) {
+          // normally knowing where $1.method() is would be enough, but some
+          // properties may omit the / (like FLUME) so the property would look
+          // like /$1.method()FailureCount instead of /$1.method()/FailureCount
+          int parensIndex = token.lastIndexOf(')');
+          if (parensIndex < token.length() - 1) {
+            // cut out $1
+            String temp = token.substring(0, methodIndex);
+
+            // append FailureCount
+            temp += token.substring(parensIndex + 1);
+
+            // $1.method()FailureCount -> $1FailureCount
+            token = temp;
+          } else {
+            token = token.substring(0, methodIndex);
+          }
         }
       }
-    } else {
-      lastPathSep = absProperty.lastIndexOf(EXTERNAL_PATH_SEP);
+
+      // only append a / if this is not the last part of the parent category
+      categoryBuilder.append(token);
+      if (i < tokens.length - 2) {
+        categoryBuilder.append('/');
+      }
     }
-    return lastPathSep == -1 ? null : absProperty.substring(0, lastPathSep);
+
+    String category = categoryBuilder.toString();
+    return category;
   }
 
   /**

ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BaseProviderTest.java

@@ -18,17 +18,23 @@
 
 package org.apache.ambari.server.controller.internal;
 
-import org.apache.ambari.server.controller.spi.Request;
-import org.apache.ambari.server.controller.spi.Resource;
-import org.apache.ambari.server.controller.utilities.PropertyHelper;
-import org.junit.Assert;
-import org.junit.Test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Set;
+import java.util.regex.Pattern;
+
+import org.apache.ambari.server.controller.spi.Request;
+import org.apache.ambari.server.controller.spi.Resource;
+import org.apache.ambari.server.controller.utilities.PropertyHelper;
+import org.junit.Test;
 
 /**
  * Base provider tests.
@@ -46,7 +52,7 @@ public class BaseProviderTest {
     BaseProvider provider = new TestProvider(propertyIds);
 
     Set<String> supportedPropertyIds = provider.getPropertyIds();
-    Assert.assertTrue(supportedPropertyIds.containsAll(propertyIds));
+    assertTrue(supportedPropertyIds.containsAll(propertyIds));
   }
 
   @Test
@@ -61,26 +67,28 @@ public class BaseProviderTest {
 
     BaseProvider provider = new TestProvider(propertyIds);
 
-    Assert.assertTrue(provider.checkPropertyIds(propertyIds).isEmpty());
+    assertTrue(provider.checkPropertyIds(propertyIds).isEmpty());
 
-    Assert.assertTrue(provider.checkPropertyIds(Collections.singleton("cat1")).isEmpty());
-    Assert.assertTrue(provider.checkPropertyIds(Collections.singleton("cat2")).isEmpty());
-    Assert.assertTrue(provider.checkPropertyIds(Collections.singleton("cat3")).isEmpty());
-    Assert.assertTrue(provider.checkPropertyIds(Collections.singleton("cat3/subcat3")).isEmpty());
-    Assert.assertTrue(provider.checkPropertyIds(Collections.singleton("cat4/subcat4/map")).isEmpty());
+    assertTrue(provider.checkPropertyIds(Collections.singleton("cat1")).isEmpty());
+    assertTrue(provider.checkPropertyIds(Collections.singleton("cat2")).isEmpty());
+    assertTrue(provider.checkPropertyIds(Collections.singleton("cat3")).isEmpty());
+    assertTrue(provider.checkPropertyIds(Collections.singleton("cat3/subcat3")).isEmpty());
+    assertTrue(provider.checkPropertyIds(
+        Collections.singleton("cat4/subcat4/map")).isEmpty());
 
     // note that key is not in the set of known property ids.  We allow it if its parent is a known property.
     // this allows for Map type properties where we want to treat the entries as individual properties
-    Assert.assertTrue(provider.checkPropertyIds(Collections.singleton("cat4/subcat4/map/key")).isEmpty());
+    assertTrue(provider.checkPropertyIds(
+        Collections.singleton("cat4/subcat4/map/key")).isEmpty());
 
     propertyIds.add("badprop");
     propertyIds.add("badcat");
 
     Set<String> unsupportedPropertyIds = provider.checkPropertyIds(propertyIds);
-    Assert.assertFalse(unsupportedPropertyIds.isEmpty());
-    Assert.assertEquals(2, unsupportedPropertyIds.size());
-    Assert.assertTrue(unsupportedPropertyIds.contains("badprop"));
-    Assert.assertTrue(unsupportedPropertyIds.contains("badcat"));
+    assertFalse(unsupportedPropertyIds.isEmpty());
+    assertEquals(2, unsupportedPropertyIds.size());
+    assertTrue(unsupportedPropertyIds.contains("badprop"));
+    assertTrue(unsupportedPropertyIds.contains("badcat"));
   }
 
   @Test
@@ -96,24 +104,24 @@ public class BaseProviderTest {
 
     Set<String> requestedPropertyIds = provider.getRequestPropertyIds(request, null);
 
-    Assert.assertEquals(1, requestedPropertyIds.size());
-    Assert.assertTrue(requestedPropertyIds.contains("foo"));
+    assertEquals(1, requestedPropertyIds.size());
+    assertTrue(requestedPropertyIds.contains("foo"));
 
     request = PropertyHelper.getReadRequest("foo", "bar");
 
     requestedPropertyIds = provider.getRequestPropertyIds(request, null);
 
-    Assert.assertEquals(2, requestedPropertyIds.size());
-    Assert.assertTrue(requestedPropertyIds.contains("foo"));
-    Assert.assertTrue(requestedPropertyIds.contains("bar"));
+    assertEquals(2, requestedPropertyIds.size());
+    assertTrue(requestedPropertyIds.contains("foo"));
+    assertTrue(requestedPropertyIds.contains("bar"));
 
     request = PropertyHelper.getReadRequest("foo", "baz", "bar", "cat", "cat1/prop1");
 
     requestedPropertyIds = provider.getRequestPropertyIds(request, null);
 
-    Assert.assertEquals(2, requestedPropertyIds.size());
-    Assert.assertTrue(requestedPropertyIds.contains("foo"));
-    Assert.assertTrue(requestedPropertyIds.contains("bar"));
+    assertEquals(2, requestedPropertyIds.size());
+    assertTrue(requestedPropertyIds.contains("foo"));
+    assertTrue(requestedPropertyIds.contains("bar"));
 
     // ask for a property that isn't specified as supported, but its category is... the property
     // should end up in the returned set for the case where the category is a Map property
@@ -121,9 +129,9 @@ public class BaseProviderTest {
 
     requestedPropertyIds = provider.getRequestPropertyIds(request, null);
 
-    Assert.assertEquals(2, requestedPropertyIds.size());
-    Assert.assertTrue(requestedPropertyIds.contains("foo"));
-    Assert.assertTrue(requestedPropertyIds.contains("cat1/sub1/prop1"));
+    assertEquals(2, requestedPropertyIds.size());
+    assertTrue(requestedPropertyIds.contains("foo"));
+    assertTrue(requestedPropertyIds.contains("cat1/sub1/prop1"));
   }
 
   @Test
@@ -140,26 +148,26 @@ public class BaseProviderTest {
 
     Resource resource = new ResourceImpl(Resource.Type.Service);
 
-    Assert.assertNull(resource.getPropertyValue("foo"));
+    assertNull(resource.getPropertyValue("foo"));
 
     BaseProvider.setResourceProperty(resource, "foo", "value1", propertyIds);
-    Assert.assertEquals("value1", resource.getPropertyValue("foo"));
+    assertEquals("value1", resource.getPropertyValue("foo"));
 
     BaseProvider.setResourceProperty(resource, "cat2/bar", "value2", propertyIds);
-    Assert.assertEquals("value2", resource.getPropertyValue("cat2/bar"));
+    assertEquals("value2", resource.getPropertyValue("cat2/bar"));
 
-    Assert.assertNull(resource.getPropertyValue("unsupported"));
+    assertNull(resource.getPropertyValue("unsupported"));
     BaseProvider.setResourceProperty(resource, "unsupported", "valueX", propertyIds);
-    Assert.assertNull(resource.getPropertyValue("unsupported"));
+    assertNull(resource.getPropertyValue("unsupported"));
 
     // we should allow anything under the category cat5/sub5
     BaseProvider.setResourceProperty(resource, "cat5/sub5/prop5", "value5", propertyIds);
-    Assert.assertEquals("value5", resource.getPropertyValue("cat5/sub5/prop5"));
+    assertEquals("value5", resource.getPropertyValue("cat5/sub5/prop5"));
     BaseProvider.setResourceProperty(resource, "cat5/sub5/sub5a/prop5a", "value5", propertyIds);
-    Assert.assertEquals("value5", resource.getPropertyValue("cat5/sub5/sub5a/prop5a"));
+    assertEquals("value5", resource.getPropertyValue("cat5/sub5/sub5a/prop5a"));
     // we shouldn't allow anything under the category cat5/sub7
     BaseProvider.setResourceProperty(resource, "cat5/sub7/unsupported", "valueX", propertyIds);
-    Assert.assertNull(resource.getPropertyValue("cat5/sub7/unsupported"));
+    assertNull(resource.getPropertyValue("cat5/sub7/unsupported"));
   }
 
   @Test
@@ -177,7 +185,7 @@ public class BaseProviderTest {
     // Adding an empty Map as a property should add the actual Map as a property
     Map<String, String> emptyMapProperty = new HashMap<String, String>();
     BaseProvider.setResourceProperty(resource, "cat1/emptyMapProperty", emptyMapProperty, propertyIds);
-    Assert.assertTrue(resource.getPropertiesMap().containsKey("cat1/emptyMapProperty"));
+    assertTrue(resource.getPropertiesMap().containsKey("cat1/emptyMapProperty"));
 
     Map<String, String> mapProperty = new HashMap<String, String>();
     mapProperty.put("key1", "value1");
@@ -187,10 +195,10 @@ public class BaseProviderTest {
     // Adding a property of type Map should add all of its keys as sub properties
     // if the map property was requested
     BaseProvider.setResourceProperty(resource, "cat1/mapProperty", mapProperty, propertyIds);
-    Assert.assertNull(resource.getPropertyValue("cat1/mapProperty"));
-    Assert.assertEquals("value1", resource.getPropertyValue("cat1/mapProperty/key1"));
-    Assert.assertEquals("value2", resource.getPropertyValue("cat1/mapProperty/key2"));
-    Assert.assertEquals("value3", resource.getPropertyValue("cat1/mapProperty/key3"));
+    assertNull(resource.getPropertyValue("cat1/mapProperty"));
+    assertEquals("value1", resource.getPropertyValue("cat1/mapProperty/key1"));
+    assertEquals("value2", resource.getPropertyValue("cat1/mapProperty/key2"));
+    assertEquals("value3", resource.getPropertyValue("cat1/mapProperty/key3"));
 
     Map<String, Map<String, String>> mapMapProperty = new HashMap<String, Map<String, String>>();
     Map<String, String> mapSubProperty1 = new HashMap<String, String>();
@@ -209,16 +217,23 @@ public class BaseProviderTest {
     // Map of maps ... adding a property of type Map should add all of its keys as sub properties
     // if the map property was requested
     BaseProvider.setResourceProperty(resource, "cat2/mapMapProperty", mapMapProperty, propertyIds);
-    Assert.assertNull(resource.getPropertyValue("cat2/mapMapProperty"));
-    Assert.assertNull(resource.getPropertyValue("cat2/mapMapProperty/subMap1"));
-    Assert.assertNull(resource.getPropertyValue("cat2/mapMapProperty/subMap2"));
-    Assert.assertTrue(resource.getPropertiesMap().containsKey("cat2/mapMapProperty/subMap3"));
-    Assert.assertEquals("value11", resource.getPropertyValue("cat2/mapMapProperty/subMap1/key1"));
-    Assert.assertEquals("value12", resource.getPropertyValue("cat2/mapMapProperty/subMap1/key2"));
-    Assert.assertEquals("value13", resource.getPropertyValue("cat2/mapMapProperty/subMap1/key3"));
-    Assert.assertEquals("value21", resource.getPropertyValue("cat2/mapMapProperty/subMap2/key1"));
-    Assert.assertEquals("value22", resource.getPropertyValue("cat2/mapMapProperty/subMap2/key2"));
-    Assert.assertEquals("value23", resource.getPropertyValue("cat2/mapMapProperty/subMap2/key3"));
+    assertNull(resource.getPropertyValue("cat2/mapMapProperty"));
+    assertNull(resource.getPropertyValue("cat2/mapMapProperty/subMap1"));
+    assertNull(resource.getPropertyValue("cat2/mapMapProperty/subMap2"));
+    assertTrue(resource.getPropertiesMap().containsKey(
+        "cat2/mapMapProperty/subMap3"));
+    assertEquals("value11",
+        resource.getPropertyValue("cat2/mapMapProperty/subMap1/key1"));
+    assertEquals("value12",
+        resource.getPropertyValue("cat2/mapMapProperty/subMap1/key2"));
+    assertEquals("value13",
+        resource.getPropertyValue("cat2/mapMapProperty/subMap1/key3"));
+    assertEquals("value21",
+        resource.getPropertyValue("cat2/mapMapProperty/subMap2/key1"));
+    assertEquals("value22",
+        resource.getPropertyValue("cat2/mapMapProperty/subMap2/key2"));
+    assertEquals("value23",
+        resource.getPropertyValue("cat2/mapMapProperty/subMap2/key3"));
 
     Map<String, String> mapProperty3 = new HashMap<String, String>();
     mapProperty3.put("key1", "value1");
@@ -229,10 +244,10 @@ public class BaseProviderTest {
     // should only add requested keys as sub properties ...
     // only "cat3/mapProperty3/key2" was requested
     BaseProvider.setResourceProperty(resource, "cat3/mapProperty3", mapProperty3, propertyIds);
-    Assert.assertNull(resource.getPropertyValue("cat3/mapProperty3"));
-    Assert.assertNull(resource.getPropertyValue("cat3/mapProperty3/key1"));
-    Assert.assertEquals("value2", resource.getPropertyValue("cat3/mapProperty3/key2"));
-    Assert.assertNull(resource.getPropertyValue("cat3/mapProperty3/key3"));
+    assertNull(resource.getPropertyValue("cat3/mapProperty3"));
+    assertNull(resource.getPropertyValue("cat3/mapProperty3/key1"));
+    assertEquals("value2", resource.getPropertyValue("cat3/mapProperty3/key2"));
+    assertNull(resource.getPropertyValue("cat3/mapProperty3/key3"));
 
     Map<String, Map<String, String>> mapMapProperty4 = new HashMap<String, Map<String, String>>();
     mapMapProperty4.put("subMap1", mapSubProperty1);
@@ -241,15 +256,19 @@ public class BaseProviderTest {
     // should only add requested keys as sub properties ...
     // only "cat4/mapMapProperty4/subMap1/key3" and "cat4/mapMapProperty4/subMap2" are requested
     BaseProvider.setResourceProperty(resource, "cat4/mapMapProperty4", mapMapProperty4, propertyIds);
-    Assert.assertNull(resource.getPropertyValue("cat4/mapMapProperty4"));
-    Assert.assertNull(resource.getPropertyValue("cat4/mapMapProperty4/subMap1"));
-    Assert.assertNull(resource.getPropertyValue("cat4/mapMapProperty4/subMap2"));
-    Assert.assertNull(resource.getPropertyValue("cat4/mapMapProperty4/subMap1/key1"));
-    Assert.assertNull(resource.getPropertyValue("cat4/mapMapProperty4/subMap1/key2"));
-    Assert.assertEquals("value13", resource.getPropertyValue("cat4/mapMapProperty4/subMap1/key3"));
-    Assert.assertEquals("value21", resource.getPropertyValue("cat4/mapMapProperty4/subMap2/key1"));
-    Assert.assertEquals("value22", resource.getPropertyValue("cat4/mapMapProperty4/subMap2/key2"));
-    Assert.assertEquals("value23", resource.getPropertyValue("cat4/mapMapProperty4/subMap2/key3"));
+    assertNull(resource.getPropertyValue("cat4/mapMapProperty4"));
+    assertNull(resource.getPropertyValue("cat4/mapMapProperty4/subMap1"));
+    assertNull(resource.getPropertyValue("cat4/mapMapProperty4/subMap2"));
+    assertNull(resource.getPropertyValue("cat4/mapMapProperty4/subMap1/key1"));
+    assertNull(resource.getPropertyValue("cat4/mapMapProperty4/subMap1/key2"));
+    assertEquals("value13",
+        resource.getPropertyValue("cat4/mapMapProperty4/subMap1/key3"));
+    assertEquals("value21",
+        resource.getPropertyValue("cat4/mapMapProperty4/subMap2/key1"));
+    assertEquals("value22",
+        resource.getPropertyValue("cat4/mapMapProperty4/subMap2/key2"));
+    assertEquals("value23",
+        resource.getPropertyValue("cat4/mapMapProperty4/subMap2/key3"));
   }
 
   @Test
@@ -265,15 +284,44 @@ public class BaseProviderTest {
     propertyIds.add(regexp2);
 
     BaseProvider provider = new TestProvider(propertyIds);
-    Assert.assertEquals(regexp, provider.getRegExpKey(propertyId));
-    Assert.assertNull(provider.getRegExpKey(incorrectPropertyId));
-    Assert.assertEquals("sub", provider.getRegexGroups(regexp, propertyId).get(0));
-    Assert.assertEquals("sub2", provider.getRegexGroups(regexp2, propertyId2).get(1));
-    Assert.assertTrue(provider.getRegexGroups(regexp, incorrectPropertyId).isEmpty());
+    Map.Entry<String, Pattern> regexEntry = provider.getRegexEntry(propertyId);
+
+    assertEquals(regexp, regexEntry.getKey());
+    assertNull(provider.getRegexEntry(incorrectPropertyId));
+    assertEquals("sub", provider.getRegexGroups(regexp, propertyId).get(0));
+    assertEquals("sub2", provider.getRegexGroups(regexp2, propertyId2).get(1));
+    assertTrue(provider.getRegexGroups(regexp, incorrectPropertyId).isEmpty());
   }
 
-  static class TestProvider extends BaseProvider {
+  @Test
+  public void testComplexMetricParsing() {
+    Set<String> propertyIds = new HashSet<String>();
+    propertyIds.add("metrics/flume/$1.substring(0)/CHANNEL/$2.replaceAll(\"[^-]+\",\"\")EventPutSuccessCount/rate/sum");
+    propertyIds.add("metrics/yarn/Queue/$1.replaceAll(\"([.])\",\"/\")/AppsCompleted");
+
+    TestProvider provider = new TestProvider(propertyIds);
+    Entry<String, Pattern> entry = provider.getRegexEntry("metrics/flume/flume");
+    assertEquals("metrics/flume/$1", entry.getKey());
+    assertEquals("metrics/flume/(\\S*)", entry.getValue().pattern());
+
+    entry = provider.getRegexEntry("metrics/flume/flume/CHANNEL");
+    assertEquals("metrics/flume/$1/CHANNEL", entry.getKey());
+    assertEquals("metrics/flume/(\\S*)/CHANNEL", entry.getValue().pattern());
+
+    entry = provider.getRegexEntry("metrics/flume/flume/CHANNEL/EventPutSuccessCount");
+    assertEquals("metrics/flume/$1/CHANNEL/$2EventPutSuccessCount", entry.getKey());
+    assertEquals("metrics/flume/(\\S*)/CHANNEL/(\\S*)EventPutSuccessCount",
+        entry.getValue().pattern());
+
+    entry = provider.getRegexEntry("metrics/flume/flume/CHANNEL/EventPutSuccessCount/rate");
+    assertEquals("metrics/flume/$1/CHANNEL/$2EventPutSuccessCount/rate",
+        entry.getKey());
+    assertEquals(
+        "metrics/flume/(\\S*)/CHANNEL/(\\S*)EventPutSuccessCount/rate",
+        entry.getValue().pattern());
+  }
 
+  static class TestProvider extends BaseProvider {
     public TestProvider(Set<String> propertyIds) {
       super(propertyIds);
     }

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

@@ -17,11 +17,6 @@
  */
 package org.apache.ambari.server.controller.utilities;
 
-import org.apache.ambari.server.controller.internal.PropertyInfo;
-import org.apache.ambari.server.controller.spi.Resource;
-import org.junit.Assert;
-import org.junit.Test;
-
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -29,6 +24,11 @@ import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 
+import org.apache.ambari.server.controller.internal.PropertyInfo;
+import org.apache.ambari.server.controller.spi.Resource;
+import org.junit.Assert;
+import org.junit.Test;
+
 
 /**
  * Property helper tests.
@@ -68,25 +68,19 @@ public class PropertyHelperTest {
   @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);
+    Assert.assertEquals("metrics/yarn/Queue/$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);
   }
 
@@ -96,7 +90,7 @@ public class PropertyHelperTest {
 
     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/$1"));
     Assert.assertTrue(categories.contains("metrics/yarn/Queue"));
     Assert.assertTrue(categories.contains("metrics/yarn"));
     Assert.assertTrue(categories.contains("metrics"));
@@ -108,7 +102,7 @@ public class PropertyHelperTest {
 
     categories = PropertyHelper.getCategories(propertyIds);
 
-    Assert.assertTrue(categories.contains("metrics/yarn/Queue/$1.replaceAll(\",q(\\d+)=\",\"/\").substring(1)"));
+    Assert.assertTrue(categories.contains("metrics/yarn/Queue/$1"));
     Assert.assertTrue(categories.contains("metrics/yarn/Queue"));
     Assert.assertTrue(categories.contains("metrics/yarn"));
     Assert.assertTrue(categories.contains("metrics"));