Cleanup query builder for inner hits construction.

* Inner hits can now only be provided and prepared via setter in the nested, has_child and has_parent query.
* Also made `score_mode` a required constructor parameter.
* Moved has_child's min_child/max_children validation from doToQuery(...) to a setter.

Author: martijnvg

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

@@ -272,4 +272,12 @@ public final QueryBuilder<?> rewrite(QueryRewriteContext queryShardContext) thro
     protected QueryBuilder<?> doRewrite(QueryRewriteContext queryShardContext) throws IOException {
         return this;
     }
+
+    // Like Objects.requireNotNull(...) but instead throws a IllegalArgumentException
+    protected static <T> T requireValue(T value, String message) {
+        if (value == null) {
+            throw new IllegalArgumentException(message);
+        }
+        return value;
+    }
 }

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

@@ -45,7 +45,7 @@
 import java.util.Objects;
 
 /**
- * A query builder for <tt>has_child</tt> queries.
+ * A query builder for <tt>has_child</tt> query.
  */
 public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuilder> {
 
@@ -68,10 +68,6 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
      * The default value for ignore_unmapped.
      */
     public static final boolean DEFAULT_IGNORE_UNMAPPED = false;
-    /*
-     * The default score mode that is used to combine score coming from multiple parent documents.
-     */
-    public static final ScoreMode DEFAULT_SCORE_MODE = ScoreMode.None;
 
     private static final ParseField QUERY_FIELD = new ParseField("query", "filter");
     private static final ParseField TYPE_FIELD = new ParseField("type", "child_type");
@@ -82,42 +78,25 @@ public class HasChildQueryBuilder extends AbstractQueryBuilder<HasChildQueryBuil
     private static final ParseField IGNORE_UNMAPPED_FIELD = new ParseField("ignore_unmapped");
 
     private final QueryBuilder<?> query;
-
     private final String type;
-
-    private ScoreMode scoreMode = DEFAULT_SCORE_MODE;
-
+    private final ScoreMode scoreMode;
+    private InnerHitBuilder innerHitBuilder;
     private int minChildren = DEFAULT_MIN_CHILDREN;
-
     private int maxChildren = DEFAULT_MAX_CHILDREN;
-
-    private InnerHitBuilder innerHitBuilder;
-
     private boolean ignoreUnmapped = false;
 
+    public HasChildQueryBuilder(String type, QueryBuilder<?> query, ScoreMode scoreMode) {
+        this(type, query, DEFAULT_MIN_CHILDREN, DEFAULT_MAX_CHILDREN, scoreMode, null);
+    }
 
-    public HasChildQueryBuilder(String type, QueryBuilder<?> query, int maxChildren, int minChildren, ScoreMode scoreMode,
+    private HasChildQueryBuilder(String type, QueryBuilder<?> query, int minChildren, int maxChildren, ScoreMode scoreMode,
                                 InnerHitBuilder innerHitBuilder) {
-        this(type, query);
-        scoreMode(scoreMode);
-        this.maxChildren = maxChildren;
-        this.minChildren = minChildren;
+        this.type = requireValue(type, "[" + NAME + "] requires 'type' field");
+        this.query = requireValue(query, "[" + NAME + "] requires 'query' field");
+        this.scoreMode = requireValue(scoreMode, "[" + NAME + "] requires 'score_mode' field");
         this.innerHitBuilder = innerHitBuilder;
-        if (this.innerHitBuilder != null) {
-            this.innerHitBuilder.setParentChildType(type);
-            this.innerHitBuilder.setQuery(query);
-        }
-    }
-
-    public HasChildQueryBuilder(String type, QueryBuilder<?> query) {
-        if (type == null) {
-            throw new IllegalArgumentException("[" + NAME + "] requires 'type' field");
-        }
-        if (query == null) {
-            throw new IllegalArgumentException("[" + NAME + "] requires 'query' field");
-        }
-        this.type = type;
-        this.query = query;
+        this.minChildren = minChildren;
+        this.maxChildren = maxChildren;
     }
 
     /**
@@ -137,64 +116,47 @@ public HasChildQueryBuilder(StreamInput in) throws IOException {
     @Override
     protected void doWriteTo(StreamOutput out) throws IOException {
         out.writeString(type);
-        out.writeInt(minChildren());
-        out.writeInt(maxChildren());
+        out.writeInt(minChildren);
+        out.writeInt(maxChildren);
         out.writeVInt(scoreMode.ordinal());
         out.writeQuery(query);
         out.writeOptionalWriteable(innerHitBuilder);
         out.writeBoolean(ignoreUnmapped);
     }
 
     /**
-     * Defines how the scores from the matching child documents are mapped into the parent document.
+     * Defines the minimum number of children that are required to match for the parent to be considered a match and
+     * the maximum number of children that are required to match for the parent to be considered a match.
      */
-    public HasChildQueryBuilder scoreMode(ScoreMode scoreMode) {
-        if (scoreMode == null) {
-            throw new IllegalArgumentException("[" + NAME + "]  requires 'score_mode' field");
-        }
-        this.scoreMode = scoreMode;
-        return this;
-    }
-
-    /**
-     * Defines the minimum number of children that are required to match for the parent to be considered a match.
-     */
-    public HasChildQueryBuilder minChildren(int minChildren) {
+    public HasChildQueryBuilder minMaxChildren(int minChildren, int maxChildren) {
         if (minChildren < 0) {
-            throw new IllegalArgumentException("[" + NAME + "]  requires non-negative 'min_children' field");
+            throw new IllegalArgumentException("[" + NAME + "] requires non-negative 'min_children' field");
         }
-        this.minChildren = minChildren;
-        return this;
-    }
-
-    /**
-     * Defines the maximum number of children that are required to match for the parent to be considered a match.
-     */
-    public HasChildQueryBuilder maxChildren(int maxChildren) {
         if (maxChildren < 0) {
-            throw new IllegalArgumentException("[" + NAME + "]  requires non-negative 'max_children' field");
+            throw new IllegalArgumentException("[" + NAME + "] requires non-negative 'max_children' field");
         }
+        if (maxChildren < minChildren) {
+            throw new IllegalArgumentException("[" + NAME + "] 'max_children' is less than 'min_children'");
+        }
+        this.minChildren = minChildren;
         this.maxChildren = maxChildren;
         return this;
     }
 
-    /**
-     * Sets the query name for the filter that can be used when searching for matched_filters per hit.
-     */
-    public HasChildQueryBuilder innerHit(InnerHitBuilder innerHitBuilder) {
-        this.innerHitBuilder = Objects.requireNonNull(innerHitBuilder);
-        this.innerHitBuilder.setParentChildType(type);
-        this.innerHitBuilder.setQuery(query);
-        return this;
-    }
-
     /**
      * Returns inner hit definition in the scope of this query and reusing the defined type and query.
      */
     public InnerHitBuilder innerHit() {
         return innerHitBuilder;
     }
 
+    public HasChildQueryBuilder innerHit(InnerHitBuilder innerHit) {
+        innerHit.setParentChildType(type);
+        innerHit.setQuery(query);
+        this.innerHitBuilder = innerHit;
+        return this;
+    }
+
     /**
      * Returns the children query to execute.
      */
@@ -270,7 +232,7 @@ public static HasChildQueryBuilder fromXContent(QueryParseContext parseContext)
         XContentParser parser = parseContext.parser();
         float boost = AbstractQueryBuilder.DEFAULT_BOOST;
         String childType = null;
-        ScoreMode scoreMode = HasChildQueryBuilder.DEFAULT_SCORE_MODE;
+        ScoreMode scoreMode = ScoreMode.None;
         int minChildren = HasChildQueryBuilder.DEFAULT_MIN_CHILDREN;
         int maxChildren = HasChildQueryBuilder.DEFAULT_MAX_CHILDREN;
         boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED;
@@ -312,7 +274,7 @@ public static HasChildQueryBuilder fromXContent(QueryParseContext parseContext)
                 }
             }
         }
