remove FetchSubPhaseContext in favour of generic fetch sub phase builder of type object

The context was an object where the parsed info are stored. That is more of what we call the builder since after the search refactoring. No need for generics in FetchSubPhaseParser then. Also the previous setHitsExecutionNeeded wasn't useful, it can be removed as well, given that once there is a parsed ext section, it will become a builder that can be retrieved by the sub fetch phase. The sub fetch phase is responsible for doing nothing in case the builder is not set, meaning that the fetch sub phase is plugged in but the request didn't have the corresponding section.
This commit is contained in:
javanna 2016-09-08 00:01:43 +02:00 committed by Luca Cavanna
parent dc2ba90f48
commit f9530dfe8f
9 changed files with 39 additions and 129 deletions

View File

@ -1,31 +0,0 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.search;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.search.internal.SearchContext;
/**
*
*/
public interface SearchParseElement {
void parse(XContentParser parser, SearchContext context) throws Exception;
}

View File

@ -63,7 +63,6 @@ import org.elasticsearch.search.dfs.DfsPhase;
import org.elasticsearch.search.dfs.DfsSearchResult;
import org.elasticsearch.search.fetch.FetchPhase;
import org.elasticsearch.search.fetch.FetchSearchResult;
import org.elasticsearch.search.fetch.FetchSubPhaseContext;
import org.elasticsearch.search.fetch.FetchSubPhaseParser;
import org.elasticsearch.search.fetch.QueryFetchSearchResult;
import org.elasticsearch.search.fetch.ScrollQueryFetchSearchResult;
@ -774,8 +773,8 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
throw new SearchParseException(context, "Unknown element [" + currentFieldName + "] in [ext]",
extParser.getTokenLocation());
} else {
FetchSubPhaseContext fetchSubPhaseContext = fetchSubPhaseParser.parse(extParser);
context.putFetchSubPhaseContext(currentFieldName, fetchSubPhaseContext);
Object fetchSubPhaseBuilder = fetchSubPhaseParser.parse(extParser);
context.putFetchSubPhaseBuilder(currentFieldName, fetchSubPhaseBuilder);
}
}
}

View File

@ -1,46 +0,0 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.search.fetch;
/**
* All configuration and context needed by the FetchSubPhase to execute on hits.
* The only required information in this base class is whether or not the sub phase needs to be run at all.
* It can be extended by FetchSubPhases to hold information the phase needs to execute on hits.
*/
public abstract class FetchSubPhaseContext {
// This is to store if the FetchSubPhase should be executed at all.
private boolean hitExecutionNeeded = false;
/**
* Set if this phase should be executed at all.
*/
public void setHitExecutionNeeded(boolean hitExecutionNeeded) {
this.hitExecutionNeeded = hitExecutionNeeded;
}
/**
* Returns if this phase be executed at all.
*/
public boolean hitExecutionNeeded() {
return hitExecutionNeeded;
}
}

View File

