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

Signed-off-by: Madhan Neethiraj <madhan@apache.org>
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.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 @@
     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 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 @@
     }
 
     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 @@
             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());
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 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 @@
         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 @@
         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 @@
         }
     }
 
-    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 @@
             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 @@
 
         return defaultValue;
     }
+
+    private boolean hasOffendingChars(String str) {
+        return StringUtils.containsAny(str, OFFENDING_CHARS);
+    }
 }