-        HasChildQueryBuilder hasChildQueryBuilder = new HasChildQueryBuilder(childType, iqb, maxChildren, minChildren,
+        HasChildQueryBuilder hasChildQueryBuilder = new HasChildQueryBuilder(childType, iqb, minChildren, maxChildren,
                 scoreMode, innerHitBuilder);
         hasChildQueryBuilder.queryName(queryName);
         hasChildQueryBuilder.boost(boost);
@@ -386,10 +348,6 @@ protected Query doToQuery(QueryShardContext context) throws IOException {
                     "[" + NAME + "] Type [" + type + "] points to a non existent parent type [" + parentType + "]");
         }
 
-        if (maxChildren > 0 && maxChildren < minChildren) {
-            throw new QueryShardException(context, "[" + NAME + "] 'max_children' is less than 'min_children'");
-        }
-
         // wrap the query with type query
         innerQuery = Queries.filtered(innerQuery, childDocMapper.typeFilter());
 
@@ -502,7 +460,7 @@ protected boolean doEquals(HasChildQueryBuilder that) {
                 && Objects.equals(scoreMode, that.scoreMode)
                 && Objects.equals(minChildren, that.minChildren)
                 && Objects.equals(maxChildren, that.maxChildren)
-                && Objects.equals(innerHitBuilder, that.innerHitBuilder) 
+                && Objects.equals(innerHitBuilder, that.innerHitBuilder)
                 && Objects.equals(ignoreUnmapped, that.ignoreUnmapped);
     }
 
