From 7d4a6499e1ff491bd60038b7b704edbe8c17c2a3 Mon Sep 17 00:00:00 2001 From: javanna Date: Wed, 10 Aug 2016 12:07:03 +0200 Subject: [PATCH] [TEST] add inline comments to AbstractQueryTestCase#unknownObjectExceptionTest --- .../test/AbstractQueryTestCase.java | 64 ++++++++++++++++--- 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 77924d4d99d..eb852ec611e 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -313,7 +313,7 @@ public abstract class AbstractQueryTestCase> /** * Test that adding an additional object within each object of the otherwise correct query always triggers some kind of - * parse exception. Some specific objects do not cause any exception as they can hold arbitray content; they can be + * parse exception. Some specific objects do not cause any exception as they can hold arbitrary content; they can be * declared by overriding {@link #getObjectsHoldingArbitraryContent()} */ public final void testUnknownObjectException() throws IOException { @@ -324,10 +324,48 @@ public abstract class AbstractQueryTestCase> } } + /** + * Traverses the json tree of the valid query provided as argument and mutates it by adding one object within each object + * encountered. Every mutation is a separate iteration, which will be followed by its corresponding assertions to verify that + * a parse exception is thrown when parsing the modified query. Some specific objects do not cause any exception as they can + * hold arbitrary content; they can be declared by overriding {@link #getObjectsHoldingArbitraryContent()}, and for those we + * will verify that no exception gets thrown instead. + * + * For instance given the following valid term query: + * { + * "term" : { + * "field" : { + * "value" : "foo" + * } + * } + * } + * + * The following two mutations will be generated, and an exception is expected when trying to parse them: + * { + * "term" : { + * "newField" : { + * "field" : { + * "value" : "foo" + * } + * } + * } + * } + * + * { + * "term" : { + * "field" : { + * "newField" : { + * "value" : "foo" + * } + * } + * } + * } + */ private void unknownObjectExceptionTest(String validQuery) throws IOException { //TODO building json by concatenating strings makes the code unmaintainable, we should rewrite this test assertThat(validQuery, containsString("{")); int level = 0; + //track whether we are within quotes as we may have randomly generated strings containing curly brackets boolean withinQuotes = false; boolean expectedException = true; int objectHoldingArbitraryContentLevel = 0; @@ -345,31 +383,36 @@ public abstract class AbstractQueryTestCase> expectedException = true; } } else if (withinQuotes == false && validQuery.charAt(insertionPosition) == '{') { + //keep track of which level we are within the json so that we can properly close the additional object level++; + //if we don't expect an exception, it means that we are within an object that can contain arbitrary content. + //in that case we ignore the whole object including its children, no need to even check where we are. if (expectedException) { - int start = -1; - int end = -1; + int startCurrentObjectName = -1; + int endCurrentObjectName = -1; + //look backwards for the current object name, to find out whether we expect an exception following its mutation for (int i = insertionPosition; i >= 0; i--) { if (validQuery.charAt(i) == '}') { break; } else if (validQuery.charAt(i) == '"') { - if (end == -1) { - end = i; - } else if (start == -1) { - start = i + 1; + if (endCurrentObjectName == -1) { + endCurrentObjectName = i; + } else if (startCurrentObjectName == -1) { + startCurrentObjectName = i + 1; } else { break; } } } - if (start >= 0 && end > 0) { - String objectName = validQuery.substring(start, end); - expectedException = getObjectsHoldingArbitraryContent().contains(objectName) == false; + if (startCurrentObjectName >= 0 && endCurrentObjectName > 0) { + String currentObjectName = validQuery.substring(startCurrentObjectName, endCurrentObjectName); + expectedException = getObjectsHoldingArbitraryContent().contains(currentObjectName) == false; } } if (expectedException == false) { objectHoldingArbitraryContentLevel++; } + //inject the start of the new object String testQuery = validQuery.substring(0, insertionPosition) + "{ \"newField\" : "; String secondPart = validQuery.substring(insertionPosition); int currentLevel = level; @@ -382,6 +425,7 @@ public abstract class AbstractQueryTestCase> } else if (quotes == false && secondPart.charAt(i) == '}') { currentLevel--; if (currentLevel == level) { + //close the additional object in the right place testQuery += secondPart.substring(0, i - 1) + "}" + secondPart.substring(i); break; }