AMBARI-1479. Query Lexer fails to parse some query strings containing 'fields'

git-svn-id: https://svn.apache.org/repos/asf/incubator/ambari/trunk@1449165 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -380,6 +380,9 @@ Trunk (unreleased changes):
 
  AMBARI-1439. rrd file location should be configurable through UI. (jaimin)
 
+ AMBARI-1479. Query Lexer sometimes fails to properly parse query strings with
+              ignored properties such as 'fields' present. (jspeidel)
+ 
  AMBARI-1446. URL used by API to invoke Ganglia rrd script may exceed max length 
               for query string for large clusters. (jspeidel)
 

ambari-server/src/main/java/org/apache/ambari/server/api/predicate/QueryLexer.java

@@ -191,11 +191,9 @@ public class QueryLexer {
     private List<Token> m_listTokens = new ArrayList<Token>();
 
     /**
-     * Whether the current expression should be ignored.
-     * This is used to ignore portions of the query string that are
-     * not query specific.
+     * If non-null, ignore all tokens up to and including this token type.
      */
-    private boolean m_ignore = false;
+    private Token.TYPE m_ignoreSegmentEndToken = null;
 
     /**
      * Constructor.
@@ -206,12 +204,12 @@ public class QueryLexer {
     }
 
     /**
-     * Set the ignore tokens flag.
+     * Ignore all subsequent tokens up to and including the provided token.
      *
-     * @param ignore  true to ignore tokens; false otherwise
+     * @param type  the last token type of the ignore segment
      */
-    public void setIgnoreTokens(boolean ignore) {
-      m_ignore = ignore;
+    public void setIgnoreSegmentEndToken(Token.TYPE type) {
+      m_ignoreSegmentEndToken = type;
     }
 
     /**
@@ -258,8 +256,10 @@ public class QueryLexer {
      * @param token  the token to add
      */
     public void addToken(Token token) {
-      if (! m_ignore) {
+      if (m_ignoreSegmentEndToken == null) {
         m_listTokens.add(token);
+      } else if (token.getType() == m_ignoreSegmentEndToken) {
+        m_ignoreSegmentEndToken = null;
       }
     }
 
@@ -335,10 +335,14 @@ public class QueryLexer {
       if (! SET_IGNORE.contains(token)) {
         ctx.setPropertyOperand(token);
       } else {
-        ctx.setIgnoreTokens(true);
-        if (!ctx.getTokenList().isEmpty()) {
-        // remove '&' token that separates ignored token and query
+        if (!ctx.getTokenList().isEmpty() ) {
+          // ignore through next value operand
+          ctx.setIgnoreSegmentEndToken(Token.TYPE.VALUE_OPERAND);
+          // remove preceding '&' token
           ctx.getTokenList().remove(ctx.getTokenList().size() -1);
+        } else {
+          // first expression.  Ignore and strip out next '&'
+          ctx.setIgnoreSegmentEndToken(Token.TYPE.LOGICAL_OPERATOR);
         }
       }
     }
@@ -465,7 +469,6 @@ public class QueryLexer {
     @Override
     public void _handleToken(String token, ScanContext ctx) throws InvalidQueryException {
       ctx.addToken(new Token(Token.TYPE.LOGICAL_OPERATOR, token));
-      ctx.setIgnoreTokens(false);
     }
 
     @Override

ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseService.java

@@ -25,6 +25,8 @@ import org.apache.ambari.server.api.resources.ResourceInstanceFactory;
 import org.apache.ambari.server.api.resources.ResourceInstanceFactoryImpl;
 import org.apache.ambari.server.api.services.serializers.ResultSerializer;
 import org.apache.ambari.server.controller.spi.Resource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.Response;
@@ -46,6 +48,11 @@ public abstract class BaseService {
    */
   private RequestHandlerFactory m_handlerFactory = new RequestHandlerFactory();
 
+  /**
+   *  Logger instance.
+   */
+  private final static Logger LOG = LoggerFactory.getLogger(BaseService.class);
+
   /**
    * All requests are funneled through this method so that common logic can be executed.
    * This consists of creating a {@link Request} instance, invoking the correct {@link RequestHandler} and
@@ -65,6 +72,8 @@ public abstract class BaseService {
     Request request = getRequestFactory().createRequest(
         headers, body, uriInfo, requestType, resource);
 
+    LOG.info("Handling API Request: '" + request.getURI() + "'");
+
     Result result = getRequestHandler(request.getRequestType()).handleRequest(request);
     if (! result.getStatus().isErrorState()) {
       request.getResultPostProcessor().process(result);

ambari-server/src/test/java/org/apache/ambari/server/api/predicate/QueryLexerTest.java

@@ -194,6 +194,79 @@ public class QueryLexerTest {
     assertArrayEquals(listTokens.toArray(new Token[listTokens.size()]), tokens);
   }
 
+  @Test
+  public void testTokens_ignore__multipleIgnoreFields() throws InvalidQueryException {
+    List<Token> listTokens = new ArrayList<Token>();
+    listTokens.add(new Token(Token.TYPE.RELATIONAL_OPERATOR, "="));
+    listTokens.add(new Token(Token.TYPE.PROPERTY_OPERAND, "foo"));
+    listTokens.add(new Token(Token.TYPE.VALUE_OPERAND, "1"));
+
+    QueryLexer lexer = new QueryLexer();
+    Token[] tokens = lexer.tokens("fields=a/b&foo=1&_=5555555");
+
+    assertArrayEquals(listTokens.toArray(new Token[listTokens.size()]), tokens);
+  }
+
+  @Test
+  public void testTokens_ignore__multipleConsecutiveIgnoreFields() throws InvalidQueryException {
+    List<Token> listTokens = new ArrayList<Token>();
+    listTokens.add(new Token(Token.TYPE.RELATIONAL_OPERATOR, "="));
+    listTokens.add(new Token(Token.TYPE.PROPERTY_OPERAND, "foo"));
+    listTokens.add(new Token(Token.TYPE.VALUE_OPERAND, "1"));
+
+    QueryLexer lexer = new QueryLexer();
+    Token[] tokens = lexer.tokens("foo=1&fields=a/b&_=5555555");
+
+    assertArrayEquals(listTokens.toArray(new Token[listTokens.size()]), tokens);
+  }
+
+  @Test
+  public void testTokens_ignore__multipleConsecutiveIgnoreFields2() throws InvalidQueryException {
+    List<Token> listTokens = new ArrayList<Token>();
+    listTokens.add(new Token(Token.TYPE.RELATIONAL_OPERATOR, "="));
+    listTokens.add(new Token(Token.TYPE.PROPERTY_OPERAND, "foo"));
+    listTokens.add(new Token(Token.TYPE.VALUE_OPERAND, "1"));
+
+    QueryLexer lexer = new QueryLexer();
+    Token[] tokens = lexer.tokens("fields=a/b&_=5555555&foo=1");
+
+    assertArrayEquals(listTokens.toArray(new Token[listTokens.size()]), tokens);
+  }
+
+  @Test
+  public void testTokens_ignore__fieldsMiddle() throws InvalidQueryException {
+    List<Token> listTokens = new ArrayList<Token>();
+    listTokens.add(new Token(Token.TYPE.RELATIONAL_OPERATOR, "="));
+    listTokens.add(new Token(Token.TYPE.PROPERTY_OPERAND, "foo"));
+    listTokens.add(new Token(Token.TYPE.VALUE_OPERAND, "1"));
+    listTokens.add(new Token(Token.TYPE.LOGICAL_OPERATOR, "&"));
+    listTokens.add(new Token(Token.TYPE.RELATIONAL_OPERATOR, "="));
+    listTokens.add(new Token(Token.TYPE.PROPERTY_OPERAND, "bar"));
+    listTokens.add(new Token(Token.TYPE.VALUE_OPERAND, "2"));
+
+    QueryLexer lexer = new QueryLexer();
+    Token[] tokens = lexer.tokens("foo=1&fields=a/b&bar=2");
+
+    assertArrayEquals(listTokens.toArray(new Token[listTokens.size()]), tokens);
+  }
+
+  @Test
+  public void testTokens_ignore__fieldsMiddle2() throws InvalidQueryException {
+    List<Token> listTokens = new ArrayList<Token>();
+    listTokens.add(new Token(Token.TYPE.RELATIONAL_OPERATOR, "="));
+    listTokens.add(new Token(Token.TYPE.PROPERTY_OPERAND, "foo"));
+    listTokens.add(new Token(Token.TYPE.VALUE_OPERAND, "1"));
+    listTokens.add(new Token(Token.TYPE.LOGICAL_OPERATOR, "&"));
+    listTokens.add(new Token(Token.TYPE.RELATIONAL_OPERATOR, "="));
+    listTokens.add(new Token(Token.TYPE.PROPERTY_OPERAND, "bar"));
+    listTokens.add(new Token(Token.TYPE.VALUE_OPERAND, "2"));
+
+    QueryLexer lexer = new QueryLexer();
+    Token[] tokens = lexer.tokens("foo=1&fields=a/b,c&_=123&bar=2");
+
+    assertArrayEquals(listTokens.toArray(new Token[listTokens.size()]), tokens);
+  }
+
   @Test
   public void testTokens_invalidRelationalOp() {
     try {