ATLAS-1925: basic search fixes for ATLAS-1917, ATLAS-1922, ATLAS-1926

Signed-off-by: Madhan Neethiraj <mad...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/atlas/repo
Commit: http://git-wip-us.apache.org/repos/asf/atlas/commit/abc4856e
Tree: http://git-wip-us.apache.org/repos/asf/atlas/tree/abc4856e
Diff: http://git-wip-us.apache.org/repos/asf/atlas/diff/abc4856e

Branch: refs/heads/feature-odf
Commit: abc4856e94e24837eee0dc5efc1763a5ceba6fc9
Parents: 3962057
Author: apoorvnaik <apoorvn...@apache.org>
Authored: Mon Jul 17 14:09:22 2017 -0700
Committer: Madhan Neethiraj <mad...@apache.org>
Committed: Mon Jul 17 17:07:53 2017 -0700

----------------------------------------------------------------------
 .../atlas/discovery/EntityDiscoveryService.java | 106 ++++++++++---------
 .../apache/atlas/discovery/SearchProcessor.java |  36 +++++--
 2 files changed, 82 insertions(+), 60 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/atlas/blob/abc4856e/repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
----------------------------------------------------------------------
diff --git 
a/repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
 
b/repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
index a4538bd..c7a624f 100644
--- 
a/repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
+++ 
b/repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
@@ -55,6 +55,7 @@ import 
org.apache.atlas.type.AtlasBuiltInTypes.AtlasObjectIdType;
 import org.apache.atlas.type.AtlasStructType.AtlasAttribute;
 import org.apache.atlas.util.AtlasGremlinQueryProvider;
 import org.apache.atlas.util.AtlasGremlinQueryProvider.AtlasGremlinQuery;
