Refactors TemplateQueryBuilder and Parser

Relates to #10217

This PR is against the query-refactoring branch.

Closes #13253

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

@@ -31,6 +31,7 @@ import org.apache.lucene.search.similarities.Similarity;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.ParseFieldMatcher;
+import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.Index;
@@ -41,7 +42,10 @@ import org.elasticsearch.index.mapper.core.StringFieldMapper;
 import org.elasticsearch.index.mapper.object.ObjectMapper;
 import org.elasticsearch.index.query.support.NestedScope;
 import org.elasticsearch.indices.cache.query.terms.TermsLookup;
+import org.elasticsearch.script.ExecutableScript;
+import org.elasticsearch.script.ScriptContext;
 import org.elasticsearch.script.ScriptService;
+import org.elasticsearch.script.Template;
 import org.elasticsearch.search.fetch.innerhits.InnerHitsContext;
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.search.lookup.SearchLookup;
@@ -323,4 +327,12 @@ public class QueryShardContext {
     public List<Object> handleTermsLookup(TermsLookup termsLookup) {
         return this.indexQueryParser.handleTermsLookup(termsLookup);
     }
+
+    /*
+    * Executes the given template, and returns the response.
+    */
+    public BytesReference executeQueryTemplate(Template template, SearchContext searchContext) {
+        ExecutableScript executable = scriptService().executable(template, ScriptContext.Standard.SEARCH, searchContext);
+        return (BytesReference) executable.run();
+    }
 }

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

@@ -18,12 +18,20 @@
  */
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.search.Query;
+import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.script.Template;
+import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
 import java.util.Map;
+import java.util.Objects;
 
 /**
  * Facilitates creating template query requests.
@@ -34,15 +42,9 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder<TemplateQueryBuil
     public static final String NAME = "template";
 
     /** Template to fill. */
-    private Template template;
+    private final Template template;
-    /** Parameters to fill the template with. */
-    private Map<String, Object> vars;
-    /** Template to fill.*/
-    private String templateString;
 
-    private ScriptService.ScriptType templateType;
+    static final TemplateQueryBuilder PROTOTYPE = new TemplateQueryBuilder(null);
-
-    static final TemplateQueryBuilder PROTOTYPE = new TemplateQueryBuilder(null, null);
 
     /**
      * @param template
@@ -52,6 +54,10 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder<TemplateQueryBuil
         this.template = template;
     }
 
+    public Template template() {
+        return template;
+    }
+
     /**
      * @param template
      *            the template to use for that query.
@@ -61,7 +67,7 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder<TemplateQueryBuil
      * */
     @Deprecated
     public TemplateQueryBuilder(String template, Map<String, Object> vars) {
-        this(template, ScriptService.ScriptType.INLINE, vars);
+        this(new Template(template, ScriptService.ScriptType.INLINE, null, null, vars));
     }
 
     /**
@@ -75,23 +81,64 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder<TemplateQueryBuil
      * */
     @Deprecated
     public TemplateQueryBuilder(String template, ScriptService.ScriptType templateType, Map<String, Object> vars) {
-        this.templateString = template;
+        this(new Template(template, templateType, null, null, vars));
-        this.vars = vars;
-        this.templateType = templateType;
     }
 
     @Override
     protected void doXContent(XContentBuilder builder, Params builderParams) throws IOException {
         builder.field(TemplateQueryBuilder.NAME);
-        if (template == null) {
+        template.toXContent(builder, builderParams);
-            new Template(templateString, templateType, null, null, this.vars).toXContent(builder, builderParams);
-        } else {
-            template.toXContent(builder, builderParams);
-        }
     }
 
     @Override
     public String getWriteableName() {
         return NAME;
     }
+
+    @Override
+    protected Query doToQuery(QueryShardContext context) throws IOException {
+        BytesReference querySource = context.executeQueryTemplate(template, SearchContext.current());
+        try (XContentParser qSourceParser = XContentFactory.xContent(querySource).createParser(querySource)) {
+            final QueryShardContext contextCopy = new QueryShardContext(context.index(), context.indexQueryParserService());
+            contextCopy.reset(qSourceParser);
+            QueryBuilder result = contextCopy.parseContext().parseInnerQueryBuilder();
+            context.combineNamedQueries(contextCopy);
+            return result.toQuery(context);
+        }
+    }
+
+    @Override
+    protected void setFinalBoost(Query query) {
+        //no-op this query doesn't support boost
+    }
+
+    @Override
+    public QueryValidationException validate() {
+        QueryValidationException validationException = null;
+        if (this.template == null) {
+            validationException = addValidationError("query template cannot be null", validationException);
+        }
+        return validationException;
+    }
+
+    @Override
+    protected TemplateQueryBuilder doReadFrom(StreamInput in) throws IOException {
+        TemplateQueryBuilder templateQueryBuilder = new TemplateQueryBuilder(Template.readTemplate(in));
+        return templateQueryBuilder;
+    }
+
+    @Override
+    protected void doWriteTo(StreamOutput out) throws IOException {
+        template.writeTo(out);
+    }
+
+    @Override
+    protected int doHashCode() {
+        return Objects.hash(template);
+    }
+
+    @Override
+    protected boolean doEquals(TemplateQueryBuilder other) {
+        return Objects.equals(template, other.template);
+    }
 }

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

