falcon-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From venkat...@apache.org
Subject [1/2] git commit: FALCON-840 Possible NPE in filteredInstanceSet method of AbstractInstanceManager. Contributed by Balu Vellanki
Date Wed, 29 Oct 2014 05:22:29 GMT
Repository: incubator-falcon
Updated Branches:
  refs/heads/master 35506d528 -> 20a13e3b4


FALCON-840 Possible NPE in filteredInstanceSet method of AbstractInstanceManager. Contributed
by Balu Vellanki


Project: http://git-wip-us.apache.org/repos/asf/incubator-falcon/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-falcon/commit/20a13e3b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-falcon/tree/20a13e3b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-falcon/diff/20a13e3b

Branch: refs/heads/master
Commit: 20a13e3b458526153a884ce6135162245ab016cd
Parents: b49360e
Author: Venkatesh Seetharam <venkatesh@apache.org>
Authored: Tue Oct 28 22:22:00 2014 -0700
Committer: Venkatesh Seetharam <venkatesh@apache.org>
Committed: Tue Oct 28 22:22:32 2014 -0700

----------------------------------------------------------------------
 CHANGES.txt                                     |   3 +
 .../falcon/resource/AbstractEntityManager.java  | 106 +++++++--------
 .../resource/AbstractInstanceManager.java       | 133 +++++++++++--------
 .../AbstractSchedulableEntityManager.java       |   1 +
 .../java/org/apache/falcon/cli/FalconCLIIT.java |  20 ++-
 5 files changed, 140 insertions(+), 123 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/20a13e3b/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 625dd43..f83e4a8 100755
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -125,6 +125,9 @@ Trunk (Unreleased)
   OPTIMIZATIONS
 
   BUG FIXES
+   FALCON-840 Possible NPE in filteredInstanceSet method of
+   AbstractInstanceManager (Balu Vellanki via Venkatesh Seetharam)
+
    FALCON-839 Authorization succeeds with invalid acl owner based on group
    membership (Venkatesh Seetharam)
 