+import org.apache.atlas.util.SearchTracker;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang.StringUtils;
@@ -84,17 +85,19 @@ public class EntityDiscoveryService implements 
AtlasDiscoveryService {
     private final AtlasGremlinQueryProvider       gremlinQueryProvider;
     private final AtlasTypeRegistry               typeRegistry;
     private final GraphBackedSearchIndexer        indexer;
+    private final SearchTracker                   searchTracker;
     private final int                             maxResultSetSize;
     private final int                             maxTypesCountInIdxQuery;
     private final int                             maxTagsCountInIdxQuery;
 
     @Inject
     EntityDiscoveryService(MetadataRepository metadataRepository, 
AtlasTypeRegistry typeRegistry,
-                           AtlasGraph graph, GraphBackedSearchIndexer indexer) 
throws AtlasException {
+                           AtlasGraph graph, GraphBackedSearchIndexer indexer, 
SearchTracker searchTracker) throws AtlasException {
         this.graph                    = graph;
         this.graphPersistenceStrategy = new 
DefaultGraphPersistenceStrategy(metadataRepository);
         this.entityRetriever          = new EntityGraphRetriever(typeRegistry);
         this.indexer                  = indexer;
+        this.searchTracker            = searchTracker;
         this.gremlinQueryProvider     = AtlasGremlinQueryProvider.INSTANCE;
         this.typeRegistry             = typeRegistry;
         this.maxResultSetSize         = 
ApplicationProperties.get().getInt(Constants.INDEX_SEARCH_MAX_RESULT_SET_SIZE, 
150);
@@ -401,73 +404,78 @@ public class EntityDiscoveryService implements 
AtlasDiscoveryService {
     public AtlasSearchResult searchWithParameters(SearchParameters 
searchParameters) throws AtlasBaseException {
         AtlasSearchResult ret = new AtlasSearchResult(searchParameters);
 
-        SearchContext context = new SearchContext(searchParameters, 
typeRegistry, graph, indexer.getVertexIndexKeys());
+        SearchContext context  = new SearchContext(searchParameters, 
typeRegistry, graph, indexer.getVertexIndexKeys());
+        String        searchID = searchTracker.add(context); // For future 
cancellations
 
-        List<AtlasVertex> resultList = context.getSearchProcessor().execute();
+        try {
+            List<AtlasVertex> resultList = 
context.getSearchProcessor().execute();
 
-        // By default any attribute that shows up in the search parameter 
should be sent back in the response
-        // If additional values are requested then the entityAttributes will 
be a superset of the all search attributes
-        // and the explicitly requested attribute(s)
-        Set<String> resultAttributes = new HashSet<>();
-        Set<String> entityAttributes = new HashSet<>();
+            // By default any attribute that shows up in the search parameter 
should be sent back in the response
+            // If additional values are requested then the entityAttributes 
will be a superset of the all search attributes
+            // and the explicitly requested attribute(s)
+            Set<String> resultAttributes = new HashSet<>();
+            Set<String> entityAttributes = new HashSet<>();
 
-        if (CollectionUtils.isNotEmpty(searchParameters.getAttributes())) {
-            resultAttributes.addAll(searchParameters.getAttributes());
-        }
+            if (CollectionUtils.isNotEmpty(searchParameters.getAttributes())) {
+                resultAttributes.addAll(searchParameters.getAttributes());
+            }
 
-        for (String resultAttribute : resultAttributes) {
-            AtlasAttribute attribute = 
context.getEntityType().getAttribute(resultAttribute);
+            for (String resultAttribute : resultAttributes) {
+                AtlasAttribute attribute = 
context.getEntityType().getAttribute(resultAttribute);
 
-            if (attribute != null) {
-                AtlasType attributeType = attribute.getAttributeType();
+                if (attribute != null) {
+                    AtlasType attributeType = attribute.getAttributeType();
 
-                if (attributeType instanceof AtlasArrayType) {
-                    attributeType = ((AtlasArrayType) 
attributeType).getElementType();
-                }
+                    if (attributeType instanceof AtlasArrayType) {
+                        attributeType = ((AtlasArrayType) 
attributeType).getElementType();
+                    }
 
-                if (attributeType instanceof AtlasEntityType || attributeType 
instanceof AtlasObjectIdType) {
-                    entityAttributes.add(resultAttribute);
+                    if (attributeType instanceof AtlasEntityType || 
attributeType instanceof AtlasObjectIdType) {
+                        entityAttributes.add(resultAttribute);
+                    }
                 }
             }
-        }
 
-        for (AtlasVertex atlasVertex : resultList) {
-            AtlasEntityHeader entity = 
entityRetriever.toAtlasEntityHeader(atlasVertex, resultAttributes);
+            for (AtlasVertex atlasVertex : resultList) {
+                AtlasEntityHeader entity = 
entityRetriever.toAtlasEntityHeader(atlasVertex, resultAttributes);
 
-            ret.addEntity(entity);
+                ret.addEntity(entity);
 
-            // populate ret.referredEntities
-            for (String entityAttribute : entityAttributes) {
-                Object attrValue = entity.getAttribute(entityAttribute);
+                // populate ret.referredEntities
+                for (String entityAttribute : entityAttributes) {
+                    Object attrValue = entity.getAttribute(entityAttribute);
 
-                if (attrValue instanceof AtlasObjectId) {
-                    AtlasObjectId objId = (AtlasObjectId)attrValue;
+                    if (attrValue instanceof AtlasObjectId) {
+                        AtlasObjectId objId = (AtlasObjectId) attrValue;
 
-                    if (ret.getReferredEntities() == null) {
-                        ret.setReferredEntities(new HashMap<String, 
AtlasEntityHeader>());
-                    }
+                        if (ret.getReferredEntities() == null) {
+                            ret.setReferredEntities(new HashMap<String, 
AtlasEntityHeader>());
+                        }
 
-                    if 
(!ret.getReferredEntities().containsKey(objId.getGuid())) {
-                        ret.getReferredEntities().put(objId.getGuid(), 
entityRetriever.toAtlasEntityHeader(objId.getGuid()));
-                    }
-                } else if (attrValue instanceof Collection) {
-                    Collection objIds = (Collection)attrValue;
+                        if 
(!ret.getReferredEntities().containsKey(objId.getGuid())) {
+                            ret.getReferredEntities().put(objId.getGuid(), 
entityRetriever.toAtlasEntityHeader(objId.getGuid()));
+                        }
+                    } else if (attrValue instanceof Collection) {
+                        Collection objIds = (Collection) attrValue;
 
-                    for (Object obj : objIds) {
-                        if (obj instanceof AtlasObjectId) {
-                            AtlasObjectId objId = (AtlasObjectId)obj;
+                        for (Object obj : objIds) {
+                            if (obj instanceof AtlasObjectId) {
+                                AtlasObjectId objId = (AtlasObjectId) obj;
 
-                            if (ret.getReferredEntities() == null) {
-                                ret.setReferredEntities(new HashMap<String, 
AtlasEntityHeader>());
-                            }
+                                if (ret.getReferredEntities() == null) {
+                                    ret.setReferredEntities(new 
HashMap<String, AtlasEntityHeader>());
+                                }
 
-                            if 
(!ret.getReferredEntities().containsKey(objId.getGuid())) {
-                                ret.getReferredEntities().put(objId.getGuid(), 
entityRetriever.toAtlasEntityHeader(objId.getGuid()));
+                                if 
(!ret.getReferredEntities().containsKey(objId.getGuid())) {
+                                    
ret.getReferredEntities().put(objId.getGuid(), 
entityRetriever.toAtlasEntityHeader(objId.getGuid()));
+                                }
                             }
                         }
                     }
                 }
             }
+        } finally {
+            searchTracker.remove(searchID);
         }
 
         return ret;
@@ -478,8 +486,8 @@ public class EntityDiscoveryService implements 
AtlasDiscoveryService {
     }
 
     private String getQueryForFullTextSearch(String userKeyedString, String 
typeName, String classification) {
-        String typeFilter          = getTypeFilter(typeRegistry, typeName, 
maxTypesCountInIdxQuery);
-        String classficationFilter = getClassificationFilter(typeRegistry, 
classification, maxTagsCountInIdxQuery);
+        String typeFilter              = getTypeFilter(typeRegistry, typeName, 
maxTypesCountInIdxQuery);
+        String classificationFilter = getClassificationFilter(typeRegistry, 
classification, maxTagsCountInIdxQuery);
 
         StringBuilder queryText = new StringBuilder();
 
@@ -495,12 +503,12 @@ public class EntityDiscoveryService implements 
AtlasDiscoveryService {
             queryText.append(typeFilter);
         }
 
-        if (! StringUtils.isEmpty(classficationFilter)) {
+        if (! StringUtils.isEmpty(classificationFilter)) {
             if (queryText.length() > 0) {
                 queryText.append(" AND ");
             }
 
-            queryText.append(classficationFilter);
+            queryText.append(classificationFilter);
         }
 
         return String.format("v.\"%s\":(%s)", 
Constants.ENTITY_TEXT_PROPERTY_KEY, queryText.toString());

http://git-wip-us.apache.org/repos/asf/atlas/blob/abc4856e/repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java
----------------------------------------------------------------------
diff --git 
a/repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java 
b/repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java
index 1a2d997..29430ef 100644
--- a/repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java
+++ b/repository/src/main/java/org/apache/atlas/discovery/SearchProcessor.java
@@ -52,6 +52,7 @@ public abstract class SearchProcessor {
     public static final String  BRACE_CLOSE_STR = " )";
 
     private static final Map<SearchParameters.Operator, String> OPERATOR_MAP = 
new HashMap<>();
+    private static final char[] OFFENDING_CHARS = {'@', '/', ' '}; // This can 
grow as we discover corner cases
 
     static
     {
@@ -60,9 +61,9 @@ public abstract class SearchProcessor {
         OPERATOR_MAP.put(SearchParameters.Operator.LTE,"v.\"%s\": [* TO %s]");
         OPERATOR_MAP.put(SearchParameters.Operator.GTE,"v.\"%s\": [%s TO *]");
         OPERATOR_MAP.put(SearchParameters.Operator.EQ,"v.\"%s\": %s");
-        OPERATOR_MAP.put(SearchParameters.Operator.NEQ,"v.\"%s\": (NOT %s)");
-        OPERATOR_MAP.put(SearchParameters.Operator.IN, "v.\"%s\": (%s)");
-        OPERATOR_MAP.put(SearchParameters.Operator.LIKE, "v.\"%s\": (%s)");
+        OPERATOR_MAP.put(SearchParameters.Operator.NEQ,"-" + "v.\"%s\": %s");
+        OPERATOR_MAP.put(SearchParameters.Operator.IN, "v.\"%s\": (%s)"); // 
this should be a list of quoted strings
+        OPERATOR_MAP.put(SearchParameters.Operator.LIKE, "v.\"%s\": (%s)"); // 
this should be regex pattern
         OPERATOR_MAP.put(SearchParameters.Operator.STARTS_WITH, "v.\"%s\": 
(%s*)");
         OPERATOR_MAP.put(SearchParameters.Operator.ENDS_WITH, "v.\"%s\": 
(*%s)");
         OPERATOR_MAP.put(SearchParameters.Operator.CONTAINS, "v.\"%s\": 
(*%s*)");
@@ -184,7 +185,7 @@ public abstract class SearchProcessor {
         if (filterCriteria != null) {
             LOG.debug("Processing Filters");
 
-            String filterQuery = toSolrQuery(type, filterCriteria, 
solrAttributes);
+            String filterQuery = toSolrQuery(type, filterCriteria, 
solrAttributes, 0);
 
             if (StringUtils.isNotEmpty(filterQuery)) {
                 solrQuery.append(AND_STR).append(filterQuery);
@@ -196,27 +197,31 @@ public abstract class SearchProcessor {
         }
     }
 
-    private String toSolrQuery(AtlasStructType type, FilterCriteria criteria, 
Set<String> solrAttributes) {
-        return toSolrQuery(type, criteria, solrAttributes, new 
StringBuilder());
+    private String toSolrQuery(AtlasStructType type, FilterCriteria criteria, 
Set<String> solrAttributes, int level) {
+        return toSolrQuery(type, criteria, solrAttributes, new 
StringBuilder(), level);
     }
 
-    private String toSolrQuery(AtlasStructType type, FilterCriteria criteria, 
Set<String> solrAttributes, StringBuilder sb) {
+    private String toSolrQuery(AtlasStructType type, FilterCriteria criteria, 
Set<String> solrAttributes, StringBuilder sb, int level) {
         if (criteria.getCondition() != null && 
CollectionUtils.isNotEmpty(criteria.getCriterion())) {
             StringBuilder nestedExpression = new StringBuilder();
 
             for (FilterCriteria filterCriteria : criteria.getCriterion()) {
-                String nestedQuery = toSolrQuery(type, filterCriteria, 
solrAttributes);
+                String nestedQuery = toSolrQuery(type, filterCriteria, 
solrAttributes, level + 1);
 
                 if (StringUtils.isNotEmpty(nestedQuery)) {
                     if (nestedExpression.length() > 0) {
                         
nestedExpression.append(SPACE_STRING).append(criteria.getCondition()).append(SPACE_STRING);
                     }
-
+                    // todo: when a neq operation is nested and occurs in the 
beginning of the query, solr has issues
                     nestedExpression.append(nestedQuery);
                 }
             }
 
-            return nestedExpression.length() > 0 ? 
sb.append(BRACE_OPEN_STR).append(nestedExpression.toString()).append(BRACE_CLOSE_STR).toString()
 : EMPTY_STRING;
+            if (level == 0) {
+                return nestedExpression.length() > 0 ? 
sb.append(nestedExpression).toString() : EMPTY_STRING;
+            } else {
+                return nestedExpression.length() > 0 ? 
sb.append(BRACE_OPEN_STR).append(nestedExpression).append(BRACE_CLOSE_STR).toString()
 : EMPTY_STRING;
+            }
         } else if (solrAttributes.contains(criteria.getAttributeName())){
             return toSolrExpression(type, criteria.getAttributeName(), 
criteria.getOperator(), criteria.getAttributeValue());
         } else {
@@ -231,7 +236,12 @@ public abstract class SearchProcessor {
             String qualifiedName = type.getQualifiedAttributeName(attrName);
 
             if (OPERATOR_MAP.get(op) != null) {
-                ret = String.format(OPERATOR_MAP.get(op), qualifiedName, 
attrVal);
+                if (hasOffendingChars(attrVal)) {
+                    // FIXME: if attrVal has offending chars & op is contains, 
endsWith, startsWith, solr doesn't like it and results are skewed
+                    ret = String.format(OPERATOR_MAP.get(op), qualifiedName, 
"\"" + attrVal + "\"");
+                } else {
+                    ret = String.format(OPERATOR_MAP.get(op), qualifiedName, 
attrVal);
+                }
             }
         } catch (AtlasBaseException ex) {
             LOG.warn(ex.getMessage());
@@ -378,4 +388,8 @@ public abstract class SearchProcessor {
 
         return defaultValue;
     }
+
+    private boolean hasOffendingChars(String str) {
+        return StringUtils.containsAny(str, OFFENDING_CHARS);
+    }
 }

Reply via email to