[Type Removal] Remove type handling in LeafFieldsLookup

Signed-off-by: Suraj Singh <surajrider@gmail.com>
opensearch-project · Apr 20, 2022 · ccfbf09 · ccfbf09
1 parent 4be4834
commit ccfbf09
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 60 deletions.
diff --git a/server/src/internalClusterTest/java/org/opensearch/search/fields/SearchFieldsIT.java b/server/src/internalClusterTest/java/org/opensearch/search/fields/SearchFieldsIT.java
@@ -471,42 +471,6 @@ public void testIdBasedScriptFields() throws Exception {
             assertThat(fields, equalTo(singleton("id")));
             assertThat(response.getHits().getAt(i).getFields().get("id").getValue(), equalTo(Integer.toString(i)));
         }
-
-        response = client().prepareSearch()
-            .setQuery(matchAllQuery())
-            .addSort("num1", SortOrder.ASC)
-            .setSize(numDocs)
-            .addScriptField("type", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_fields._type.value", Collections.emptyMap()))
-            .get();
-
-        assertNoFailures(response);
-
-        assertThat(response.getHits().getTotalHits().value, equalTo((long) numDocs));
-        for (int i = 0; i < numDocs; i++) {
-            assertThat(response.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
-            Set<String> fields = new HashSet<>(response.getHits().getAt(i).getFields().keySet());
-            assertThat(fields, equalTo(singleton("type")));
-            assertThat(response.getHits().getAt(i).getFields().get("type").getValue(), equalTo(MapperService.SINGLE_MAPPING_NAME));
-        }
-
-        response = client().prepareSearch()
-            .setQuery(matchAllQuery())
-            .addSort("num1", SortOrder.ASC)
-            .setSize(numDocs)
-            .addScriptField("id", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_fields._id.value", Collections.emptyMap()))
-            .addScriptField("type", new Script(ScriptType.INLINE, CustomScriptPlugin.NAME, "_fields._type.value", Collections.emptyMap()))
-            .get();
-
-        assertNoFailures(response);
-
-        assertThat(response.getHits().getTotalHits().value, equalTo((long) numDocs));
-        for (int i = 0; i < numDocs; i++) {
-            assertThat(response.getHits().getAt(i).getId(), equalTo(Integer.toString(i)));
-            Set<String> fields = new HashSet<>(response.getHits().getAt(i).getFields().keySet());
-            assertThat(fields, equalTo(newHashSet("type", "id")));
-            assertThat(response.getHits().getAt(i).getFields().get("type").getValue(), equalTo(MapperService.SINGLE_MAPPING_NAME));
-            assertThat(response.getHits().getAt(i).getFields().get("id").getValue(), equalTo(Integer.toString(i)));
-        }
     }
 
     public void testScriptFieldUsingSource() throws Exception {

diff --git a/server/src/main/java/org/opensearch/index/mapper/TypeFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/TypeFieldMapper.java
@@ -54,7 +54,7 @@
 import java.util.Objects;
 import java.util.function.Supplier;
 
-// Todo: Remove TypeFieldMapper once we have nested field mapping name implementation in place
+// Todo: Remove TypeFieldMapper once we have NestedFieldMapper implementation
 public class TypeFieldMapper extends MetadataFieldMapper {
 
     public static final String NAME = "_type";

diff --git a/server/src/main/java/org/opensearch/search/lookup/LeafFieldsLookup.java b/server/src/main/java/org/opensearch/search/lookup/LeafFieldsLookup.java
@@ -34,10 +34,8 @@
 import org.apache.lucene.index.LeafReader;
 import org.opensearch.OpenSearchParseException;
 import org.opensearch.index.fieldvisitor.SingleFieldsVisitor;
-import org.opensearch.index.mapper.DocumentMapper;
 import org.opensearch.index.mapper.MappedFieldType;
 import org.opensearch.index.mapper.MapperService;
-import org.opensearch.index.mapper.TypeFieldMapper;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -147,21 +145,12 @@ private FieldLookup loadFieldData(String name) {
             cachedFieldData.put(name, data);
         }
         if (data.fields() == null) {
-            List<Object> values;
-            if (TypeFieldMapper.NAME.equals(data.fieldType().name())) {
-                values = new ArrayList<>(1);
-                final DocumentMapper mapper = mapperService.documentMapper();
-                if (mapper != null) {
-                    values.add(mapper.type());
-                }
-            } else {
-                values = new ArrayList<Object>(2);
-                SingleFieldsVisitor visitor = new SingleFieldsVisitor(data.fieldType(), values);
-                try {
-                    reader.document(docId, visitor);
-                } catch (IOException e) {
-                    throw new OpenSearchParseException("failed to load field [{}]", e, name);
-                }
+            List<Object> values = new ArrayList<>(2);
+            SingleFieldsVisitor visitor = new SingleFieldsVisitor(data.fieldType(), values);
+            try {
+                reader.document(docId, visitor);
+            } catch (IOException e) {
+                throw new OpenSearchParseException("failed to load field [{}]", e, name);
             }
             data.fields(singletonMap(data.fieldType().name(), values));
         }

diff --git a/server/src/test/java/org/opensearch/index/get/GetResultTests.java b/server/src/test/java/org/opensearch/index/get/GetResultTests.java
@@ -46,7 +46,6 @@
 import org.opensearch.index.mapper.IndexFieldMapper;
 import org.opensearch.index.mapper.SeqNoFieldMapper;
 import org.opensearch.index.mapper.SourceFieldMapper;
-import org.opensearch.index.mapper.TypeFieldMapper;
 import org.opensearch.index.mapper.VersionFieldMapper;
 import org.opensearch.test.OpenSearchTestCase;
 import org.opensearch.test.RandomObjects;
@@ -372,9 +371,8 @@ public static Tuple<Map<String, DocumentField>, Map<String, DocumentField>> rand
         Map<String, DocumentField> fields = new HashMap<>(numFields);
         Map<String, DocumentField> expectedFields = new HashMap<>(numFields);
         // As we are using this to construct a GetResult object that already contains
-        // index, type, id, version, seqNo, and source fields, we need to exclude them from random fields
-        Predicate<String> excludeMetaFieldFilter = field -> field.equals(TypeFieldMapper.NAME)
-            || field.equals(IndexFieldMapper.NAME)
+        // index, id, version, seqNo, and source fields, we need to exclude them from random fields
+        Predicate<String> excludeMetaFieldFilter = field -> field.equals(IndexFieldMapper.NAME)
             || field.equals(IdFieldMapper.NAME)
             || field.equals(VersionFieldMapper.NAME)
             || field.equals(SourceFieldMapper.NAME)

diff --git a/server/src/test/java/org/opensearch/indices/IndicesModuleTests.java b/server/src/test/java/org/opensearch/indices/IndicesModuleTests.java
@@ -45,7 +45,6 @@
 import org.opensearch.index.mapper.SeqNoFieldMapper;
 import org.opensearch.index.mapper.SourceFieldMapper;
 import org.opensearch.index.mapper.TextFieldMapper;
-import org.opensearch.index.mapper.TypeFieldMapper;
 import org.opensearch.index.mapper.VersionFieldMapper;
 import org.opensearch.indices.mapper.MapperRegistry;
 import org.opensearch.plugins.MapperPlugin;
@@ -95,7 +94,6 @@ public Map<String, MetadataFieldMapper.TypeParser> getMetadataMappers() {
         IndexFieldMapper.NAME,
         DataStreamFieldMapper.NAME,
         SourceFieldMapper.NAME,
-        TypeFieldMapper.NAME,
         VersionFieldMapper.NAME,
         SeqNoFieldMapper.NAME,
         FieldNamesFieldMapper.NAME };