Browse Source

Rename FieldValueRetriever -> FieldFetcher. (#62795)

The name `FieldFetcher` fits better with the 'fetch' terminology we use
elsewhere, for example `FetchFieldsPhase` and `ValueFetcher`.

This PR also moves the construction of the fetcher off the context and onto
`FetchFieldsPhase`, which feels like a more natural place for it, and fixes a
TODO in javadocs.
Julie Tibshirani 5 years ago
parent
commit
22ca36489b

+ 1 - 1
server/src/main/java/org/elasticsearch/action/search/SearchRequestBuilder.java

@@ -318,7 +318,7 @@ public class SearchRequestBuilder extends ActionRequestBuilder<SearchRequest, Se
      * Adds a field to load and return. The field must be present in the document _source.
      *
      * @param name The field to load
-     * @param format TODO(jtibs): fill this in
+     * @param format an optional format string used when formatting values, for example a date format.
      */
     public SearchRequestBuilder addFetchField(String name, String format) {
         sourceBuilder().fetchField(name, format);

+ 2 - 10
server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsContext.java

@@ -18,9 +18,6 @@
  */
 package org.elasticsearch.search.fetch.subphase;
 
-import org.elasticsearch.index.mapper.MapperService;
-import org.elasticsearch.search.lookup.SearchLookup;
-
 import java.util.List;
 
 /**
@@ -33,12 +30,7 @@ public class FetchFieldsContext {
         this.fields = fields;
     }
 
-    public FieldValueRetriever fieldValueRetriever(String indexName, MapperService mapperService, SearchLookup searchLookup) {
-        if (mapperService.documentMapper().sourceMapper().enabled() == false) {
-            throw new IllegalArgumentException(
-                "Unable to retrieve the requested [fields] since _source is disabled in the mappings for index [" + indexName + "]"
-            );
-        }
-        return FieldValueRetriever.create(mapperService, searchLookup, fields);
+    public List<FieldAndFormat> fields() {
+        return fields;
     }
 }

+ 13 - 7
server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhase.java

@@ -22,10 +22,12 @@ package org.elasticsearch.search.fetch.subphase;
 import org.apache.lucene.index.LeafReaderContext;
 import org.elasticsearch.common.document.DocumentField;
 import org.elasticsearch.index.mapper.IgnoredFieldMapper;
+import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.search.fetch.FetchContext;
 import org.elasticsearch.search.fetch.FetchSubPhase;
 import org.elasticsearch.search.fetch.FetchSubPhaseProcessor;
+import org.elasticsearch.search.lookup.SearchLookup;
 import org.elasticsearch.search.lookup.SourceLookup;
 
 import java.io.IOException;
@@ -45,15 +47,19 @@ public final class FetchFieldsPhase implements FetchSubPhase {
         if (fetchFieldsContext == null) {
             return null;
         }
-        FieldValueRetriever retriever = fetchFieldsContext.fieldValueRetriever(
-            fetchContext.getIndexName(),
-            fetchContext.mapperService(),
-            fetchContext.searchLookup()
-        );
+
+        MapperService mapperService = fetchContext.mapperService();
+        SearchLookup searchLookup = fetchContext.searchLookup();
+        if (fetchContext.mapperService().documentMapper().sourceMapper().enabled() == false) {
+            throw new IllegalArgumentException("Unable to retrieve the requested [fields] since _source is disabled " +
+                "in the mappings for index [" + fetchContext.getIndexName() + "]");
+        }
+
+        FieldFetcher fieldFetcher = FieldFetcher.create(mapperService, searchLookup, fetchFieldsContext.fields());
         return new FetchSubPhaseProcessor() {
             @Override
             public void setNextReader(LeafReaderContext readerContext) {
-                retriever.setNextReader(readerContext);
+                fieldFetcher.setNextReader(readerContext);
             }
 
             @Override
@@ -62,7 +68,7 @@ public final class FetchFieldsPhase implements FetchSubPhase {
                 SourceLookup sourceLookup = hitContext.sourceLookup();
 
                 Set<String> ignoredFields = getIgnoredFields(hit);
-                Map<String, DocumentField> documentFields = retriever.retrieve(sourceLookup, ignoredFields);
+                Map<String, DocumentField> documentFields = fieldFetcher.fetch(sourceLookup, ignoredFields);
                 for (Map.Entry<String, DocumentField> entry : documentFields.entrySet()) {
                     hit.setDocumentField(entry.getKey(), entry.getValue());
                 }

+ 7 - 9
server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldValueRetriever.java → server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldFetcher.java

@@ -42,12 +42,10 @@ import java.util.Set;
  * A helper class to {@link FetchFieldsPhase} that's initialized with a list of field patterns to fetch.
  * Then given a specific document, it can retrieve the corresponding fields from the document's source.
  */
-public class FieldValueRetriever {
-    public static FieldValueRetriever create(
-        MapperService mapperService,
-        SearchLookup searchLookup,
-        Collection<FieldAndFormat> fieldAndFormats
-    ) {
+public class FieldFetcher {
+    public static FieldFetcher create(MapperService mapperService,
+                                      SearchLookup searchLookup,
+                                      Collection<FieldAndFormat> fieldAndFormats) {
         MappingLookup fieldMappers = mapperService.documentMapper().mappers();
         List<FieldContext> fieldContexts = new ArrayList<>();
 
@@ -74,16 +72,16 @@ public class FieldValueRetriever {
             }
         }
 
-        return new FieldValueRetriever(fieldContexts);
+        return new FieldFetcher(fieldContexts);
     }
 
     private final List<FieldContext> fieldContexts;
 
-    private FieldValueRetriever(List<FieldContext> fieldContexts) {
+    private FieldFetcher(List<FieldContext> fieldContexts) {
         this.fieldContexts = fieldContexts;
     }
 
-    public Map<String, DocumentField> retrieve(SourceLookup sourceLookup, Set<String> ignoredFields) throws IOException {
+    public Map<String, DocumentField> fetch(SourceLookup sourceLookup, Set<String> ignoredFields) throws IOException {
         Map<String, DocumentField> documentFields = new HashMap<>();
         for (FieldContext context : fieldContexts) {
             String field = context.fieldName;

+ 26 - 26
server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java → server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java

@@ -37,7 +37,7 @@ import java.util.Set;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasItems;
 
-public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
+public class FieldFetcherTests extends ESSingleNodeTestCase {
 
     public void testLeafValues() throws IOException {
         MapperService mapperService = createMapperService();
@@ -51,7 +51,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
         List<FieldAndFormat> fieldAndFormats = List.of(
             new FieldAndFormat("field", null),
             new FieldAndFormat("object.field", null));
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, fieldAndFormats);
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, fieldAndFormats);
         assertThat(fields.size(), equalTo(2));
 
         DocumentField field = fields.get("field");
@@ -74,7 +74,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .endObject()
         .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "float_range");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "float_range");
         assertThat(fields.size(), equalTo(1));
 
         DocumentField rangeField = fields.get("float_range");
@@ -89,7 +89,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .field("field", "value")
         .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "non-existent");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "non-existent");
         assertThat(fields.size(), equalTo(0));
     }
 
@@ -99,11 +99,11 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .field("field", "value")
         .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "_routing");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "_routing");
         assertTrue(fields.isEmpty());
     }
 
