Make reset() in QueryShardContext private

The query shard reset() method resets some internal state in the
query shard context, like clearing query names, the filter flag
or named queries. The problem with this method being public is
that it currently (miss?) used for modifying an existing context
for recursive invocatiob, but the contexts that have been reseted
that way cannot be properly set back to their previous state.

This PR is a step towards removing reset() entirely by first making
it only be used internally in QueryShardContext. In places where
reset() was used we can either create new QueryShardContexts or
modify the existing context because it is discarded afterwards anyway.

core/src/main/java/org/elasticsearch/cluster/metadata/AliasValidator.java

@@ -139,15 +139,9 @@ public class AliasValidator extends AbstractComponent {
         }
     }
 
-    private void validateAliasFilter(XContentParser parser, QueryShardContext queryShardContext) throws IOException {
+    private static void validateAliasFilter(XContentParser parser, QueryShardContext queryShardContext) throws IOException {
-        try {
+        QueryParseContext queryParseContext = queryShardContext.newParseContext(parser);
-            queryShardContext.reset();
+        QueryBuilder<?> queryBuilder = QueryBuilder.rewriteQuery(queryParseContext.parseInnerQueryBuilder(), queryShardContext);
-            QueryParseContext queryParseContext = queryShardContext.newParseContext(parser);
+        queryBuilder.toFilter(queryShardContext);
-            QueryBuilder<?> queryBuilder = QueryBuilder.rewriteQuery(queryParseContext.parseInnerQueryBuilder(), queryShardContext);
-            queryBuilder.toFilter(queryShardContext);
-        } finally {
-            queryShardContext.reset();
-            parser.close();
-        }
     }
 }

core/src/main/java/org/elasticsearch/index/percolator/PercolatorFieldMapper.java

