Query refactoring: validate inner queries whenever supported

Added validation for all inner queries that any already refactored query may hold. Added also tests around this. At the end of the refactoring validate will be called by SearchRequest#validate during TransportSearchAction execution, which will call validate against the top level query builder that will need to go and validate its data plus all of its inner queries, and so on.

Closes #11889

core/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java

@@ -76,6 +76,10 @@ public abstract class AbstractQueryBuilder<QB extends QueryBuilder> 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<QB extends QueryBuilder> extends ToXC
             xContentBuilder.endObject();
         }
     }
+
+    protected static QueryValidationException validateInnerQueries(List<QueryBuilder> 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;
+    }
 }

core/src/main/java/org/elasticsearch/index/query/AndQueryBuilder.java

@@ -125,8 +125,7 @@ public class AndQueryBuilder extends AbstractQueryBuilder<AndQueryBuilder> {
 
     @Override
     public QueryValidationException validate() {
-        // nothing to validate
+        return validateInnerQueries(filters, null);
-        return null;
     }
 
     @Override

core/src/main/java/org/elasticsearch/index/query/BaseTermQueryBuilder.java

@@ -178,10 +178,10 @@ public abstract class BaseTermQueryBuilder<QB extends BaseTermQueryBuilder<QB>>
     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;
     }

core/src/main/java/org/elasticsearch/index/query/BoolQueryBuilder.java

@@ -315,8 +315,12 @@ public class BoolQueryBuilder extends AbstractQueryBuilder<BoolQueryBuilder> imp
 
     @Override
     public QueryValidationException validate() {
-        // nothing to validate, clauses are optional, see hasClauses(), other parameters have defaults
+        QueryValidationException validationException = null;
-        return 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<QueryBuilder> clauses, Occur occurs)

core/src/main/java/org/elasticsearch/index/query/BoostingQueryBuilder.java

@@ -132,9 +132,10 @@ public class BoostingQueryBuilder extends AbstractQueryBuilder<BoostingQueryBuil
     public QueryValidationException validate() {
         QueryValidationException validationException = null;
         if (negativeBoost < 0) {
-            validationException = QueryValidationException
+            validationException = addValidationError("query requires negativeBoost to be set to positive value", validationException);
-                    .addValidationError("[boosting] query requires negativeBoost to be set to positive value", validationException);
         }
+        validationException = validateInnerQuery(negativeQuery, validationException);
+        validationException = validateInnerQuery(positiveQuery, validationException);
         return validationException;
     };
 

core/src/main/java/org/elasticsearch/index/query/CommonTermsQueryBuilder.java

@@ -323,10 +323,10 @@ public class CommonTermsQueryBuilder extends AbstractQueryBuilder<CommonTermsQue
     public QueryValidationException validate() {
         QueryValidationException validationException = null;
         if (Strings.isEmpty(this.fieldName)) {
-            validationException = QueryValidationException.addValidationError("field name cannot be null or empty.", validationException);
+            validationException = addValidationError("field name cannot be null or empty.", validationException);
         }
         if (this.text == null) {
-            validationException = QueryValidationException.addValidationError("query text cannot be null", validationException);
+            validationException = addValidationError("query text cannot be null", validationException);
         }
         return validationException;
     }

core/src/main/java/org/elasticsearch/index/query/ConstantScoreQueryBuilder.java

@@ -104,8 +104,7 @@ public class ConstantScoreQueryBuilder extends AbstractQueryBuilder<ConstantScor
 
     @Override
     public QueryValidationException validate() {
-        // nothing to validate
+        return validateInnerQuery(filterBuilder, null);
-        return null;
     }
 
     @Override

core/src/main/java/org/elasticsearch/index/query/DisMaxQueryBuilder.java

