You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@falcon.apache.org by so...@apache.org on 2015/09/17 00:25:32 UTC
falcon git commit: FALCON-1342 Do not allow duplicate properties in
entities. Contributed by Balu Vellanki.
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 <sr...@hortonworks.com>
Authored: Wed Sep 16 15:25:29 2015 -0700
Committer: Sowmya Ramesh <sr...@hortonworks.com>
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>