-    public void testRetrieveAllFields() throws IOException {
+    public void testFetchAllFields() throws IOException {
         MapperService mapperService = createMapperService();
         XContentBuilder source = XContentFactory.jsonBuilder().startObject()
             .field("field", "value")
@@ -112,7 +112,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .endObject()
         .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "*");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "*");
         assertThat(fields.size(), equalTo(2));
     }
 
@@ -124,7 +124,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .endArray()
         .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "field");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "field");
         DocumentField field = fields.get("field");
         assertNotNull(field);
         assertThat(field.getValues().size(), equalTo(2));
@@ -138,7 +138,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .endArray()
             .endObject();
 
-        fields = retrieveFields(mapperService, source, "object.field");
+        fields = fetchFields(mapperService, source, "object.field");
         field = fields.get("object.field");
         assertNotNull(field);
         assertThat(field.getValues().size(), equalTo(4));
@@ -152,7 +152,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .array("geo_point", 27.1, 42.0)
         .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "geo_point");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "geo_point");
         assertThat(fields.size(), equalTo(1));
 
         DocumentField field = fields.get("geo_point");
@@ -167,7 +167,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .endArray()
         .endObject();
 
-        fields = retrieveFields(mapperService, source, "geo_point");
+        fields = fetchFields(mapperService, source, "geo_point");
         assertThat(fields.size(), equalTo(1));
 
         field = fields.get("geo_point");
