From 69ef9f5194e0334f5a8378b8e25247ebdc16f794 Mon Sep 17 00:00:00 2001 From: mibo Date: Fri, 20 Nov 2015 14:43:24 +0100 Subject: [PATCH] [OLINGO-568] Added tests and exception messages --- .../core/uri/parser/search/SearchParser.java | 40 ++++++++++------- .../parser/search/SearchParserException.java | 10 ++++- .../uri/parser/search/SearchTokenizer.java | 30 ++++++++----- .../search/SearchTokenizerException.java | 8 ++-- .../server-core-exceptions-i18n.properties | 9 ++++ .../search/SearchParserAndTokenizerTest.java | 29 +++++++++--- .../uri/parser/search/SearchParserTest.java | 11 ++++- .../parser/search/SearchTokenizerTest.java | 45 +++++++++++-------- 8 files changed, 126 insertions(+), 56 deletions(-) diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParser.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParser.java index 3e164447c..4d93f73b6 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParser.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParser.java @@ -54,7 +54,12 @@ public class SearchParser { if (token == null) { throw new SearchParserException("No search String", SearchParserException.MessageKeys.NO_EXPRESSION_FOUND); } - return processSearchExpression(null); + SearchExpression se = processSearchExpression(null); + if(!isEof()) { + throw new SearchParserException("Token left after end of search query parsing.", + SearchParserException.MessageKeys.INVALID_END_OF_QUERY_TOKEN_LEFT, token.getToken().name()); + } + return se; } private SearchExpression processSearchExpression(SearchExpression left) throws SearchParserException { @@ -71,16 +76,15 @@ public class SearchParser { if (isToken(SearchQueryToken.Token.OPEN)) { processOpen(); expression = processSearchExpression(left); - validateToken(SearchQueryToken.Token.CLOSE); + if (expression == null) { + throw new SearchParserException("Brackets must contain an expression.", + SearchParserException.MessageKeys.NO_EXPRESSION_FOUND); + } processClose(); } else if (isTerm()) { expression = processTerm(); } - 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); @@ -106,18 +110,15 @@ public class SearchParser { return token != null && token.getToken() == toCheckToken; } - private void validateToken(SearchQueryToken.Token toValidateToken) throws SearchParserException { - if (!isToken(toValidateToken)) { - String actualToken = token == null ? "null" : token.getToken().toString(); - throw new SearchParserException("Expected " + toValidateToken + " but was " + actualToken, - SearchParserException.MessageKeys.EXPECTED_DIFFERENT_TOKEN, toValidateToken.toString(), actualToken); + private void processClose() throws SearchParserException { + if (isToken(Token.CLOSE)) { + nextToken(); + } else { + throw new SearchParserException("Missing close bracket after open bracket.", + SearchParserException.MessageKeys.MISSING_CLOSE); } } - private void processClose() { - nextToken(); - } - private void processOpen() { nextToken(); } @@ -137,7 +138,10 @@ public class SearchParser { } 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()); + SearchParserException.MessageKeys.INVALID_OPERATOR_AFTER_AND, token.getToken().name()); + } else if(isEof()) { + throw new SearchParserException("Missing search expression after AND (found end of search query)", + SearchParserException.MessageKeys.INVALID_END_OF_QUERY, Token.AND.name()); } se = processSearchExpression(se); return new SearchBinaryImpl(left, SearchBinaryOperatorKind.AND, se); @@ -148,6 +152,10 @@ public class SearchParser { if (isToken(SearchQueryToken.Token.OR)) { nextToken(); } + if(isEof()) { + throw new SearchParserException("Missing search expression after OR (found end of search query)", + SearchParserException.MessageKeys.INVALID_END_OF_QUERY, Token.OR.name()); + } SearchExpression se = processSearchExpression(left); return new SearchBinaryImpl(left, SearchBinaryOperatorKind.OR, se); } diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParserException.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParserException.java index 9e612a097..1bc60dc1d 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParserException.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchParserException.java @@ -25,17 +25,23 @@ public class SearchParserException extends UriParserSyntaxException { private static final long serialVersionUID = 5781553037561337795L; public enum MessageKeys implements MessageKey { + NO_EXPRESSION_FOUND, /** parameter: message */ TOKENIZER_EXCEPTION, /** parameter: tokenCharacter */ INVALID_TOKEN_CHARACTER_FOUND, /** parameter: operatorkind */ - INVALID_BINARY_OPERATOR_POSITION, + INVALID_BINARY_OPERATOR_POSITION, /** parameter: operatorkind */ INVALID_NOT_OPERAND, + /** parameters: - */ + MISSING_CLOSE, /** parameters: expectedToken actualToken */ EXPECTED_DIFFERENT_TOKEN, - NO_EXPRESSION_FOUND, + /** parameters: actualToken */ + INVALID_END_OF_QUERY, + /** parameters: left_over_token */ + INVALID_END_OF_QUERY_TOKEN_LEFT, /** parameter: operatorkind */ INVALID_OPERATOR_AFTER_AND; diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizer.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizer.java index ffff1b5e5..5c42e6ddf 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizer.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizer.java @@ -84,6 +84,7 @@ public class SearchTokenizer { this.finished = true; return this; } + public State finishAs(Token token) { this.finished = true; return changeToken(token); @@ -97,6 +98,13 @@ public class SearchTokenizer { return token; } + public String getTokenName() { + if(token == null) { + return "NULL"; + } + return token.name(); + } + public State close() throws SearchTokenizerException { return this; } @@ -260,7 +268,7 @@ public class SearchTokenizer { @Override public String toString() { - return this.getToken() + "=>{" + getLiteral() + "}"; + return getToken() + "=>{" + getLiteral() + "}"; } } @@ -292,7 +300,7 @@ public class SearchTokenizer { public State init(char c) throws SearchTokenizerException { if (isFinished()) { throw new SearchTokenizerException(toString() + " is already finished.", - SearchTokenizerException.MessageKeys.ALREADY_FINISHED); + SearchTokenizerException.MessageKeys.ALREADY_FINISHED, getTokenName()); } literal.append(c); return this; @@ -348,10 +356,9 @@ public class SearchTokenizer { public SearchWordState(State toConsume) throws SearchTokenizerException { super(Token.WORD, toConsume.getLiteral()); - char[] chars = literal.toString().toCharArray(); - for (char aChar : chars) { - if (!isAllowedWord(aChar)) { - forbidden(aChar); + for (int i = 0; i < literal.length(); i++) { + if (!isAllowedWord(literal.charAt(i))) { + forbidden(literal.charAt(i)); } } } @@ -479,7 +486,8 @@ public class SearchTokenizer { changeToken(Token.WORD).finish(); return new RwsState(); } - return new SearchWordState(this).nextChar(c); + literal.append(c); + return new SearchWordState(this); } @Override public State close() throws SearchTokenizerException { @@ -511,7 +519,8 @@ public class SearchTokenizer { changeToken(Token.WORD).finish(); return new RwsState(); } - return new SearchWordState(this).nextChar(c); + literal.append(c); + return new SearchWordState(this); } @Override public State close() throws SearchTokenizerException { @@ -540,7 +549,8 @@ public class SearchTokenizer { changeToken(Token.WORD).finish(); return new RwsState(); } - return new SearchWordState(this).nextChar(c); + literal.append(c); + return new SearchWordState(this); } @Override public State close() throws SearchTokenizerException { @@ -620,7 +630,7 @@ public class SearchTokenizer { states.add(state); } else { throw new SearchTokenizerException("Last parsed state '" + state.toString() + "' is not finished.", - SearchTokenizerException.MessageKeys.NOT_FINISHED_QUERY); + SearchTokenizerException.MessageKeys.NOT_FINISHED_QUERY, state.getTokenName()); } return states; diff --git a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerException.java b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerException.java index abdf84c44..6ebec5ae8 100644 --- a/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerException.java +++ b/lib/server-core/src/main/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerException.java @@ -25,15 +25,15 @@ public class SearchTokenizerException extends UriParserSyntaxException { private static final long serialVersionUID = -8295456415309640166L; public enum MessageKeys implements MessageKey { - /** parameter: character */ + /** parameter: character, TOKEN */ FORBIDDEN_CHARACTER, /** parameter: TOKEN */ NOT_EXPECTED_TOKEN, - /** parameter: - */ + /** parameter: TOKEN */ NOT_FINISHED_QUERY, - /** parameter: - */ + /** parameter: TOKEN */ INVALID_TOKEN_STATE, - /** parameter: - */ + /** parameter: TOKEN */ ALREADY_FINISHED; @Override diff --git a/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties b/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties index bfb9440b3..a2baf0fc3 100644 --- a/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties +++ b/lib/server-core/src/main/resources/server-core-exceptions-i18n.properties @@ -44,6 +44,15 @@ SearchParserException.INVALID_NOT_OPERAND=Invalid not operand for kind '%1$s' fo SearchParserException.EXPECTED_DIFFERENT_TOKEN=Expected token '%1$s' but found '%2$s'. SearchParserException.NO_EXPRESSION_FOUND=No expression found. SearchParserException.INVALID_OPERATOR_AFTER_AND=Invalid operator after AND found of kind '%1$s'. +SearchParserException.INVALID_END_OF_QUERY=Invalid end of search query after '%1$s' (query must end with a search phrase or word). +SearchParserException.INVALID_END_OF_QUERY_TOKEN_LEFT=Invalid end of search query. Found not processed token '%1$s' at the end. +SearchParserException.MISSING_CLOSE=Missing closing bracket after an opening bracket. + +SearchTokenizerException.FORBIDDEN_CHARACTER=Not allowed character '%1$s' found for token '%2$s'. +SearchTokenizerException.NOT_EXPECTED_TOKEN=Not expected token '%1$s' found. +SearchTokenizerException.NOT_FINISHED_QUERY=Search query end in an invalid state after token '%1$s'. +SearchTokenizerException.INVALID_TOKEN_STATE=Token '%1$s' is in an invalid state. +SearchTokenizerException.ALREADY_FINISHED=Token '%1$s' is already in finished state. UriParserSemanticException.FUNCTION_NOT_FOUND=The function import '%1$s' has no function with parameters '%2$s'. diff --git a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserAndTokenizerTest.java b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserAndTokenizerTest.java index 0aa79ab02..f6722b832 100644 --- a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserAndTokenizerTest.java +++ b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserAndTokenizerTest.java @@ -18,7 +18,6 @@ */ package org.apache.olingo.server.core.uri.parser.search; -import org.apache.olingo.server.api.ODataLibraryException; import org.apache.olingo.server.api.uri.queryoption.SearchOption; import org.apache.olingo.server.api.uri.queryoption.search.SearchExpression; import org.junit.Assert; @@ -71,6 +70,22 @@ public class SearchParserAndTokenizerTest { assertQuery("NOT").resultsIn(SearchParserException.MessageKeys.INVALID_NOT_OPERAND); assertQuery("AND").resultsIn(SearchParserException.MessageKeys.INVALID_BINARY_OPERATOR_POSITION); assertQuery("OR").resultsIn(SearchParserException.MessageKeys.INVALID_BINARY_OPERATOR_POSITION); + + assertQuery("NOT a AND").resultsIn(SearchParserException.MessageKeys.INVALID_END_OF_QUERY); + assertQuery("NOT a OR").resultsIn(SearchParserException.MessageKeys.INVALID_END_OF_QUERY); + assertQuery("a AND").resultsIn(SearchParserException.MessageKeys.INVALID_END_OF_QUERY); + assertQuery("a OR").resultsIn(SearchParserException.MessageKeys.INVALID_END_OF_QUERY); + + assertQuery("a OR b)").resultsIn(SearchParserException.MessageKeys.INVALID_END_OF_QUERY_TOKEN_LEFT); + assertQuery("a NOT b)").resultsIn(SearchParserException.MessageKeys.INVALID_END_OF_QUERY_TOKEN_LEFT); + assertQuery("a AND b)").resultsIn(SearchParserException.MessageKeys.INVALID_END_OF_QUERY_TOKEN_LEFT); + + assertQuery("(a OR b").resultsIn(SearchParserException.MessageKeys.MISSING_CLOSE); + assertQuery("(a NOT b").resultsIn(SearchParserException.MessageKeys.MISSING_CLOSE); + assertQuery("((a AND b)").resultsIn(SearchParserException.MessageKeys.MISSING_CLOSE); + assertQuery("((a AND b OR c)").resultsIn(SearchParserException.MessageKeys.MISSING_CLOSE); + assertQuery("a AND (b OR c").resultsIn(SearchParserException.MessageKeys.MISSING_CLOSE); + assertQuery("(a AND ((b OR c)").resultsIn(SearchParserException.MessageKeys.MISSING_CLOSE); } private static Validator assertQuery(String searchQuery) { @@ -95,13 +110,17 @@ public class SearchParserAndTokenizerTest { return this; } - private void resultsIn(ODataLibraryException.MessageKey key) + private void resultsIn(SearchParserException.MessageKey key) throws SearchTokenizerException { try { resultsIn(searchQuery); - } catch (ODataLibraryException e) { - Assert.assertEquals(SearchParserException.class, e.getClass()); - Assert.assertEquals(key, e.getMessageKey()); + } catch (SearchParserException e) { + Assert.assertEquals("SearchParserException with unexpected message '" + e.getMessage() + + "' was thrown.", key, e.getMessageKey()); + if(log) { + System.out.println("Caught SearchParserException with message key " + + e.getMessageKey() + " and message " + e.getMessage()); + } return; } Assert.fail("SearchParserException with message key " + key.getKey() + " was not thrown."); diff --git a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserTest.java b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserTest.java index 5d3f2dc7d..578f89fa8 100644 --- a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserTest.java +++ b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchParserTest.java @@ -187,6 +187,15 @@ public class SearchParserTest extends SearchParser { runEx(SearchParserException.MessageKeys.INVALID_OPERATOR_AFTER_AND, Token.WORD, Token.AND, Token.AND, Token.WORD); } + @Test + public void invalidQueryEnds() { + runEx(MessageKeys.INVALID_END_OF_QUERY, Token.WORD, Token.AND); + runEx(MessageKeys.INVALID_END_OF_QUERY, Token.WORD, Token.OR); + runEx(MessageKeys.INVALID_END_OF_QUERY, Token.NOT, Token.WORD, Token.OR); + runEx(MessageKeys.INVALID_END_OF_QUERY, Token.NOT, Token.WORD, Token.AND); + runEx(MessageKeys.INVALID_END_OF_QUERY_TOKEN_LEFT, Token.WORD, Token.AND, Token.WORD, Token.CLOSE); + } + @Test public void singleAnd() { runEx(SearchParserException.MessageKeys.INVALID_BINARY_OPERATOR_POSITION, Token.AND); @@ -194,7 +203,7 @@ public class SearchParserTest extends SearchParser { @Test public void singleOpenBracket() { - runEx(SearchParserException.MessageKeys.EXPECTED_DIFFERENT_TOKEN, Token.OPEN); + runEx(SearchParserException.MessageKeys.NO_EXPRESSION_FOUND, Token.OPEN); } @Test diff --git a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerTest.java b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerTest.java index 133bb2e16..abf06b7ee 100644 --- a/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerTest.java +++ b/lib/server-core/src/test/java/org/apache/olingo/server/core/uri/parser/search/SearchTokenizerTest.java @@ -290,11 +290,14 @@ public class SearchTokenizerTest { @Test public void tokenizeInvalid() throws SearchTokenizerException { // - assertQuery("( abc AND) OR something").resultsIn(SearchTokenizerException.class); + assertQuery("( abc AND) OR something").resultsIn(SearchTokenizerException.MessageKeys.FORBIDDEN_CHARACTER); - assertQuery("\"phrase\"word").resultsIn(SearchTokenizerException.class); - assertQuery("\"p\"w").resultsIn(SearchTokenizerException.class); - assertQuery("\"\"").resultsIn(SearchTokenizerException.class); + assertQuery("\"phrase\"word").resultsIn(SearchTokenizerException.MessageKeys.FORBIDDEN_CHARACTER); + assertQuery("\"p\"w").resultsIn(SearchTokenizerException.MessageKeys.FORBIDDEN_CHARACTER); + assertQuery("\"\"").resultsIn(SearchTokenizerException.MessageKeys.INVALID_TOKEN_STATE); + assertQuery("some AND)").resultsIn(SearchTokenizerException.MessageKeys.FORBIDDEN_CHARACTER); + assertQuery("some OR)").resultsIn(SearchTokenizerException.MessageKeys.FORBIDDEN_CHARACTER); + assertQuery("some NOT)").enableLogging().resultsIn(SearchTokenizerException.MessageKeys.FORBIDDEN_CHARACTER); } @Test @@ -302,9 +305,15 @@ public class SearchTokenizerTest { assertQuery("AND").resultsIn(AND); assertQuery("OR").resultsIn(OR); assertQuery("NOT").resultsIn(NOT); + assertQuery("a AND").resultsIn(WORD, AND); + assertQuery("o OR").resultsIn(WORD, OR); + assertQuery("n NOT").resultsIn(WORD, NOT); assertQuery("NOT AND").resultsIn(NOT, AND); + assertQuery("NOT and AND").resultsIn(NOT, WORD, AND); assertQuery("NOT OR").resultsIn(NOT, OR); + assertQuery("NOT a OR").resultsIn(NOT, WORD, OR); assertQuery("NOT NOT").resultsIn(NOT, NOT); + assertQuery("some AND other)").resultsIn(WORD, AND, WORD, CLOSE); assertQuery("abc AND OR something").resultsIn(WORD, AND, OR, WORD); assertQuery("abc AND \"something\" )").resultsIn(WORD, AND, PHRASE, CLOSE); } @@ -359,10 +368,6 @@ public class SearchTokenizerTest { this.searchQuery = searchQuery; } - private static Validator init(String searchQuery) { - return new Validator(searchQuery); - } - @SuppressWarnings("unused") private Validator enableLogging() { log = true; @@ -378,22 +383,26 @@ public class SearchTokenizerTest { } return this; } - private void resultsIn(Class exception) throws SearchTokenizerException { - try { - new SearchTokenizer().tokenize(searchQuery); - } catch (Exception e) { - Assert.assertEquals(exception, e.getClass()); - return; - } - Assert.fail("Expected exception " + exception.getClass().getSimpleName() + " was not thrown."); - } +// private void resultsIn(Class exception) throws SearchTokenizerException { +// try { +// validate(); +// } catch (Exception e) { +// Assert.assertEquals(exception, e.getClass()); +// return; +// } +// Assert.fail("Expected exception " + exception.getClass().getSimpleName() + " was not thrown."); +// } private void resultsIn(SearchTokenizerException.MessageKey key) throws SearchTokenizerException { try { - init(searchQuery).validate(); + validate(); } catch (SearchTokenizerException e) { Assert.assertEquals("SearchTokenizerException with unexpected message was thrown.", key, e.getMessageKey()); + if(log) { + System.out.println("Caught SearchTokenizerException with message key " + + e.getMessageKey() + " and message " + e.getMessage()); + } return; } Assert.fail("No SearchTokenizerException was not thrown.");