Browse Source

Remove MapperService searchAnalyzer and searchQuoteAnalyzer (#63406)

Everywhere that pulls the searchAnalyzer or searchQuoteAnalyzer from
MapperService can either instead pull an analyzer from the MappedFieldType
they are working with (which will always have a valid analyzer set) or can
call MapperService#getIndexAnalyzers()#getDefaultSearchAnalyzer if they
need a default.
Alan Woodward 5 years ago
parent
commit
3c752acfec

+ 0 - 14
server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -110,8 +110,6 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
     private final Version indexVersionCreated;
 
     private final MapperAnalyzerWrapper indexAnalyzer;
-    private final MapperAnalyzerWrapper searchAnalyzer;
-    private final MapperAnalyzerWrapper searchQuoteAnalyzer;
 
     final MapperRegistry mapperRegistry;
 
@@ -127,10 +125,6 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
         this.documentParser = new DocumentMapperParser(indexSettings, this, xContentRegistry, similarityService, mapperRegistry,
                 queryShardContextSupplier, scriptService);
         this.indexAnalyzer = new MapperAnalyzerWrapper(indexAnalyzers.getDefaultIndexAnalyzer(), MappedFieldType::indexAnalyzer);
-        this.searchAnalyzer = new MapperAnalyzerWrapper(indexAnalyzers.getDefaultSearchAnalyzer(),
-            p -> p.getTextSearchInfo().getSearchAnalyzer());
-        this.searchQuoteAnalyzer = new MapperAnalyzerWrapper(indexAnalyzers.getDefaultSearchQuoteAnalyzer(),
-            p -> p.getTextSearchInfo().getSearchQuoteAnalyzer());
         this.mapperRegistry = mapperRegistry;
         this.idFieldDataEnabled = idFieldDataEnabled;
     }
@@ -425,14 +419,6 @@ public class MapperService extends AbstractIndexComponent implements Closeable {
         return this.indexAnalyzer;
     }
 
-    public Analyzer searchAnalyzer() {
-        return this.searchAnalyzer;
-    }
-
-    public Analyzer searchQuoteAnalyzer() {
-        return this.searchQuoteAnalyzer;
-    }
-
     /**
      * Returns <code>true</code> if fielddata is enabled for the {@link IdFieldMapper} field, <code>false</code> otherwise.
      */

+ 1 - 1
server/src/main/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilder.java

@@ -971,7 +971,7 @@ public class MoreLikeThisQueryBuilder extends AbstractQueryBuilder<MoreLikeThisQ
         // set analyzer
         Analyzer analyzerObj = context.getIndexAnalyzers().get(analyzer);
         if (analyzerObj == null) {
-            analyzerObj = context.getSearchAnalyzer();
+            analyzerObj = context.getIndexAnalyzer();
         }
         mltQuery.setAnalyzer(analyzerObj);
 

+ 4 - 29
server/src/main/java/org/elasticsearch/index/query/QueryShardContext.java

@@ -249,35 +249,6 @@ public class QueryShardContext extends QueryRewriteContext {
         return mapperService.getObjectMapper(name);
     }
 