@@ -185,7 +185,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .endObject()
         .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "*field");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "*field");
         assertThat(fields.size(), equalTo(3));
 
         DocumentField field = fields.get("field");
@@ -211,7 +211,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .field("date_field", "1990-12-29T00:00:00.000Z")
         .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, List.of(
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, List.of(
             new FieldAndFormat("field", null),
             new FieldAndFormat("date_field", "yyyy/MM/dd")));
         assertThat(fields.size(), equalTo(2));
@@ -241,14 +241,14 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
         XContentBuilder source = XContentFactory.jsonBuilder().startObject()
             .array("field", "value", "other_value", "really_really_long_value")
             .endObject();
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "field");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "field");
         DocumentField field = fields.get("field");
         assertThat(field.getValues().size(), equalTo(2));
 
         source = XContentFactory.jsonBuilder().startObject()
             .array("field", "really_really_long_value")
             .endObject();
-        fields = retrieveFields(mapperService, source, "field");
+        fields = fetchFields(mapperService, source, "field");
         assertFalse(fields.containsKey("field"));
     }
 
@@ -270,7 +270,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .field("field", "value")
             .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "alias_field");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "alias_field");
         assertThat(fields.size(), equalTo(1));
 
         DocumentField field = fields.get("alias_field");
@@ -278,7 +278,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
         assertThat(field.getValues().size(), equalTo(1));
         assertThat(field.getValues(), hasItems("value"));
 
-        fields = retrieveFields(mapperService, source, "*field");
+        fields = fetchFields(mapperService, source, "*field");
         assertThat(fields.size(), equalTo(2));
         assertTrue(fields.containsKey("alias_field"));
         assertTrue(fields.containsKey("field"));
@@ -303,7 +303,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .field("field", 42)
             .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "field.keyword");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "field.keyword");
         assertThat(fields.size(), equalTo(1));
 
         DocumentField field = fields.get("field.keyword");
@@ -311,7 +311,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
         assertThat(field.getValues().size(), equalTo(1));
         assertThat(field.getValues(), hasItems("42"));
 
-        fields = retrieveFields(mapperService, source, "field*");
+        fields = fetchFields(mapperService, source, "field*");
         assertThat(fields.size(), equalTo(2));
         assertTrue(fields.containsKey("field"));
         assertTrue(fields.containsKey("field.keyword"));
@@ -338,7 +338,7 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .array("other_field", 1, 2, 3)
             .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "field");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "field");
         assertThat(fields.size(), equalTo(1));
 
         DocumentField field = fields.get("field");
@@ -356,25 +356,25 @@ public class FieldValueRetrieverTests extends ESSingleNodeTestCase {
             .endObject()
         .endObject();
 
-        Map<String, DocumentField> fields = retrieveFields(mapperService, source, "object");
+        Map<String, DocumentField> fields = fetchFields(mapperService, source, "object");
         assertFalse(fields.containsKey("object"));
     }
 
-    private Map<String, DocumentField> retrieveFields(MapperService mapperService, XContentBuilder source, String fieldPattern)
+    private Map<String, DocumentField> fetchFields(MapperService mapperService, XContentBuilder source, String fieldPattern)
         throws IOException {
 
         List<FieldAndFormat> fields = List.of(new FieldAndFormat(fieldPattern, null));
-        return retrieveFields(mapperService, source, fields);
+        return fetchFields(mapperService, source, fields);
     }
 
-    private Map<String, DocumentField> retrieveFields(MapperService mapperService, XContentBuilder source, List<FieldAndFormat> fields)
+    private Map<String, DocumentField> fetchFields(MapperService mapperService, XContentBuilder source, List<FieldAndFormat> fields)
         throws IOException {
 
         SourceLookup sourceLookup = new SourceLookup();
         sourceLookup.setSource(BytesReference.bytes(source));
 
-        FieldValueRetriever fetchFieldsLookup = FieldValueRetriever.create(mapperService, null, fields);
-        return fetchFieldsLookup.retrieve(sourceLookup, Set.of());
+        FieldFetcher fieldFetcher = FieldFetcher.create(mapperService, null, fields);
+        return fieldFetcher.fetch(sourceLookup, Set.of());
     }
 
     public MapperService createMapperService() throws IOException {