Remove code duplication in FieldsVisitor. #18218

It currently has twice the same method, once with a MapperService instance and
once with a DocumentMapper. This commits only keeps the former.

core/src/main/java/org/elasticsearch/index/fieldvisitor/FieldsVisitor.java

@@ -23,8 +23,6 @@ import org.apache.lucene.index.StoredFieldVisitor;
 import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
-import org.elasticsearch.index.mapper.DocumentMapper;
-import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.Uid;
@@ -84,19 +82,11 @@ public class FieldsVisitor extends StoredFieldVisitor {
     }
 
     public void postProcess(MapperService mapperService) {
-        if (uid != null) {
-            DocumentMapper documentMapper = mapperService.documentMapper(uid.type());
-            if (documentMapper != null) {
-                // we can derive the exact type for the mapping
-                postProcess(documentMapper);
-                return;
-            }
-        }
-        // can't derive exact mapping type
         for (Map.Entry<String, List<Object>> entry : fields().entrySet()) {
             MappedFieldType fieldType = mapperService.fullName(entry.getKey());
             if (fieldType == null) {
-                continue;
+                throw new IllegalStateException("Field [" + entry.getKey()
+                    + "] exists in the index but not in mappings");
             }
             List<Object> fieldValues = entry.getValue();
             for (int i = 0; i < fieldValues.size(); i++) {
@@ -105,30 +95,6 @@ public class FieldsVisitor extends StoredFieldVisitor {
         }
     }
 
-    public void postProcess(DocumentMapper documentMapper) {
-        for (Map.Entry<String, List<Object>> entry : fields().entrySet()) {
-            String indexName = entry.getKey();
-            FieldMapper fieldMapper = documentMapper.mappers().getMapper(indexName);
-            if (fieldMapper == null) {
-                // it's possible index name doesn't match field name (legacy feature)
-                for (FieldMapper mapper : documentMapper.mappers()) {
-                    if (mapper.fieldType().name().equals(indexName)) {
-                        fieldMapper = mapper;
-                        break;
-                    }
-                }
-                if (fieldMapper == null) {
-                    // no index name or full name found, so skip
-                    continue;
-                }
-            }
-            List<Object> fieldValues = entry.getValue();
-            for (int i = 0; i < fieldValues.size(); i++) {
-                fieldValues.set(i, fieldMapper.fieldType().valueForSearch(fieldValues.get(i)));
-            }
-        }
-    }
-
     @Override
     public void binaryField(FieldInfo fieldInfo, byte[] value) throws IOException {
         if (SourceFieldMapper.NAME.equals(fieldInfo.name)) {

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

@@ -118,13 +118,8 @@ public final class ShardGetService extends AbstractIndexShardComponent {
         currentMetric.inc();
         try {
             long now = System.nanoTime();
-            DocumentMapper docMapper = mapperService.documentMapper(type);
-            if (docMapper == null) {
-                missingMetric.inc(System.nanoTime() - now);
-                return new GetResult(shardId.getIndexName(), type, id, -1, false, null, null);
-            }
             fetchSourceContext = normalizeFetchSourceContent(fetchSourceContext, fields);
-            GetResult getResult = innerGetLoadFromStoredFields(type, id, fields, fetchSourceContext, engineGetResult, docMapper, ignoreErrorsOnGeneratedFields);
+            GetResult getResult = innerGetLoadFromStoredFields(type, id, fields, fetchSourceContext, engineGetResult, mapperService, ignoreErrorsOnGeneratedFields);
             if (getResult.isExists()) {
                 existsMetric.inc(System.nanoTime() - now);
             } else {
@@ -185,16 +180,10 @@ public final class ShardGetService extends AbstractIndexShardComponent {
             }
         }
 
-        DocumentMapper docMapper = mapperService.documentMapper(type);
-        if (docMapper == null) {
-            get.release();
-            return new GetResult(shardId.getIndexName(), type, id, -1, false, null, null);
-        }
-
         try {
             // break between having loaded it from translog (so we only have _source), and having a document to load
             if (get.docIdAndVersion() != null) {
-                return innerGetLoadFromStoredFields(type, id, gFields, fetchSourceContext, get, docMapper, ignoreErrorsOnGeneratedFields);
+                return innerGetLoadFromStoredFields(type, id, gFields, fetchSourceContext, get, mapperService, ignoreErrorsOnGeneratedFields);
             } else {
                 Translog.Source source = get.source();
 
@@ -205,6 +194,7 @@ public final class ShardGetService extends AbstractIndexShardComponent {
                 Set<String> neededFields = new HashSet<>();
                 // add meta fields
                 neededFields.add(RoutingFieldMapper.NAME);
+                DocumentMapper docMapper = mapperService.documentMapper(type);
                 if (docMapper.parentFieldMapper().active()) {
                     neededFields.add(ParentFieldMapper.NAME);
                 }
@@ -326,7 +316,7 @@ public final class ShardGetService extends AbstractIndexShardComponent {
         }
     }
 
-    private GetResult innerGetLoadFromStoredFields(String type, String id, String[] gFields, FetchSourceContext fetchSourceContext, Engine.GetResult get, DocumentMapper docMapper, boolean ignoreErrorsOnGeneratedFields) {
+    private GetResult innerGetLoadFromStoredFields(String type, String id, String[] gFields, FetchSourceContext fetchSourceContext, Engine.GetResult get, MapperService mapperService, boolean ignoreErrorsOnGeneratedFields) {
         Map<String, GetField> fields = null;
         BytesReference source = null;
         Versions.DocIdAndVersion docIdAndVersion = get.docIdAndVersion();
@@ -340,7 +330,7 @@ public final class ShardGetService extends AbstractIndexShardComponent {
             source = fieldVisitor.source();
 
             if (!fieldVisitor.fields().isEmpty()) {
-                fieldVisitor.postProcess(docMapper);
+                fieldVisitor.postProcess(mapperService);
                 fields = new HashMap<>(fieldVisitor.fields().size());
                 for (Map.Entry<String, List<Object>> entry : fieldVisitor.fields().entrySet()) {
                     fields.put(entry.getKey(), new GetField(entry.getKey(), entry.getValue()));
@@ -348,6 +338,7 @@ public final class ShardGetService extends AbstractIndexShardComponent {
             }
         }
 
+        DocumentMapper docMapper = mapperService.documentMapper(type);
         if (docMapper.parentFieldMapper().active()) {
             String parentId = ParentFieldSubFetchPhase.getParentId(docMapper.parentFieldMapper(), docIdAndVersion.context.reader(), docIdAndVersion.docId);
             if (fields == null) {

core/src/test/java/org/elasticsearch/index/mapper/lucene/StoredNumericValuesTests.java

@@ -29,6 +29,8 @@ import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.fieldvisitor.CustomFieldsVisitor;
 import org.elasticsearch.index.mapper.DocumentMapper;
+import org.elasticsearch.index.mapper.MapperService;
+import org.elasticsearch.index.mapper.MapperService.MergeReason;
 import org.elasticsearch.index.mapper.ParsedDocument;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 
@@ -61,7 +63,8 @@ public class StoredNumericValuesTests extends ESSingleNodeTestCase {
                     .endObject()
                 .endObject()
                 .string();
-        DocumentMapper mapper = createIndex("test").mapperService().documentMapperParser().parse("type", new CompressedXContent(mapping));
+        MapperService mapperService = createIndex("test").mapperService();
+        DocumentMapper mapper = mapperService.merge("type", new CompressedXContent(mapping), MergeReason.MAPPING_UPDATE, false);
 
         ParsedDocument doc = mapper.parse("test", "type", "1", XContentFactory.jsonBuilder()
                 .startObject()
@@ -86,7 +89,7 @@ public class StoredNumericValuesTests extends ESSingleNodeTestCase {
         CustomFieldsVisitor fieldsVisitor = new CustomFieldsVisitor(
                 Collections.emptySet(), Collections.singletonList("field*"), false);
         searcher.doc(0, fieldsVisitor);
-        fieldsVisitor.postProcess(mapper);
+        fieldsVisitor.postProcess(mapperService);
         assertThat(fieldsVisitor.fields().size(), equalTo(10));
         assertThat(fieldsVisitor.fields().get("field1").size(), equalTo(1));
         assertThat(fieldsVisitor.fields().get("field1").get(0), equalTo((byte) 1));