@ -24,7 +24,7 @@ import org.elasticsearch.common.xcontent.XContentParser;
/**
* Parser for the ext section of a search request, which can hold custom fetch sub phase
*/
public interface FetchSubPhaseParser<Context extends FetchSubPhaseContext> {
public interface FetchSubPhaseParser {
Context parse(XContentParser parser) throws Exception;
Object parse(XContentParser parser) throws Exception;
}

View File

@ -58,7 +58,6 @@ import org.elasticsearch.search.aggregations.SearchContextAggregations;
import org.elasticsearch.search.dfs.DfsSearchResult;
import org.elasticsearch.search.fetch.FetchPhase;
import org.elasticsearch.search.fetch.FetchSearchResult;
import org.elasticsearch.search.fetch.FetchSubPhaseContext;
import org.elasticsearch.search.fetch.StoredFieldsContext;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
@ -146,7 +145,7 @@ public class DefaultSearchContext extends SearchContext {
private volatile long lastAccessTime = -1;
private Profilers profilers;
private final Map<String, FetchSubPhaseContext> subPhaseContexts = new HashMap<>();
private final Map<String, Object> subPhaseBuilders = new HashMap<>();
private final Map<Class<?>, Collector> queryCollectors = new HashMap<>();
private final QueryShardContext queryShardContext;
private FetchPhase fetchPhase;
@ -386,14 +385,13 @@ public class DefaultSearchContext extends SearchContext {
}
@Override
@SuppressWarnings("unchecked")
public <SubPhaseContext extends FetchSubPhaseContext> SubPhaseContext getFetchSubPhaseContext(String name) {
return (SubPhaseContext) subPhaseContexts.get(name);
public Object getFetchSubPhaseBuilder(String name) {
return subPhaseBuilders.get(name);
}
@Override
public <SubPhaseContext extends FetchSubPhaseContext> void putFetchSubPhaseContext(String name, SubPhaseContext subPhaseContext) {
subPhaseContexts.put(name, subPhaseContext);
public void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder) {
subPhaseBuilders.put(name, fetchSubPhaseBuilder);
}
@Override

View File

@ -43,7 +43,6 @@ import org.elasticsearch.search.aggregations.SearchContextAggregations;
import org.elasticsearch.search.dfs.DfsSearchResult;
import org.elasticsearch.search.fetch.FetchPhase;
import org.elasticsearch.search.fetch.FetchSearchResult;
import org.elasticsearch.search.fetch.FetchSubPhaseContext;
import org.elasticsearch.search.fetch.StoredFieldsContext;
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
import org.elasticsearch.search.fetch.subphase.InnerHitsContext;
@ -511,13 +510,13 @@ public abstract class FilteredSearchContext extends SearchContext {
}
@Override
public <SubPhaseContext extends FetchSubPhaseContext> SubPhaseContext getFetchSubPhaseContext(String name) {
return in.getFetchSubPhaseContext(name);
public void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder) {
in.putFetchSubPhaseBuilder(name, fetchSubPhaseBuilder);
}
@Override
public <SubPhaseContext extends FetchSubPhaseContext> void putFetchSubPhaseContext(String name, SubPhaseContext subPhaseContext) {
in.putFetchSubPhaseContext(name, subPhaseContext);
public Object getFetchSubPhaseBuilder(String name) {
return in.getFetchSubPhaseBuilder(name);
}
@Override

View File

@ -49,7 +49,6 @@ import org.elasticsearch.search.aggregations.SearchContextAggregations;
import org.elasticsearch.search.dfs.DfsSearchResult;
import org.elasticsearch.search.fetch.FetchPhase;
import org.elasticsearch.search.fetch.FetchSearchResult;
import org.elasticsearch.search.fetch.FetchSubPhaseContext;
import org.elasticsearch.search.fetch.StoredFieldsContext;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
@ -185,10 +184,9 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas
public abstract SearchContext aggregations(SearchContextAggregations aggregations);
public abstract <SubPhaseContext extends FetchSubPhaseContext> void putFetchSubPhaseContext(String name,
SubPhaseContext subPhaseContext);
public abstract void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder);
public abstract <SubPhaseContext extends FetchSubPhaseContext> SubPhaseContext getFetchSubPhaseContext(String name);
public abstract Object getFetchSubPhaseBuilder(String name);
public abstract SearchContextHighlight highlight();

View File

@ -26,6 +26,7 @@ import org.apache.lucene.util.BytesRef;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.termvectors.TermVectorsRequest;
import org.elasticsearch.action.termvectors.TermVectorsResponse;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.logging.ESLoggerFactory;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
@ -62,6 +63,7 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase {
return Collections.singletonList(FetchTermVectorsPlugin.class);
}
@SuppressWarnings("unchecked")
public void testPlugin() throws Exception {
client().admin()
.indices()
@ -104,22 +106,20 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase {
}
public static final class TermVectorsFetchSubPhase implements FetchSubPhase {
public static final String NAME = "term_vectors_fetch";
private static final String NAME = "term_vectors_fetch";
@Override
public Map<String, ? extends FetchSubPhaseParser> parsers() {
return singletonMap("term_vectors_fetch", new TermVectorsFetchParser());
return singletonMap(NAME, new TermVectorsFetchParser());
}
@Override
public void hitExecute(SearchContext context, HitContext hitContext) {
TermVectorsFetchContext fetchSubPhaseContext = context.getFetchSubPhaseContext(NAME);
if (fetchSubPhaseContext.hitExecutionNeeded() == false) {
TermVectorsFetchBuilder fetchSubPhaseBuilder = (TermVectorsFetchBuilder)context.getFetchSubPhaseBuilder(NAME);
if (fetchSubPhaseBuilder == null) {
return;
}
String field = fetchSubPhaseContext.getField();
String field = fetchSubPhaseBuilder.getField();
if (hitContext.hit().fieldsOrNull() == null) {
hitContext.hit().fields(new HashMap<>());
}
@ -145,32 +145,27 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase {
}
}
public static class TermVectorsFetchParser implements FetchSubPhaseParser<TermVectorsFetchContext> {
public static class TermVectorsFetchParser implements FetchSubPhaseParser {
@Override
public TermVectorsFetchContext parse(XContentParser parser) throws Exception {
// this is to make sure that the SubFetchPhase knows it should execute
TermVectorsFetchContext termVectorsFetchContext = new TermVectorsFetchContext();
termVectorsFetchContext.setHitExecutionNeeded(true);
public TermVectorsFetchBuilder parse(XContentParser parser) throws Exception {
String field;
XContentParser.Token token = parser.currentToken();
if (token == XContentParser.Token.VALUE_STRING) {
String fieldName = parser.text();
termVectorsFetchContext.setField(fieldName);
field = parser.text();
} else {
throw new IllegalStateException("Expected a VALUE_STRING but got " + token);
throw new ParsingException(parser.getTokenLocation(), "Expected a VALUE_STRING but got " + token);
}
return termVectorsFetchContext;
if (field == null) {
throw new ParsingException(parser.getTokenLocation(), "no fields specified for " + TermVectorsFetchSubPhase.NAME);
}
return new TermVectorsFetchBuilder(field);
}
}
public static class TermVectorsFetchContext extends FetchSubPhaseContext {
public static class TermVectorsFetchBuilder {
private final String field;
private String field = null;
public TermVectorsFetchContext() {
}
public void setField(String field) {
private TermVectorsFetchBuilder(String field) {
this.field = field;
}

View File

@ -44,7 +44,6 @@ import org.elasticsearch.search.aggregations.SearchContextAggregations;
import org.elasticsearch.search.dfs.DfsSearchResult;
import org.elasticsearch.search.fetch.FetchPhase;
import org.elasticsearch.search.fetch.FetchSearchResult;
import org.elasticsearch.search.fetch.FetchSubPhaseContext;
import org.elasticsearch.search.fetch.StoredFieldsContext;
import org.elasticsearch.search.fetch.subphase.DocValueFieldsContext;
import org.elasticsearch.search.fetch.subphase.FetchSourceContext;
@ -90,7 +89,7 @@ public class TestSearchContext extends SearchContext {
private SearchContextAggregations aggregations;
private final long originNanoTime = System.nanoTime();
private final Map<String, FetchSubPhaseContext> subPhaseContexts = new HashMap<>();
private final Map<String, Object> subPhaseContexts = new HashMap<>();
public TestSearchContext(ThreadPool threadPool, BigArrays bigArrays, ScriptService scriptService, IndexService indexService) {
super(ParseFieldMatcher.STRICT);
@ -197,14 +196,13 @@ public class TestSearchContext extends SearchContext {
}
@Override
@SuppressWarnings("unchecked")
public <SubPhaseContext extends FetchSubPhaseContext> SubPhaseContext getFetchSubPhaseContext(String name) {
return (SubPhaseContext) subPhaseContexts.get(name);
public void putFetchSubPhaseBuilder(String name, Object fetchSubPhaseBuilder) {
subPhaseContexts.put(name, fetchSubPhaseBuilder);
}
@Override
public <SubPhaseContext extends FetchSubPhaseContext> void putFetchSubPhaseContext(String name, SubPhaseContext subPhaseContext) {
subPhaseContexts.put(name, subPhaseContext);
public Object getFetchSubPhaseBuilder(String name) {
return subPhaseContexts.get(name);
}
@Override