@@ -515,12 +473,7 @@ protected int doHashCode() {
     protected QueryBuilder<?> doRewrite(QueryRewriteContext queryRewriteContext) throws IOException {
         QueryBuilder<?> rewrite = query.rewrite(queryRewriteContext);
         if (rewrite != query) {
-            HasChildQueryBuilder hasChildQueryBuilder = new HasChildQueryBuilder(type, rewrite);
-            hasChildQueryBuilder.minChildren(minChildren);
-            hasChildQueryBuilder.maxChildren(maxChildren);
-            hasChildQueryBuilder.scoreMode(scoreMode);
-            hasChildQueryBuilder.innerHit(innerHitBuilder);
-            return hasChildQueryBuilder;
+            return new HasChildQueryBuilder(type, rewrite, minChildren, minChildren, scoreMode, innerHitBuilder);
         }
         return this;
     }

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

@@ -48,8 +48,6 @@ public class HasParentQueryBuilder extends AbstractQueryBuilder<HasParentQueryBu
     public static final String NAME = "has_parent";
     public static final ParseField QUERY_NAME_FIELD = new ParseField(NAME);
 
-    public static final boolean DEFAULT_SCORE = false;
-
     /**
      * The default value for ignore_unmapped.
      */
@@ -64,33 +62,19 @@ public class HasParentQueryBuilder extends AbstractQueryBuilder<HasParentQueryBu
 
     private final QueryBuilder<?> query;
     private final String type;
-    private boolean score = DEFAULT_SCORE;
+    private final boolean score;
     private InnerHitBuilder innerHit;
     private boolean ignoreUnmapped = false;
 
-    /**
-     * @param type  The parent type
-     * @param query The query that will be matched with parent documents
-     */
-    public HasParentQueryBuilder(String type, QueryBuilder<?> query) {
-        if (type == null) {
-            throw new IllegalArgumentException("[" + NAME + "] requires 'parent_type' field");
-        }
-        if (query == null) {
-            throw new IllegalArgumentException("[" + NAME + "] requires 'query' field");
-        }
-        this.type = type;
-        this.query = query;
+    public HasParentQueryBuilder(String type, QueryBuilder<?> query, boolean score) {
+        this(type, query, score, null);
     }
 
-    public HasParentQueryBuilder(String type, QueryBuilder<?> query, boolean score, InnerHitBuilder innerHit) {
-        this(type, query);
+    private HasParentQueryBuilder(String type, QueryBuilder<?> query, boolean score, InnerHitBuilder innerHit) {
+        this.type = requireValue(type, "[" + NAME + "] requires 'type' field");
+        this.query = requireValue(query, "[" + NAME + "] requires 'query' field");
         this.score = score;
         this.innerHit = innerHit;
-        if (this.innerHit != null) {
-            this.innerHit.setParentChildType(type);
-            this.innerHit.setQuery(query);
-        }
     }
 
     /**
@@ -114,24 +98,6 @@ protected void doWriteTo(StreamOutput out) throws IOException {
         out.writeBoolean(ignoreUnmapped);
     }
 
-    /**
-     * Defines if the parent score is mapped into the child documents.
-     */
-    public HasParentQueryBuilder score(boolean score) {
-        this.score = score;
-        return this;
-    }
-
-    /**
-     * Sets inner hit definition in the scope of this query and reusing the defined type and query.
-     */
-    public HasParentQueryBuilder innerHit(InnerHitBuilder innerHit) {
-        this.innerHit = Objects.requireNonNull(innerHit);
-        this.innerHit.setParentChildType(type);
-        this.innerHit.setQuery(query);
-        return this;
-    }
-
     /**
      * Returns the query to execute.
      */
@@ -160,6 +126,13 @@ public InnerHitBuilder innerHit() {
         return innerHit;
     }
 
+    public HasParentQueryBuilder innerHit(InnerHitBuilder innerHit) {
+        innerHit.setParentChildType(type);
+        innerHit.setQuery(query);
+        this.innerHit = innerHit;
+        return this;
+    }
+
     /**
      * Sets whether the query builder should ignore unmapped types (and run a
      * {@link MatchNoDocsQuery} in place of this query) or throw an exception if
@@ -264,7 +237,7 @@ public static HasParentQueryBuilder fromXContent(QueryParseContext parseContext)
         XContentParser parser = parseContext.parser();
         float boost = AbstractQueryBuilder.DEFAULT_BOOST;
         String parentType = null;
-        boolean score = HasParentQueryBuilder.DEFAULT_SCORE;
+        boolean score = false;
         String queryName = null;
         InnerHitBuilder innerHits = null;
         boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED;
@@ -323,7 +296,7 @@ protected boolean doEquals(HasParentQueryBuilder that) {
         return Objects.equals(query, that.query)
                 && Objects.equals(type, that.type)
                 && Objects.equals(score, that.score)
-                && Objects.equals(innerHit, that.innerHit) 
+                && Objects.equals(innerHit, that.innerHit)
                 && Objects.equals(ignoreUnmapped, that.ignoreUnmapped);
     }
 
@@ -336,10 +309,7 @@ protected int doHashCode() {
     protected QueryBuilder<?> doRewrite(QueryRewriteContext queryShardContext) throws IOException {
         QueryBuilder<?> rewrite = query.rewrite(queryShardContext);
         if (rewrite != query) {
-            HasParentQueryBuilder hasParentQueryBuilder = new HasParentQueryBuilder(type, rewrite);
-            hasParentQueryBuilder.score(score);
-            hasParentQueryBuilder.innerHit(innerHit);
-            return hasParentQueryBuilder;
+            return new HasParentQueryBuilder(type, rewrite, score, innerHit);
         }
         return this;
     }