Refactor how to determine if a field is metafield

Before to determine if a field is meta-field, a static method of MapperService
isMetadataField was used. This method was using an outdated static list
of meta-fields.

This PR instead changes this method to the instance method that
is also aware of meta-fields in all registered plugins.

Related elastic#38373, elastic#41656
Closes elastic#24422

modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorMatchedSlotSubFetchPhase.java

@@ -32,15 +32,14 @@
 import org.apache.lucene.util.BitSetIterator;
 import org.elasticsearch.common.document.DocumentField;
 import org.elasticsearch.common.lucene.search.Queries;
+import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.fetch.FetchSubPhase;
 import org.elasticsearch.search.internal.SearchContext;
 
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
@@ -57,11 +56,12 @@ final class PercolatorMatchedSlotSubFetchPhase implements FetchSubPhase {
 
     @Override
     public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOException {
-        innerHitsExecute(context.query(), context.searcher(), hits);
+        innerHitsExecute(context.query(), context.searcher(), context.mapperService(), hits);
     }
 
     static void innerHitsExecute(Query mainQuery,
                                  IndexSearcher indexSearcher,
+                                 MapperService mapperService,
                                  SearchHit[] hits) throws IOException {
         List<PercolateQuery> percolateQueries = locatePercolatorQuery(mainQuery);
         if (percolateQueries.isEmpty()) {
@@ -101,13 +101,9 @@ static void innerHitsExecute(Query mainQuery,
                     continue;
                 }
 
-                Map<String, DocumentField> fields = hit.fieldsOrNull();
-                if (fields == null) {
-                    fields = new HashMap<>();
-                    hit.fields(fields);
-                }
                 IntStream slots = convertTopDocsToSlots(topDocs, rootDocsBySlot);
-                hit.setField(fieldName, new DocumentField(fieldName, slots.boxed().collect(Collectors.toList())));
+                hit.setField(fieldName, new DocumentField(fieldName, slots.boxed().collect(Collectors.toList())),
+                    mapperService.isMetadataField(fieldName));
             }
         }
     }

modules/percolator/src/test/java/org/elasticsearch/percolator/PercolatorMatchedSlotSubFetchPhaseTests.java

@@ -34,16 +34,20 @@
 import org.apache.lucene.search.TotalHits;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.util.FixedBitSet;