@@ -152,8 +152,7 @@ public class DisMaxQueryBuilder extends AbstractQueryBuilder<DisMaxQueryBuilder>
 
     @Override
     public QueryValidationException validate() {
-        // nothing to validate, clauses are optional
+        return validateInnerQueries(queries, null);
-        return null;
     }
 
     @Override

core/src/main/java/org/elasticsearch/index/query/FQueryFilterBuilder.java

@@ -105,8 +105,7 @@ public class FQueryFilterBuilder extends AbstractQueryBuilder<FQueryFilterBuilde
 
     @Override
     public QueryValidationException validate() {
-        // nothing to validate
+        return validateInnerQuery(queryBuilder, null);
-        return null;
     }
 
     @Override

core/src/main/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilder.java

@@ -41,7 +41,12 @@ public class FieldMaskingSpanQueryBuilder extends AbstractQueryBuilder<FieldMask
 
     private String queryName;
 
-    static final FieldMaskingSpanQueryBuilder PROTOTYPE = new FieldMaskingSpanQueryBuilder(null, null);
+    static final FieldMaskingSpanQueryBuilder PROTOTYPE = new FieldMaskingSpanQueryBuilder();
+
+    private FieldMaskingSpanQueryBuilder() {
+        this.queryBuilder = null;
+        this.fieldName = null;
+    }
 
     /**
      * Constructs a new {@link FieldMaskingSpanQueryBuilder} given an inner {@link SpanQueryBuilder} for
@@ -50,7 +55,7 @@ public class FieldMaskingSpanQueryBuilder extends AbstractQueryBuilder<FieldMask
      * @param fieldName the field name
      */
     public FieldMaskingSpanQueryBuilder(SpanQueryBuilder queryBuilder, String fieldName) {
-        this.queryBuilder = queryBuilder;
+        this.queryBuilder = Objects.requireNonNull(queryBuilder);
         this.fieldName = fieldName;
     }
 
@@ -127,12 +132,9 @@ public class FieldMaskingSpanQueryBuilder extends AbstractQueryBuilder<FieldMask
 
     @Override
     public QueryValidationException validate() {
-        QueryValidationException validationExceptions = null;
+        QueryValidationException validationExceptions = validateInnerQuery(queryBuilder, null);
-        if (queryBuilder == null) {
-            validationExceptions = QueryValidationException.addValidationError("[field_masking_span] must have inner span query clause", validationExceptions);
-        }
         if (fieldName == null || fieldName.isEmpty()) {
-            validationExceptions = QueryValidationException.addValidationError("[field_masking_span] must have set field name", validationExceptions);
+            validationExceptions = addValidationError("field name is null or empty", validationExceptions);
         }
         return validationExceptions;
     }

core/src/main/java/org/elasticsearch/index/query/FieldMaskingSpanQueryParser.java

