[OLINGO-568] SearchParser negative tests

lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParser.java

@@ -21,6 +21,7 @@ package org.apache.olingo.server.core.uri.parser.search;
 import org.apache.olingo.server.api.uri.queryoption.SearchOption;
 import org.apache.olingo.server.api.uri.queryoption.search.SearchBinaryOperatorKind;
 import org.apache.olingo.server.api.uri.queryoption.search.SearchExpression;
+import org.apache.olingo.server.core.uri.parser.search.SearchQueryToken.Token;
 import org.apache.olingo.server.core.uri.queryoption.SearchOptionImpl;
 
 import java.util.Iterator;
@@ -31,13 +32,11 @@ public class SearchParser {
   private Iterator<SearchQueryToken> tokens;
   private SearchQueryToken token;
 
-  public SearchOption parse(String path, String value) {
+  public SearchOption parse(String path, String value) throws SearchParserException, SearchTokenizerException {
     SearchTokenizer tokenizer = new SearchTokenizer();
     SearchExpression searchExpression;
     try {
-      tokens = tokenizer.tokenize(value).iterator();
+      searchExpression = parseInternal(tokenizer.tokenize(value));
-      nextToken();
-      searchExpression = processSearchExpression(null);
     } catch (SearchTokenizerException e) {
       return null;
     }
@@ -46,17 +45,25 @@ public class SearchParser {
     return searchOption;
   }
 
-  protected SearchExpression parseInternal(List<SearchQueryToken> tokens) {
+  protected SearchExpression parseInternal(List<SearchQueryToken> tokens) throws SearchParserException {
     this.tokens = tokens.iterator();
     nextToken();
+    if (token == null) {
+      throw new SearchParserException("No search String", SearchParserException.MessageKeys.NO_EXPRESSION_FOUND);
+    }
     return processSearchExpression(null);
   }
 
-  private SearchExpression processSearchExpression(SearchExpression left) {
+  private SearchExpression processSearchExpression(SearchExpression left) throws SearchParserException {
     if (token == null) {
       return left;
     }
 
+    if (left == null && (isToken(SearchQueryToken.Token.AND) || isToken(SearchQueryToken.Token.OR))) {
+      throw new SearchParserException(token.getToken() + " needs a left operand.",
+          SearchParserException.MessageKeys.INVALID_BINARY_OPERATOR_POSITION, token.getToken().toString());
+    }
+
     SearchExpression expression = left;
     if (isToken(SearchQueryToken.Token.OPEN)) {
       processOpen();
@@ -67,7 +74,12 @@ public class SearchParser {
       expression = processTerm();
     }
 
-    if(isToken(SearchQueryToken.Token.AND) || isTerm()) {
+    if (expression == null) {
+      throw new SearchParserException("Brackets must contain an expression.",
+          SearchParserException.MessageKeys.NO_EXPRESSION_FOUND);
+    }
+
+    if (isToken(SearchQueryToken.Token.AND) || isToken(SearchQueryToken.Token.OPEN) || isTerm()) {
       expression = processAnd(expression);
     } else if (isToken(SearchQueryToken.Token.OR)) {
       expression = processOr(expression);
@@ -94,9 +106,11 @@ public class SearchParser {
     return token.getToken() == toCheckToken;
   }
 
-  private void validateToken(SearchQueryToken.Token toValidateToken) {
+  private void validateToken(SearchQueryToken.Token toValidateToken) throws SearchParserException {
     if (!isToken(toValidateToken)) {
-      throw illegalState();
+      String actualToken = token == null ? "null" : token.getToken().toString();
+      throw new SearchParserException("Expected " + toValidateToken + " but was " + actualToken,
+          SearchParserException.MessageKeys.EXPECTED_DIFFERENT_TOKEN, toValidateToken.toString(), actualToken);
     }
   }
 
@@ -108,7 +122,7 @@ public class SearchParser {
     nextToken();
   }
 
-  private SearchExpression processAnd(SearchExpression left) {
+  private SearchExpression processAnd(SearchExpression left) throws SearchParserException {
     if (isToken(SearchQueryToken.Token.AND)) {
       nextToken();
     }
@@ -118,12 +132,16 @@ public class SearchParser {
       se = new SearchBinaryImpl(left, SearchBinaryOperatorKind.AND, se);
       return processSearchExpression(se);
     } else {
+      if (isToken(SearchQueryToken.Token.AND) || isToken(SearchQueryToken.Token.OR)) {
+        throw new SearchParserException("Operators must not be followed by an AND or an OR",
+            SearchParserException.MessageKeys.INVALID_OPERATOR_AFTER_AND, token.getToken().toString());
+      }
       se = processSearchExpression(se);
       return new SearchBinaryImpl(left, SearchBinaryOperatorKind.AND, se);
     }
   }
 
-  public SearchExpression processOr(SearchExpression left) {
+  public SearchExpression processOr(SearchExpression left) throws SearchParserException {
     if (isToken(SearchQueryToken.Token.OR)) {
       nextToken();
     }
@@ -135,13 +153,14 @@ public class SearchParser {
     return new RuntimeException();
   }
 
-  private SearchExpression processNot() {
+  private SearchExpression processNot() throws SearchParserException {
     nextToken();
+    if (isToken(Token.WORD) || isToken(Token.PHRASE)) {
       SearchExpression searchExpression = processTerm();
-    if(searchExpression.isSearchTerm()) {
       return new SearchUnaryImpl(searchExpression.asSearchTerm());
     }
-    throw illegalState();
+    throw new SearchParserException("NOT must be followed by a term not a " + token.getToken(),
+        SearchParserException.MessageKeys.INVALID_NOT_OPERAND, token.getToken().toString());
   }
 
   private void nextToken() {
@@ -152,7 +171,7 @@ public class SearchParser {
     }
   }
 
-  private SearchExpression processTerm() {
+  private SearchExpression processTerm() throws SearchParserException {
     if (isToken(SearchQueryToken.Token.NOT)) {
       return processNot();
     }

lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParserException.java

@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.olingo.server.core.uri.parser.search;
+
+import org.apache.olingo.server.core.uri.parser.UriParserSyntaxException;
+
+public class SearchParserException extends UriParserSyntaxException {
+
+  private static final long serialVersionUID = 5781553037561337795L;
+
+  public static enum MessageKeys implements MessageKey {
+    /** parameter: operatorkind */
+    INVALID_BINARY_OPERATOR_POSITION, 
+    /** parameter: operatorkind */
+    INVALID_NOT_OPERAND,
+    /** parameters: expectedToken actualToken */
+    EXPECTED_DIFFERENT_TOKEN,
+    NO_EXPRESSION_FOUND, 
+    /** parameter: operatorkind */
+    INVALID_OPERATOR_AFTER_AND;
+
+    @Override
+    public String getKey() {
+      return name();
+    }
+  }
+
+  public SearchParserException(final String developmentMessage, final MessageKey messageKey,
+      final String... parameters) {
+    super(developmentMessage, messageKey, parameters);
+  }
+
+  public SearchParserException(final String developmentMessage, final Throwable cause, final MessageKey messageKey,
+      final String... parameters) {
+    super(developmentMessage, cause, messageKey, parameters);
+  }
+  
+}

lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizer.java

@@ -65,7 +65,8 @@ public class SearchTokenizer {
     }
 
     public State forbidden(char c) throws SearchTokenizerException {
-      throw new SearchTokenizerException("Forbidden character for " + this.getClass().getName() + "->" + c);
+      throw new SearchTokenizerException("Forbidden character for " + this.getClass().getName() + "->" + c,
+          SearchTokenizerException.MessageKeys.FORBIDDEN_CHARACTER, "" + c);
     }
 
     public State finish() {
@@ -158,21 +159,26 @@ public class SearchTokenizer {
 
   private static abstract class LiteralState extends State {
     protected final StringBuilder literal = new StringBuilder();
+
     public LiteralState(Token t) {
       super(t);
     }
+
     public LiteralState(Token t, char c) throws SearchTokenizerException {
       super(t);
       init(c);
     }
+
     public LiteralState(Token t, String initLiteral) {
       super(t);
       literal.append(initLiteral);
     }
+
     public State allowed(char c) {
       literal.append(c);
       return this;
     }
+
     @Override
     public String getLiteral() {
       return literal.toString();
@@ -180,7 +186,8 @@ public class SearchTokenizer {
 
     public State init(char c) throws SearchTokenizerException {
       if (isFinished()) {
-        throw new SearchTokenizerException(toString() + " is already finished.");
+        throw new SearchTokenizerException(toString() + " is already finished.",
+            SearchTokenizerException.MessageKeys.ALREADY_FINISHED);
       }
       literal.append(c);
       return this;
@@ -191,6 +198,7 @@ public class SearchTokenizer {
     public SearchExpressionState() {
       super(null);
     }
+
     @Override
     public State nextChar(char c) throws SearchTokenizerException {
       if (c == CHAR_OPEN) {
@@ -214,6 +222,7 @@ public class SearchTokenizer {
     public SearchTermState() {
       super(Token.TERM);
     }
+
     @Override
     public State nextChar(char c) throws SearchTokenizerException {
       if (c == CHAR_N) {
@@ -225,6 +234,7 @@ public class SearchTokenizer {
       }
       return forbidden(c);
     }
+
     @Override
     public State init(char c) throws SearchTokenizerException {
       return nextChar(c);
@@ -238,6 +248,7 @@ public class SearchTokenizer {
         forbidden(c);
       }
     }
+
     public SearchWordState(State toConsume) throws SearchTokenizerException {
       super(Token.WORD, toConsume.getLiteral());
       char[] chars = literal.toString().toCharArray();
@@ -298,6 +309,7 @@ public class SearchTokenizer {
       super(Token.OPEN);
       finish();
     }
+
     @Override
     public State nextChar(char c) throws SearchTokenizerException {
       finish();
@@ -327,6 +339,7 @@ public class SearchTokenizer {
         forbidden(c);
       }
     }
+
     @Override
     public State nextChar(char c) throws SearchTokenizerException {
       if (literal.length() == 1 && c == CHAR_O) {
@@ -340,6 +353,7 @@ public class SearchTokenizer {
       return forbidden(c);
     }
   }
+
   private class AndState extends LiteralState {
     public AndState(char c) throws SearchTokenizerException {
       super(Token.AND, c);
@@ -347,6 +361,7 @@ public class SearchTokenizer {
         forbidden(c);
       }
     }
+
     @Override
     public State nextChar(char c) throws SearchTokenizerException {
       if (literal.length() == 1 && c == CHAR_N) {
@@ -361,6 +376,7 @@ public class SearchTokenizer {
       }
     }
   }
+
   private class OrState extends LiteralState {
     public OrState(char c) throws SearchTokenizerException {
       super(Token.OR, c);
@@ -368,6 +384,7 @@ public class SearchTokenizer {
         forbidden(c);
       }
     }
+
     @Override
     public State nextChar(char c) throws SearchTokenizerException {
       if (literal.length() == 1 && (c == CHAR_R)) {
@@ -387,6 +404,7 @@ public class SearchTokenizer {
     public BeforeSearchExpressionRwsState() {
       super(null);
     }
+
     @Override
     public State nextChar(char c) throws SearchTokenizerException {
       if (isWhitespace(c)) {
@@ -401,6 +419,7 @@ public class SearchTokenizer {
     public BeforePhraseOrWordRwsState() {
       super(null);
     }
+
     @Override
     public State nextChar(char c) throws SearchTokenizerException {
       if (isWhitespace(c)) {
@@ -417,6 +436,7 @@ public class SearchTokenizer {
     public RwsState() {
       super(null);
     }
+
     @Override
     public State nextChar(char c) throws SearchTokenizerException {
       if (isWhitespace(c)) {

lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerException.java

@@ -18,11 +18,30 @@
  */
 package org.apache.olingo.server.core.uri.parser.search;
 
-public class SearchTokenizerException extends Exception {
+import org.apache.olingo.server.core.uri.parser.UriParserSyntaxException;
+
+public class SearchTokenizerException extends UriParserSyntaxException {
 
   private static final long serialVersionUID = -8295456415309640166L;
 
-  public SearchTokenizerException(String message) {
+  public static enum MessageKeys implements MessageKey {
-    super(message);
+    /** parameter: character */
+    FORBIDDEN_CHARACTER, 
+    ALREADY_FINISHED;
+
+    @Override
+    public String getKey() {
+      return name();
+    }
+  }
+
+  public SearchTokenizerException(final String developmentMessage, final MessageKey messageKey,
+      final String... parameters) {
+    super(developmentMessage, messageKey, parameters);
+  }
+
+  public SearchTokenizerException(final String developmentMessage, final Throwable cause, final MessageKey messageKey,
+      final String... parameters) {
+    super(developmentMessage, cause, messageKey, parameters);
   }
 }

lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserAndTokenizerTest.java

@@ -33,7 +33,7 @@ import org.junit.Test;
 public class SearchParserAndTokenizerTest {
 
   @Test
-  public void basicParsing() throws SearchTokenizerException {
+  public void basicParsing() throws Exception {
     SearchExpressionValidator.init("a")
         .validate(with("a"));
     SearchExpressionValidator.init("a AND b")
@@ -172,17 +172,18 @@ public class SearchParserAndTokenizerTest {
       Assert.fail("Expected exception " + exception.getClass().getSimpleName() + " was not thrown.");
     }
 
-    private void validate(SearchExpression expectedSearchExpression) throws SearchTokenizerException {
+    private void validate(SearchExpression expectedSearchExpression) throws SearchTokenizerException,
+        SearchParserException {
       final SearchExpression searchExpression = getSearchExpression();
       Assert.assertEquals(expectedSearchExpression.toString(), searchExpression.toString());
     }
 
-    private void validate(String expectedSearchExpression) throws SearchTokenizerException {
+    private void validate(String expectedSearchExpression) throws SearchTokenizerException, SearchParserException {
       final SearchExpression searchExpression = getSearchExpression();
       Assert.assertEquals(expectedSearchExpression, searchExpression.toString());
     }
 
-    private SearchExpression getSearchExpression() {
+    private SearchExpression getSearchExpression() throws SearchParserException, SearchTokenizerException {
       SearchParser tokenizer = new SearchParser();
       SearchOption result = tokenizer.parse(null, searchQuery);
       Assert.assertNotNull(result);
@@ -195,5 +196,4 @@ public class SearchParserAndTokenizerTest {
     }
   }
 
-  
 }

lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserTest.java

@@ -21,6 +21,7 @@ package org.apache.olingo.server.core.uri.parser.search;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -29,14 +30,14 @@ import java.util.List;
 
 import org.apache.olingo.server.api.uri.queryoption.search.SearchBinaryOperatorKind;
 import org.apache.olingo.server.api.uri.queryoption.search.SearchExpression;
+import org.apache.olingo.server.core.uri.parser.search.SearchParserException.MessageKeys;
 import org.apache.olingo.server.core.uri.parser.search.SearchQueryToken.Token;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class SearchParserTest extends SearchParser {
 
   @Test
-  public void simple() {
+  public void simple() throws Exception {
     SearchExpression se = run(Token.WORD);
     assertEquals("'word1'", se.toString());
     assertTrue(se.isSearchTerm());
@@ -50,7 +51,7 @@ public class SearchParserTest extends SearchParser {
   }
 
   @Test
-  public void simpleAnd() {
+  public void simpleAnd() throws Exception {
     SearchExpression se = run(Token.WORD, Token.AND, Token.WORD);
     assertEquals("{'word1' AND 'word2'}", se.toString());
     assertTrue(se.isSearchBinary());
@@ -67,7 +68,7 @@ public class SearchParserTest extends SearchParser {
   }
 
   @Test
-  public void simpleOr() {
+  public void simpleOr() throws Exception {
     SearchExpression se = run(Token.WORD, Token.OR, Token.WORD);
     assertEquals("{'word1' OR 'word2'}", se.toString());
     assertTrue(se.isSearchBinary());
@@ -84,7 +85,7 @@ public class SearchParserTest extends SearchParser {
   }
 
   @Test
-  public void simpleImplicitAnd() {
+  public void simpleImplicitAnd() throws Exception {
     SearchExpression se = run(Token.WORD, Token.WORD);
     assertEquals("{'word1' AND 'word2'}", se.toString());
     assertTrue(se.isSearchBinary());
@@ -101,7 +102,7 @@ public class SearchParserTest extends SearchParser {
   }
 
   @Test
-  public void simpleBrackets() {
+  public void simpleBrackets() throws Exception {
     SearchExpression se = run(Token.OPEN, Token.WORD, Token.CLOSE);
     assertEquals("'word1'", se.toString());
     assertTrue(se.isSearchTerm());
@@ -114,7 +115,7 @@ public class SearchParserTest extends SearchParser {
   }
 
   @Test
-  public void simpleNot() {
+  public void simpleNot() throws Exception {
     SearchExpression se = run(Token.NOT, Token.WORD);
     assertEquals("{NOT 'word1'}", se.toString());
     assertTrue(se.isSearchUnary());
@@ -127,21 +128,21 @@ public class SearchParserTest extends SearchParser {
   }
 
   @Test
-  public void precedenceLast() {
+  public void precedenceLast() throws Exception {
     // word1 AND (word2 AND word3)
     SearchExpression se = run(Token.WORD, Token.AND, Token.OPEN, Token.WORD, Token.AND, Token.WORD, Token.CLOSE);
     assertEquals("{'word1' AND {'word2' AND 'word3'}}", se.toString());
   }
 
   @Test
-  public void precedenceFirst() {
+  public void precedenceFirst() throws Exception {
     // (word1 AND word2) AND word3
     SearchExpression se = run(Token.OPEN, Token.WORD, Token.AND, Token.WORD, Token.CLOSE, Token.AND, Token.WORD);
     assertEquals("{{'word1' AND 'word2'} AND 'word3'}", se.toString());
   }
 
   @Test
-  public void combinationAndOr() {
+  public void combinationAndOr() throws Exception {
     // word1 AND word2 OR word3
     SearchExpression se = run(Token.WORD, Token.AND, Token.WORD, Token.OR, Token.WORD);
     assertEquals("{{'word1' AND 'word2'} OR 'word3'}", se.toString());
@@ -150,8 +151,68 @@ public class SearchParserTest extends SearchParser {
     assertEquals("{'word1' OR {'word2' AND 'word3'}}", se.toString());
   }
 
+  @Test
+  public void unnecessaryBrackets() throws Exception {
+    // (word1) (word2)
+    SearchExpression se = run(Token.OPEN, Token.WORD, Token.CLOSE, Token.OPEN, Token.WORD, Token.CLOSE);
+    assertEquals("{'word1' AND 'word2'}", se.toString());
+  }
 
-  private SearchExpression run(SearchQueryToken.Token... tokenArray) {
+  @Test
+  public void complex() throws Exception {
+    // ((word1 word2) word3) OR word4
+    SearchExpression se =
+        run(Token.OPEN, Token.OPEN, Token.WORD, Token.WORD, Token.CLOSE, Token.WORD, Token.CLOSE, Token.OR, Token.WORD);
+    assertEquals("{{{'word1' AND 'word2'} AND 'word3'} OR 'word4'}", se.toString());
+  }
+
+  @Test
+  public void doubleNot() throws Exception {
+    SearchExpression se = run(Token.NOT, Token.WORD, Token.AND, Token.NOT, Token.PHRASE);
+    assertEquals("{{NOT 'word1'} AND {NOT 'phrase1'}}", se.toString());
+  }
+
+  @Test
+  public void notAnd() throws Exception {
+    runEx(SearchParserException.MessageKeys.INVALID_NOT_OPERAND, Token.NOT, Token.AND);
+  }
+
+  @Test
+  public void doubleAnd() throws Exception {
+    runEx(SearchParserException.MessageKeys.INVALID_OPERATOR_AFTER_AND, Token.WORD, Token.AND, Token.AND, Token.WORD);
+  }
+
+  @Test
+  public void singleAnd() {
+    runEx(SearchParserException.MessageKeys.INVALID_BINARY_OPERATOR_POSITION, Token.AND);
+  }
+
+  @Test
+  public void singleOpenBracket() {
+    runEx(SearchParserException.MessageKeys.EXPECTED_DIFFERENT_TOKEN, Token.OPEN);
+  }
+
+  @Test
+  public void emptyBrackets() {
+    runEx(SearchParserException.MessageKeys.NO_EXPRESSION_FOUND, Token.OPEN, Token.CLOSE);
+  }
+
+  @Test
+  public void empty() {
+    Token[] emptyArray = new Token[0];
+    runEx(SearchParserException.MessageKeys.NO_EXPRESSION_FOUND, emptyArray);
+  }
+
+  private void runEx(MessageKeys key, Token... tokenArray) {
+    try {
+      run(tokenArray);
+      fail("Expected UriParserSyntaxException with key " + key);
+    } catch (SearchParserException e) {
+      assertEquals(key, e.getMessageKey());
+    }
+  }
+
+  private SearchExpression run(SearchQueryToken.Token... tokenArray) throws SearchParserException {
     List<SearchQueryToken> tokenList = prepareTokens(tokenArray);
     SearchExpression se = parseInternal(tokenList);
     assertNotNull(se);
@@ -172,6 +233,7 @@ public class SearchParserTest extends SearchParser {
         when(token.getLiteral()).thenReturn("phrase" + phraseNumber);
         phraseNumber++;
       }
+      when(token.toString()).thenReturn("" + tokenArray[i]);
       tokenList.add(token);
     }
     return tokenList;