@@ -18,18 +18,11 @@
  */
 package org.elasticsearch.index.query;
 
-import org.apache.lucene.search.Query;
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.ParseFieldMatcher;
-import org.elasticsearch.common.bytes.BytesReference;
-import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
-import org.elasticsearch.script.ExecutableScript;
-import org.elasticsearch.script.ScriptContext;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.script.Template;
-import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
 import java.util.HashMap;
@@ -39,12 +32,7 @@ import java.util.Map;
  * In the simplest case, parse template string and variables from the request,
  * compile the template and execute the template against the given variables.
  * */
-public class TemplateQueryParser extends BaseQueryParserTemp {
+public class TemplateQueryParser extends BaseQueryParser<TemplateQueryBuilder> {
-
-    /** Name of query parameter containing the template string. */
-    public static final String QUERY = "query";
-
-    private final ScriptService scriptService;
 
     private final static Map<String, ScriptService.ScriptType> parametersToTypes = new HashMap<>();
     static {
@@ -53,11 +41,6 @@ public class TemplateQueryParser extends BaseQueryParserTemp {
         parametersToTypes.put("id", ScriptService.ScriptType.INDEXED);
     }
 
-    @Inject
-    public TemplateQueryParser(ScriptService scriptService) {
-        this.scriptService = scriptService;
-    }
-
     @Override
     public String[] names() {
         return new String[] {TemplateQueryBuilder.NAME};
@@ -68,28 +51,17 @@ public class TemplateQueryParser extends BaseQueryParserTemp {
      * values. Handles both submitting the template as part of the request as
      * well as referencing only the template name.
      *
-     * @param context
+     * @param parseContext parse context containing the templated query.
-     *            parse context containing the templated query.
      */
     @Override
     @Nullable
-    public Query parse(QueryShardContext context) throws IOException {
+    public TemplateQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException {
-        QueryParseContext parseContext = context.parseContext();
         XContentParser parser = parseContext.parser();
         Template template = parse(parser, parseContext.parseFieldMatcher());
-        ExecutableScript executable = this.scriptService.executable(template, ScriptContext.Standard.SEARCH, SearchContext.current());
+        return new TemplateQueryBuilder(template);
-
-        BytesReference querySource = (BytesReference) executable.run();
-
-        try (XContentParser qSourceParser = XContentFactory.xContent(querySource).createParser(querySource)) {
-            final QueryShardContext contextCopy = new QueryShardContext(context.index(), context.indexQueryParserService());
-            contextCopy.reset(qSourceParser);
-            return contextCopy.parseContext().parseInnerQuery();
-        }
     }
 
     public static Template parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher, String... parameters) throws IOException {
-
         Map<String, ScriptService.ScriptType> parameterMap = new HashMap<>(parametersToTypes);
         for (String parameter : parameters) {
             parameterMap.put(parameter, ScriptService.ScriptType.INLINE);

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

@@ -16,23 +16,60 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.search.Query;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.script.ScriptService.ScriptType;
 import org.elasticsearch.script.Template;
-import org.elasticsearch.test.ESTestCase;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 
-/**
+import static org.hamcrest.Matchers.is;
- * Test building and serialising a template search request.
+
- * */
+public class TemplateQueryBuilderTests extends BaseQueryTestCase<TemplateQueryBuilder> {
-public class TemplateQueryBuilderTests extends ESTestCase {
+
+    /**
+     * The query type all template tests will be based on.
+     */
+    private static QueryBuilder<?> templateBase;
+
+    @BeforeClass
+    public static void setupClass() {
+        templateBase = RandomQueryBuilder.createQuery(getRandom());
+    }
+
+    @Override
+    protected boolean supportsBoostAndQueryName() {
+        return false;
+    }
+
+    @Override
+    protected TemplateQueryBuilder doCreateTestQueryBuilder() {
+        return new TemplateQueryBuilder(new Template(templateBase.toString()));
+    }
+
+    @Override
+    protected void doAssertLuceneQuery(TemplateQueryBuilder queryBuilder, Query query, QueryShardContext context) throws IOException {
+        assertEquals(templateBase.toQuery(createShardContext()), query);
+    }
+
+    @Test
+    public void testValidate() {
+        TemplateQueryBuilder templateQueryBuilder = new TemplateQueryBuilder(null);
+        assertThat(templateQueryBuilder.validate().validationErrors().size(), is(1));
+    }
+
+    @Override
+    protected void assertBoost(TemplateQueryBuilder queryBuilder, Query query) throws IOException {
+        //no-op boost is checked already above as part of doAssertLuceneQuery as we rely on lucene equals impl
+    }
 
     @Test
     public void testJSONGeneration() throws IOException {