@@ -57,7 +57,7 @@ public class FieldMaskingSpanQueryParser extends BaseQueryParser {
                 if ("query".equals(currentFieldName)) {
                     QueryBuilder query = parseContext.parseInnerQueryBuilder();
                     if (!(query instanceof SpanQueryBuilder)) {
-                        throw new QueryParsingException(parseContext, "[field_masking_span] query] must be of type span query");
+                        throw new QueryParsingException(parseContext, "[field_masking_span] query must be of type span query");
                     }
                     inner = (SpanQueryBuilder) query;
                 } else {

core/src/main/java/org/elasticsearch/index/query/NotQueryBuilder.java

@@ -97,8 +97,7 @@ public class NotQueryBuilder extends AbstractQueryBuilder<NotQueryBuilder> {
 
     @Override
     public QueryValidationException validate() {
-        // nothing to validate.
+        return validateInnerQuery(filter, null);
-        return null;
     }
 
     @Override

core/src/main/java/org/elasticsearch/index/query/OrQueryBuilder.java

@@ -122,8 +122,7 @@ public class OrQueryBuilder extends AbstractQueryBuilder<OrQueryBuilder> {
 
     @Override
     public QueryValidationException validate() {
-        // nothing to validate
+        return validateInnerQueries(filters, null);
-        return null;
     }
 
     @Override

core/src/main/java/org/elasticsearch/index/query/QueryFilterBuilder.java

@@ -78,8 +78,7 @@ public class QueryFilterBuilder extends AbstractQueryBuilder<QueryFilterBuilder>
 
     @Override
     public QueryValidationException validate() {
-        // nothing to validate
+        return validateInnerQuery(queryBuilder, null);
-        return null;
     }
 
     @Override

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<String> 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<String> validationErrors, QueryValidationException validationException) {
+        if (validationException == null) {
+            validationException = new QueryValidationException(validationErrors);
+        } else {
+            validationException.addValidationErrors(validationErrors);
+        }
+        return validationException;
+    }
 }

core/src/main/java/org/elasticsearch/index/query/RangeQueryBuilder.java

@@ -326,13 +326,13 @@ public class RangeQueryBuilder extends MultiTermQueryBuilder<RangeQueryBuilder>
     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<RangeQueryBuilder>
             try {
                 Joda.forPattern(this.format);
             } catch (Exception e) {
-                validationException = QueryValidationException.addValidationError("error parsing format." + e.getMessage(),
+                validationException = addValidationError("error parsing format." + e.getMessage(),
                         validationException);
             }
         }

core/src/main/java/org/elasticsearch/index/query/SimpleQueryStringBuilder.java

@@ -277,11 +277,9 @@ public class SimpleQueryStringBuilder extends AbstractQueryBuilder<SimpleQuerySt
     @Override
     public QueryValidationException validate() {
         QueryValidationException validationException = null;
-
         // Query text is required
         if (queryText == null) {
-            validationException = QueryValidationException.addValidationError("[" + SimpleQueryStringBuilder.NAME + "] query text missing",
+            validationException = addValidationError("query text missing", validationException);
-                    validationException);
         }
 
         return validationException;

core/src/test/java/org/elasticsearch/index/query/AndQueryBuilderTest.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;
@@ -56,7 +56,7 @@ public class AndQueryBuilderTest extends BaseQueryTestCase<AndQueryBuilder> {
         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<AndQueryBuilder> {
         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);
+    }
 }

core/src/test/java/org/elasticsearch/index/query/BaseQueryTestCase.java

@@ -273,4 +273,14 @@ public abstract class BaseQueryTestCase<QB extends QueryBuilder<QB>> 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());
+        }
+    }
 }

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<BoolQueryBuilder> {
 
@@ -63,19 +64,19 @@ public class BoolQueryBuilderTest extends BaseQueryTestCase<BoolQueryBuilder> {
         }
         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<BoolQueryBuilder> {
             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);
+    }
 }

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<BoostingQueryBuilder> {
 
     @Override
     protected BoostingQueryBuilder createTestQueryBuilder() {
         BoostingQueryBuilder query = new BoostingQueryBuilder();
-        query.positive(RandomQueryBuilder.create(random()));
+        query.positive(RandomQueryBuilder.createQuery(random()));
-        query.negative(RandomQueryBuilder.create(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<BoostingQueryBui
 
     @Test
     public void testValidate() {
-        BoostingQueryBuilder boostingQueryBuilder = new BoostingQueryBuilder();
+        BoostingQueryBuilder boostingQuery = new BoostingQueryBuilder();
-        // check for error with negative `negative boost` factor
+        int totalExpectedErrors = 0;
-        boostingQueryBuilder.negativeBoost(-0.5f);
+        if (randomBoolean()) {
-        assertThat(boostingQueryBuilder.validate().validationErrors().size(), is(1));
+            boostingQuery.negative(RandomQueryBuilder.createInvalidQuery(random()));
+            totalExpectedErrors++;
+        } else if(rarely()) {
+            boostingQuery.negative(RandomQueryBuilder.createQuery(random()));
+        }
+        if (randomBoolean()) {
+            boostingQuery.positive(RandomQueryBuilder.createInvalidQuery(random()));
+            totalExpectedErrors++;
+        } else if(rarely()) {
+            boostingQuery.positive(RandomQueryBuilder.createQuery(random()));
+        }
+        if (frequently()) {
+            boostingQuery.negativeBoost(0.5f);
+        } else {
+            boostingQuery.negativeBoost(-0.5f);
+            totalExpectedErrors++;
+        }
+        assertValidate(boostingQuery, totalExpectedErrors);
     }
 }

core/src/test/java/org/elasticsearch/index/query/ConstantScoreQueryBuilderTest.java

@@ -41,7 +41,7 @@ public class ConstantScoreQueryBuilderTest extends BaseQueryTestCase<ConstantSco
      */
     @Override
     protected ConstantScoreQueryBuilder createTestQueryBuilder() {
-        ConstantScoreQueryBuilder query = new ConstantScoreQueryBuilder(RandomQueryBuilder.create(random()));
+        ConstantScoreQueryBuilder query = new ConstantScoreQueryBuilder(RandomQueryBuilder.createQuery(random()));
         if (randomBoolean()) {
             query.boost(2.0f / randomIntBetween(1, 20));
         }
@@ -79,4 +79,18 @@ public class ConstantScoreQueryBuilderTest extends BaseQueryTestCase<ConstantSco
         assertNull(queryBuilder.query());
         assertNull(queryBuilder.toQuery(createContext()));
     }
+
+    @Test
+    public void testValidate() {
+        QueryBuilder innerQuery;
+        int totalExpectedErrors = 0;
+        if (randomBoolean()) {
+            innerQuery = RandomQueryBuilder.createInvalidQuery(random());
+            totalExpectedErrors++;
+        } else {
+            innerQuery = RandomQueryBuilder.createQuery(random());
+        }
+        ConstantScoreQueryBuilder constantScoreQuery = new ConstantScoreQueryBuilder(innerQuery);
+        assertValidate(constantScoreQuery, totalExpectedErrors);
+    }
 }

core/src/test/java/org/elasticsearch/index/query/DisMaxQueryBuilderTest.java

@@ -47,7 +47,7 @@ public class DisMaxQueryBuilderTest extends BaseQueryTestCase<DisMaxQueryBuilder
         DisMaxQueryBuilder dismax = new DisMaxQueryBuilder();
         int clauses = randomIntBetween(1, 5);
         for (int i = 0; i < clauses; i++) {
-            dismax.add(RandomQueryBuilder.create(random()));
+            dismax.add(RandomQueryBuilder.createQuery(random()));
         }
         if (randomBoolean()) {
             dismax.boost(2.0f / randomIntBetween(1, 20));
@@ -92,4 +92,20 @@ public class DisMaxQueryBuilderTest extends BaseQueryTestCase<DisMaxQueryBuilder
         DisMaxQueryBuilder disMaxBuilder = new DisMaxQueryBuilder().add(innerQueryBuilder);
         assertNull(disMaxBuilder.toQuery(context));
     }
+
+    @Test
+    public void testValidate() {
+        DisMaxQueryBuilder disMaxQuery = new DisMaxQueryBuilder();
+        int iters = randomIntBetween(0, 5);
+        int totalExpectedErrors = 0;
+        for (int i = 0; i < iters; i++) {
+            if (randomBoolean()) {
+                disMaxQuery.add(RandomQueryBuilder.createInvalidQuery(random()));
+                totalExpectedErrors++;
+            } else {
+                disMaxQuery.add(RandomQueryBuilder.createQuery(random()));
+            }
+        }
+        assertValidate(disMaxQuery, totalExpectedErrors);
+    }
 }

core/src/test/java/org/elasticsearch/index/query/FQueryFilterBuilderTest.java

@@ -42,7 +42,7 @@ public class FQueryFilterBuilderTest extends BaseQueryTestCase<FQueryFilterBuild
      */
     @Override
     protected FQueryFilterBuilder createTestQueryBuilder() {
-        QueryBuilder innerQuery = RandomQueryBuilder.create(random());
+        QueryBuilder innerQuery = RandomQueryBuilder.createQuery(random());
         FQueryFilterBuilder testQuery = new FQueryFilterBuilder(innerQuery);
         testQuery.queryName(randomAsciiOfLengthBetween(1, 10));
         return testQuery;
@@ -81,4 +81,18 @@ public class FQueryFilterBuilderTest extends BaseQueryTestCase<FQueryFilterBuild
         FQueryFilterBuilder queryFilterQuery = new FQueryFilterBuilder(innerQuery);
         assertNull(queryFilterQuery.toQuery(createContext()));
     }
+
+    @Test
+    public void testValidate() {
+        QueryBuilder innerQuery;
+        int totalExpectedErrors = 0;
+        if (randomBoolean()) {
+            innerQuery = RandomQueryBuilder.createInvalidQuery(random());
+            totalExpectedErrors++;
+        } else {
+            innerQuery = RandomQueryBuilder.createQuery(random());
+        }
+        FQueryFilterBuilder fQueryFilter = new FQueryFilterBuilder(innerQuery);
+        assertValidate(fQueryFilter, totalExpectedErrors);
+    }
 }

