From c2ccb2157c22a885a9b892c3e6f728d2e87724d6 Mon Sep 17 00:00:00 2001 From: Alex Ksikes Date: Tue, 1 Sep 2015 18:22:39 +0200 Subject: [PATCH] Refactors TemplateQueryBuilder and Parser Relates to #10217 This PR is against the query-refactoring branch. Closes #13253 --- .../index/query/QueryShardContext.java | 12 +++ .../index/query/TemplateQueryBuilder.java | 81 +++++++++++++++---- .../index/query/TemplateQueryParser.java | 36 +-------- .../query/TemplateQueryBuilderTests.java | 47 +++++++++-- 4 files changed, 122 insertions(+), 54 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 0ce87a2cd20..025f24b36d2 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/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 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(); + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java index 63721c005a1..5c912de5346 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TemplateQueryBuilder.java +++ b/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 vars; - /** Template to fill.*/ - private String templateString; + private final Template template; - private ScriptService.ScriptType templateType; - - static final TemplateQueryBuilder PROTOTYPE = new TemplateQueryBuilder(null, null); + static final TemplateQueryBuilder PROTOTYPE = new TemplateQueryBuilder(null); /** * @param template @@ -52,6 +54,10 @@ public class TemplateQueryBuilder extends AbstractQueryBuilder 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 vars) { - this.templateString = template; - this.vars = vars; - this.templateType = templateType; + this(new Template(template, templateType, null, null, vars)); } @Override protected void doXContent(XContentBuilder builder, Params builderParams) throws IOException { builder.field(TemplateQueryBuilder.NAME); - if (template == null) { - new Template(templateString, templateType, null, null, this.vars).toXContent(builder, builderParams); - } else { - template.toXContent(builder, builderParams); - } + 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); + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/TemplateQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/TemplateQueryParser.java index a4c3ef96771..b74e2767077 100644 --- a/core/src/main/java/org/elasticsearch/index/query/TemplateQueryParser.java +++ b/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 { - - /** Name of query parameter containing the template string. */ - public static final String QUERY = "query"; - - private final ScriptService scriptService; +public class TemplateQueryParser extends BaseQueryParser { private final static Map 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 - * parse context containing the templated query. + * @param parseContext parse context containing the templated query. */ @Override @Nullable - public Query parse(QueryShardContext context) throws IOException { - QueryParseContext parseContext = context.parseContext(); + public TemplateQueryBuilder fromXContent(QueryParseContext parseContext) throws IOException, QueryParsingException { XContentParser parser = parseContext.parser(); Template template = parse(parser, parseContext.parseFieldMatcher()); - ExecutableScript executable = this.scriptService.executable(template, ScriptContext.Standard.SEARCH, SearchContext.current()); - - 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(); - } + return new TemplateQueryBuilder(template); } public static Template parse(XContentParser parser, ParseFieldMatcher parseFieldMatcher, String... parameters) throws IOException { - Map parameterMap = new HashMap<>(parametersToTypes); for (String parameter : parameters) { parameterMap.put(parameter, ScriptService.ScriptType.INLINE); diff --git a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java index 647ac44c673..e18a0f83d21 100644 --- a/core/src/test/java/org/elasticsearch/index/query/TemplateQueryBuilderTests.java +++ b/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; -/** - * Test building and serialising a template search request. - * */ -public class TemplateQueryBuilderTests extends ESTestCase { +import static org.hamcrest.Matchers.is; + +public class TemplateQueryBuilderTests extends BaseQueryTestCase { + + /** + * 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 {