Repository: falcon Updated Branches: refs/heads/master 52586b3e3 -> e0c08e767
FALCON-1342 Do not allow duplicate properties in entities. Contributed by Balu Vellanki. Project: http://git-wip-us.apache.org/repos/asf/falcon/repo Commit: http://git-wip-us.apache.org/repos/asf/falcon/commit/e0c08e76 Tree: http://git-wip-us.apache.org/repos/asf/falcon/tree/e0c08e76 Diff: http://git-wip-us.apache.org/repos/asf/falcon/diff/e0c08e76 Branch: refs/heads/master Commit: e0c08e767c6b55b6e4432518054a097d4aeda717 Parents: 52586b3 Author: Sowmya Ramesh <[email protected]> Authored: Wed Sep 16 15:25:29 2015 -0700 Committer: Sowmya Ramesh <[email protected]> Committed: Wed Sep 16 15:25:29 2015 -0700 ---------------------------------------------------------------------- CHANGES.txt | 2 + .../entity/parser/ClusterEntityParser.java | 29 ++++++++++++- .../falcon/entity/parser/FeedEntityParser.java | 25 ++++++++++- .../entity/parser/ProcessEntityParser.java | 27 +++++++++++- .../entity/parser/ClusterEntityParserTest.java | 44 ++++++++++++++++++++ .../entity/parser/FeedEntityParserTest.java | 42 +++++++++++++++++++ .../entity/parser/ProcessEntityParserTest.java | 42 +++++++++++++++++++ .../src/test/resources/config/feed/feed-0.1.xml | 6 +++ 8 files changed, 213 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/falcon/blob/e0c08e76/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index b7a2831..fad2d49 100755 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -23,6 +23,8 @@ Trunk (Unreleased) OPTIMIZATIONS BUG FIXES + FALCON-1342 Do not allow duplicate properties in entities(Balu Vellanki via Sowmya Ramesh) + FALCON-1461 NPE in DateValidator validate(Raghav Kumar Gautam via Sowmya Ramesh) FALCON-1446 Flaky TaskLogRetrieverYarnTest(Narayan Periwal via Pallavi Rao) http://git-wip-us.apache.org/repos/asf/falcon/blob/e0c08e76/common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java b/common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java index 5756f84..6a8ec25 100644 --- a/common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java +++ b/common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java @@ -19,6 +19,7 @@ package org.apache.falcon.entity.parser; import org.apache.commons.lang.Validate; +import org.apache.commons.lang3.StringUtils; import org.apache.falcon.FalconException; import org.apache.falcon.catalog.CatalogServiceFactory; import org.apache.falcon.entity.ClusterHelper; @@ -30,6 +31,8 @@ import org.apache.falcon.entity.v0.cluster.ClusterLocationType; import org.apache.falcon.entity.v0.cluster.Interface; import org.apache.falcon.entity.v0.cluster.Interfacetype; import org.apache.falcon.entity.v0.cluster.Location; +import org.apache.falcon.entity.v0.cluster.Properties; +import org.apache.falcon.entity.v0.cluster.Property; import org.apache.falcon.hadoop.HadoopClientFactory; import org.apache.falcon.security.SecurityUtil; import org.apache.falcon.util.StartupProperties; @@ -48,6 +51,8 @@ import org.slf4j.LoggerFactory; import javax.jms.ConnectionFactory; import java.io.IOException; import java.net.URI; +import java.util.HashSet; +import java.util.List; /** * Parser that parses cluster entity definition. @@ -87,8 +92,8 @@ public class ClusterEntityParser extends EntityParser<Cluster> { validateWorkflowInterface(cluster); validateMessagingInterface(cluster); validateRegistryInterface(cluster); - validateLocations(cluster); + validateProperties(cluster); } private void validateScheme(Cluster cluster, Interfacetype interfacetype) @@ -257,7 +262,7 @@ public class ClusterEntityParser extends EntityParser<Cluster> { * @param cluster cluster entity * @throws ValidationException */ - private void validateLocations(Cluster cluster) throws ValidationException { + protected void validateLocations(Cluster cluster) throws ValidationException { Configuration conf = ClusterHelper.getConfiguration(cluster); FileSystem fs; try { @@ -327,6 +332,26 @@ public class ClusterEntityParser extends EntityParser<Cluster> { } + protected void validateProperties(Cluster cluster) throws ValidationException { + Properties properties = cluster.getProperties(); + if (properties == null) { + return; // Cluster has no properties to validate. + } + + List<Property> propertyList = cluster.getProperties().getProperties(); + HashSet<String> propertyKeys = new HashSet<String>(); + for (Property prop : propertyList) { + if (StringUtils.isBlank(prop.getName())) { + throw new ValidationException("Property name and value cannot be empty for Cluster: " + + cluster.getName()); + } + if (!propertyKeys.add(prop.getName())) { + throw new ValidationException("Multiple properties with same name found for Cluster: " + + cluster.getName()); + } + } + } + private void checkPathOwnerAndPermission(String clusterName, String location, FileSystem fs, FsPermission expectedPermission) throws ValidationException { http://git-wip-us.apache.org/repos/asf/falcon/blob/e0c08e76/common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java b/common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java index f22a343..992fc51 100644 --- a/common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java +++ b/common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java @@ -32,6 +32,8 @@ import org.apache.falcon.entity.v0.Entity; import org.apache.falcon.entity.v0.EntityGraph; import org.apache.falcon.entity.v0.EntityType; import org.apache.falcon.entity.v0.Frequency; +import org.apache.falcon.entity.v0.feed.Properties; +import org.apache.falcon.entity.v0.feed.Property; import org.apache.falcon.entity.v0.feed.ACL; import org.apache.falcon.entity.v0.feed.Cluster; import org.apache.falcon.entity.v0.feed.ClusterType; @@ -92,6 +94,7 @@ public class FeedEntityParser extends EntityParser<Feed> { validateFeedGroups(feed); validateFeedSLA(feed); validateACL(feed); + validateProperties(feed); // Seems like a good enough entity object for a new one // But is this an update ? @@ -447,7 +450,7 @@ public class FeedEntityParser extends EntityParser<Feed> { * @param feed Feed entity * @throws ValidationException */ - private void validateACL(Feed feed) throws FalconException { + protected void validateACL(Feed feed) throws FalconException { if (isAuthorizationDisabled) { return; } @@ -476,6 +479,26 @@ public class FeedEntityParser extends EntityParser<Feed> { } } + protected void validateProperties(Feed feed) throws ValidationException { + Properties properties = feed.getProperties(); + if (properties == null) { + return; // feed has no properties to validate. + } + + List<Property> propertyList = feed.getProperties().getProperties(); + HashSet<String> propertyKeys = new HashSet<String>(); + for (Property prop : propertyList) { + if (StringUtils.isBlank(prop.getName())) { + throw new ValidationException("Property name and value cannot be empty for Feed : " + + feed.getName()); + } + if (!propertyKeys.add(prop.getName())) { + throw new ValidationException("Multiple properties with same name found for Feed : " + + feed.getName()); + } + } + } + /** * Validate if FileSystem based feed contains location type data. * http://git-wip-us.apache.org/repos/asf/falcon/blob/e0c08e76/common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java ---------------------------------------------------------------------- diff --git a/common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java b/common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java index 56cc4ca..48a4286 100644 --- a/common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java +++ b/common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java @@ -28,6 +28,8 @@ import org.apache.falcon.entity.store.ConfigurationStore; import org.apache.falcon.entity.v0.EntityType; import org.apache.falcon.entity.v0.Frequency; import org.apache.falcon.entity.v0.cluster.Cluster; +import org.apache.falcon.entity.v0.process.Properties; +import org.apache.falcon.entity.v0.process.Property; import org.apache.falcon.entity.v0.feed.Feed; import org.apache.falcon.entity.v0.process.ACL; import org.apache.falcon.entity.v0.process.Input; @@ -47,6 +49,7 @@ import java.io.IOException; import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.TimeZone; @@ -78,6 +81,7 @@ public class ProcessEntityParser extends EntityParser<Process> { validateEntityExists(EntityType.CLUSTER, clusterName); validateProcessValidity(cluster.getValidity().getStart(), cluster.getValidity().getEnd()); validateHDFSPaths(process, clusterName); + validateProperties(process); if (process.getInputs() != null) { for (Input input : process.getInputs().getInputs()) { @@ -266,7 +270,7 @@ public class ProcessEntityParser extends EntityParser<Process> { * @param process process entity * @throws ValidationException */ - private void validateACL(Process process) throws FalconException { + protected void validateACL(Process process) throws FalconException { if (isAuthorizationDisabled) { return; } @@ -285,4 +289,25 @@ public class ProcessEntityParser extends EntityParser<Process> { throw new ValidationException(e); } } + + protected void validateProperties(Process process) throws ValidationException { + Properties properties = process.getProperties(); + if (properties == null) { + return; // Cluster has no properties to validate. + } + + List<Property> propertyList = process.getProperties().getProperties(); + HashSet<String> propertyKeys = new HashSet<String>(); + for (Property prop : propertyList) { + if (StringUtils.isBlank(prop.getName())) { + throw new ValidationException("Property name and value cannot be empty for Process : " + + process.getName()); + } + if (!propertyKeys.add(prop.getName())) { + throw new ValidationException("Multiple properties with same name found for Process : " + + process.getName()); + } + } + } + } http://git-wip-us.apache.org/repos/asf/falcon/blob/e0c08e76/common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java ---------------------------------------------------------------------- diff --git a/common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java b/common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java index 638cef9..2bafac9 100644 --- a/common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java +++ b/common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java @@ -31,6 +31,7 @@ import org.apache.falcon.entity.v0.cluster.Interface; import org.apache.falcon.entity.v0.cluster.Interfacetype; import org.apache.falcon.entity.v0.cluster.Location; import org.apache.falcon.entity.v0.cluster.Locations; +import org.apache.falcon.entity.v0.cluster.Property; import org.apache.falcon.hadoop.HadoopClientFactory; import org.apache.falcon.util.StartupProperties; import org.apache.hadoop.fs.FileStatus; @@ -150,6 +151,49 @@ public class ClusterEntityParserTest extends AbstractTestBase { Assert.assertEquals(catalog.getVersion(), "0.1"); } + @Test + public void testValidateClusterProperties() throws Exception { + ClusterEntityParser clusterEntityParser = Mockito + .spy((ClusterEntityParser) EntityParserFactory.getParser(EntityType.CLUSTER)); + InputStream stream = this.getClass().getResourceAsStream("/config/cluster/cluster-0.1.xml"); + Cluster cluster = parser.parse(stream); + + Mockito.doNothing().when(clusterEntityParser).validateWorkflowInterface(cluster); + Mockito.doNothing().when(clusterEntityParser).validateMessagingInterface(cluster); + Mockito.doNothing().when(clusterEntityParser).validateRegistryInterface(cluster); + Mockito.doNothing().when(clusterEntityParser).validateLocations(cluster); + + // Good set of properties, should work + clusterEntityParser.validate(cluster); + + // add duplicate property, should throw validation exception. + Property property1 = new Property(); + property1.setName("field1"); + property1.setValue("any value"); + cluster.getProperties().getProperties().add(property1); + try { + clusterEntityParser.validate(cluster); + Assert.fail(); // should not reach here + } catch (ValidationException e) { + // Do nothing + } + + // Remove duplicate property. It should not throw exception anymore + cluster.getProperties().getProperties().remove(property1); + clusterEntityParser.validate(cluster); + + // add empty property name, should throw validation exception. + property1.setName(""); + cluster.getProperties().getProperties().add(property1); + try { + clusterEntityParser.validate(cluster); + Assert.fail(); // should not reach here + } catch (ValidationException e) { + // Do nothing + } + + } + /** * A positive test for validating tags key value pair regex: key=value, key=value. * @throws FalconException http://git-wip-us.apache.org/repos/asf/falcon/blob/e0c08e76/common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java ---------------------------------------------------------------------- diff --git a/common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java b/common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java index 754fb06..d203b7c 100644 --- a/common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java +++ b/common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java @@ -38,12 +38,14 @@ import org.apache.falcon.entity.v0.feed.LocationType; import org.apache.falcon.entity.v0.feed.Locations; import org.apache.falcon.entity.v0.feed.Partition; import org.apache.falcon.entity.v0.feed.Partitions; +import org.apache.falcon.entity.v0.feed.Property; import org.apache.falcon.entity.v0.feed.Validity; import org.apache.falcon.group.FeedGroupMapTest; import org.apache.falcon.security.CurrentUser; import org.apache.falcon.util.FalconTestUtil; import org.apache.falcon.util.StartupProperties; import org.apache.hadoop.fs.Path; +import org.mockito.Mockito; import org.testng.Assert; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -923,4 +925,44 @@ public class FeedEntityParserTest extends AbstractTestBase { StartupProperties.get().setProperty("falcon.security.authorization.enabled", "false"); } } + + @Test + public void testValidateFeedProperties() throws Exception { + FeedEntityParser feedEntityParser = Mockito + .spy((FeedEntityParser) EntityParserFactory.getParser(EntityType.FEED)); + InputStream stream = this.getClass().getResourceAsStream("/config/feed/feed-0.1.xml"); + Feed feed = parser.parse(stream); + + Mockito.doNothing().when(feedEntityParser).validateACL(feed); + + // Good set of properties, should work + feedEntityParser.validate(feed); + + // add duplicate property, should throw validation exception. + Property property1 = new Property(); + property1.setName("field1"); + property1.setValue("any value"); + feed.getProperties().getProperties().add(property1); + try { + feedEntityParser.validate(feed); + Assert.fail(); // should not reach here + } catch (ValidationException e) { + // Do nothing + } + + // Remove duplicate property. It should not throw exception anymore + feed.getProperties().getProperties().remove(property1); + feedEntityParser.validate(feed); + + // add empty property name, should throw validation exception. + property1.setName(""); + feed.getProperties().getProperties().add(property1); + try { + feedEntityParser.validate(feed); + Assert.fail(); // should not reach here + } catch (ValidationException e) { + // Do nothing + } + } + } http://git-wip-us.apache.org/repos/asf/falcon/blob/e0c08e76/common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java ---------------------------------------------------------------------- diff --git a/common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java b/common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java index 6612b74..77f6a77 100644 --- a/common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java +++ b/common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java @@ -25,6 +25,7 @@ import org.apache.falcon.entity.v0.EntityType; import org.apache.falcon.entity.v0.Frequency; import org.apache.falcon.entity.v0.SchemaHelper; import org.apache.falcon.entity.v0.feed.Feed; +import org.apache.falcon.entity.v0.process.Property; import org.apache.falcon.entity.v0.process.Cluster; import org.apache.falcon.entity.v0.process.Input; import org.apache.falcon.entity.v0.process.Process; @@ -32,6 +33,7 @@ import org.apache.falcon.security.CurrentUser; import org.apache.falcon.util.FalconTestUtil; import org.apache.falcon.util.StartupProperties; import org.apache.hadoop.fs.Path; +import org.mockito.Mockito; import org.testng.Assert; import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeClass; @@ -551,4 +553,44 @@ public class ProcessEntityParserTest extends AbstractTestBase { process.getOutputs().getOutputs().get(0).setInstance("today(120,0)"); parser.validate(process); } + + @Test + public void testValidateProcessProperties() throws Exception { + ProcessEntityParser processEntityParser = Mockito + .spy((ProcessEntityParser) EntityParserFactory.getParser(EntityType.PROCESS)); + InputStream stream = this.getClass().getResourceAsStream("/config/process/process-0.1.xml"); + Process process = parser.parse(stream); + + Mockito.doNothing().when(processEntityParser).validateACL(process); + + // Good set of properties, should work + processEntityParser.validate(process); + + // add duplicate property, should throw validation exception. + Property property1 = new Property(); + property1.setName("name1"); + property1.setValue("any value"); + process.getProperties().getProperties().add(property1); + try { + processEntityParser.validate(process); + Assert.fail(); // should not reach here + } catch (ValidationException e) { + // Do nothing + } + + // Remove duplicate property. It should not throw exception anymore + process.getProperties().getProperties().remove(property1); + processEntityParser.validate(process); + + // add empty property name, should throw validation exception. + property1.setName(""); + process.getProperties().getProperties().add(property1); + try { + processEntityParser.validate(process); + Assert.fail(); // should not reach here + } catch (ValidationException e) { + // Do nothing + } + } + } http://git-wip-us.apache.org/repos/asf/falcon/blob/e0c08e76/common/src/test/resources/config/feed/feed-0.1.xml ---------------------------------------------------------------------- diff --git a/common/src/test/resources/config/feed/feed-0.1.xml b/common/src/test/resources/config/feed/feed-0.1.xml index 6448803..ee2607a 100644 --- a/common/src/test/resources/config/feed/feed-0.1.xml +++ b/common/src/test/resources/config/feed/feed-0.1.xml @@ -60,4 +60,10 @@ <ACL owner="testuser-ut-user" group="group" permission="0x755"/> <schema location="/schema/clicks" provider="protobuf"/> + + <properties> + <property name="field1" value="value1"/> + <property name="field2" value="value2"/> + </properties> + </feed>