core/src/test/java/org/elasticsearch/index/query/FieldMaskingSpanQueryBuilderTest.java

@@ -27,9 +27,7 @@ import org.junit.Test;
 
 import java.io.IOException;
 
-import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.*;
-
-import static org.hamcrest.Matchers.equalTo;
 
 public class FieldMaskingSpanQueryBuilderTest extends BaseQueryTestCase<FieldMaskingSpanQueryBuilder> {
 
@@ -60,7 +58,7 @@ public class FieldMaskingSpanQueryBuilderTest extends BaseQueryTestCase<FieldMas
 
     @Override
     protected FieldMaskingSpanQueryBuilder createTestQueryBuilder() {
-        String fieldName = null;
+        String fieldName;
         if (randomBoolean()) {
             fieldName = randomFrom(mappedFieldNames);
         } else {
@@ -79,16 +77,22 @@ public class FieldMaskingSpanQueryBuilderTest extends BaseQueryTestCase<FieldMas
 
     @Test
     public void testValidate() {
-        FieldMaskingSpanQueryBuilder queryBuilder = new FieldMaskingSpanQueryBuilder(new SpanTermQueryBuilder("name", "value"), "fieldName");
+        String fieldName;
-        assertNull(queryBuilder.validate());
+        SpanQueryBuilder spanQueryBuilder;
-
+        int totalExpectedErrors = 0;
-        queryBuilder = new FieldMaskingSpanQueryBuilder(null, "fieldName");
+        if (randomBoolean()) {
-        assertThat(queryBuilder.validate().validationErrors().size(), is(1));
+            fieldName = "fieldName";
-
+        } else {
-        queryBuilder = new FieldMaskingSpanQueryBuilder(null, "");
+            fieldName = randomBoolean() ? "" : null;
-        assertThat(queryBuilder.validate().validationErrors().size(), is(2));
+            totalExpectedErrors++;
-
+        }
-        queryBuilder = new FieldMaskingSpanQueryBuilder(null, null);
+        if (randomBoolean()) {
-        assertThat(queryBuilder.validate().validationErrors().size(), is(2));
+            spanQueryBuilder = new SpanTermQueryBuilder("", "test");
+            totalExpectedErrors++;
+        } else {
+            spanQueryBuilder = new SpanTermQueryBuilder("name", "value");
+        }
+        FieldMaskingSpanQueryBuilder queryBuilder = new FieldMaskingSpanQueryBuilder(spanQueryBuilder, fieldName);
+        assertValidate(queryBuilder, totalExpectedErrors);
     }
 }

core/src/test/java/org/elasticsearch/index/query/NotQueryBuilderTest.java

@@ -44,7 +44,7 @@ public class NotQueryBuilderTest extends BaseQueryTestCase<NotQueryBuilder> {
      */
     @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<NotQueryBuilder> {
         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);
+    }
 }

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> {
         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<OrQueryBuilder> {
         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);
+    }
 }