-    /**
-     * Gets the search analyzer for the given field, or the default if there is none present for the field
-     * TODO: remove this by moving defaults into mappers themselves
-     */
-    public Analyzer getSearchAnalyzer(MappedFieldType fieldType) {
-        if (fieldType.getTextSearchInfo().getSearchAnalyzer() != null) {
-            return fieldType.getTextSearchInfo().getSearchAnalyzer();
-        }
-        return mapperService.searchAnalyzer();
-    }
-
-    /**
-     * Returns the default search analyzer
-     */
-    public Analyzer getSearchAnalyzer() {
-        return this.mapperService.searchAnalyzer();
-    }
-
-    /**
-     * Gets the search quote analyzer for the given field, or the default if there is none present for the field
-     * TODO: remove this by moving defaults into mappers themselves
-     */
-    public Analyzer getSearchQuoteAnalyzer(MappedFieldType fieldType) {
-        if (fieldType.getTextSearchInfo().getSearchQuoteAnalyzer() != null) {
-            return fieldType.getTextSearchInfo().getSearchQuoteAnalyzer();
-        }
-        return mapperService.searchQuoteAnalyzer();
-    }
-
     /**
      * Given a type (eg. long, string, ...), returns an anonymous field type that can be used for search operations.
      * Generally used to handle unmapped fields in the context of sorting.
@@ -301,6 +272,10 @@ public class QueryShardContext extends QueryRewriteContext {
         return mapperService.getIndexAnalyzers();
     }
 
+    public Analyzer getIndexAnalyzer() {
+        return mapperService.indexAnalyzer();
+    }
+
     public ValuesSourceRegistry getValuesSourceRegistry() {
         return valuesSourceRegistry;
     }

+ 9 - 1
server/src/main/java/org/elasticsearch/index/search/MatchQuery.java

@@ -52,6 +52,7 @@ import org.elasticsearch.common.unit.Fuzziness;
 import org.elasticsearch.index.mapper.KeywordFieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.TextFieldMapper;
+import org.elasticsearch.index.mapper.TextSearchInfo;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.index.query.support.QueryParsers;
 
@@ -296,8 +297,15 @@ public class MatchQuery {
     }
 
     protected Analyzer getAnalyzer(MappedFieldType fieldType, boolean quoted) {
+        // We check here that the field supports text searches and therefore has an analyzer -
+        // if it doesn't, we can bail out early without doing any further parsing.
+        TextSearchInfo tsi = fieldType.getTextSearchInfo();
+        if (tsi == TextSearchInfo.NONE) {
+            throw new IllegalArgumentException("Field [" + fieldType.name() + "] of type [" + fieldType.typeName() +
+                " does not support match queries");
+        }
         if (analyzer == null) {
-            return quoted ? context.getSearchQuoteAnalyzer(fieldType) : context.getSearchAnalyzer(fieldType);
+            return quoted ? tsi.getSearchQuoteAnalyzer() : tsi.getSearchAnalyzer();
         } else {
             return analyzer;
         }

+ 12 - 12
server/src/main/java/org/elasticsearch/index/search/QueryStringQueryParser.java

@@ -105,7 +105,7 @@ public class QueryStringQueryParser extends XQueryParser {
      * @param defaultField The default field for query terms.
      */
     public QueryStringQueryParser(QueryShardContext context, String defaultField) {
-        this(context, defaultField, Collections.emptyMap(), false, context.getSearchAnalyzer());
+        this(context, defaultField, Collections.emptyMap(), false);
     }
 
     /**
@@ -114,7 +114,7 @@ public class QueryStringQueryParser extends XQueryParser {
      * @param lenient If set to `true` will cause format based failures (like providing text to a numeric field) to be ignored.
      */
     public QueryStringQueryParser(QueryShardContext context, String defaultField, boolean lenient) {
-        this(context, defaultField, Collections.emptyMap(), lenient, context.getSearchAnalyzer());
+        this(context, defaultField, Collections.emptyMap(), lenient);
     }
 
     /**
@@ -122,7 +122,7 @@ public class QueryStringQueryParser extends XQueryParser {
      * @param fieldsAndWeights The default fields and weights expansion for query terms
      */
     public QueryStringQueryParser(QueryShardContext context, Map<String, Float> fieldsAndWeights) {
-        this(context, null, fieldsAndWeights, false, context.getSearchAnalyzer());
+        this(context, null, fieldsAndWeights, false);
     }
 
     /**
@@ -131,7 +131,7 @@ public class QueryStringQueryParser extends XQueryParser {
      * @param lenient If set to `true` will cause format based failures (like providing text to a numeric field) to be ignored.
      */
     public QueryStringQueryParser(QueryShardContext context, Map<String, Float> fieldsAndWeights, boolean lenient) {
-        this(context, null, fieldsAndWeights, lenient, context.getSearchAnalyzer());
+        this(context, null, fieldsAndWeights, lenient);
     }
 
     /**
@@ -142,13 +142,13 @@ public class QueryStringQueryParser extends XQueryParser {
     public QueryStringQueryParser(QueryShardContext context, boolean lenient) {
         this(context, "*",
             resolveMappingField(context, "*", 1.0f, false, false, null),
-            lenient, context.getSearchAnalyzer());
+            lenient);
     }
 
     private QueryStringQueryParser(QueryShardContext context, String defaultField,
                                    Map<String, Float> fieldsAndWeights,
-                                   boolean lenient, Analyzer analyzer) {
-        super(defaultField, analyzer);
+                                   boolean lenient) {
+        super(defaultField, context.getIndexAnalyzers().getDefaultSearchAnalyzer());
         this.context = context;
         this.fieldsAndWeights = Collections.unmodifiableMap(fieldsAndWeights);
         this.queryBuilder = new MultiMatchQuery(context);
@@ -417,11 +417,11 @@ public class QueryStringQueryParser extends XQueryParser {
     private Query getRangeQuerySingle(String field, String part1, String part2,
                                       boolean startInclusive, boolean endInclusive, QueryShardContext context) {
         MappedFieldType currentFieldType = context.getFieldType(field);
-        if (currentFieldType == null) {
+        if (currentFieldType == null || currentFieldType.getTextSearchInfo() == TextSearchInfo.NONE) {
             return newUnmappedFieldQuery(field);
         }
         try {
-            Analyzer normalizer = forceAnalyzer == null ? queryBuilder.context.getSearchAnalyzer(currentFieldType) : forceAnalyzer;
+            Analyzer normalizer = forceAnalyzer == null ? currentFieldType.getTextSearchInfo().getSearchAnalyzer() : forceAnalyzer;
             BytesRef part1Binary = part1 == null ? null : normalizer.normalize(field, part1);
             BytesRef part2Binary = part2 == null ? null : normalizer.normalize(field, part2);
             Query rangeQuery = currentFieldType.rangeQuery(part1Binary, part2Binary,
@@ -467,11 +467,11 @@ public class QueryStringQueryParser extends XQueryParser {
 
     private Query getFuzzyQuerySingle(String field, String termStr, int minSimilarity) throws ParseException {
         MappedFieldType currentFieldType = context.getFieldType(field);
-        if (currentFieldType == null) {
+        if (currentFieldType == null || currentFieldType.getTextSearchInfo() == TextSearchInfo.NONE) {
             return newUnmappedFieldQuery(field);
         }
         try {
-            Analyzer normalizer = forceAnalyzer == null ? queryBuilder.context.getSearchAnalyzer(currentFieldType) : forceAnalyzer;
+            Analyzer normalizer = forceAnalyzer == null ? currentFieldType.getTextSearchInfo().getSearchAnalyzer() : forceAnalyzer;
             BytesRef term = termStr == null ? null : normalizer.normalize(field, termStr);
             return currentFieldType.fuzzyQuery(term, Fuzziness.fromEdits(minSimilarity),
                 getFuzzyPrefixLength(), fuzzyMaxExpansions, fuzzyTranspositions, context);
@@ -522,7 +522,7 @@ public class QueryStringQueryParser extends XQueryParser {
             if (currentFieldType == null || currentFieldType.getTextSearchInfo() == TextSearchInfo.NONE) {
                 return newUnmappedFieldQuery(field);
             }
-            setAnalyzer(forceAnalyzer == null ? queryBuilder.context.getSearchAnalyzer(currentFieldType) : forceAnalyzer);
+            setAnalyzer(forceAnalyzer == null ? currentFieldType.getTextSearchInfo().getSearchAnalyzer() : forceAnalyzer);
             Query query = null;
             if (currentFieldType.getTextSearchInfo().isTokenized() == false) {
                 query = currentFieldType.prefixQuery(termStr, getMultiTermRewriteMethod(), context);

+ 1 - 1
server/src/main/java/org/elasticsearch/search/suggest/SuggestionBuilder.java

@@ -306,7 +306,7 @@ public abstract class SuggestionBuilder<T extends SuggestionBuilder<T>> implemen
 
         MappedFieldType fieldType = context.getFieldType(field);
         if (analyzer == null) {
-            suggestionContext.setAnalyzer(context.getSearchAnalyzer(fieldType));
+            suggestionContext.setAnalyzer(fieldType.getTextSearchInfo().getSearchAnalyzer());
         } else {
             Analyzer luceneAnalyzer = context.getIndexAnalyzers().get(analyzer);
             if (luceneAnalyzer == null) {

+ 16 - 0
server/src/test/java/org/elasticsearch/index/query/MoreLikeThisQueryBuilderTests.java

@@ -19,7 +19,9 @@
 
 package org.elasticsearch.index.query;
 
+import org.apache.lucene.analysis.TokenStream;
 import org.apache.lucene.analysis.core.WhitespaceAnalyzer;
+import org.apache.lucene.analysis.tokenattributes.CharTermAttribute;
 import org.apache.lucene.index.Fields;
 import org.apache.lucene.index.memory.MemoryIndex;
 import org.apache.lucene.search.BooleanClause;
@@ -296,6 +298,20 @@ public class MoreLikeThisQueryBuilderTests extends AbstractQueryTestCase<MoreLik
         assertThat(e.getMessage(), containsString("more_like_this only supports text/keyword fields"));
     }
 
+    public void testUsesIndexAnalyzer() throws IOException {
+        MoreLikeThisQueryBuilder qb
+            = new MoreLikeThisQueryBuilder(new String[]{KEYWORD_FIELD_NAME}, new String[]{"some text"}, null);
+        MoreLikeThisQuery q = (MoreLikeThisQuery) qb.toQuery(createShardContext());
+        try (TokenStream ts = q.getAnalyzer().tokenStream(KEYWORD_FIELD_NAME, "some text")) {
+            ts.reset();
+            CharTermAttribute termAtt = ts.addAttribute(CharTermAttribute.class);
+            assertTrue(ts.incrementToken());
+            assertEquals("some text", termAtt.toString());
+            assertFalse(ts.incrementToken());
+            ts.end();
+        }
+    }
+
     public void testDefaultField() throws IOException {
         QueryShardContext context = createShardContext();
 

+ 4 - 12
server/src/test/java/org/elasticsearch/search/suggest/AbstractSuggestionBuilderTestCase.java

@@ -165,10 +165,7 @@ public abstract class AbstractSuggestionBuilderTestCase<SB extends SuggestionBui
                     indexSettings);
             MapperService mapperService = mock(MapperService.class);
             ScriptService scriptService = mock(ScriptService.class);
-            boolean fieldTypeSearchAnalyzerSet = randomBoolean();
-            MappedFieldType fieldType = mockFieldType(suggestionBuilder.field(), fieldTypeSearchAnalyzerSet);
-            when(mapperService.searchAnalyzer())
-                .thenReturn(new NamedAnalyzer("mapperServiceSearchAnalyzer", AnalyzerScope.INDEX, new SimpleAnalyzer()));
+            MappedFieldType fieldType = mockFieldType(suggestionBuilder.field());
             when(mapperService.fieldType(any(String.class))).thenReturn(fieldType);
             IndexAnalyzers indexAnalyzers = new IndexAnalyzers(
                 new HashMap<>() {
@@ -202,10 +199,8 @@ public abstract class AbstractSuggestionBuilderTestCase<SB extends SuggestionBui
             assertSame(mockShardContext, suggestionContext.getShardContext());
             if (suggestionBuilder.analyzer() != null) {
                 assertEquals(suggestionBuilder.analyzer(), ((NamedAnalyzer) suggestionContext.getAnalyzer()).name());
-            } else if (fieldTypeSearchAnalyzerSet) {
-                assertEquals("fieldSearchAnalyzer", ((NamedAnalyzer) suggestionContext.getAnalyzer()).name());
             } else {
-                assertEquals("mapperServiceSearchAnalyzer", ((NamedAnalyzer) suggestionContext.getAnalyzer()).name());
+                assertEquals("fieldSearchAnalyzer", ((NamedAnalyzer) suggestionContext.getAnalyzer()).name());
             }
             assertSuggestionContext(suggestionBuilder, suggestionContext);
         }
@@ -222,8 +217,6 @@ public abstract class AbstractSuggestionBuilderTestCase<SB extends SuggestionBui
         MapperService mapperService = mock(MapperService.class);
         ScriptService scriptService = mock(ScriptService.class);
 
-        when(mapperService.searchAnalyzer())
-            .thenReturn(new NamedAnalyzer("mapperServiceSearchAnalyzer", AnalyzerScope.INDEX, new SimpleAnalyzer()));
         when(mapperService.getNamedAnalyzer(any(String.class))).then(
             invocation -> new NamedAnalyzer((String) invocation.getArguments()[0], AnalyzerScope.INDEX, new SimpleAnalyzer()));
         QueryShardContext mockShardContext = new QueryShardContext(0, idxSettings, BigArrays.NON_RECYCLING_INSTANCE, null,
@@ -243,11 +236,10 @@ public abstract class AbstractSuggestionBuilderTestCase<SB extends SuggestionBui
      */
     protected abstract void assertSuggestionContext(SB builder, SuggestionContext context) throws IOException;
 
