Updated Branches: refs/heads/trunk 1851aa3dd -> 34c85a689
AMBARI-2926 - PropertyHelper.getPropertyCategory() does not account for replacement tokens in property id Project: http://git-wip-us.apache.org/repos/asf/incubator-ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ambari/commit/34c85a68 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ambari/tree/34c85a68 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ambari/diff/34c85a68 Branch: refs/heads/trunk Commit: 34c85a689acf604a330be3a3d34b7b0c0e96c032 Parents: 1851aa3 Author: tbeerbower <[email protected]> Authored: Thu Aug 15 21:38:08 2013 -0400 Committer: tbeerbower <[email protected]> Committed: Fri Aug 16 03:51:25 2013 -0400 ---------------------------------------------------------------------- .../controller/internal/BaseProvider.java | 8 +-- .../controller/utilities/PropertyHelper.java | 36 +++++++++- .../utilities/PropertyHelperTest.java | 70 ++++++++++++++++++++ 3 files changed, 106 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/34c85a68/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java index a28b224..3e5e8d7 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BaseProvider.java @@ -60,11 +60,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. */ protected final static Logger LOG = @@ -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); } /** http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/34c85a68/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java index 6bf738a..999c534 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/utilities/PropertyHelper.java +++ b/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. @@ -54,6 +56,11 @@ public class PropertyHelper { 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. */ public enum MetricsVersion { @@ -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); } @@ -163,6 +184,19 @@ public class PropertyHelper { } /** + * 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. * * @param request the request http://git-wip-us.apache.org/repos/asf/incubator-ambari/blob/34c85a68/ambari-server/src/test/java/org/apache/ambari/server/controller/utilities/PropertyHelperTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/utilities/PropertyHelperTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/utilities/PropertyHelperTest.java index 8879268..3ac7551 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/controller/utilities/PropertyHelperTest.java +++ b/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")); + } }