http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/20a13e3b/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java
----------------------------------------------------------------------
diff --git a/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java b/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java
index 23e0535..c4c6493 100644
--- a/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java
+++ b/prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java
@@ -513,6 +513,7 @@ public abstract class AbstractEntityManager {
                                     String orderBy, String sortOrder, Integer offset, Integer
resultsPerPage) {
 
         HashSet<String> fields = new HashSet<String>(Arrays.asList(fieldStr.toLowerCase().split(",")));
+        validateEntityFilterByClause(filterBy);
         List<Entity> entities;
         try {
             entities = getEntities(type, "", "", "", filterBy, filterTags, orderBy, sortOrder,
offset, resultsPerPage);
@@ -526,11 +527,23 @@ public abstract class AbstractEntityManager {
                 : new EntityList(buildEntityElements(fields, entities));
     }
 
+    protected void validateEntityFilterByClause(String entityFilterByClause) {
+        Map<String, String> filterByFieldsValues = getFilterByFieldsValues(entityFilterByClause);
+        for (Map.Entry<String, String> entry : filterByFieldsValues.entrySet()) {
+            try {
+                EntityList.EntityFilterByFields.valueOf(entry.getKey().toUpperCase());
+            } catch (IllegalArgumentException e) {
+                throw FalconWebException.newInstanceException(
+                        "Invalid filter key: " + entry.getKey(), Response.Status.BAD_REQUEST);
+            }
+        }
+    }
+
     protected List<Entity> getEntities(String type, String startDate, String endDate,
String cluster,
                                        String filterBy, String filterTags, String orderBy,
String sortOrder,
                                        int offset, int resultsPerPage) throws FalconException
{
-        final HashMap<String, String> filterByFieldsValues = getFilterByFieldsValues(filterBy);
-        final ArrayList<String> filterByTags = getFilterByTags(filterTags);
+        final Map<String, String> filterByFieldsValues = getFilterByFieldsValues(filterBy);
+        final List<String> filterByTags = getFilterByTags(filterTags);
 
         EntityType entityType = EntityType.valueOf(type.toUpperCase());
         Collection<String> entityNames = configStore.getEntities(entityType);
@@ -538,7 +551,7 @@ public abstract class AbstractEntityManager {
             return Collections.emptyList();
         }
 
-        ArrayList<Entity> entities = new ArrayList<Entity>();
+        List<Entity> entities = new ArrayList<Entity>();
         for (String entityName : entityNames) {
             Entity entity;
             try {
@@ -603,25 +616,24 @@ public abstract class AbstractEntityManager {
         return false;
     }
 
-    protected static HashMap<String, String> getFilterByFieldsValues(String filterBy)
{
-        //Filter the results by specific field:value
-        HashMap<String, String> filterByFieldValues = new HashMap<String, String>();
+    protected static Map<String, String> getFilterByFieldsValues(String filterBy) {
+        // Filter the results by specific field:value, eliminate empty values
+        Map<String, String> filterByFieldValues = new HashMap<String, String>();
         if (!StringUtils.isEmpty(filterBy)) {
             String[] fieldValueArray = filterBy.split(",");
             for (String fieldValue : fieldValueArray) {
                 String[] splits = fieldValue.split(":", 2);
                 String filterByField = splits[0];
-                if (splits.length == 2) {
+                if (splits.length == 2 && !splits[1].equals("")) {
                     filterByFieldValues.put(filterByField, splits[1]);
-                } else {
-                    filterByFieldValues.put(filterByField, "");
                 }
             }
         }
+
         return filterByFieldValues;
     }
 
-    private static ArrayList<String> getFilterByTags(String filterTags) {
+    private static List<String> getFilterByTags(String filterTags) {
         ArrayList<String> filterTagsList = new ArrayList<String>();
         if (!StringUtils.isEmpty(filterTags)) {
             String[] splits = filterTags.split(",");
@@ -644,7 +656,7 @@ public abstract class AbstractEntityManager {
     }
 
     private boolean filterEntity(Entity entity, String entityStatus,
-                                 HashMap<String, String> filterByFieldsValues, ArrayList<String>
filterByTags,
+                                 Map<String, String> filterByFieldsValues, List<String>
filterByTags,
                                  List<String> tags, List<String> pipelines) {
         if (SecurityUtil.isAuthorizationEnabled() && !isEntityAuthorized(entity))
{
             // the user who requested list query has no permission to access this entity.
Skip this entity
@@ -670,7 +682,7 @@ public abstract class AbstractEntityManager {
         return true;
     }
 
-    private boolean filterEntityByTags(ArrayList<String> filterTagsList, List<String>
tags) {
+    private boolean filterEntityByTags(List<String> filterTagsList, List<String>
tags) {
         boolean filterEntity = false;
         for (String tag : filterTagsList) {
             if (!tags.contains(tag)) {
@@ -682,64 +694,40 @@ public abstract class AbstractEntityManager {
         return filterEntity;
     }
 
-    private boolean filterEntityByFields(Entity entity, HashMap<String, String> filterKeyVals,
+    private boolean filterEntityByFields(Entity entity, Map<String, String> filterKeyVals,
                                          String status, List<String> pipelines) {
-        boolean filterEntity = false;
-
-        if (filterKeyVals.size() != 0) {
-            String filterValue;
-            for (Map.Entry<String, String> pair : filterKeyVals.entrySet()) {
-                filterValue = pair.getValue();
-                if (StringUtils.isEmpty(filterValue)) {
-                    continue; // nothing to filter
-                }
-                EntityList.EntityFilterByFields filter =
-                        EntityList.EntityFilterByFields.valueOf(pair.getKey().toUpperCase());
-                switch (filter) {
+        for (Map.Entry<String, String> pair : filterKeyVals.entrySet()) {
+            String filterValue = pair.getValue();
+            if (StringUtils.isEmpty(filterValue)) {
+                continue; // nothing to filter
+            }
+            EntityList.EntityFilterByFields filter =
+                    EntityList.EntityFilterByFields.valueOf(pair.getKey().toUpperCase());
+            switch (filter) {
 
-                case TYPE:
-                    if (!entity.getEntityType().toString().equalsIgnoreCase(filterValue))
{
-                        filterEntity = true;
-                    }
-                    break;
+            case TYPE:
+                return !entity.getEntityType().toString().equalsIgnoreCase(filterValue);
 
-                case NAME:
-                    if (!entity.getName().equalsIgnoreCase(filterValue)) {
-                        filterEntity = true;
-                    }
-                    break;
+            case NAME:
+                return !entity.getName().equalsIgnoreCase(filterValue);
 
-                case STATUS:
-                    if (!status.equalsIgnoreCase(filterValue)) {
-                        filterEntity = true;
-                    }
-                    break;
+            case STATUS:
+                return  !status.equalsIgnoreCase(filterValue);
 
-                case PIPELINES:
-                    if (entity.getEntityType().equals(EntityType.PROCESS)
-                            && !pipelines.contains(filterValue)) {
-                        filterEntity = true;
-                    }
-                    break;
+            case PIPELINES:
+                return  entity.getEntityType().equals(EntityType.PROCESS) && !pipelines.contains(filterValue);
 
-                case CLUSTER:
-                    Set<String> clusters = EntityUtil.getClustersDefined(entity);
-                    if (!clusters.contains(filterValue)) {
-                        filterEntity = true;
-                    }
+            case CLUSTER:
+                return  !EntityUtil.getClustersDefined(entity).contains(filterValue);
 
-                default:
-                    break;
-                }
-                if (filterEntity) {
-                    break;
-                }
+            default:
+                break;
             }
         }
-        return filterEntity;
+        return false;
     }
 
-    private ArrayList<Entity> sortEntities(ArrayList<Entity> entities, String
orderBy, String sortOrder) {
+    private List<Entity> sortEntities(List<Entity> entities, String orderBy,
String sortOrder) {
         // Sort the ArrayList using orderBy param
         if (!StringUtils.isEmpty(orderBy)) {
             EntityList.EntityFieldList orderByField = EntityList.EntityFieldList.valueOf(orderBy.toUpperCase());

http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/20a13e3b/prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java
----------------------------------------------------------------------
diff --git a/prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java b/prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java
index fce28ad..8dd57ed 100644
--- a/prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java
+++ b/prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java
@@ -95,6 +95,7 @@ public abstract class AbstractInstanceManager extends AbstractEntityManager
{
         try {
             lifeCycles = checkAndUpdateLifeCycle(lifeCycles, type);
             validateNotEmpty("entityName", entity);
+            validateInstanceFilterByClause(filterBy);
             AbstractWorkflowEngine wfEngine = getWorkflowEngine();
             Entity entityObject = EntityUtil.getEntity(type, entity);
             return getInstanceResultSubset(wfEngine.getRunningInstances(entityObject, lifeCycles),
@@ -105,6 +106,25 @@ public abstract class AbstractInstanceManager extends AbstractEntityManager
{
         }
     }
 
+    protected void validateInstanceFilterByClause(String entityFilterByClause) {
+        Map<String, String> filterByFieldsValues = getFilterByFieldsValues(entityFilterByClause);
+        for (Map.Entry<String, String> entry : filterByFieldsValues.entrySet()) {
+            try {
+                InstancesResult.InstanceFilterFields filterKey =
+                        InstancesResult.InstanceFilterFields .valueOf(entry.getKey());
+                if (filterKey == InstancesResult.InstanceFilterFields.STARTEDAFTER) {
+                    EntityUtil.parseDateUTC(entry.getValue());
+                }
+            } catch (IllegalArgumentException e) {
+                throw FalconWebException.newInstanceException(
+                        "Invalid filter key: " + entry.getKey(), Response.Status.BAD_REQUEST);
+            } catch (FalconException e) {
+                throw FalconWebException.newInstanceException(
+                        "Invalid date value for key: " + entry.getKey(), Response.Status.BAD_REQUEST);
+            }
+        }
+    }
+
     //SUSPEND CHECKSTYLE CHECK ParameterNumberCheck
     public InstancesResult getInstances(String type, String entity, String startStr, String
endStr,
                                         String colo, List<LifeCycle> lifeCycles,
@@ -123,6 +143,7 @@ public abstract class AbstractInstanceManager extends AbstractEntityManager
{
         try {
             lifeCycles = checkAndUpdateLifeCycle(lifeCycles, type);
             validateParams(type, entity);
+            validateInstanceFilterByClause(filterBy);
             Entity entityObject = EntityUtil.getEntity(type, entity);
             Pair<Date, Date> startAndEndDate = getStartAndEndDate(entityObject, startStr,
endStr);
 
@@ -149,7 +170,8 @@ public abstract class AbstractInstanceManager extends AbstractEntityManager
{
             Pair<Date, Date> startAndEndDate = getStartAndEndDate(entityObject, startStr,
endStr);
 
             AbstractWorkflowEngine wfEngine = getWorkflowEngine();
-            return wfEngine.getSummary(entityObject, startAndEndDate.first, startAndEndDate.second,
lifeCycles);
+            return wfEngine.getSummary(entityObject, startAndEndDate.first, startAndEndDate.second,
+                    lifeCycles);
         } catch (Throwable e) {
             LOG.error("Failed to get instances status", e);
             throw FalconWebException.newInstanceSummaryException(e, Response.Status.BAD_REQUEST);
@@ -179,10 +201,8 @@ public abstract class AbstractInstanceManager extends AbstractEntityManager
{
     }
 
     private InstancesResult getInstanceResultSubset(InstancesResult resultSet, String filterBy,
-                                                    String orderBy, String sortOrder,
-                                                    Integer offset, Integer numResults) {
-
-        ArrayList<Instance> instanceSet = new ArrayList<Instance>();
+                                                    String orderBy, String sortOrder, Integer
offset,
+                                                    Integer numResults) throws FalconException
{
         if (resultSet.getInstances() == null) {
             // return the empty resultSet
             resultSet.setInstances(new Instance[0]);
@@ -190,7 +210,8 @@ public abstract class AbstractInstanceManager extends AbstractEntityManager
{
         }
 
         // Filter instances
-        instanceSet = filteredInstanceSet(resultSet, instanceSet, getFilterByFieldsValues(filterBy));
+        Map<String, String> filterByFieldsValues = getFilterByFieldsValues(filterBy);
+        List<Instance> instanceSet = getFilteredInstanceSet(resultSet, filterByFieldsValues);
 
         int pageCount = super.getRequiredNumberOfResults(instanceSet.size(), offset, numResults);
         InstancesResult result = new InstancesResult(resultSet.getStatus(), resultSet.getMessage());
@@ -201,69 +222,65 @@ public abstract class AbstractInstanceManager extends AbstractEntityManager
{
         }
         // Sort the ArrayList using orderBy
         instanceSet = sortInstances(instanceSet, orderBy, sortOrder);
-        result.setCollection(instanceSet.subList(offset, (offset+pageCount)).toArray(new
Instance[pageCount]));
+        result.setCollection(instanceSet.subList(
+                offset, (offset+pageCount)).toArray(new Instance[pageCount]));
         return result;
     }
 
-    private ArrayList<Instance> filteredInstanceSet(InstancesResult resultSet, ArrayList<Instance>
instanceSet,
-                                                  HashMap<String, String> filterByFieldsValues)
{
-
-        for (Instance instance : resultSet.getInstances()) {
-            boolean addInstance = true;
-            // If filterBy is empty, return all instances. Else return instances with matching
filter.
-            if (filterByFieldsValues.size() > 0) {
-                String filterValue;
-                for (Map.Entry<String, String> pair : filterByFieldsValues.entrySet())
{
-                    filterValue = pair.getValue();
-                    if (filterValue.equals("")) {
-                        continue;
-                    }
-                    try {
-                        switch (InstancesResult.InstanceFilterFields.valueOf(pair.getKey().toUpperCase()))
{
-                        case STATUS:
-                            String status = "";
-                            if (instance.getStatus() != null) {
-                                status = instance.getStatus().toString();
-                            }
-                            if (!status.equalsIgnoreCase(filterValue)) {
-                                addInstance = false;
-                            }
-                            break;
-                        case CLUSTER:
-                            if (!instance.getCluster().equalsIgnoreCase(filterValue)) {
-                                addInstance = false;
-                            }
-                            break;
-                        case SOURCECLUSTER:
-                            if (!instance.getSourceCluster().equalsIgnoreCase(filterValue))
{
-                                addInstance = false;
-                            }
-                            break;
-                        case STARTEDAFTER:
-                            if (instance.getStartTime().before(EntityUtil.parseDateUTC(filterValue)))
{
-                                addInstance = false;
-                            }
-                            break;
-                        default:
-                            break;
-                        }
-                    } catch (Exception e) {
-                        LOG.error("Invalid entry for filterBy field", e);
-                        throw FalconWebException.newInstanceException(e, Response.Status.BAD_REQUEST);
-                    }
-                    if (!addInstance) {
-                        break;
-                    }
+    private List<Instance> getFilteredInstanceSet(InstancesResult resultSet,
+                                                  Map<String, String> filterByFieldsValues)
+        throws FalconException {
+        // If filterBy is empty, return all instances. Else return instances with matching
filter.
+        if (filterByFieldsValues.size() == 0) {
+            return Arrays.asList(resultSet.getInstances());
+        }
+
+        List<Instance> instanceSet = new ArrayList<Instance>();
+        for (Instance instance : resultSet.getInstances()) {   // for each instance
+            boolean isInstanceFiltered = false;
+
+            // for each filter
+            for (Map.Entry<String, String> pair : filterByFieldsValues.entrySet())
{
+                if (isInstanceFiltered(instance, pair)) { // wait until all filters are applied
+                    isInstanceFiltered = true;
+                    break;  // no use to continue other filters as the current one filtered
this
                 }
             }
-            if (addInstance) {
+
+            if (!isInstanceFiltered) {  // survived all filters
                 instanceSet.add(instance);
             }
         }
+
         return instanceSet;
     }
 
-    private ArrayList<Instance> sortInstances(ArrayList<Instance> instanceSet,
+    private boolean isInstanceFiltered(Instance instance,
+                                       Map.Entry<String, String> pair) throws FalconException
{
+        final String filterValue = pair.getValue();
+        switch (InstancesResult.InstanceFilterFields.valueOf(pair.getKey().toUpperCase()))
{
+        case STATUS:
+            return instance.getStatus() == null
+                    || !instance.getStatus().toString().equalsIgnoreCase(filterValue);
+
+        case CLUSTER:
+            return instance.getCluster() == null
+                    || !instance.getCluster().equalsIgnoreCase(filterValue);
+
+        case SOURCECLUSTER:
+            return instance.getSourceCluster() == null
+                    || !instance.getSourceCluster().equalsIgnoreCase(filterValue);
+
+        case STARTEDAFTER:
+            return instance.getStartTime() == null
+                    || instance.getStartTime().before(EntityUtil.parseDateUTC(filterValue));
+
+        default:
+            return true;
+        }
+    }
+
+    private List<Instance> sortInstances(List<Instance> instanceSet,
                                               String orderBy, String sortOrder) {
         final String order = getValidSortOrder(sortOrder, orderBy);
         if (orderBy.equals("status")) {

http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/20a13e3b/prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java
----------------------------------------------------------------------
diff --git a/prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java
b/prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java
index eb89f5f..687fde9 100644
--- a/prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java
+++ b/prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java
@@ -176,6 +176,7 @@ public abstract class AbstractSchedulableEntityManager extends AbstractInstanceM
                                                 Integer resultsPerPage, Integer numInstances)
{
         HashSet<String> fieldSet = new HashSet<String>(Arrays.asList(fields.toLowerCase().split(",")));
         Pair<Date, Date> startAndEndDates = getStartEndDatesForSummary(startDate, endDate);
+        validateEntityFilterByClause(filterBy);
 
         List<Entity> entities;
         String colo;

http://git-wip-us.apache.org/repos/asf/incubator-falcon/blob/20a13e3b/webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java
----------------------------------------------------------------------
diff --git a/webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java b/webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java
index c9b03c5..88aee0c 100644
--- a/webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java
+++ b/webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java
@@ -29,10 +29,10 @@ import org.testng.annotations.Test;
 
 import java.io.File;
 import java.io.FileOutputStream;
+import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.PrintStream;
-import java.io.IOException;
 import java.util.Date;
 import java.util.Map;
 import java.util.Properties;
@@ -483,10 +483,23 @@ public class FalconCLIIT {
                         + " -filterBy STATUS:SUCCEEDED,STARTEDAFTER:"+START_INSTANCE
                         + " -orderBy startTime -sortOrder desc -offset 0 -numResults 1"));
         Assert.assertEquals(0,
+                executeWithURL("instance -status -type process -name "
+                        + overlay.get("processName")
+                        + " -start "+ START_INSTANCE
+                        + " -filterBy SOURCECLUSTER:"+ overlay.get("cluster")
+                        + " -orderBy startTime -sortOrder desc -offset 0 -numResults 1"));
+
+        Assert.assertEquals(0,
                 executeWithURL("instance -list -type feed -lifecycle eviction -name "
                         + overlay.get("outputFeedName")
                         + " -start "+ SchemaHelper.getDateFormat().format(new Date())
                         +" -filterBy STATUS:SUCCEEDED -orderBy startTime -offset 0 -numResults
1"));
+        Assert.assertEquals(0,
+                executeWithURL("instance -list -type feed -lifecycle eviction -name "
+                        + overlay.get("outputFeedName")
+                        + " -start "+ SchemaHelper.getDateFormat().format(new Date())
+                        +" -filterBy SOURCECLUSTER:" + overlay.get("src.cluster.name")
+                        + " -orderBy startTime -offset 0 -numResults 1"));
         Assert.assertEquals(-1,
                 executeWithURL("instance -status -type feed -lifecycle eviction -name "
                         + overlay.get("outputFeedName")
@@ -502,11 +515,6 @@ public class FalconCLIIT {
                         + overlay.get("outputFeedName")
                         + " -start "+ SchemaHelper.getDateFormat().format(new Date())
                         +" -filterBy STATUS:SUCCEEDED -orderBy startTime -offset 1 -numResults
1"));
-        Assert.assertEquals(0,
-                executeWithURL("instance -list -type feed -lifecycle eviction -name "
-                        + overlay.get("outputFeedName")
-                        + " -start "+ SchemaHelper.getDateFormat().format(new Date())
-                        +" -filterBy STATUS:SUCCEEDED -offset 0 -numResults 1"));
         // When you get a cluster for which there are no feed entities,
         Assert.assertEquals(0,
                 executeWithURL("entity -summary -type feed -cluster " + overlay.get("cluster")
+ " -fields status,tags"


Mime
View raw message