@@ -207,7 +207,6 @@ public class PercolatorFieldMapper extends FieldMapper {
     }
 
     static Query toQuery(QueryShardContext context, boolean mapUnmappedFieldsAsString, QueryBuilder<?> queryBuilder) throws IOException {
-        context.reset();
         // This means that fields in the query need to exist in the mapping prior to registering this query
         // The reason that this is required, is that if a field doesn't exist then the query assumes defaults, which may be undesired.
         //
@@ -222,11 +221,7 @@ public class PercolatorFieldMapper extends FieldMapper {
         // as an analyzed string.
         context.setAllowUnmappedFields(false);
         context.setMapUnmappedFieldAsString(mapUnmappedFieldsAsString);
-        try {
+        return queryBuilder.toQuery(context);
-            return queryBuilder.toQuery(context);
-        } finally {
-            context.reset();
-        }
     }
 
     static QueryBuilder<?> parseQueryBuilder(QueryParseContext context, XContentLocation location) {

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

@@ -109,12 +109,12 @@ public abstract class AbstractQueryBuilder<QB extends AbstractQueryBuilder<QB>>
     @Override
     public final Query toFilter(QueryShardContext context) throws IOException {
         Query result = null;
-            final boolean originalIsFilter = context.isFilter;
+            final boolean originalIsFilter = context.isFilter();
             try {
-                context.isFilter = true;
+                context.setIsFilter(true);
                 result = toQuery(context);
             } finally {
-                context.isFilter = originalIsFilter;
+                context.setIsFilter(originalIsFilter);
             }
         return result;
     }

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

@@ -91,7 +91,7 @@ public class QueryShardContext extends QueryRewriteContext {
     private boolean allowUnmappedFields;
     private boolean mapUnmappedFieldAsString;
     private NestedScope nestedScope;
-    boolean isFilter; // pkg private for testing
+    private boolean isFilter;
 
     public QueryShardContext(IndexSettings indexSettings, BitsetFilterCache bitsetFilterCache, IndexFieldDataService indexFieldDataService,
                              MapperService mapperService, SimilarityService similarityService, ScriptService scriptService,
@@ -116,7 +116,7 @@ public class QueryShardContext extends QueryRewriteContext {
         this.types = source.getTypes();
     }
 
-    public void reset() {
+    private void reset() {
         allowUnmappedFields = indexSettings.isDefaultAllowUnmappedFields();
         this.lookup = null;
         this.namedQueries.clear();
@@ -183,6 +183,10 @@ public class QueryShardContext extends QueryRewriteContext {
         return isFilter;
     }
 
+    void setIsFilter(boolean isFilter) {
+        this.isFilter = isFilter;
+    }
+
     public Collection<String> simpleMatchToIndexNames(String pattern) {
         return mapperService.simpleMatchToIndexNames(pattern);
     }
@@ -369,5 +373,4 @@ public class QueryShardContext extends QueryRewriteContext {
     public final Index index() {
         return indexSettings.getIndex();
     }
-
 }

modules/lang-mustache/src/test/java/org/elasticsearch/messy/tests/TemplateQueryParserTests.java

@@ -74,6 +74,7 @@ import org.junit.Before;
 import java.io.IOException;
 import java.lang.reflect.Proxy;
 import java.util.Collections;
+import java.util.function.Supplier;
 
 import static org.hamcrest.Matchers.containsString;
 
@@ -84,7 +85,7 @@ import static org.hamcrest.Matchers.containsString;
 public class TemplateQueryParserTests extends ESTestCase {
 
     private Injector injector;
-    private QueryShardContext context;
+    private Supplier<QueryShardContext> contextFactory;
 
     @Before
     public void setup() throws IOException {
@@ -134,7 +135,8 @@ public class TemplateQueryParserTests extends ESTestCase {
         ScriptService scriptService = injector.getInstance(ScriptService.class);
         SimilarityService similarityService = new SimilarityService(idxSettings, Collections.emptyMap());
         MapperRegistry mapperRegistry = new IndicesModule().getMapperRegistry();
-        MapperService mapperService = new MapperService(idxSettings, analysisService, similarityService, mapperRegistry, () -> context);
+        MapperService mapperService = new MapperService(idxSettings, analysisService, similarityService, mapperRegistry, () ->
+            contextFactory.get());
         IndicesFieldDataCache cache = new IndicesFieldDataCache(settings, new IndexFieldDataCache.Listener() {});
         IndexFieldDataService indexFieldDataService =new IndexFieldDataService(idxSettings, cache, injector.getInstance(CircuitBreakerService.class), mapperService);
         BitsetFilterCache bitsetFilterCache = new BitsetFilterCache(idxSettings, new BitsetFilterCache.Listener() {
@@ -149,7 +151,7 @@ public class TemplateQueryParserTests extends ESTestCase {
             }
         });
         IndicesQueriesRegistry indicesQueriesRegistry = injector.getInstance(IndicesQueriesRegistry.class);
-        context = new QueryShardContext(idxSettings, bitsetFilterCache, indexFieldDataService, mapperService,
+        contextFactory =  () -> new QueryShardContext(idxSettings, bitsetFilterCache, indexFieldDataService, mapperService,
                 similarityService, scriptService, indicesQueriesRegistry, proxy, null, null, null);
     }
 
@@ -164,7 +166,7 @@ public class TemplateQueryParserTests extends ESTestCase {
         String templateString = "{" + "\"query\":{\"match_{{template}}\": {}}," + "\"params\":{\"template\":\"all\"}" + "}";
 
         XContentParser templateSourceParser = XContentFactory.xContent(templateString).createParser(templateString);
-        context.reset();
+        QueryShardContext context = contextFactory.get();
         templateSourceParser.nextToken();
 
         Query query = QueryBuilder.rewriteQuery(TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)),
@@ -176,7 +178,7 @@ public class TemplateQueryParserTests extends ESTestCase {
         String templateString = "{" + "  \"inline\" : \"{ \\\"match_{{#use_it}}{{template}}{{/use_it}}\\\":{} }\"," + "  \"params\":{"
                 + "    \"template\":\"all\"," + "    \"use_it\": true" + "  }" + "}";
         XContentParser templateSourceParser = XContentFactory.xContent(templateString).createParser(templateString);
-        context.reset();
+        QueryShardContext context = contextFactory.get();
 
         Query query = QueryBuilder.rewriteQuery(TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)), context).toQuery(context);
         assertTrue("Parsing template query failed.", query instanceof MatchAllDocsQuery);
@@ -192,7 +194,7 @@ public class TemplateQueryParserTests extends ESTestCase {
                 + "  \"params\":{" + "    \"size\":2" + "  }\n" + "}";
 
         XContentParser templateSourceParser = XContentFactory.xContent(templateString).createParser(templateString);
-        context.reset();
+        QueryShardContext context = contextFactory.get();
 
         try {
             TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)).rewrite(context);
@@ -206,7 +208,7 @@ public class TemplateQueryParserTests extends ESTestCase {
         String templateString = "{ \"file\": \"storedTemplate\" ,\"params\":{\"template\":\"all\" } } ";
 
         XContentParser templateSourceParser = XContentFactory.xContent(templateString).createParser(templateString);
-        context.reset();
+        QueryShardContext context = contextFactory.get();
         templateSourceParser.nextToken();
 
 
@@ -219,7 +221,7 @@ public class TemplateQueryParserTests extends ESTestCase {
         String templateString = "{ \"file\": \"storedTemplate\" ,\"params\":{\"template\":\"all\" } } ";
 
         XContentParser templateSourceParser = XContentFactory.xContent(templateString).createParser(templateString);
-        context.reset();
+        QueryShardContext context = contextFactory.get();
         templateSourceParser.nextToken();
         try {
             TemplateQueryBuilder.fromXContent(context.newParseContext(templateSourceParser)).toQuery(context);