diff --git a/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java index 124396a0cc5..7cd3c4e3b33 100644 --- a/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java @@ -76,6 +76,10 @@ public abstract class AbstractQueryBuilder extends ToXC public void writeTo(StreamOutput out) throws IOException { } + protected final QueryValidationException addValidationError(String validationError, QueryValidationException validationException) { + return QueryValidationException.addValidationError(getName(), validationError, validationException); + } + @Override public final boolean equals(Object obj) { if (this == obj) { @@ -159,4 +163,23 @@ public abstract class AbstractQueryBuilder extends ToXC xContentBuilder.endObject(); } } + + protected static QueryValidationException validateInnerQueries(List queryBuilders, QueryValidationException initialValidationException) { + QueryValidationException validationException = initialValidationException; + for (QueryBuilder queryBuilder : queryBuilders) { + validationException = validateInnerQuery(queryBuilder, validationException); + } + return validationException; + } + + protected static QueryValidationException validateInnerQuery(QueryBuilder queryBuilder, QueryValidationException initialValidationException) { + QueryValidationException validationException = initialValidationException; + if (queryBuilder != null) { + QueryValidationException queryValidationException = queryBuilder.validate(); + if (queryValidationException != null) { + validationException = QueryValidationException.addValidationErrors(queryValidationException.validationErrors(), validationException); + } + } + return validationException; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java index b0c8a572556..b37bd93de87 100644 --- a/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java @@ -125,8 +125,7 @@ public class AndQueryBuilder extends AbstractQueryBuilder { @Override public QueryValidationException validate() { - // nothing to validate - return null; + return validateInnerQueries(filters, null); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/BaseTermQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BaseTermQueryBuilder.java index 8b81d17b68f..3d5db49bb61 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BaseTermQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BaseTermQueryBuilder.java @@ -178,10 +178,10 @@ public abstract class BaseTermQueryBuilder> public QueryValidationException validate() { QueryValidationException validationException = null; if (fieldName == null || fieldName.isEmpty()) { - validationException = QueryValidationException.addValidationError("field name cannot be null or empty.", validationException); + validationException = addValidationError("field name cannot be null or empty.", validationException); } if (value == null) { - validationException = QueryValidationException.addValidationError("value cannot be null.", validationException); + validationException = addValidationError("value cannot be null.", validationException); } return validationException; } diff --git a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java index 2b482a7c7b3..397e5a473d2 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java @@ -315,8 +315,12 @@ public class BoolQueryBuilder extends AbstractQueryBuilder imp @Override public QueryValidationException validate() { - // nothing to validate, clauses are optional, see hasClauses(), other parameters have defaults - return null; + QueryValidationException validationException = null; + validationException = validateInnerQueries(mustClauses, validationException); + validationException = validateInnerQueries(shouldClauses, validationException); + validationException = validateInnerQueries(mustNotClauses, validationException); + validationException = validateInnerQueries(filterClauses, validationException); + return validationException; } private static void addBooleanClauses(QueryParseContext parseContext, BooleanQuery booleanQuery, List clauses, Occur occurs) diff --git a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java index 3e6aeea0901..70886f7225b 100644 --- a/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java @@ -132,9 +132,10 @@ public class BoostingQueryBuilder extends AbstractQueryBuilder @Override public QueryValidationException validate() { - // nothing to validate, clauses are optional - return null; + return validateInnerQueries(queries, null); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java b/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java index 9ecbc21dcbb..a5bd11f8dc6 100644 --- a/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java @@ -105,8 +105,7 @@ public class FQueryFilterBuilder extends AbstractQueryBuilder { @Override public QueryValidationException validate() { - // nothing to validate. - return null; + return validateInnerQuery(filter, null); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java index d6f5d2fbdcd..5cbf71c09cb 100644 --- a/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java @@ -122,8 +122,7 @@ public class OrQueryBuilder extends AbstractQueryBuilder { @Override public QueryValidationException validate() { - // nothing to validate - return null; + return validateInnerQueries(filters, null); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java b/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java index b308cdfc6e9..ee1837a55fc 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java @@ -78,8 +78,7 @@ public class QueryFilterBuilder extends AbstractQueryBuilder @Override public QueryValidationException validate() { - // nothing to validate - return null; + return validateInnerQuery(queryBuilder, null); } @Override diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryValidationException.java b/core/src/main/java/org/elasticsearch/index/query/QueryValidationException.java index c6aa7f662a5..3761a8247fd 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryValidationException.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryValidationException.java @@ -34,6 +34,11 @@ public class QueryValidationException extends IllegalArgumentException { validationErrors.add(error); } + public QueryValidationException(List errors) { + super("query validation failed"); + validationErrors.addAll(errors); + } + public void addValidationError(String error) { validationErrors.add(error); } @@ -62,16 +67,34 @@ public class QueryValidationException extends IllegalArgumentException { /** * Helper method than can be used to add error messages to an existing {@link QueryValidationException}. * When passing {@code null} as the initial exception, a new exception is created. + * + * @param queryId * @param validationError the error message to add to an initial exception * @param validationException an initial exception. Can be {@code null}, in which case a new exception is created. * @return a {@link QueryValidationException} with added validation error message */ - public static QueryValidationException addValidationError(String validationError, QueryValidationException validationException) { + public static QueryValidationException addValidationError(String queryId, String validationError, QueryValidationException validationException) { if (validationException == null) { - validationException = new QueryValidationException(validationError); + validationException = new QueryValidationException("[" + queryId + "] " + validationError); } else { validationException.addValidationError(validationError); } return validationException; } + + /** + * Helper method than can be used to add error messages to an existing {@link QueryValidationException}. + * When passing {@code null} as the initial exception, a new exception is created. + * @param validationErrors the error messages to add to an initial exception + * @param validationException an initial exception. Can be {@code null}, in which case a new exception is created. + * @return a {@link QueryValidationException} with added validation error message + */ + public static QueryValidationException addValidationErrors(List validationErrors, QueryValidationException validationException) { + if (validationException == null) { + validationException = new QueryValidationException(validationErrors); + } else { + validationException.addValidationErrors(validationErrors); + } + return validationException; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java index f8d0e306271..c3b5c859dd5 100644 --- a/core/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java @@ -326,13 +326,13 @@ public class RangeQueryBuilder extends MultiTermQueryBuilder public QueryValidationException validate() { QueryValidationException validationException = null; if (this.fieldName == null || this.fieldName.isEmpty()) { - validationException = QueryValidationException.addValidationError("field name cannot be null or empty.", validationException); + validationException = addValidationError("field name cannot be null or empty.", validationException); } if (this.timeZone != null) { try { DateTimeZone.forID(this.timeZone); } catch (Exception e) { - validationException = QueryValidationException.addValidationError("error parsing timezone." + e.getMessage(), + validationException = addValidationError("error parsing timezone." + e.getMessage(), validationException); } } @@ -340,7 +340,7 @@ public class RangeQueryBuilder extends MultiTermQueryBuilder try { Joda.forPattern(this.format); } catch (Exception e) { - validationException = QueryValidationException.addValidationError("error parsing format." + e.getMessage(), + validationException = addValidationError("error parsing format." + e.getMessage(), validationException); } } diff --git a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java index ce883a396e1..f88aecd6d5b 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java @@ -277,11 +277,9 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder { AndQueryBuilder query = new AndQueryBuilder(); int subQueries = randomIntBetween(1, 5); for (int i = 0; i < subQueries; i++ ) { - query.add(RandomQueryBuilder.create(random())); + query.add(RandomQueryBuilder.createQuery(random())); } if (randomBoolean()) { query.queryName(randomAsciiOfLengthBetween(1, 10)); @@ -90,4 +90,20 @@ public class AndQueryBuilderTest extends BaseQueryTestCase { assertQueryHeader(parser, AndQueryBuilder.PROTOTYPE.getName()); context.indexQueryParserService().queryParser(AndQueryBuilder.PROTOTYPE.getName()).fromXContent(context); } + + @Test + public void testValidate() { + AndQueryBuilder andQuery = new AndQueryBuilder(); + int iters = randomIntBetween(0, 5); + int totalExpectedErrors = 0; + for (int i = 0; i < iters; i++) { + if (randomBoolean()) { + andQuery.add(RandomQueryBuilder.createInvalidQuery(random())); + totalExpectedErrors++; + } else { + andQuery.add(RandomQueryBuilder.createQuery(random())); + } + } + assertValidate(andQuery, totalExpectedErrors); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java b/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java index f10ba2799fe..f6d7f6a9df7 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java +++ b/core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java @@ -273,4 +273,14 @@ public abstract class BaseQueryTestCase> extends Ela assertThat(parser.currentName(), is(expectedParserName)); assertThat(parser.nextToken(), is(XContentParser.Token.START_OBJECT)); } + + protected static void assertValidate(QueryBuilder queryBuilder, int totalExpectedErrors) { + QueryValidationException queryValidationException = queryBuilder.validate(); + if (totalExpectedErrors > 0) { + assertThat(queryValidationException, notNullValue()); + assertThat(queryValidationException.validationErrors().size(), equalTo(totalExpectedErrors)); + } else { + assertThat(queryValidationException, nullValue()); + } + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java index b55c822185f..51f71bbee97 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTest.java @@ -20,17 +20,18 @@ package org.elasticsearch.index.query; import org.apache.lucene.search.BooleanClause; +import org.apache.lucene.search.BooleanClause.Occur; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; -import org.apache.lucene.search.BooleanClause.Occur; import org.elasticsearch.common.lucene.search.Queries; +import org.junit.Test; import java.io.IOException; import java.util.List; -import static org.hamcrest.Matchers.equalTo; import static org.elasticsearch.common.lucene.search.Queries.fixNegativeQueryIfNeeded; +import static org.hamcrest.Matchers.equalTo; public class BoolQueryBuilderTest extends BaseQueryTestCase { @@ -63,19 +64,19 @@ public class BoolQueryBuilderTest extends BaseQueryTestCase { } int mustClauses = randomIntBetween(0, 3); for (int i = 0; i < mustClauses; i++) { - query.must(RandomQueryBuilder.create(random())); + query.must(RandomQueryBuilder.createQuery(random())); } int mustNotClauses = randomIntBetween(0, 3); for (int i = 0; i < mustNotClauses; i++) { - query.mustNot(RandomQueryBuilder.create(random())); + query.mustNot(RandomQueryBuilder.createQuery(random())); } int shouldClauses = randomIntBetween(0, 3); for (int i = 0; i < shouldClauses; i++) { - query.should(RandomQueryBuilder.create(random())); + query.should(RandomQueryBuilder.createQuery(random())); } int filterClauses = randomIntBetween(0, 3); for (int i = 0; i < filterClauses; i++) { - query.filter(RandomQueryBuilder.create(random())); + query.filter(RandomQueryBuilder.createQuery(random())); } if (randomBoolean()) { query.queryName(randomUnicodeOfLengthBetween(3, 15)); @@ -107,4 +108,45 @@ public class BoolQueryBuilderTest extends BaseQueryTestCase { booleanQuery.add(new BooleanClause(query.toQuery(parseContext), occurs)); } } + + @Test + public void testValidate() { + BoolQueryBuilder booleanQuery = new BoolQueryBuilder(); + int iters = randomIntBetween(0, 3); + int totalExpectedErrors = 0; + for (int i = 0; i < iters; i++) { + if (randomBoolean()) { + booleanQuery.must(RandomQueryBuilder.createInvalidQuery(random())); + totalExpectedErrors++; + } else { + booleanQuery.must(RandomQueryBuilder.createQuery(random())); + } + } + iters = randomIntBetween(0, 3); + for (int i = 0; i < iters; i++) { + if (randomBoolean()) { + booleanQuery.should(RandomQueryBuilder.createInvalidQuery(random())); + totalExpectedErrors++; + } else { + booleanQuery.should(RandomQueryBuilder.createQuery(random())); + } + } + for (int i = 0; i < iters; i++) { + if (randomBoolean()) { + booleanQuery.mustNot(RandomQueryBuilder.createInvalidQuery(random())); + totalExpectedErrors++; + } else { + booleanQuery.mustNot(RandomQueryBuilder.createQuery(random())); + } + } + for (int i = 0; i < iters; i++) { + if (randomBoolean()) { + booleanQuery.filter(RandomQueryBuilder.createInvalidQuery(random())); + totalExpectedErrors++; + } else { + booleanQuery.filter(RandomQueryBuilder.createQuery(random())); + } + } + assertValidate(booleanQuery, totalExpectedErrors); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java index 95e40551f99..017cfde8505 100644 --- a/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/BoostingQueryBuilderTest.java @@ -28,15 +28,13 @@ import org.junit.Test; import java.io.IOException; -import static org.hamcrest.Matchers.is; - public class BoostingQueryBuilderTest extends BaseQueryTestCase { @Override protected BoostingQueryBuilder createTestQueryBuilder() { BoostingQueryBuilder query = new BoostingQueryBuilder(); - query.positive(RandomQueryBuilder.create(random())); - query.negative(RandomQueryBuilder.create(random())); + query.positive(RandomQueryBuilder.createQuery(random())); + query.negative(RandomQueryBuilder.createQuery(random())); query.negativeBoost(2.0f / randomIntBetween(1, 20)); if (randomBoolean()) { query.boost(2.0f / randomIntBetween(1, 20)); @@ -121,9 +119,26 @@ public class BoostingQueryBuilderTest extends BaseQueryTestCase { @@ -60,7 +58,7 @@ public class FieldMaskingSpanQueryBuilderTest extends BaseQueryTestCase { */ @Override protected NotQueryBuilder createTestQueryBuilder() { - NotQueryBuilder query = new NotQueryBuilder(RandomQueryBuilder.create(random())); + NotQueryBuilder query = new NotQueryBuilder(RandomQueryBuilder.createQuery(random())); if (randomBoolean()) { query.queryName(randomAsciiOfLengthBetween(1, 10)); } @@ -92,4 +92,18 @@ public class NotQueryBuilderTest extends BaseQueryTestCase { assertQueryHeader(parser, NotQueryBuilder.PROTOTYPE.getName()); context.indexQueryParserService().queryParser(NotQueryBuilder.PROTOTYPE.getName()).fromXContent(context); } + + @Test + public void testValidate() { + QueryBuilder innerQuery; + int totalExpectedErrors = 0; + if (randomBoolean()) { + innerQuery = RandomQueryBuilder.createInvalidQuery(random()); + totalExpectedErrors++; + } else { + innerQuery = RandomQueryBuilder.createQuery(random()); + } + NotQueryBuilder notQuery = new NotQueryBuilder(innerQuery); + assertValidate(notQuery, totalExpectedErrors); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java index db9b7ba1b27..78feca5b37b 100644 --- a/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/OrQueryBuilderTest.java @@ -19,9 +19,9 @@ package org.elasticsearch.index.query; +import org.apache.lucene.search.BooleanClause.Occur; import org.apache.lucene.search.BooleanQuery; import org.apache.lucene.search.Query; -import org.apache.lucene.search.BooleanClause.Occur; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; import org.junit.Test; @@ -57,7 +57,7 @@ public class OrQueryBuilderTest extends BaseQueryTestCase { OrQueryBuilder query = new OrQueryBuilder(); int subQueries = randomIntBetween(1, 5); for (int i = 0; i < subQueries; i++ ) { - query.add(RandomQueryBuilder.create(random())); + query.add(RandomQueryBuilder.createQuery(random())); } if (randomBoolean()) { query.queryName(randomAsciiOfLengthBetween(1, 10)); @@ -91,4 +91,20 @@ public class OrQueryBuilderTest extends BaseQueryTestCase { assertQueryHeader(parser, OrQueryBuilder.PROTOTYPE.getName()); context.indexQueryParserService().queryParser(OrQueryBuilder.PROTOTYPE.getName()).fromXContent(context); } + + @Test + public void testValidate() { + OrQueryBuilder orQuery = new OrQueryBuilder(); + int iters = randomIntBetween(0, 5); + int totalExpectedErrors = 0; + for (int i = 0; i < iters; i++) { + if (randomBoolean()) { + orQuery.add(RandomQueryBuilder.createInvalidQuery(random())); + totalExpectedErrors++; + } else { + orQuery.add(RandomQueryBuilder.createQuery(random())); + } + } + assertValidate(orQuery, totalExpectedErrors); + } } diff --git a/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java b/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java index 93e1ca0b6df..85b3912da2e 100644 --- a/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java +++ b/core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java @@ -40,7 +40,7 @@ public class QueryFilterBuilderTest extends BaseQueryTestCase