-    protected MappedFieldType mockFieldType(String fieldName, boolean analyzerSet) {
+    protected MappedFieldType mockFieldType(String fieldName) {
         MappedFieldType fieldType = mock(MappedFieldType.class);
         when(fieldType.name()).thenReturn(fieldName);
-        NamedAnalyzer searchAnalyzer = analyzerSet ?
-            new NamedAnalyzer("fieldSearchAnalyzer", AnalyzerScope.INDEX, new SimpleAnalyzer()) : null;
+        NamedAnalyzer searchAnalyzer = new NamedAnalyzer("fieldSearchAnalyzer", AnalyzerScope.INDEX, new SimpleAnalyzer());
         TextSearchInfo tsi = new TextSearchInfo(TextFieldMapper.Defaults.FIELD_TYPE, null, searchAnalyzer, searchAnalyzer);
         when(fieldType.getTextSearchInfo()).thenReturn(tsi);
         return fieldType;

+ 1 - 6
server/src/test/java/org/elasticsearch/search/suggest/completion/CompletionSuggesterBuilderTests.java

@@ -166,12 +166,7 @@ public class CompletionSuggesterBuilderTests extends AbstractSuggestionBuilderTe
     }
 
     @Override
-    protected MappedFieldType mockFieldType(String fieldName, boolean analyzerSet) {
-        if (analyzerSet == false) {
-            CompletionFieldType completionFieldType = new CompletionFieldType(fieldName, null, Collections.emptyMap());
-            completionFieldType.setContextMappings(new ContextMappings(contextMappings));
-            return completionFieldType;
-        }
+    protected MappedFieldType mockFieldType(String fieldName) {
         CompletionFieldType completionFieldType = new CompletionFieldType(fieldName,
             new NamedAnalyzer("fieldSearchAnalyzer", AnalyzerScope.INDEX, new SimpleAnalyzer()),
             Collections.emptyMap());

+ 0 - 2
test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java

@@ -228,8 +228,6 @@ public abstract class MapperServiceTestCase extends ESTestCase {
         when(queryShardContext.isFieldMapped(anyString()))
             .thenAnswer(inv -> mapperService.fieldType(inv.getArguments()[0].toString()) != null);
         when(queryShardContext.getIndexAnalyzers()).thenReturn(mapperService.getIndexAnalyzers());
-        when(queryShardContext.getSearchQuoteAnalyzer(anyObject())).thenCallRealMethod();
-        when(queryShardContext.getSearchAnalyzer(anyObject())).thenCallRealMethod();
         when(queryShardContext.getIndexSettings()).thenReturn(mapperService.getIndexSettings());
         when(queryShardContext.simpleMatchToIndexNames(anyObject())).thenAnswer(
             inv -> mapperService.simpleMatchToFullName(inv.getArguments()[0].toString())

+ 0 - 2
x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/AbstractScriptFieldTypeTestCase.java

@@ -20,7 +20,6 @@ import java.io.IOException;
 import java.util.function.BiConsumer;
 
 import static org.hamcrest.Matchers.equalTo;
-import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -73,7 +72,6 @@ abstract class AbstractScriptFieldTypeTestCase extends ESTestCase {
         QueryShardContext context = mock(QueryShardContext.class);
         if (mappedFieldType != null) {
             when(context.getFieldType(anyString())).thenReturn(mappedFieldType);
-            when(context.getSearchAnalyzer(any())).thenReturn(mappedFieldType.getTextSearchInfo().getSearchAnalyzer());
         }
         when(context.allowExpensiveQueries()).thenReturn(allowExpensiveQueries);
         SearchLookup lookup = new SearchLookup(