+import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.SeqNoFieldMapper;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.test.ESTestCase;
+import org.mockito.Mockito;
 
 import java.util.Collections;
 import java.util.stream.IntStream;
 
 public class PercolatorMatchedSlotSubFetchPhaseTests extends ESTestCase {
 
     public void testHitsExecute() throws Exception {
+        MapperService mapperService = Mockito.mock(MapperService.class);
+        Mockito.when(mapperService.isMetadataField(Mockito.anyString())).thenReturn(false);
         try (Directory directory = newDirectory()) {
             // Need a one doc index:
             try (RandomIndexWriter indexWriter = new RandomIndexWriter(random(), directory)) {
@@ -64,7 +68,7 @@ public void testHitsExecute() throws Exception {
                     PercolateQuery percolateQuery =  new PercolateQuery("_name", queryStore, Collections.emptyList(),
                         new MatchAllDocsQuery(), memoryIndex.createSearcher(), null, new MatchNoDocsQuery());
 
-                    PercolatorMatchedSlotSubFetchPhase.innerHitsExecute(percolateQuery, indexSearcher, hits);
+                    PercolatorMatchedSlotSubFetchPhase.innerHitsExecute(percolateQuery, indexSearcher, mapperService, hits);
                     assertNotNull(hits[0].field(PercolatorMatchedSlotSubFetchPhase.FIELD_NAME_PREFIX));
                     assertEquals(0, (int) hits[0].field(PercolatorMatchedSlotSubFetchPhase.FIELD_NAME_PREFIX).getValue());
                 }
@@ -79,7 +83,7 @@ public void testHitsExecute() throws Exception {
                     PercolateQuery percolateQuery =  new PercolateQuery("_name", queryStore, Collections.emptyList(),
                         new MatchAllDocsQuery(), memoryIndex.createSearcher(), null, new MatchNoDocsQuery());
 
-                    PercolatorMatchedSlotSubFetchPhase.innerHitsExecute(percolateQuery, indexSearcher, hits);
+                    PercolatorMatchedSlotSubFetchPhase.innerHitsExecute(percolateQuery, indexSearcher, mapperService, hits);
                     assertNull(hits[0].field(PercolatorMatchedSlotSubFetchPhase.FIELD_NAME_PREFIX));
                 }
 
@@ -93,7 +97,7 @@ public void testHitsExecute() throws Exception {
                     PercolateQuery percolateQuery =  new PercolateQuery("_name", queryStore, Collections.emptyList(),
                         new MatchAllDocsQuery(), memoryIndex.createSearcher(), null, new MatchNoDocsQuery());
 
-                    PercolatorMatchedSlotSubFetchPhase.innerHitsExecute(percolateQuery, indexSearcher, hits);
+                    PercolatorMatchedSlotSubFetchPhase.innerHitsExecute(percolateQuery, indexSearcher, mapperService, hits);
                     assertNull(hits[0].field(PercolatorMatchedSlotSubFetchPhase.FIELD_NAME_PREFIX));
                 }
             }

modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedSearchHitTests.java

@@ -32,7 +32,6 @@
 import java.util.OptionalInt;
 
 import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
-import static org.elasticsearch.test.XContentTestUtils.insertRandomFields;
 
 public class RatedSearchHitTests extends ESTestCase {
 
@@ -74,8 +73,7 @@ public void testXContentRoundtrip() throws IOException {
         RatedSearchHit testItem = randomRatedSearchHit();
         XContentType xContentType = randomFrom(XContentType.values());
         BytesReference originalBytes = toShuffledXContent(testItem, xContentType, ToXContent.EMPTY_PARAMS, randomBoolean());
-        BytesReference withRandomFields = insertRandomFields(xContentType, originalBytes, null, random());
-        try (XContentParser parser = createParser(xContentType.xContent(), withRandomFields)) {
+        try (XContentParser parser = createParser(xContentType.xContent(), originalBytes)) {
             RatedSearchHit parsedItem = RatedSearchHit.parse(parser);
             assertNotSame(testItem, parsedItem);
             assertEquals(testItem, parsedItem);

server/src/internalClusterTest/java/org/elasticsearch/get/GetActionIT.java

@@ -594,14 +594,12 @@ public void testGetFieldsComplexField() throws Exception {
         String field = "field1.field2.field3.field4";
         GetResponse getResponse = client().prepareGet("my-index", "1").setStoredFields(field).get();
         assertThat(getResponse.isExists(), equalTo(true));
-        assertThat(getResponse.getField(field).isMetadataField(), equalTo(false));
         assertThat(getResponse.getField(field).getValues().size(), equalTo(2));
         assertThat(getResponse.getField(field).getValues().get(0).toString(), equalTo("value1"));
         assertThat(getResponse.getField(field).getValues().get(1).toString(), equalTo("value2"));
 
         getResponse = client().prepareGet("my-index", "1").setStoredFields(field).get();
         assertThat(getResponse.isExists(), equalTo(true));
-        assertThat(getResponse.getField(field).isMetadataField(), equalTo(false));
         assertThat(getResponse.getField(field).getValues().size(), equalTo(2));
         assertThat(getResponse.getField(field).getValues().get(0).toString(), equalTo("value1"));
         assertThat(getResponse.getField(field).getValues().get(1).toString(), equalTo("value2"));
@@ -629,7 +627,6 @@ public void testGetFieldsComplexField() throws Exception {
 
         getResponse = client().prepareGet("my-index", "1").setStoredFields(field).get();
         assertThat(getResponse.isExists(), equalTo(true));
-        assertThat(getResponse.getField(field).isMetadataField(), equalTo(false));
         assertThat(getResponse.getField(field).getValues().size(), equalTo(2));
         assertThat(getResponse.getField(field).getValues().get(0).toString(), equalTo("value1"));
         assertThat(getResponse.getField(field).getValues().get(1).toString(), equalTo("value2"));

server/src/internalClusterTest/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java

@@ -119,13 +119,10 @@ public void hitExecute(SearchContext context, HitContext hitContext) {
                 return;
             }
             String field = fetchSubPhaseBuilder.getField();
-            if (hitContext.hit().fieldsOrNull() == null) {
-                hitContext.hit().fields(new HashMap<>());
-            }
             DocumentField hitField = hitContext.hit().getFields().get(NAME);
             if (hitField == null) {
                 hitField = new DocumentField(NAME, new ArrayList<>(1));
-                hitContext.hit().setField(NAME, hitField);
+                hitContext.hit().setField(NAME, hitField, false);
             }
             TermVectorsRequest termVectorsRequest = new TermVectorsRequest(context.indexShard().shardId().getIndex().getName(),
                     hitContext.hit().getId());

server/src/internalClusterTest/java/org/elasticsearch/search/fields/SearchFieldsIT.java

@@ -659,7 +659,7 @@ public void testSearchFieldsMetadata() throws Exception {
 
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
         assertThat(searchResponse.getHits().getAt(0).field("field1"), nullValue());
-        assertThat(searchResponse.getHits().getAt(0).field("_routing").isMetadataField(), equalTo(true));
+        assertThat(searchResponse.getHits().getAt(0).isMetadataField("_routing"), equalTo(true));
         assertThat(searchResponse.getHits().getAt(0).field("_routing").getValue().toString(), equalTo("1"));
     }
 
@@ -735,7 +735,6 @@ public void testGetFieldsComplexField() throws Exception {
 
         SearchResponse searchResponse = client().prepareSearch("my-index").addStoredField(field).get();
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
-        assertThat(searchResponse.getHits().getAt(0).field(field).isMetadataField(), equalTo(false));
         assertThat(searchResponse.getHits().getAt(0).field(field).getValues().size(), equalTo(2));
         assertThat(searchResponse.getHits().getAt(0).field(field).getValues().get(0).toString(), equalTo("value1"));
         assertThat(searchResponse.getHits().getAt(0).field(field).getValues().get(1).toString(), equalTo("value2"));
@@ -1187,7 +1186,7 @@ public void testLoadMetadata() throws Exception {
         Map<String, DocumentField> fields = response.getHits().getAt(0).getFields();
 
         assertThat(fields.get("field1"), nullValue());
-        assertThat(fields.get("_routing").isMetadataField(), equalTo(true));
+        assertThat(response.getHits().getAt(0).isMetadataField("_routing"), equalTo(true));
         assertThat(fields.get("_routing").getValue().toString(), equalTo("1"));
     }
 }

server/src/main/java/org/elasticsearch/common/document/DocumentField.java

@@ -26,7 +26,6 @@
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.index.get.GetResult;
-import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.search.SearchHit;
 
 import java.io.IOException;
@@ -88,13 +87,6 @@ public List<Object> getValues() {
         return values;
     }
 
-    /**
-     * @return The field is a metadata field
-     */
-    public boolean isMetadataField() {
-        return MapperService.isMetadataField(name);
-    }
-
     @Override
     public Iterator<Object> iterator() {
         return values.iterator();

server/src/main/java/org/elasticsearch/index/get/GetResult.java

@@ -93,7 +93,8 @@ public GetResult(StreamInput in) throws IOException {
                 Map<String, DocumentField> fields = readFields(in);
                 documentFields = new HashMap<>();
                 metaFields = new HashMap<>();
-                splitFieldsByMetadata(fields, documentFields, metaFields);
+                fields.forEach((fieldName, docField) ->
+                    (MapperService.isMetadataFieldStatic(fieldName) ? metaFields : documentFields).put(fieldName, docField));
             }
         } else {
             metaFields = Collections.emptyMap();
@@ -386,10 +387,10 @@ public static GetResult fromXContent(XContentParser parser) throws IOException {
     }
 
     private Map<String, DocumentField> readFields(StreamInput in) throws IOException {
-        Map<String, DocumentField> fields = null;
+        Map<String, DocumentField> fields;
         int size = in.readVInt();
         if (size == 0) {
-            fields = new HashMap<>();
+            fields = emptyMap();
         } else {
             fields = new HashMap<>(size);
             for (int i = 0; i < size; i++) {
@@ -400,20 +401,6 @@ private Map<String, DocumentField> readFields(StreamInput in) throws IOException
         return fields;
     }
 
-    static void splitFieldsByMetadata(Map<String, DocumentField> fields, Map<String, DocumentField> outOther,
-                                       Map<String, DocumentField> outMetadata) {
-        if (fields == null) {
-            return;
-        }
-        for (Map.Entry<String, DocumentField> fieldEntry: fields.entrySet()) {
-            if (fieldEntry.getValue().isMetadataField()) {
-                outMetadata.put(fieldEntry.getKey(), fieldEntry.getValue());
-            } else {
-                outOther.put(fieldEntry.getKey(), fieldEntry.getValue());
-            }
-        }
-    }
-
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeString(index);

server/src/main/java/org/elasticsearch/index/get/ShardGetService.java

@@ -278,7 +278,7 @@ private GetResult innerGetLoadFromStoredFields(String id, String[] storedFields,
                 documentFields = new HashMap<>();
                 metadataFields = new HashMap<>();
                 for (Map.Entry<String, List<Object>> entry : fieldVisitor.fields().entrySet()) {
-                    if (MapperService.isMetadataField(entry.getKey())) {
+                    if (mapperService.isMetadataField(entry.getKey())) {
                         metadataFields.put(entry.getKey(), new DocumentField(entry.getKey(), entry.getValue()));
                     } else {
                         documentFields.put(entry.getKey(), new DocumentField(entry.getKey(), entry.getValue()));

server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java

@@ -389,7 +389,7 @@ private static void innerParseObject(ParseContext context, ObjectMapper mapper,
             if (token == XContentParser.Token.FIELD_NAME) {
                 currentFieldName = parser.currentName();
                 paths = splitAndValidatePath(currentFieldName);
-                if (MapperService.isMetadataField(context.path().pathAsText(currentFieldName))) {
+                if (context.mapperService().isMetadataField(context.path().pathAsText(currentFieldName))) {
                     throw new MapperParsingException("Field [" + currentFieldName + "] is a metadata field and cannot be added inside"
                         + " a document. Use the index API request parameters.");
                 } else if (containsDisabledObjectMapper(mapper, paths)) {

server/src/main/java/org/elasticsearch/index/mapper/MapperService.java

@@ -19,11 +19,11 @@
 
 package org.elasticsearch.index.mapper;
 
-import com.carrotsearch.hppc.ObjectHashSet;
 import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.apache.lucene.analysis.Analyzer;
 import org.apache.lucene.analysis.DelegatingAnalyzerWrapper;
 import org.elasticsearch.Assertions;
+import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.cluster.metadata.MappingMetadata;
 import org.elasticsearch.common.Strings;
@@ -50,14 +50,14 @@
 import org.elasticsearch.index.mapper.Mapper.BuilderContext;
 import org.elasticsearch.index.query.QueryShardContext;
 import org.elasticsearch.index.similarity.SimilarityService;
+import org.elasticsearch.indices.IndicesModule;
 import org.elasticsearch.indices.InvalidTypeNameException;
 import org.elasticsearch.indices.mapper.MapperRegistry;
 import org.elasticsearch.search.suggest.completion.context.ContextMapping;
 
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -107,14 +107,6 @@ public enum MergeReason {
     public static final Setting<Long> INDEX_MAPPING_FIELD_NAME_LENGTH_LIMIT_SETTING =
         Setting.longSetting("index.mapping.field_name_length.limit", Long.MAX_VALUE, 1L, Property.Dynamic, Property.IndexScope);
 
-    //TODO this needs to be cleaned up: _timestamp and _ttl are not supported anymore, _field_names, _seq_no, _version and _source are
-    //also missing, not sure if on purpose. See IndicesModule#getMetadataMappers
-    private static final String[] SORTED_META_FIELDS = new String[]{
-        "_id", IgnoredFieldMapper.NAME, "_index", "_nested_path", "_routing", "_size", "_timestamp", "_ttl", "_type"
-    };
-
-    private static final ObjectHashSet<String> META_FIELDS = ObjectHashSet.from(SORTED_META_FIELDS);
-
     private final IndexAnalyzers indexAnalyzers;
 
     private volatile DocumentMapper mapper;
@@ -124,6 +116,7 @@ public enum MergeReason {
     private boolean hasNested = false; // updated dynamically to true when a nested object is added
 
     private final DocumentMapperParser documentParser;
+    private final Version indexVersionCreated;
 
     private final MapperAnalyzerWrapper indexAnalyzer;
     private final MapperAnalyzerWrapper searchAnalyzer;
@@ -139,6 +132,7 @@ public MapperService(IndexSettings indexSettings, IndexAnalyzers indexAnalyzers,
                          SimilarityService similarityService, MapperRegistry mapperRegistry,
                          Supplier<QueryShardContext> queryShardContextSupplier, BooleanSupplier idFieldDataEnabled) {
         super(indexSettings);
+        this.indexVersionCreated = indexSettings.getIndexVersionCreated();
         this.indexAnalyzers = indexAnalyzers;
         this.fieldTypes = new FieldTypeLookup();
         this.documentParser = new DocumentMapperParser(indexSettings, this, xContentRegistry, similarityService, mapperRegistry,
@@ -640,14 +634,25 @@ public void close() throws IOException {
     }
 
     /**
-     * @return Whether a field is a metadata field.
+     * @return Whether a field is a metadata field for older indexes
+     * TODO: remove in v 9.0
+     * @deprecated  Use an instance method isMetadataField instead
      */
-    public static boolean isMetadataField(String fieldName) {
-        return META_FIELDS.contains(fieldName);
+    @Deprecated
+    public static boolean isMetadataFieldStatic(String fieldName) {
+        if (IndicesModule.getBuiltInMetadataFields().contains(fieldName)) {
+            return true;
+        }
+        // adding _size field as meta-field for bwc
+        return fieldName.equals("_size");
     }
 
-    public static String[] getAllMetaFields() {
-        return Arrays.copyOf(SORTED_META_FIELDS, SORTED_META_FIELDS.length);
+    /**
+     * @return Whether a field is a metadata field.
+     * this method considers all mapper plugins
+     */
+    public boolean isMetadataField(String field) {
+        return mapperRegistry.isMetadataField(indexVersionCreated, field);
     }
 
     /** An analyzer wrapper that can lookup fields within the index mappings */

server/src/main/java/org/elasticsearch/indices/IndicesModule.java

@@ -141,6 +141,8 @@ public static Map<String, Mapper.TypeParser> getMappers(List<MapperPlugin> mappe
 
     private static final Map<String, MetadataFieldMapper.TypeParser> builtInMetadataMappers = initBuiltInMetadataMappers();
 
+    private static Set<String> builtInMetadataFields = Collections.unmodifiableSet(builtInMetadataMappers.keySet());
+
     private static Map<String, MetadataFieldMapper.TypeParser> initBuiltInMetadataMappers() {
         Map<String, MetadataFieldMapper.TypeParser> builtInMetadataMappers;
         // Use a LinkedHashMap for metadataMappers because iteration order matters
@@ -198,7 +200,7 @@ public static Map<String, MetadataFieldMapper.TypeParser> getMetadataMappers(Lis
      * Returns a set containing all of the builtin metadata fields
      */
     public static Set<String> getBuiltInMetadataFields() {
-        return builtInMetadataMappers.keySet();
+        return builtInMetadataFields;
     }
 
     private static Function<String, Predicate<String>> getFieldFilter(List<MapperPlugin> mapperPlugins) {