diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/search/330_fetch_fields.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/search/330_fetch_fields.yml index 4600e71580258..f716e42f9e7eb 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/search/330_fetch_fields.yml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/search/330_fetch_fields.yml @@ -2,33 +2,34 @@ setup: - skip: version: " - 7.99.99" reason: "fields retrieval is currently only implemented on master" + +--- +"Test basic field retrieval": - do: indices.create: - index: test - body: - mappings: - properties: - keyword: - type: keyword - integer_range: - type: integer_range + index: test + body: + mappings: + properties: + keyword: + type: keyword + integer_range: + type: integer_range - do: index: - index: test - id: 1 - body: - keyword: [ "first", "second" ] - integer_range: - gte: 0 - lte: 42 + index: test + id: 1 + body: + keyword: [ "first", "second" ] + integer_range: + gte: 0 + lte: 42 - do: indices.refresh: - index: [ test ] + index: [ test ] ---- -"Test basic field retrieval": - do: search: index: test @@ -43,3 +44,35 @@ setup: - match: { hits.hits.0.fields.integer_range.0.gte: 0 } - match: { hits.hits.0.fields.integer_range.0.lte: 42 } + +--- +"Test disable source": + - do: + indices.create: + index: test + body: + settings: + number_of_shards: 1 + mappings: + _source: + enabled: false + properties: + keyword: + type: keyword + + - do: + index: + index: test + id: 1 + body: + keyword: [ "value" ] + + - do: + catch: bad_request + search: + index: test + body: + fields: [keyword] + - match: { error.root_cause.0.type: "illegal_argument_exception" } + - match: { error.root_cause.0.reason: "Unable to retrieve the requested [fields] since _source is disabled + in the mappings for index [test]" } diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhase.java index 8b3343ae5defc..1ede570a3bc41 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhase.java @@ -22,20 +22,13 @@ import org.apache.lucene.index.LeafReaderContext; import org.apache.lucene.index.ReaderUtil; import org.elasticsearch.common.document.DocumentField; -import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.fetch.FetchSubPhase; import org.elasticsearch.search.internal.SearchContext; import org.elasticsearch.search.lookup.SourceLookup; -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; import java.util.Map; -import java.util.Set; -import java.util.function.Function; /** * A fetch sub-phase for high-level field retrieval. Given a list of fields, it @@ -45,14 +38,6 @@ public final class FetchFieldsPhase implements FetchSubPhase { @Override public void hitsExecute(SearchContext context, SearchHit[] hits) { - hitsExecute(context, hit -> getSourceLookup(context, hit), hits); - } - - // Visible for testing. - @SuppressWarnings("unchecked") - void hitsExecute(SearchContext context, - Function sourceProvider, - SearchHit[] hits) { FetchFieldsContext fetchFieldsContext = context.fetchFieldsContext(); if (fetchFieldsContext == null || fetchFieldsContext.fields().isEmpty()) { return; @@ -64,52 +49,18 @@ void hitsExecute(SearchContext context, "disabled in the mappings for index [" + context.indexShard().shardId().getIndexName() + "]"); } - Set fields = new HashSet<>(); - for (String fieldPattern : context.fetchFieldsContext().fields()) { - if (documentMapper.objectMappers().containsKey(fieldPattern)) { - continue; - } - Collection concreteFields = context.mapperService().simpleMatchToFullName(fieldPattern); - fields.addAll(concreteFields); - } + SourceLookup sourceLookup = context.lookup().source(); + FieldValueRetriever fieldValueRetriever = FieldValueRetriever.create( + context.mapperService(), + fetchFieldsContext.fields()); for (SearchHit hit : hits) { - SourceLookup sourceLookup = sourceProvider.apply(hit); - Map valuesByField = extractValues(sourceLookup, fields); - - for (Map.Entry entry : valuesByField.entrySet()) { - String field = entry.getKey(); - Object value = entry.getValue(); - List values = value instanceof List - ? (List) value - : List.of(value); - - DocumentField documentField = new DocumentField(field, values); - hit.setField(field, documentField); - } - } - } - - private SourceLookup getSourceLookup(SearchContext context, SearchHit hit) { - SourceLookup sourceLookup = context.lookup().source(); - int readerIndex = ReaderUtil.subIndex(hit.docId(), context.searcher().getIndexReader().leaves()); - LeafReaderContext readerContext = context.searcher().getIndexReader().leaves().get(readerIndex); - sourceLookup.setSegmentAndDocument(readerContext, hit.docId()); - return sourceLookup; - } + int readerIndex = ReaderUtil.subIndex(hit.docId(), context.searcher().getIndexReader().leaves()); + LeafReaderContext readerContext = context.searcher().getIndexReader().leaves().get(readerIndex); + sourceLookup.setSegmentAndDocument(readerContext, hit.docId()); - /** - * For each of the provided paths, return its value in the source. Note that in contrast with - * {@link SourceLookup#extractRawValues}, array and object values can be returned. - */ - private Map extractValues(SourceLookup sourceLookup, Collection paths) { - Map result = new HashMap<>(paths.size()); - for (String path : paths) { - Object value = XContentMapValues.extractValue(path, sourceLookup); - if (value != null) { - result.put(path, value); - } + Map fieldValues = fieldValueRetriever.retrieve(sourceLookup); + hit.fields(fieldValues); } - return result; } } diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldValueRetriever.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldValueRetriever.java new file mode 100644 index 0000000000000..9c1e74983c171 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FieldValueRetriever.java @@ -0,0 +1,87 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.search.fetch.subphase; + +import org.elasticsearch.common.document.DocumentField; +import org.elasticsearch.common.xcontent.support.XContentMapValues; +import org.elasticsearch.index.mapper.DocumentMapper; +import org.elasticsearch.index.mapper.MapperService; +import org.elasticsearch.search.lookup.SourceLookup; + +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +public class FieldValueRetriever { + private final Set fields; + + public static FieldValueRetriever create(MapperService mapperService, + Collection fieldPatterns) { + Set fields = new HashSet<>(); + DocumentMapper documentMapper = mapperService.documentMapper(); + + for (String fieldPattern : fieldPatterns) { + if (documentMapper.objectMappers().containsKey(fieldPattern)) { + continue; + } + Collection concreteFields = mapperService.simpleMatchToFullName(fieldPattern); + fields.addAll(concreteFields); + } + return new FieldValueRetriever(fields); + } + + private FieldValueRetriever(Set fields) { + this.fields = fields; + } + + @SuppressWarnings("unchecked") + public Map retrieve(SourceLookup sourceLookup) { + Map result = new HashMap<>(); + Map sourceValues = extractValues(sourceLookup, this.fields); + + for (Map.Entry entry : sourceValues.entrySet()) { + String field = entry.getKey(); + Object value = entry.getValue(); + List values = value instanceof List + ? (List) value + : List.of(value); + result.put(field, new DocumentField(field, values)); + } + return result; + } + + /** + * For each of the provided paths, return its value in the source. Note that in contrast with + * {@link SourceLookup#extractRawValues}, array and object values can be returned. + */ + private static Map extractValues(SourceLookup sourceLookup, Collection paths) { + Map result = new HashMap<>(paths.size()); + for (String path : paths) { + Object value = XContentMapValues.extractValue(path, sourceLookup); + if (value != null) { + result.put(path, value); + } + } + return result; + } +} diff --git a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhaseTests.java b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java similarity index 56% rename from server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhaseTests.java rename to server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java index 73f156f81ac36..99ebf1a8807d1 100644 --- a/server/src/test/java/org/elasticsearch/search/fetch/subphase/FetchFieldsPhaseTests.java +++ b/server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldValueRetrieverTests.java @@ -25,27 +25,23 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.index.IndexService; -import org.elasticsearch.search.SearchHit; -import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.search.lookup.SourceLookup; import org.elasticsearch.test.ESSingleNodeTestCase; -import org.elasticsearch.test.TestSearchContext; import org.junit.Before; import java.io.IOException; import java.util.List; import java.util.Map; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItems; -public class FetchFieldsPhaseTests extends ESSingleNodeTestCase { - - private IndexService indexService; +public class FieldValueRetrieverTests extends ESSingleNodeTestCase { + private MapperService mapperService; @Before - public void setupIndex() throws IOException { + public void createMapperService() throws IOException { XContentBuilder mapping = XContentFactory.jsonBuilder().startObject() .startObject("properties") .startObject("field").field("type", "keyword").endObject() @@ -59,7 +55,9 @@ public void setupIndex() throws IOException { .startObject("field_that_does_not_match").field("type", "keyword").endObject() .endObject() .endObject(); - indexService = createIndex("index", Settings.EMPTY, mapping); + + IndexService indexService = createIndex("index", Settings.EMPTY, mapping); + this.mapperService = indexService.mapperService(); } public void testLeafValues() throws IOException { @@ -70,15 +68,15 @@ public void testLeafValues() throws IOException { .endObject() .endObject(); - SearchHit hit = fetchFields(indexService, source, List.of("field", "object.field")); - assertThat(hit.getFields().size(), equalTo(2)); + Map fields = retrieveFields(source, List.of("field", "object.field")); + assertThat(fields.size(), equalTo(2)); - DocumentField field = hit.getFields().get("field"); + DocumentField field = fields.get("field"); assertNotNull(field); assertThat(field.getValues().size(), equalTo(2)); assertThat(field.getValues(), hasItems("first", "second")); - DocumentField objectField = hit.getFields().get("object.field"); + DocumentField objectField = fields.get("object.field"); assertNotNull(objectField); assertThat(objectField.getValues().size(), equalTo(1)); assertThat(objectField.getValues(), hasItems("third")); @@ -92,10 +90,10 @@ public void testObjectValues() throws IOException { .endObject() .endObject(); - SearchHit hit = fetchFields(indexService, source, "float_range"); - assertThat(hit.getFields().size(), equalTo(1)); + Map fields = retrieveFields(source, "float_range"); + assertThat(fields.size(), equalTo(1)); - DocumentField rangeField = hit.getFields().get("float_range"); + DocumentField rangeField = fields.get("float_range"); assertNotNull(rangeField); assertThat(rangeField.getValues().size(), equalTo(1)); assertThat(rangeField.getValue(), equalTo(Map.of("gte", 0.0, "lte", 2.718))); @@ -110,43 +108,25 @@ public void testFieldNamesWithWildcard() throws IOException { .endObject() .endObject(); - SearchHit hit = fetchFields(indexService, source, "*field"); - assertThat(hit.getFields().size(), equalTo(3)); + Map fields = retrieveFields(source, "*field"); + assertThat(fields.size(), equalTo(3)); - DocumentField field = hit.getFields().get("field"); + DocumentField field = fields.get("field"); assertNotNull(field); assertThat(field.getValues().size(), equalTo(2)); assertThat(field.getValues(), hasItems("first", "second")); - DocumentField otherField = hit.getFields().get("integer_field"); + DocumentField otherField = fields.get("integer_field"); assertNotNull(otherField); assertThat(otherField.getValues().size(), equalTo(1)); assertThat(otherField.getValues(), hasItems("third")); - DocumentField objectField = hit.getFields().get("object.field"); + DocumentField objectField = fields.get("object.field"); assertNotNull(objectField); assertThat(objectField.getValues().size(), equalTo(1)); assertThat(objectField.getValues(), hasItems("fourth")); } - public void testSourceDisabled() throws IOException { - XContentBuilder mapping = XContentFactory.jsonBuilder().startObject() - .startObject("_source").field("enabled", false).endObject() - .startObject("properties") - .startObject("field").field("type", "keyword").endObject() - .endObject() - .endObject(); - - IndexService sourceDisabledIndexService = createIndex("disabled-index", Settings.EMPTY, mapping); - XContentBuilder source = XContentFactory.jsonBuilder().startObject() - .field("field", "value") - .endObject(); - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> fetchFields(sourceDisabledIndexService, source, "field")); - assertThat(e.getMessage(), containsString("Unable to retrieve the requested [fields] since _source is disabled in the mapping")); - } - public void testObjectFields() throws IOException { XContentBuilder source = XContentFactory.jsonBuilder().startObject() .array("field", "first", "second") @@ -155,39 +135,19 @@ public void testObjectFields() throws IOException { .endObject() .endObject(); - SearchHit hit = fetchFields(indexService, source, "object"); - assertFalse(hit.getFields().containsKey("object")); + Map fields = retrieveFields(source, "object"); + assertFalse(fields.containsKey("object")); } - private SearchHit fetchFields(IndexService indexService, XContentBuilder source, String field) { - return fetchFields(indexService, source, List.of(field)); + private Map retrieveFields(XContentBuilder source, String fieldPattern) { + return retrieveFields(source, List.of(fieldPattern)); } - private SearchHit fetchFields(IndexService indexService, XContentBuilder source, List fields) { - FetchFieldsContext fetchFieldsContext = new FetchFieldsContext(fields); - SearchContext searchContext = new FetchFieldsTestSearchContext(indexService, fetchFieldsContext); - + private Map retrieveFields(XContentBuilder source, List fieldPatterns) { SourceLookup sourceLookup = new SourceLookup(); sourceLookup.setSource(BytesReference.bytes(source)); - FetchFieldsPhase phase = new FetchFieldsPhase(); - SearchHit searchHit = new SearchHit(1, null, null, null, null); - phase.hitsExecute(searchContext, hit -> sourceLookup, new SearchHit[]{ searchHit }); - return searchHit; - } - - private static class FetchFieldsTestSearchContext extends TestSearchContext { - private final FetchFieldsContext context; - - FetchFieldsTestSearchContext(IndexService indexService, - FetchFieldsContext context) { - super(indexService.getBigArrays(), indexService); - this.context = context; - } - - @Override - public FetchFieldsContext fetchFieldsContext() { - return context; - } + FieldValueRetriever fetchFieldsLookup = FieldValueRetriever.create(mapperService, fieldPatterns); + return fetchFieldsLookup.retrieve(sourceLookup); } }