diff --git a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java index 50f07d04f6f..e884d32f3e4 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -160,16 +160,12 @@ public class FetchPhase implements SearchPhase { hits[index] = searchHit; hitContext.reset(searchHit, subReaderContext, subDocId, context.searcher()); for (FetchSubPhase fetchSubPhase : fetchSubPhases) { - if (fetchSubPhase.hitExecutionNeeded(context)) { - fetchSubPhase.hitExecute(context, hitContext); - } + fetchSubPhase.hitExecute(context, hitContext); } } for (FetchSubPhase fetchSubPhase : fetchSubPhases) { - if (fetchSubPhase.hitsExecutionNeeded(context)) { - fetchSubPhase.hitsExecute(context, hits); - } + fetchSubPhase.hitsExecute(context, hits); } context.fetchResult().hits(new InternalSearchHits(hits, context.queryResult().topDocs().totalHits, context.queryResult().topDocs().getMaxScore())); diff --git a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java index 5ba75b84fcc..b55afe642ec 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/FetchSubPhase.java @@ -36,7 +36,7 @@ import java.util.Map; */ public interface FetchSubPhase { - public static class HitContext { + class HitContext { private InternalSearchHit hit; private IndexSearcher searcher; private LeafReaderContext readerContext; @@ -87,16 +87,13 @@ public interface FetchSubPhase { return Collections.emptyMap(); } - boolean hitExecutionNeeded(SearchContext context); - /** * Executes the hit level phase, with a reader and doc id (note, its a low level reader, and the matching doc). */ - void hitExecute(SearchContext context, HitContext hitContext); + default void hitExecute(SearchContext context, HitContext hitContext) {} - boolean hitsExecutionNeeded(SearchContext context); - void hitsExecute(SearchContext context, InternalSearchHit[] hits); + default void hitsExecute(SearchContext context, InternalSearchHit[] hits) {} /** * This interface is in the fetch phase plugin mechanism. @@ -104,16 +101,16 @@ public interface FetchSubPhase { * Fetch phases that use the plugin mechanism must provide a ContextFactory to the SearchContext that creates the fetch phase context and also associates them with a name. * See {@link SearchContext#getFetchSubPhaseContext(FetchSubPhase.ContextFactory)} */ - public interface ContextFactory { + interface ContextFactory { /** * The name of the context. */ - public String getName(); + String getName(); /** * Creates a new instance of a FetchSubPhaseContext that holds all information a FetchSubPhase needs to execute on hits. */ - public SubPhaseContext newContextInstance(); + SubPhaseContext newContextInstance(); } } diff --git a/core/src/main/java/org/elasticsearch/search/fetch/explain/ExplainFetchSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/explain/ExplainFetchSubPhase.java index 0d8b4ef3271..e560b815d5a 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/explain/ExplainFetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/explain/ExplainFetchSubPhase.java @@ -30,24 +30,13 @@ import java.io.IOException; /** * */ -public class ExplainFetchSubPhase implements FetchSubPhase { - - @Override - public boolean hitsExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { - } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return context.explain(); - } +public final class ExplainFetchSubPhase implements FetchSubPhase { @Override public void hitExecute(SearchContext context, HitContext hitContext) { + if (context.explain() == false) { + return; + } try { final int topLevelDocId = hitContext.hit().docId(); Explanation explanation = context.searcher().explain(context.query(), topLevelDocId); diff --git a/core/src/main/java/org/elasticsearch/search/fetch/fielddata/FieldDataFieldsFetchSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/fielddata/FieldDataFieldsFetchSubPhase.java index f8034719a1d..81907d0b906 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/fielddata/FieldDataFieldsFetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/fielddata/FieldDataFieldsFetchSubPhase.java @@ -18,13 +18,11 @@ */ package org.elasticsearch.search.fetch.fielddata; -import org.elasticsearch.common.inject.Inject; import org.elasticsearch.index.fielddata.AtomicFieldData; import org.elasticsearch.index.fielddata.ScriptDocValues; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.search.SearchHitField; import org.elasticsearch.search.fetch.FetchSubPhase; -import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.InternalSearchHitField; import org.elasticsearch.search.internal.SearchContext; @@ -37,7 +35,7 @@ import java.util.HashMap; * * Specifying {@code "fielddata_fields": ["field1", "field2"]} */ -public class FieldDataFieldsFetchSubPhase implements FetchSubPhase { +public final class FieldDataFieldsFetchSubPhase implements FetchSubPhase { public static final String[] NAMES = {"fielddata_fields", "fielddataFields"}; public static final ContextFactory CONTEXT_FACTORY = new ContextFactory() { @@ -53,29 +51,14 @@ public class FieldDataFieldsFetchSubPhase implements FetchSubPhase { } }; - @Inject - public FieldDataFieldsFetchSubPhase() { - } - - @Override - public boolean hitsExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { - } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return context.getFetchSubPhaseContext(CONTEXT_FACTORY).hitExecutionNeeded(); - } - @Override public void hitExecute(SearchContext context, HitContext hitContext) { + if (context.getFetchSubPhaseContext(CONTEXT_FACTORY).hitExecutionNeeded() == false) { + return; + } for (FieldDataFieldsContext.FieldDataField field : context.getFetchSubPhaseContext(CONTEXT_FACTORY).fields()) { if (hitContext.hit().fieldsOrNull() == null) { - hitContext.hit().fields(new HashMap(2)); + hitContext.hit().fields(new HashMap<>(2)); } SearchHitField hitField = hitContext.hit().fields().get(field.name()); if (hitField == null) { diff --git a/core/src/main/java/org/elasticsearch/search/fetch/innerhits/InnerHitsFetchSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/innerhits/InnerHitsFetchSubPhase.java index 1382e8e2194..35a25b522c0 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/innerhits/InnerHitsFetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/innerhits/InnerHitsFetchSubPhase.java @@ -23,7 +23,6 @@ import org.apache.lucene.search.FieldDoc; import org.apache.lucene.search.ScoreDoc; import org.apache.lucene.search.TopDocs; import org.elasticsearch.ExceptionsHelper; -import org.elasticsearch.search.SearchParseElement; import org.elasticsearch.search.fetch.FetchPhase; import org.elasticsearch.search.fetch.FetchSearchResult; import org.elasticsearch.search.fetch.FetchSubPhase; @@ -32,13 +31,10 @@ import org.elasticsearch.search.internal.InternalSearchHits; import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; -import java.util.Collections; import java.util.HashMap; import java.util.Map; -/** - */ -public class InnerHitsFetchSubPhase implements FetchSubPhase { +public final class InnerHitsFetchSubPhase implements FetchSubPhase { private final FetchPhase fetchPhase; @@ -46,20 +42,11 @@ public class InnerHitsFetchSubPhase implements FetchSubPhase { this.fetchPhase = fetchPhase; } - @Override - public Map parseElements() { - // SearchParse elements needed because everything is parsed by InnerHitBuilder and eventually put - // into the search context. - return Collections.emptyMap(); - } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return context.innerHits() != null && context.innerHits().getInnerHits().size() > 0; - } - @Override public void hitExecute(SearchContext context, HitContext hitContext) { + if ((context.innerHits() != null && context.innerHits().getInnerHits().size() > 0) == false) { + return; + } Map results = new HashMap<>(); for (Map.Entry entry : context.innerHits().getInnerHits().entrySet()) { InnerHitsContext.BaseInnerHits innerHits = entry.getValue(); @@ -92,13 +79,4 @@ public class InnerHitsFetchSubPhase implements FetchSubPhase { } hitContext.hit().setInnerHits(results); } - - @Override - public boolean hitsExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { - } } diff --git a/core/src/main/java/org/elasticsearch/search/fetch/matchedqueries/MatchedQueriesFetchSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/matchedqueries/MatchedQueriesFetchSubPhase.java index 983e131215d..59225f93a61 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/matchedqueries/MatchedQueriesFetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/matchedqueries/MatchedQueriesFetchSubPhase.java @@ -18,6 +18,7 @@ */ package org.elasticsearch.search.fetch.matchedqueries; +import org.apache.lucene.index.IndexReader; import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.ReaderUtil; import org.apache.lucene.search.Query; @@ -26,7 +27,6 @@ import org.apache.lucene.search.Weight; import org.apache.lucene.util.Bits; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.lucene.Lucene; -import org.elasticsearch.search.SearchParseElement; import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.SearchContext; @@ -39,22 +39,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import static java.util.Collections.emptyMap; - -/** - * - */ -public class MatchedQueriesFetchSubPhase implements FetchSubPhase { - - @Override - public Map parseElements() { - return emptyMap(); - } - - @Override - public boolean hitsExecutionNeeded(SearchContext context) { - return true; // we short-circuit in hitsExecute - } +public final class MatchedQueriesFetchSubPhase implements FetchSubPhase { @Override public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { @@ -82,12 +67,13 @@ public class MatchedQueriesFetchSubPhase implements FetchSubPhase { int docBase = -1; Weight weight = context.searcher().createNormalizedWeight(query, false); Bits matchingDocs = null; + final IndexReader indexReader = context.searcher().getIndexReader(); for (int i = 0; i < hits.length; ++i) { InternalSearchHit hit = hits[i]; - int hitReaderIndex = ReaderUtil.subIndex(hit.docId(), context.searcher().getIndexReader().leaves()); + int hitReaderIndex = ReaderUtil.subIndex(hit.docId(), indexReader.leaves()); if (readerIndex != hitReaderIndex) { readerIndex = hitReaderIndex; - LeafReaderContext ctx = context.searcher().getIndexReader().leaves().get(readerIndex); + LeafReaderContext ctx = indexReader.leaves().get(readerIndex); docBase = ctx.docBase; // scorers can be costly to create, so reuse them across docs of the same segment Scorer scorer = weight.scorer(ctx); @@ -99,7 +85,7 @@ public class MatchedQueriesFetchSubPhase implements FetchSubPhase { } } for (int i = 0; i < hits.length; ++i) { - hits[i].matchedQueries(matchedQueries[i].toArray(new String[0])); + hits[i].matchedQueries(matchedQueries[i].toArray(new String[matchedQueries[i].size()])); } } catch (IOException e) { throw ExceptionsHelper.convertToElastic(e); @@ -107,14 +93,4 @@ public class MatchedQueriesFetchSubPhase implements FetchSubPhase { SearchContext.current().clearReleasables(Lifetime.COLLECTION); } } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitExecute(SearchContext context, HitContext hitContext) { - // we do everything in hitsExecute - } } diff --git a/core/src/main/java/org/elasticsearch/search/fetch/parent/ParentFieldSubFetchPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/parent/ParentFieldSubFetchPhase.java index 41fe7176424..6ace9a86a3e 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/parent/ParentFieldSubFetchPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/parent/ParentFieldSubFetchPhase.java @@ -25,10 +25,7 @@ import org.apache.lucene.util.BytesRef; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.index.mapper.internal.ParentFieldMapper; import org.elasticsearch.search.SearchHitField; -import org.elasticsearch.search.SearchParseElement; import org.elasticsearch.search.fetch.FetchSubPhase; -import org.elasticsearch.search.fetch.innerhits.InnerHitsContext; -import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.InternalSearchHitField; import org.elasticsearch.search.internal.SearchContext; @@ -37,17 +34,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; -public class ParentFieldSubFetchPhase implements FetchSubPhase { - - @Override - public Map parseElements() { - return Collections.emptyMap(); - } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return true; - } +public final class ParentFieldSubFetchPhase implements FetchSubPhase { @Override public void hitExecute(SearchContext context, HitContext hitContext) { @@ -65,15 +52,6 @@ public class ParentFieldSubFetchPhase implements FetchSubPhase { fields.put(ParentFieldMapper.NAME, new InternalSearchHitField(ParentFieldMapper.NAME, Collections.singletonList(parentId))); } - @Override - public boolean hitsExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { - } - public static String getParentId(ParentFieldMapper fieldMapper, LeafReader reader, int docId) { try { SortedDocValues docValues = reader.getSortedDocValues(fieldMapper.name()); diff --git a/core/src/main/java/org/elasticsearch/search/fetch/script/ScriptFieldsFetchSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/script/ScriptFieldsFetchSubPhase.java index 540ce9aa66b..0838467bb6c 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/script/ScriptFieldsFetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/script/ScriptFieldsFetchSubPhase.java @@ -21,7 +21,6 @@ package org.elasticsearch.search.fetch.script; import org.elasticsearch.script.LeafSearchScript; import org.elasticsearch.search.SearchHitField; import org.elasticsearch.search.fetch.FetchSubPhase; -import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.InternalSearchHitField; import org.elasticsearch.search.internal.SearchContext; @@ -32,27 +31,13 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; -/** - * - */ -public class ScriptFieldsFetchSubPhase implements FetchSubPhase { - - @Override - public boolean hitsExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { - } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return context.hasScriptFields(); - } +public final class ScriptFieldsFetchSubPhase implements FetchSubPhase { @Override public void hitExecute(SearchContext context, HitContext hitContext) { + if (context.hasScriptFields() == false) { + return; + } for (ScriptFieldsContext.ScriptField scriptField : context.scriptFields().fields()) { LeafSearchScript leafScript; try { @@ -62,10 +47,9 @@ public class ScriptFieldsFetchSubPhase implements FetchSubPhase { } leafScript.setDocument(hitContext.docId()); - Object value; + final Object value; try { - value = leafScript.run(); - value = leafScript.unwrap(value); + value = leafScript.unwrap(leafScript.run()); } catch (RuntimeException e) { if (scriptField.ignoreException()) { continue; @@ -74,7 +58,7 @@ public class ScriptFieldsFetchSubPhase implements FetchSubPhase { } if (hitContext.hit().fieldsOrNull() == null) { - hitContext.hit().fields(new HashMap(2)); + hitContext.hit().fields(new HashMap<>(2)); } SearchHitField hitField = hitContext.hit().fields().get(scriptField.name()); @@ -84,7 +68,7 @@ public class ScriptFieldsFetchSubPhase implements FetchSubPhase { values = Collections.emptyList(); } else if (value instanceof Collection) { // TODO: use diamond operator once JI-9019884 is fixed - values = new ArrayList((Collection) value); + values = new ArrayList<>((Collection) value); } else { values = Collections.singletonList(value); } diff --git a/core/src/main/java/org/elasticsearch/search/fetch/source/FetchSourceSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/source/FetchSourceSubPhase.java index 30220d12694..900402bda13 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/source/FetchSourceSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/source/FetchSourceSubPhase.java @@ -23,32 +23,18 @@ import org.elasticsearch.ElasticsearchException; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.search.fetch.FetchSubPhase; -import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.search.lookup.SourceLookup; import java.io.IOException; -/** - */ -public class FetchSourceSubPhase implements FetchSubPhase { - - @Override - public boolean hitsExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { - } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return context.sourceRequested(); - } +public final class FetchSourceSubPhase implements FetchSubPhase { @Override public void hitExecute(SearchContext context, HitContext hitContext) { + if (context.sourceRequested() == false) { + return; + } FetchSourceContext fetchSourceContext = context.fetchSourceContext(); assert fetchSourceContext.fetchSource(); if (fetchSourceContext.includes().length == 0 && fetchSourceContext.excludes().length == 0) { diff --git a/core/src/main/java/org/elasticsearch/search/fetch/version/VersionFetchSubPhase.java b/core/src/main/java/org/elasticsearch/search/fetch/version/VersionFetchSubPhase.java index c5511352ec2..77a0e954b2d 100644 --- a/core/src/main/java/org/elasticsearch/search/fetch/version/VersionFetchSubPhase.java +++ b/core/src/main/java/org/elasticsearch/search/fetch/version/VersionFetchSubPhase.java @@ -25,36 +25,21 @@ import org.elasticsearch.common.lucene.uid.Versions; import org.elasticsearch.index.mapper.Uid; import org.elasticsearch.index.mapper.internal.UidFieldMapper; import org.elasticsearch.search.fetch.FetchSubPhase; -import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.SearchContext; import java.io.IOException; -/** - * - */ -public class VersionFetchSubPhase implements FetchSubPhase { - - @Override - public boolean hitsExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { - } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return context.version(); - } +public final class VersionFetchSubPhase implements FetchSubPhase { @Override public void hitExecute(SearchContext context, HitContext hitContext) { + if (context.version() == false) { + return; + } // it might make sense to cache the TermDocs on a shared fetch context and just skip here) // it is going to mean we work on the high level multi reader and not the lower level reader as is // the case below... - long version; + final long version; try { BytesRef uid = Uid.createUidAsBytes(hitContext.hit().type(), hitContext.hit().id()); version = Versions.loadVersion( @@ -64,10 +49,6 @@ public class VersionFetchSubPhase implements FetchSubPhase { } catch (IOException e) { throw new ElasticsearchException("Could not query index for _version", e); } - - if (version < 0) { - version = -1; - } - hitContext.hit().version(version); + hitContext.hit().version(version < 0 ? -1 : version); } } diff --git a/core/src/main/java/org/elasticsearch/search/highlight/HighlightPhase.java b/core/src/main/java/org/elasticsearch/search/highlight/HighlightPhase.java index 701cd588892..7952addbe8c 100644 --- a/core/src/main/java/org/elasticsearch/search/highlight/HighlightPhase.java +++ b/core/src/main/java/org/elasticsearch/search/highlight/HighlightPhase.java @@ -21,7 +21,6 @@ package org.elasticsearch.search.highlight; import org.apache.lucene.search.Query; import org.elasticsearch.common.component.AbstractComponent; -import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.regex.Regex; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.mapper.DocumentMapper; @@ -31,9 +30,7 @@ import org.elasticsearch.index.mapper.core.StringFieldMapper; import org.elasticsearch.index.mapper.core.TextFieldMapper; import org.elasticsearch.index.mapper.internal.SourceFieldMapper; import org.elasticsearch.search.Highlighters; -import org.elasticsearch.search.SearchParseElement; import org.elasticsearch.search.fetch.FetchSubPhase; -import org.elasticsearch.search.internal.InternalSearchHit; import org.elasticsearch.search.internal.SearchContext; import java.util.Arrays; @@ -43,45 +40,21 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -/** - * - */ public class HighlightPhase extends AbstractComponent implements FetchSubPhase { private static final List STANDARD_HIGHLIGHTERS_BY_PRECEDENCE = Arrays.asList("fvh", "postings", "plain"); private final Highlighters highlighters; - @Inject public HighlightPhase(Settings settings, Highlighters highlighters) { super(settings); this.highlighters = highlighters; } - /** - * highlighters do not have a parse element, they use - * {@link HighlightBuilder#fromXContent(org.elasticsearch.index.query.QueryParseContext)} for parsing instead. - */ - @Override - public Map parseElements() { - return Collections.emptyMap(); - } - - @Override - public boolean hitsExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { - } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return context.highlight() != null; - } - @Override public void hitExecute(SearchContext context, HitContext hitContext) { + if (context.highlight() == null) { + return; + } Map highlightFields = new HashMap<>(); for (SearchContextHighlight.Field field : context.highlight().fields()) { Collection fieldNamesToHighlight; diff --git a/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java b/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java index 65da6b58d20..81c923ccf6d 100644 --- a/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java +++ b/core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java @@ -116,7 +116,7 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { } } - public static class TermVectorsFetchSubPhase implements FetchSubPhase { + public final static class TermVectorsFetchSubPhase implements FetchSubPhase { public static final ContextFactory CONTEXT_FACTORY = new ContextFactory() { @@ -138,22 +138,11 @@ public class FetchSubPhasePluginIT extends ESIntegTestCase { return singletonMap("term_vectors_fetch", new TermVectorsFetchParseElement()); } - @Override - public boolean hitsExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { - } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return context.getFetchSubPhaseContext(CONTEXT_FACTORY).hitExecutionNeeded(); - } - @Override public void hitExecute(SearchContext context, HitContext hitContext) { + if (context.getFetchSubPhaseContext(CONTEXT_FACTORY).hitExecutionNeeded() == false) { + return; + } String field = context.getFetchSubPhaseContext(CONTEXT_FACTORY).getField(); if (hitContext.hit().fieldsOrNull() == null) { diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhase.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhase.java index ccf33a69739..4a110172d77 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhase.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhase.java @@ -32,7 +32,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.text.Text; import org.elasticsearch.index.query.ParsedQuery; import org.elasticsearch.search.Highlighters; -import org.elasticsearch.search.SearchParseElement; import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.highlight.HighlightPhase; import org.elasticsearch.search.highlight.SearchContextHighlight; @@ -43,23 +42,27 @@ import org.elasticsearch.search.internal.SubSearchContext; import java.io.IOException; import java.util.Collections; import java.util.List; -import java.util.Map; -// Highlighting in the case of the percolate query is a bit different, because the PercolateQuery itself doesn't get highlighted, -// but the source of the PercolateQuery gets highlighted by each hit containing a query. -public class PercolatorHighlightSubFetchPhase extends HighlightPhase { +/** + * Highlighting in the case of the percolate query is a bit different, because the PercolateQuery itself doesn't get highlighted, + * but the source of the PercolateQuery gets highlighted by each hit containing a query. + */ +public final class PercolatorHighlightSubFetchPhase extends HighlightPhase { public PercolatorHighlightSubFetchPhase(Settings settings, Highlighters highlighters) { super(settings, highlighters); } - @Override - public boolean hitsExecutionNeeded(SearchContext context) { + + boolean hitsExecutionNeeded(SearchContext context) { // for testing return context.highlight() != null && locatePercolatorQuery(context.query()) != null; } @Override public void hitsExecute(SearchContext context, InternalSearchHit[] hits) { + if (hitsExecutionNeeded(context) == false) { + return; + } PercolateQuery percolateQuery = locatePercolatorQuery(context.query()); if (percolateQuery == null) { // shouldn't happen as we checked for the existence of a percolator query in hitsExecutionNeeded(...) @@ -97,20 +100,6 @@ public class PercolatorHighlightSubFetchPhase extends HighlightPhase { } } - @Override - public Map parseElements() { - return Collections.emptyMap(); - } - - @Override - public boolean hitExecutionNeeded(SearchContext context) { - return false; - } - - @Override - public void hitExecute(SearchContext context, HitContext hitContext) { - } - static PercolateQuery locatePercolatorQuery(Query query) { if (query instanceof PercolateQuery) { return (PercolateQuery) query; diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhaseTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhaseTests.java index 95f64132ea1..1dfd941e8e0 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhaseTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorHighlightSubFetchPhaseTests.java @@ -53,10 +53,6 @@ public class PercolatorHighlightSubFetchPhaseTests extends ESTestCase { Mockito.when(searchContext.query()).thenReturn(new MatchAllDocsQuery()); assertThat(subFetchPhase.hitsExecutionNeeded(searchContext), is(false)); - IllegalStateException exception = expectThrows(IllegalStateException.class, - () -> subFetchPhase.hitsExecute(searchContext, null)); - assertThat(exception.getMessage(), equalTo("couldn't locate percolator query")); - Mockito.when(searchContext.query()).thenReturn(percolateQuery); assertThat(subFetchPhase.hitsExecutionNeeded(searchContext), is(true)); }