core/src/test/java/org/elasticsearch/index/query/QueryFilterBuilderTest.java

@@ -40,7 +40,7 @@ public class QueryFilterBuilderTest extends BaseQueryTestCase<QueryFilterBuilder
      */
     @Override
     protected QueryFilterBuilder createTestQueryBuilder() {
-        QueryBuilder innerQuery = RandomQueryBuilder.create(random());
+        QueryBuilder innerQuery = RandomQueryBuilder.createQuery(random());
         QueryFilterBuilder testQuery = new QueryFilterBuilder(innerQuery);
         return testQuery;
     }
@@ -72,4 +72,18 @@ public class QueryFilterBuilderTest extends BaseQueryTestCase<QueryFilterBuilder
         QueryFilterBuilder queryFilterQuery = new QueryFilterBuilder(innerQuery);
         assertNull(queryFilterQuery.toQuery(createContext()));
     }
+
+    @Test
+    public void testValidate() {
+        QueryBuilder innerQuery;
+        int totalExpectedErrors = 0;
+        if (randomBoolean()) {
+            innerQuery = RandomQueryBuilder.createInvalidQuery(random());
+            totalExpectedErrors++;
+        } else {
+            innerQuery = RandomQueryBuilder.createQuery(random());
+        }
+        QueryFilterBuilder fQueryFilter = new QueryFilterBuilder(innerQuery);
+        assertValidate(fQueryFilter, totalExpectedErrors);
+    }
 }

core/src/test/java/org/elasticsearch/index/query/RandomQueryBuilder.java

@@ -31,19 +31,41 @@ import java.util.Random;
 public class RandomQueryBuilder {
 
     /**
+     * Create a new query of a random type
      * @param r random seed
      * @return a random {@link QueryBuilder}
      */
-    public static QueryBuilder create(Random r) {
+    public static QueryBuilder createQuery(Random r) {
-        QueryBuilder query = null;
         switch (RandomInts.randomIntBetween(r, 0, 2)) {
-        case 0:
+            case 0:
-            return new MatchAllQueryBuilderTest().createTestQueryBuilder();
+                return new MatchAllQueryBuilderTest().createTestQueryBuilder();
-        case 1:
+            case 1:
-            return new TermQueryBuilderTest().createTestQueryBuilder();
+                return new TermQueryBuilderTest().createTestQueryBuilder();
-        case 2:
+            case 2:
-            return new IdsQueryBuilderTest().createTestQueryBuilder();
+                return new IdsQueryBuilderTest().createTestQueryBuilder();
+            default:
+                throw new UnsupportedOperationException();
+        }
+    }
+
+    /**
+     * Create a new invalid query of a random type
+     * @param r random seed
+     * @return a random {@link QueryBuilder} that is invalid, meaning that calling validate against it
+     * will return an error. We can rely on the fact that a single error will be returned per query.
+     */
+    public static QueryBuilder createInvalidQuery(Random r) {
+        switch (RandomInts.randomIntBetween(r, 0, 3)) {
+            case 0:
+                return new TermQueryBuilder("", "test");
+            case 1:
+                return new BoostingQueryBuilder().negativeBoost(-1f);
+            case 2:
+                return new CommonTermsQueryBuilder("", "text");
+            case 3:
+                return new SimpleQueryStringBuilder(null);
+            default:
+                throw new UnsupportedOperationException();
         }
-        return query;
     }
 }