From 70e63a365a658aaec360f9fb6b0616a381d76eb0 Mon Sep 17 00:00:00 2001 From: Mayya Sharipova Date: Mon, 8 Jun 2020 09:16:18 -0400 Subject: [PATCH] Refactor how to determine if a field is metafield (#57378) (#57771) 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 #38373, #41656 Closes #24422 --- .../RankFeatureMetaFieldMapperTests.java | 18 ++- .../PercolatorMatchedSlotSubFetchPhase.java | 10 +- .../index/rankeval/RatedSearchHitTests.java | 4 +- .../org/elasticsearch/get/GetActionIT.java | 3 - .../elasticsearch/get/LegacyGetActionIT.java | 4 - .../search/fetch/FetchSubPhasePluginIT.java | 5 +- .../search/fields/SearchFieldsIT.java | 3 - .../common/document/DocumentField.java | 8 -- .../elasticsearch/index/get/GetResult.java | 28 ++-- .../index/get/ShardGetService.java | 2 +- .../index/mapper/DocumentParser.java | 2 +- .../index/mapper/MapperService.java | 36 ++--- .../elasticsearch/indices/IndicesModule.java | 4 +- .../org/elasticsearch/search/SearchHit.java | 127 +++++------------- .../search/fetch/FetchPhase.java | 90 +++++++------ .../fetch/subphase/FetchDocValuesPhase.java | 10 +- .../fetch/subphase/ScriptFieldsPhase.java | 9 +- .../completion/CompletionSuggestion.java | 3 +- .../index/get/DocumentFieldTests.java | 18 +-- .../index/get/GetResultTests.java | 43 +++--- .../index/mapper/DocumentParserTests.java | 15 ++- .../elasticsearch/search/SearchHitTests.java | 21 +-- .../aggregations/AggregationsTests.java | 6 +- .../CompletionSuggestionOptionTests.java | 6 +- .../search/suggest/SuggestionEntryTests.java | 5 +- .../search/suggest/SuggestionTests.java | 5 +- .../test/InternalAggregationTestCase.java | 5 +- .../SecurityIndexReaderWrapperUnitTests.java | 4 +- .../scroll/ScrollDataExtractorTests.java | 3 +- .../xpack/ml/test/SearchHitBuilder.java | 5 +- .../extractor/ComputingExtractorTests.java | 5 +- .../extractor/FieldHitExtractorTests.java | 20 +-- 32 files changed, 232 insertions(+), 295 deletions(-) diff --git a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeatureMetaFieldMapperTests.java b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeatureMetaFieldMapperTests.java index f293e10f9e9ea..f855eb0fe2c8e 100644 --- a/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeatureMetaFieldMapperTests.java +++ b/modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/RankFeatureMetaFieldMapperTests.java @@ -20,8 +20,10 @@ package org.elasticsearch.index.mapper; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.IndexService; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.ESSingleNodeTestCase; @@ -44,7 +46,7 @@ public void setup() { protected Collection> getPlugins() { return pluginList(MapperExtrasPlugin.class); } - + public void testBasics() throws Exception { String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type") .startObject("properties").startObject("field").field("type", "rank_feature").endObject().endObject() @@ -55,4 +57,18 @@ public void testBasics() throws Exception { assertEquals(mapping, mapper.mappingSource().toString()); assertNotNull(mapper.metadataMapper(RankFeatureMetaFieldMapper.class)); } + + /** + * Check that meta-fields are picked from plugins (in this case MapperExtrasPlugin), + * and parsing of a document fails if the document contains these meta-fields. + */ + public void testDocumentParsingFailsOnMetaField() throws Exception { + String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("_doc").endObject().endObject()); + DocumentMapper mapper = parser.parse("_doc", new CompressedXContent(mapping)); + String rfMetaField = RankFeatureMetaFieldMapper.CONTENT_TYPE; + BytesReference bytes = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field(rfMetaField, 0).endObject()); + MapperParsingException e = expectThrows(MapperParsingException.class, () -> + mapper.parse(new SourceToParse("test", "_doc", "1", bytes, XContentType.JSON))); + assertTrue(e.getMessage().contains("Field ["+ rfMetaField + "] is a metadata field and cannot be added inside a document.")); + } } diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorMatchedSlotSubFetchPhase.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorMatchedSlotSubFetchPhase.java index bebaf856d1f98..5548cc90e1f68 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorMatchedSlotSubFetchPhase.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorMatchedSlotSubFetchPhase.java @@ -39,9 +39,7 @@ 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; @@ -104,13 +102,9 @@ static void innerHitsExecute(Query mainQuery, IndexSearcher indexSearcher, Searc continue; } - Map 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()))); + // _percolator_document_slot fields are document fields and should be under "fields" section in a hit + hit.setDocumentField(fieldName, new DocumentField(fieldName, slots.boxed().collect(Collectors.toList()))); } } } diff --git a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedSearchHitTests.java b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedSearchHitTests.java index 7d8121c0ee540..b615a3c019aae 100644 --- a/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedSearchHitTests.java +++ b/modules/rank-eval/src/test/java/org/elasticsearch/index/rankeval/RatedSearchHitTests.java @@ -34,7 +34,6 @@ import java.util.OptionalInt; import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; -import static org.elasticsearch.test.XContentTestUtils.insertRandomFields; public class RatedSearchHitTests extends ESTestCase { @@ -76,8 +75,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); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/get/GetActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/get/GetActionIT.java index 8be9a991d17e9..b2ab5e78e7ff3 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/get/GetActionIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/get/GetActionIT.java @@ -596,14 +596,12 @@ public void testGetFieldsComplexField() throws Exception { String field = "field1.field2.field3.field4"; GetResponse getResponse = client().prepareGet("my-index", "my-type", "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", "my-type", "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")); @@ -631,7 +629,6 @@ public void testGetFieldsComplexField() throws Exception { getResponse = client().prepareGet("my-index", "my-type", "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")); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/get/LegacyGetActionIT.java b/server/src/internalClusterTest/java/org/elasticsearch/get/LegacyGetActionIT.java index 9ade1d9106c4d..5f3e312e466b5 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/get/LegacyGetActionIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/get/LegacyGetActionIT.java @@ -63,9 +63,7 @@ public void testGetFieldsMetadataWithRouting() throws Exception { .setStoredFields("field1") .get(); assertThat(getResponse.isExists(), equalTo(true)); - assertThat(getResponse.getField("field1").isMetadataField(), equalTo(false)); assertThat(getResponse.getField("field1").getValue().toString(), equalTo("value")); - assertThat(getResponse.getField("_routing").isMetadataField(), equalTo(true)); assertThat(getResponse.getField("_routing").getValue().toString(), equalTo("1")); } @@ -78,9 +76,7 @@ public void testGetFieldsMetadataWithRouting() throws Exception { .setRouting("1") .get(); assertThat(getResponse.isExists(), equalTo(true)); - assertThat(getResponse.getField("field1").isMetadataField(), equalTo(false)); assertThat(getResponse.getField("field1").getValue().toString(), equalTo("value")); - assertThat(getResponse.getField("_routing").isMetadataField(), equalTo(true)); assertThat(getResponse.getField("_routing").getValue().toString(), equalTo("1")); } } diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java index b85aea575fe97..6e56974df8c40 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java @@ -125,13 +125,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().setDocumentField(NAME, hitField); } TermVectorsRequest termVectorsRequest = new TermVectorsRequest(context.indexShard().shardId().getIndex().getName(), hitContext.hit().getType(), hitContext.hit().getId()); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/fields/SearchFieldsIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/fields/SearchFieldsIT.java index 0c67e2968a7de..65502369dd186 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/fields/SearchFieldsIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/fields/SearchFieldsIT.java @@ -660,7 +660,6 @@ 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).field("_routing").getValue().toString(), equalTo("1")); } @@ -736,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")); @@ -1223,7 +1221,6 @@ public void testLoadMetadata() throws Exception { Map fields = response.getHits().getAt(0).getFields(); assertThat(fields.get("field1"), nullValue()); - assertThat(fields.get("_routing").isMetadataField(), equalTo(true)); assertThat(fields.get("_routing").getValue().toString(), equalTo("1")); } } diff --git a/server/src/main/java/org/elasticsearch/common/document/DocumentField.java b/server/src/main/java/org/elasticsearch/common/document/DocumentField.java index 61a5b829c5e48..93ef37ed4a295 100644 --- a/server/src/main/java/org/elasticsearch/common/document/DocumentField.java +++ b/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; @@ -87,13 +86,6 @@ public List getValues() { return values; } - /** - * @return The field is a metadata field - */ - public boolean isMetadataField() { - return MapperService.isMetadataField(name); - } - @Override public Iterator iterator() { return values.iterator(); diff --git a/server/src/main/java/org/elasticsearch/index/get/GetResult.java b/server/src/main/java/org/elasticsearch/index/get/GetResult.java index e9eba92db24f5..063e934d0386f 100644 --- a/server/src/main/java/org/elasticsearch/index/get/GetResult.java +++ b/server/src/main/java/org/elasticsearch/index/get/GetResult.java @@ -33,6 +33,7 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.mapper.IgnoredFieldMapper; +import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.SourceFieldMapper; import org.elasticsearch.search.lookup.SourceLookup; @@ -97,7 +98,8 @@ public GetResult(StreamInput in) throws IOException { Map 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(); @@ -363,7 +365,7 @@ public static GetResult fromXContentEmbedded(XContentParser parser, String index } else if (FOUND.equals(currentFieldName)) { found = parser.booleanValue(); } else { - metaFields.put(currentFieldName, new DocumentField(currentFieldName, + metaFields.put(currentFieldName, new DocumentField(currentFieldName, Collections.singletonList(parser.objectText()))); } } else if (token == XContentParser.Token.START_OBJECT) { @@ -401,10 +403,10 @@ public static GetResult fromXContent(XContentParser parser) throws IOException { } private Map readFields(StreamInput in) throws IOException { - Map fields = null; + Map 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++) { @@ -414,20 +416,6 @@ private Map readFields(StreamInput in) throws IOException } return fields; } - - static void splitFieldsByMetadata(Map fields, Map outOther, - Map outMetadata) { - if (fields == null) { - return; - } - for (Map.Entry 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 { @@ -446,11 +434,11 @@ public void writeTo(StreamOutput out) throws IOException { writeFields(out, documentFields); writeFields(out, metaFields); } else { - writeFields(out, this.getFields()); + writeFields(out, this.getFields()); } } } - + private void writeFields(StreamOutput out, Map fields) throws IOException { if (fields == null) { out.writeVInt(0); diff --git a/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java b/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java index cf92cab48e321..80b007124a55f 100644 --- a/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java +++ b/server/src/main/java/org/elasticsearch/index/get/ShardGetService.java @@ -285,7 +285,7 @@ private GetResult innerGetLoadFromStoredFields(String type, String id, String[] documentFields = new HashMap<>(); metadataFields = new HashMap<>(); for (Map.Entry> 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())); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 4de97fb4f68fd..b60e222122e68 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -404,7 +404,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)) { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java index 5580059fdb4d9..4d2954d914eaa 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/MapperService.java @@ -19,7 +19,6 @@ package org.elasticsearch.index.mapper; -import com.carrotsearch.hppc.ObjectHashSet; import com.carrotsearch.hppc.cursors.ObjectCursor; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.message.ParameterizedMessage; @@ -56,6 +55,7 @@ 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; @@ -63,7 +63,6 @@ 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; @@ -121,14 +120,6 @@ public enum MergeReason { Setting.boolSetting("index.mapper.dynamic", INDEX_MAPPER_DYNAMIC_DEFAULT, Property.Dynamic, Property.IndexScope, Property.Deprecated); - //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", "_routing", "_size", "_timestamp", "_ttl", "_type" - }; - - private static final ObjectHashSet META_FIELDS = ObjectHashSet.from(SORTED_META_FIELDS); - private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(MapperService.class)); static final String DEFAULT_MAPPING_ERROR_MESSAGE = "[_default_] mappings are not allowed on new indices and should no " + "longer be used. See [https://www.elastic.co/guide/en/elasticsearch/reference/current/breaking-changes-7.0.html" + @@ -146,6 +137,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; @@ -161,6 +153,7 @@ public MapperService(IndexSettings indexSettings, IndexAnalyzers indexAnalyzers, SimilarityService similarityService, MapperRegistry mapperRegistry, Supplier 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, @@ -812,14 +805,27 @@ public void close() throws IOException { } /** - * @return Whether a field is a metadata field. + * @return Whether a field is a metadata field + * Deserialization of SearchHit objects sent from pre 7.8 nodes and GetResults objects sent from pre 7.3 nodes, + * uses this method to divide fields into meta and document fields. + * 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; + } + // if a node had Size Plugin installed, _size field should also be considered a meta-field + 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); } /** diff --git a/server/src/main/java/org/elasticsearch/indices/IndicesModule.java b/server/src/main/java/org/elasticsearch/indices/IndicesModule.java index 583a856da4172..17a701b4bf33e 100644 --- a/server/src/main/java/org/elasticsearch/indices/IndicesModule.java +++ b/server/src/main/java/org/elasticsearch/indices/IndicesModule.java @@ -150,6 +150,8 @@ public static Map getMappers(List mappe private static final Map builtInMetadataMappers = initBuiltInMetadataMappers(); + private static Set builtInMetadataFields = Collections.unmodifiableSet(builtInMetadataMappers.keySet()); + private static Map initBuiltInMetadataMappers() { Map builtInMetadataMappers; // Use a LinkedHashMap for metadataMappers because iteration order matters @@ -206,7 +208,7 @@ public static Map getMetadataMappers(Lis * Returns a set containing all of the builtin metadata fields */ public static Set getBuiltInMetadataFields() { - return builtInMetadataMappers.keySet(); + return builtInMetadataFields; } private static Function> getFieldFilter(List mapperPlugins) { diff --git a/server/src/main/java/org/elasticsearch/search/SearchHit.java b/server/src/main/java/org/elasticsearch/search/SearchHit.java index 6063d192ace8a..a3afb7baa654b 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchHit.java +++ b/server/src/main/java/org/elasticsearch/search/SearchHit.java @@ -71,7 +71,6 @@ import static org.elasticsearch.common.xcontent.ConstructingObjectParser.optionalConstructorArg; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureFieldName; -import static org.elasticsearch.common.xcontent.XContentParserUtils.parseFieldsValue; /** * A single search hit. @@ -97,7 +96,7 @@ public final class SearchHit implements Writeable, ToXContentObject, Iterable documentFields; - private Map metaFields; + private final Map metaFields; private Map highlightFields = null; @@ -139,15 +138,8 @@ public SearchHit(int nestedTopDocId, String id, Text type, NestedIdentity nested } this.type = type; this.nestedIdentity = nestedIdentity; - this.documentFields = documentFields; - if (this.documentFields == null) { - this.documentFields = new HashMap<>(); - } - - this.metaFields = metaFields; - if (this.metaFields == null) { - this.metaFields = new HashMap<>(); - } + this.documentFields = documentFields == null ? emptyMap() : documentFields; + this.metaFields = metaFields == null ? emptyMap() : metaFields; } public SearchHit(StreamInput in) throws IOException { @@ -175,7 +167,8 @@ public SearchHit(StreamInput in) throws IOException { Map fields = readFields(in); documentFields = new HashMap<>(); metaFields = new HashMap<>(); - SearchHit.splitFieldsByMetadata(fields, documentFields, metaFields); + fields.forEach((fieldName, docField) -> + (MapperService.isMetadataFieldStatic(fieldName) ? metaFields : documentFields).put(fieldName, docField)); } int size = in.readVInt(); @@ -457,19 +450,21 @@ public Iterator iterator() { * The hit field matching the given field name. */ public DocumentField field(String fieldName) { - return getFields().get(fieldName); + DocumentField result = documentFields.get(fieldName); + if (result != null) { + return result; + } else { + return metaFields.get(fieldName); + } } /* * Adds a new DocumentField to the map in case both parameters are not null. * */ - public void setField(String fieldName, DocumentField field) { + public void setDocumentField(String fieldName, DocumentField field) { if (fieldName == null || field == null) return; - if (field.isMetadataField()) { - this.metaFields.put(fieldName, field); - } else { - this.documentFields.put(fieldName, field); - } + if (documentFields.size() == 0) this.documentFields = new HashMap<>(); + this.documentFields.put(fieldName, field); } /** @@ -477,27 +472,13 @@ public void setField(String fieldName, DocumentField field) { * were required to be loaded. */ public Map getFields() { - Map fields = new HashMap<>(); - fields.putAll(metaFields); - fields.putAll(documentFields); - return fields; - } - - // returns the fields without handling null cases - public Map fieldsOrNull() { - return getFields(); - } - - public void fields(Map fields) { - Objects.requireNonNull(fields); - this.metaFields = new HashMap<>(); - this.documentFields = new HashMap<>(); - for (Map.Entry fieldEntry: fields.entrySet()) { - if (fieldEntry.getValue().isMetadataField()) { - this.metaFields.put(fieldEntry.getKey(), fieldEntry.getValue()); - } else { - this.documentFields.put(fieldEntry.getKey(), fieldEntry.getValue()); - } + if (metaFields.size() > 0 || documentFields.size() > 0) { + final Map fields = new HashMap<>(); + fields.putAll(metaFields); + fields.putAll(documentFields); + return fields; + } else { + return emptyMap(); } } @@ -597,22 +578,6 @@ public void setInnerHits(Map innerHits) { this.innerHits = innerHits; } - public static void splitFieldsByMetadata(Map fields, - Map documentFields, - Map metaFields) { - // documentFields and metaFields must be non-empty maps - if (fields == null) { - return; - } - for (Map.Entry fieldEntry: fields.entrySet()) { - if (fieldEntry.getValue().isMetadataField()) { - metaFields.put(fieldEntry.getKey(), fieldEntry.getValue()); - } else { - documentFields.put(fieldEntry.getKey(), fieldEntry.getValue()); - } - } - } - public static class Fields { static final String _INDEX = "_index"; static final String _TYPE = "_type"; @@ -732,6 +697,18 @@ public XContentBuilder toInnerXContent(XContentBuilder builder, Params params) t return builder; } + // All fields on the root level of the parsed SearhHit are interpreted as metadata fields + // public because we use it in a completion suggestion option + public static final ObjectParser.UnknownFieldConsumer> unknownMetaFieldConsumer = (map, fieldName, fieldValue) -> { + Map fieldMap = (Map) map.computeIfAbsent( + METADATA_FIELDS, v -> new HashMap()); + if (fieldName.equals(IgnoredFieldMapper.NAME)) { + fieldMap.put(fieldName, new DocumentField(fieldName, (List) fieldValue)); + } else { + fieldMap.put(fieldName, new DocumentField(fieldName, Collections.singletonList(fieldValue))); + } + }; + /** * This parser outputs a temporary map of the objects needed to create the * SearchHit instead of directly creating the SearchHit. The reason for this @@ -742,7 +719,8 @@ public XContentBuilder toInnerXContent(XContentBuilder builder, Params params) t * of the included search hit. The output of the map is used to create the * actual SearchHit instance via {@link #createFromMap(Map)} */ - private static final ObjectParser, Void> MAP_PARSER = new ObjectParser<>("innerHitParser", true, HashMap::new); + private static final ObjectParser, Void> MAP_PARSER = new ObjectParser<>("innerHitParser", + unknownMetaFieldConsumer, HashMap::new); static { declareInnerHitsParseFields(MAP_PARSER); @@ -753,7 +731,6 @@ public static SearchHit fromXContent(XContentParser parser) { } public static void declareInnerHitsParseFields(ObjectParser, Void> parser) { - declareMetadataFields(parser); parser.declareString((map, value) -> map.put(Fields._TYPE, new Text(value)), new ParseField(Fields._TYPE)); parser.declareString((map, value) -> map.put(Fields._INDEX, value), new ParseField(Fields._INDEX)); parser.declareString((map, value) -> map.put(Fields._ID, value), new ParseField(Fields._ID)); @@ -852,40 +829,6 @@ private static BytesReference parseSourceBytes(XContentParser parser) throws IOE } } - /** - * we need to declare parse fields for each metadata field, except for _ID, _INDEX and _TYPE which are - * handled individually. All other fields are parsed to an entry in the fields map - */ - private static void declareMetadataFields(ObjectParser, Void> parser) { - /* TODO: This method and its usage in declareInnerHitsParseFields() must be replaced by - calling an UnknownFieldConsumer. All fields on the root level of the parsed SearhHit - should be interpreted as metadata fields. - */ - for (String metadatafield : MapperService.getAllMetaFields()) { - if (metadatafield.equals(Fields._ID) == false && metadatafield.equals(Fields._INDEX) == false - && metadatafield.equals(Fields._TYPE) == false) { - if (metadatafield.equals(IgnoredFieldMapper.NAME)) { - parser.declareObjectArray((map, list) -> { - @SuppressWarnings("unchecked") - Map fieldMap = (Map) map.computeIfAbsent(METADATA_FIELDS, - v -> new HashMap()); - DocumentField field = new DocumentField(metadatafield, list); - fieldMap.put(field.getName(), field); - }, (p, c) -> parseFieldsValue(p), - new ParseField(metadatafield)); - } else { - parser.declareField((map, field) -> { - @SuppressWarnings("unchecked") - Map fieldMap = (Map) map.computeIfAbsent(METADATA_FIELDS, - v -> new HashMap()); - fieldMap.put(field.getName(), field); - }, (p, c) -> new DocumentField(metadatafield, Collections.singletonList(parseFieldsValue(p))), - new ParseField(metadatafield), ValueType.VALUE); - } - } - } - } - private static Map parseFields(XContentParser parser) throws IOException { Map fields = new HashMap<>(); while (parser.nextToken() != XContentParser.Token.END_OBJECT) { diff --git a/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java index a4a32b3a7c513..3bd74558d5903 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/FetchPhase.java @@ -69,6 +69,7 @@ import java.util.Map; import java.util.Set; +import static java.util.Collections.emptyMap; /** * Fetch phase of a search request, used to fetch the actual top matching documents to be returned to the client, identified @@ -211,15 +212,17 @@ private SearchHit createSearchHit(SearchContext context, if (fieldsVisitor == null) { return new SearchHit(docId, null, typeText, null, null); } - - Map searchFields = getSearchFields(context, fieldsVisitor, subDocId, - storedToRequestedFields, subReaderContext); - - Map metaFields = new HashMap<>(); - Map documentFields = new HashMap<>(); - SearchHit.splitFieldsByMetadata(searchFields, documentFields, metaFields); - - SearchHit searchHit = new SearchHit(docId, fieldsVisitor.uid().id(), typeText, documentFields, metaFields); + loadStoredFields(context.shardTarget(), subReaderContext, fieldsVisitor, subDocId); + fieldsVisitor.postProcess(context.mapperService()); + SearchHit searchHit; + if (fieldsVisitor.fields().isEmpty() == false) { + Map docFields = new HashMap<>(); + Map metaFields = new HashMap<>(); + fillDocAndMetaFields(context, fieldsVisitor, storedToRequestedFields, docFields, metaFields); + searchHit = new SearchHit(docId, fieldsVisitor.uid().id(), typeText, docFields, metaFields); + } else { + searchHit = new SearchHit(docId, fieldsVisitor.uid().id(), typeText, emptyMap(), emptyMap()); + } // Set _source if requested. SourceLookup sourceLookup = context.lookup().source(); sourceLookup.setSegmentAndDocument(subReaderContext, subDocId); @@ -229,34 +232,6 @@ private SearchHit createSearchHit(SearchContext context, return searchHit; } - private Map getSearchFields(SearchContext context, - FieldsVisitor fieldsVisitor, - int subDocId, - Map> storedToRequestedFields, - LeafReaderContext subReaderContext) { - loadStoredFields(context.shardTarget(), subReaderContext, fieldsVisitor, subDocId); - fieldsVisitor.postProcess(context.mapperService()); - - if (fieldsVisitor.fields().isEmpty()) { - return null; - } - - Map searchFields = new HashMap<>(fieldsVisitor.fields().size()); - for (Map.Entry> entry : fieldsVisitor.fields().entrySet()) { - String storedField = entry.getKey(); - List storedValues = entry.getValue(); - - if (storedToRequestedFields.containsKey(storedField)) { - for (String requestedField : storedToRequestedFields.get(storedField)) { - searchFields.put(requestedField, new DocumentField(requestedField, storedValues)); - } - } else { - searchFields.put(storedField, new DocumentField(storedField, storedValues)); - } - } - return searchFields; - } - private SearchHit createNestedSearchHit(SearchContext context, int nestedTopDocId, int nestedSubDocId, @@ -281,11 +256,17 @@ private SearchHit createNestedSearchHit(SearchContext context, source = null; } - Map searchFields = null; + Map docFields = emptyMap(); + Map metaFields = emptyMap(); if (context.hasStoredFields() && !context.storedFieldsContext().fieldNames().isEmpty()) { FieldsVisitor nestedFieldsVisitor = new CustomFieldsVisitor(storedToRequestedFields.keySet(), false); - searchFields = getSearchFields(context, nestedFieldsVisitor, nestedSubDocId, - storedToRequestedFields, subReaderContext); + loadStoredFields(context.shardTarget(), subReaderContext, nestedFieldsVisitor, nestedSubDocId); + nestedFieldsVisitor.postProcess(context.mapperService()); + if (nestedFieldsVisitor.fields().isEmpty() == false) { + docFields = new HashMap<>(); + metaFields = new HashMap<>(); + fillDocAndMetaFields(context, nestedFieldsVisitor, storedToRequestedFields, docFields, metaFields); + } } DocumentMapper documentMapper = context.mapperService().documentMapper(); @@ -345,10 +326,7 @@ private SearchHit createNestedSearchHit(SearchContext context, XContentType contentType = tuple.v1(); context.lookup().source().setSourceContentType(contentType); } - Map metaFields = new HashMap<>(); - Map documentFields = new HashMap<>(); - SearchHit.splitFieldsByMetadata(searchFields, documentFields, metaFields); - return new SearchHit(nestedTopDocId, uid.id(), documentMapper.typeText(), nestedIdentity, documentFields, metaFields); + return new SearchHit(nestedTopDocId, uid.id(), documentMapper.typeText(), nestedIdentity, docFields, metaFields); } private SearchHit.NestedIdentity getInternalNestedIdentity(SearchContext context, int nestedSubDocId, @@ -435,4 +413,28 @@ private void loadStoredFields(SearchShardTarget shardTarget, LeafReaderContext r throw new FetchPhaseExecutionException(shardTarget, "Failed to fetch doc id [" + docId + "]", e); } } + + private static void fillDocAndMetaFields(SearchContext context, FieldsVisitor fieldsVisitor, + Map> storedToRequestedFields, + Map docFields, Map metaFields) { + for (Map.Entry> entry : fieldsVisitor.fields().entrySet()) { + String storedField = entry.getKey(); + List storedValues = entry.getValue(); + if (storedToRequestedFields.containsKey(storedField)) { + for (String requestedField : storedToRequestedFields.get(storedField)) { + if (context.mapperService().isMetadataField(requestedField)) { + metaFields.put(requestedField, new DocumentField(requestedField, storedValues)); + } else { + docFields.put(requestedField, new DocumentField(requestedField, storedValues)); + } + } + } else { + if (context.mapperService().isMetadataField(storedField)) { + metaFields.put(storedField, new DocumentField(storedField, storedValues)); + } else { + docFields.put(storedField, new DocumentField(storedField, storedValues)); + } + } + } + } } diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchDocValuesPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchDocValuesPhase.java index 76409b16fb73a..fc3f89fcc9545 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchDocValuesPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/FetchDocValuesPhase.java @@ -43,7 +43,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.Comparator; -import java.util.HashMap; import java.util.List; import java.util.Objects; @@ -143,13 +142,12 @@ public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOExcept binaryValues = data.getBytesValues(); } } - if (hit.fieldsOrNull() == null) { - hit.fields(new HashMap<>(2)); - } - DocumentField hitField = hit.getFields().get(field); + DocumentField hitField = hit.field(field); if (hitField == null) { hitField = new DocumentField(field, new ArrayList<>(2)); - hit.setField(field, hitField); + // even if we request a doc values of a meta-field (e.g. _routing), + // docValues fields will still be document fields, and put under "fields" section of a hit. + hit.setDocumentField(field, hitField); } final List values = hitField.getValues(); diff --git a/server/src/main/java/org/elasticsearch/search/fetch/subphase/ScriptFieldsPhase.java b/server/src/main/java/org/elasticsearch/search/fetch/subphase/ScriptFieldsPhase.java index affe1920f4817..281517a2fb3f8 100644 --- a/server/src/main/java/org/elasticsearch/search/fetch/subphase/ScriptFieldsPhase.java +++ b/server/src/main/java/org/elasticsearch/search/fetch/subphase/ScriptFieldsPhase.java @@ -34,7 +34,6 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; -import java.util.HashMap; import java.util.List; public final class ScriptFieldsPhase implements FetchSubPhase { @@ -72,11 +71,8 @@ public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOExcept } throw e; } - if (hit.fieldsOrNull() == null) { - hit.fields(new HashMap<>(2)); - } String scriptFieldName = scriptFields.get(i).name(); - DocumentField hitField = hit.getFields().get(scriptFieldName); + DocumentField hitField = hit.field(scriptFieldName); if (hitField == null) { final List values; if (value instanceof Collection) { @@ -85,7 +81,8 @@ public void hitsExecute(SearchContext context, SearchHit[] hits) throws IOExcept values = Collections.singletonList(value); } hitField = new DocumentField(scriptFieldName, values); - hit.setField(scriptFieldName, hitField); + // script fields are never meta-fields + hit.setDocumentField(scriptFieldName, hitField); } } diff --git a/server/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestion.java b/server/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestion.java index 02b0f988eb9a6..8545043444624 100644 --- a/server/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestion.java +++ b/server/src/main/java/org/elasticsearch/search/suggest/completion/CompletionSuggestion.java @@ -46,6 +46,7 @@ import java.util.Set; import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpectedToken; +import static org.elasticsearch.search.SearchHit.unknownMetaFieldConsumer; import static org.elasticsearch.search.suggest.Suggest.COMPARATOR; /** @@ -362,7 +363,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws } private static final ObjectParser, Void> PARSER = new ObjectParser<>("CompletionOptionParser", - true, HashMap::new); + unknownMetaFieldConsumer, HashMap::new); static { SearchHit.declareInnerHitsParseFields(PARSER); diff --git a/server/src/test/java/org/elasticsearch/index/get/DocumentFieldTests.java b/server/src/test/java/org/elasticsearch/index/get/DocumentFieldTests.java index e2e3d4df67cf7..691f0ed9de14c 100644 --- a/server/src/test/java/org/elasticsearch/index/get/DocumentFieldTests.java +++ b/server/src/test/java/org/elasticsearch/index/get/DocumentFieldTests.java @@ -26,11 +26,8 @@ import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; -import org.elasticsearch.index.mapper.IdFieldMapper; import org.elasticsearch.index.mapper.IgnoredFieldMapper; -import org.elasticsearch.index.mapper.IndexFieldMapper; -import org.elasticsearch.index.mapper.MapperService; -import org.elasticsearch.index.mapper.TypeFieldMapper; +import org.elasticsearch.indices.IndicesModule; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.RandomObjects; @@ -38,6 +35,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.function.Predicate; import java.util.function.Supplier; import static org.elasticsearch.common.xcontent.XContentHelper.toXContent; @@ -101,10 +99,14 @@ private static DocumentField mutateDocumentField(DocumentField documentField) { } public static Tuple randomDocumentField(XContentType xContentType) { - if (randomBoolean()) { - String metaField = randomValueOtherThanMany(field -> field.equals(TypeFieldMapper.NAME) - || field.equals(IndexFieldMapper.NAME) || field.equals(IdFieldMapper.NAME), - () -> randomFrom(MapperService.getAllMetaFields())); + return randomDocumentField(xContentType, randomBoolean(), fieldName -> false); // don't exclude any meta-fields + } + + public static Tuple randomDocumentField(XContentType xContentType, boolean isMetafield, + Predicate excludeMetaFieldFilter) { + if (isMetafield) { + String metaField = randomValueOtherThanMany(excludeMetaFieldFilter, + () -> randomFrom(IndicesModule.getBuiltInMetadataFields())); DocumentField documentField; if (metaField.equals(IgnoredFieldMapper.NAME)) { int numValues = randomIntBetween(1, 3); diff --git a/server/src/test/java/org/elasticsearch/index/get/GetResultTests.java b/server/src/test/java/org/elasticsearch/index/get/GetResultTests.java index 26f76111a6e62..ff4456786ae20 100644 --- a/server/src/test/java/org/elasticsearch/index/get/GetResultTests.java +++ b/server/src/test/java/org/elasticsearch/index/get/GetResultTests.java @@ -29,6 +29,12 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.index.mapper.IdFieldMapper; +import org.elasticsearch.index.mapper.IndexFieldMapper; +import org.elasticsearch.index.mapper.SeqNoFieldMapper; +import org.elasticsearch.index.mapper.SourceFieldMapper; +import org.elasticsearch.index.mapper.TypeFieldMapper; +import org.elasticsearch.index.mapper.VersionFieldMapper; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.RandomObjects; @@ -38,6 +44,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Predicate; import java.util.function.Supplier; import static java.util.Collections.singletonList; @@ -195,7 +202,7 @@ public static GetResult mutateGetResult(GetResult getResult) { RandomObjects.randomSource(random()), getResult.getFields(), null)); mutations.add(() -> new GetResult(getResult.getIndex(), getResult.getType(), getResult.getId(), getResult.getSeqNo(), getResult.getPrimaryTerm(), getResult.getVersion(), - getResult.isExists(), getResult.internalSourceRef(), randomDocumentFields(XContentType.JSON).v1(), null)); + getResult.isExists(), getResult.internalSourceRef(), randomDocumentFields(XContentType.JSON, randomBoolean()).v1(), null)); return randomFrom(mutations).get(); } @@ -208,8 +215,8 @@ public static Tuple randomGetResult(XContentType xContentT final long primaryTerm; final boolean exists; BytesReference source = null; - Map fields = null; - Map expectedFields = null; + Map docFields = null; + Map expectedDocFields = null; Map metaFields = null; Map expectedMetaFields = null; if (frequently()) { @@ -221,14 +228,13 @@ public static Tuple randomGetResult(XContentType xContentT source = RandomObjects.randomSource(random()); } if (randomBoolean()) { - Tuple, Map> tuple = randomDocumentFields(xContentType); - fields = new HashMap<>(); - metaFields = new HashMap<>(); - GetResult.splitFieldsByMetadata(tuple.v1(), fields, metaFields); + Tuple, Map> tuple = randomDocumentFields(xContentType, false); + docFields = tuple.v1(); + expectedDocFields = tuple.v2(); - expectedFields = new HashMap<>(); - expectedMetaFields = new HashMap<>(); - GetResult.splitFieldsByMetadata(tuple.v2(), expectedFields, expectedMetaFields); + tuple = randomDocumentFields(xContentType, true); + metaFields = tuple.v1(); + expectedMetaFields = tuple.v2(); } } else { seqNo = UNASSIGNED_SEQ_NO; @@ -236,18 +242,25 @@ public static Tuple randomGetResult(XContentType xContentT version = -1; exists = false; } - GetResult getResult = new GetResult(index, type, id, seqNo, primaryTerm, version, exists, source, fields, metaFields); + GetResult getResult = new GetResult(index, type, id, seqNo, primaryTerm, version, exists, source, docFields, metaFields); GetResult expectedGetResult = new GetResult(index, type, id, seqNo, primaryTerm, version, exists, source, - expectedFields, expectedMetaFields); + expectedDocFields, expectedMetaFields); return Tuple.tuple(getResult, expectedGetResult); } - public static Tuple,Map> randomDocumentFields(XContentType xContentType) { - int numFields = randomIntBetween(2, 10); + public static Tuple,Map> randomDocumentFields( + XContentType xContentType, boolean isMetaFields) { + int numFields = isMetaFields? randomIntBetween(1, 3) : randomIntBetween(2, 10); Map fields = new HashMap<>(numFields); Map 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 excludeMetaFieldFilter = field -> + field.equals(TypeFieldMapper.NAME) || field.equals(IndexFieldMapper.NAME) || + field.equals(IdFieldMapper.NAME) || field.equals(VersionFieldMapper.NAME) || + field.equals(SourceFieldMapper.NAME) || field.equals(SeqNoFieldMapper.NAME) ; while (fields.size() < numFields) { - Tuple tuple = randomDocumentField(xContentType); + Tuple tuple = randomDocumentField(xContentType, isMetaFields, excludeMetaFieldFilter); DocumentField getField = tuple.v1(); DocumentField expectedGetField = tuple.v2(); if (fields.putIfAbsent(getField.getName(), getField) == null) { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java index ebd89f993c4a7..f17ba56802f6e 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/DocumentParserTests.java @@ -1146,17 +1146,18 @@ public void testDynamicStrictDottedFieldNameObject() throws Exception { public void testDocumentContainsMetadataField() throws Exception { DocumentMapperParser mapperParser = createIndex("test").mapperService().documentMapperParser(); - String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type").endObject().endObject()); - DocumentMapper mapper = mapperParser.parse("type", new CompressedXContent(mapping)); + String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("_doc").endObject().endObject()); + DocumentMapper mapper = mapperParser.parse("_doc", new CompressedXContent(mapping)); - BytesReference bytes = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("_ttl", 0).endObject()); + BytesReference bytes = BytesReference.bytes(XContentFactory.jsonBuilder().startObject().field("_field_names", 0).endObject()); MapperParsingException e = expectThrows(MapperParsingException.class, () -> - mapper.parse(new SourceToParse("test", "type", "1", bytes, XContentType.JSON))); - assertTrue(e.getMessage(), e.getMessage().contains("cannot be added inside a document")); + mapper.parse(new SourceToParse("test", "_doc", "1", bytes, XContentType.JSON))); + assertTrue(e.getMessage(), + e.getMessage().contains("Field [_field_names] is a metadata field and cannot be added inside a document.")); BytesReference bytes2 = BytesReference.bytes(XContentFactory.jsonBuilder().startObject() - .field("foo._ttl", 0).endObject()); - mapper.parse(new SourceToParse("test", "type", "1", bytes2, XContentType.JSON)); // parses without error + .field("foo._field_names", 0).endObject()); + mapper.parse(new SourceToParse("test", "_doc", "1", bytes2, XContentType.JSON)); // parses without error } public void testSimpleMapper() throws Exception { diff --git a/server/src/test/java/org/elasticsearch/search/SearchHitTests.java b/server/src/test/java/org/elasticsearch/search/SearchHitTests.java index ba33bf3cd0cda..39ad8fac77cd0 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchHitTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchHitTests.java @@ -73,20 +73,12 @@ public static SearchHit createTestItem(XContentType xContentType, boolean withOp if (randomBoolean()) { nestedIdentity = NestedIdentityTests.createTestItem(randomIntBetween(0, 2)); } - Map fields = new HashMap<>(); + Map metaFields = new HashMap<>(); + Map documentFields = new HashMap<>(); if (frequently()) { - fields = new HashMap<>(); if (randomBoolean()) { - fields = GetResultTests.randomDocumentFields(xContentType).v2(); - } - } - HashMap metaFields = new HashMap<>(); - HashMap documentFields = new HashMap<>(); - for (Map.Entry fieldEntry: fields.entrySet()) { - if (fieldEntry.getValue().isMetadataField()) { - metaFields.put(fieldEntry.getKey(), fieldEntry.getValue()); - } else { - documentFields.put(fieldEntry.getKey(), fieldEntry.getValue()); + metaFields = GetResultTests.randomDocumentFields(xContentType, true).v2(); + documentFields = GetResultTests.randomDocumentFields(xContentType, false).v2(); } } @@ -184,14 +176,15 @@ public void testFromXContent() throws IOException { * objects allow arbitrary keys (the field names that are queries). Also we want to exclude * to add anything under "_source" since it is not parsed, and avoid complexity by excluding * everything under "inner_hits". They are also keyed by arbitrary names and contain SearchHits, - * which are already tested elsewhere. + * which are already tested elsewhere. We also exclude the root level, as all unknown fields + * on a root level are interpreted as meta-fields and will be kept. */ public void testFromXContentLenientParsing() throws IOException { XContentType xContentType = randomFrom(XContentType.values()); SearchHit searchHit = createTestItem(xContentType, true, true); BytesReference originalBytes = toXContent(searchHit, xContentType, true); Predicate pathsToExclude = path -> (path.endsWith("highlight") || path.endsWith("fields") || path.contains("_source") - || path.contains("inner_hits")); + || path.contains("inner_hits") || path.isEmpty()); BytesReference withRandomFields = insertRandomFields(xContentType, originalBytes, pathsToExclude, random()); SearchHit parsed; diff --git a/server/src/test/java/org/elasticsearch/search/aggregations/AggregationsTests.java b/server/src/test/java/org/elasticsearch/search/aggregations/AggregationsTests.java index 3408015cf7d3a..51c77a430c87b 100644 --- a/server/src/test/java/org/elasticsearch/search/aggregations/AggregationsTests.java +++ b/server/src/test/java/org/elasticsearch/search/aggregations/AggregationsTests.java @@ -229,6 +229,9 @@ private void parseAndAssert(boolean addRandomFields) throws IOException { * * - we cannot insert into ExtendedMatrixStats "covariance" or "correlation" fields, their syntax is strict * + * - we cannot insert random values in top_hits, as all unknown fields + * on a root level of SearchHit are interpreted as meta-fields and will be kept + * * - exclude "key", it can be an array of objects and we need strict values */ Predicate excludes = path -> (path.isEmpty() || path.endsWith("aggregations") @@ -236,7 +239,8 @@ private void parseAndAssert(boolean addRandomFields) throws IOException { || path.endsWith(Aggregation.CommonFields.BUCKETS.getPreferredName()) || path.endsWith(CommonFields.VALUES.getPreferredName()) || path.endsWith("covariance") || path.endsWith("correlation") || path.contains(CommonFields.VALUE.getPreferredName()) - || path.endsWith(CommonFields.KEY.getPreferredName())); + || path.endsWith(CommonFields.KEY.getPreferredName())) + || path.contains("top_hits"); mutated = insertRandomFields(xContentType, originalBytes, excludes, random()); } else { mutated = originalBytes; diff --git a/server/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestionOptionTests.java b/server/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestionOptionTests.java index 41b4d69ebf4e1..99a5bc211ecc1 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestionOptionTests.java +++ b/server/src/test/java/org/elasticsearch/search/suggest/CompletionSuggestionOptionTests.java @@ -85,9 +85,11 @@ private void doTestFromXContent(boolean addRandomFields) throws IOException { if (addRandomFields) { // "contexts" is an object consisting of key/array pairs, we shouldn't add anything random there // also there can be inner search hits fields inside this option, we need to exclude another couple of paths - // where we cannot add random stuff + // where we cannot add random stuff. We also exclude the root level, this is done for SearchHits as all unknown fields + // for SearchHit on a root level are interpreted as meta-fields and will be kept Predicate excludeFilter = (path) -> (path.endsWith(CompletionSuggestion.Entry.Option.CONTEXTS.getPreferredName()) - || path.endsWith("highlight") || path.endsWith("fields") || path.contains("_source") || path.contains("inner_hits")); + || path.endsWith("highlight") || path.endsWith("fields") || path.contains("_source") || path.contains("inner_hits") + || path.isEmpty()); mutated = insertRandomFields(xContentType, originalBytes, excludeFilter, random()); } else { mutated = originalBytes; diff --git a/server/src/test/java/org/elasticsearch/search/suggest/SuggestionEntryTests.java b/server/src/test/java/org/elasticsearch/search/suggest/SuggestionEntryTests.java index ac2aff2957fdd..8e7991a02f4f9 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/SuggestionEntryTests.java +++ b/server/src/test/java/org/elasticsearch/search/suggest/SuggestionEntryTests.java @@ -102,9 +102,12 @@ private void doTestFromXContent(boolean addRandomFields) throws IOException { // "contexts" is an object consisting of key/array pairs, we shouldn't add anything random there // also there can be inner search hits fields inside this option, we need to exclude another couple of paths // where we cannot add random stuff + // exclude "options" which contain SearchHits, + // on root level of SearchHit fields are interpreted as meta-fields and will be kept Predicate excludeFilter = ( path) -> (path.endsWith(CompletionSuggestion.Entry.Option.CONTEXTS.getPreferredName()) || path.endsWith("highlight") - || path.endsWith("fields") || path.contains("_source") || path.contains("inner_hits")); + || path.endsWith("fields") || path.contains("_source") || path.contains("inner_hits") + || path.contains("options")); mutated = insertRandomFields(xContentType, originalBytes, excludeFilter, random()); } else { mutated = originalBytes; diff --git a/server/src/test/java/org/elasticsearch/search/suggest/SuggestionTests.java b/server/src/test/java/org/elasticsearch/search/suggest/SuggestionTests.java index e53aab10b9d02..cf3d33559c1a4 100644 --- a/server/src/test/java/org/elasticsearch/search/suggest/SuggestionTests.java +++ b/server/src/test/java/org/elasticsearch/search/suggest/SuggestionTests.java @@ -122,9 +122,12 @@ private void doTestFromXContent(boolean addRandomFields) throws IOException { // - "contexts" is an object consisting of key/array pairs, we shouldn't add anything random there // - there can be inner search hits fields inside this option where we cannot add random stuff // - the root object should be excluded since it contains the named suggestion arrays + // We also exclude options that contain SearchHits, as all unknown fields + // on a root level of SearchHit are interpreted as meta-fields and will be kept. Predicate excludeFilter = path -> (path.isEmpty() || path.endsWith(CompletionSuggestion.Entry.Option.CONTEXTS.getPreferredName()) || path.endsWith("highlight") - || path.endsWith("fields") || path.contains("_source") || path.contains("inner_hits")); + || path.endsWith("fields") || path.contains("_source") || path.contains("inner_hits") + || path.contains("options")); mutated = insertRandomFields(xContentType, originalBytes, excludeFilter, random()); } else { mutated = originalBytes; diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalAggregationTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/InternalAggregationTestCase.java index a00456142c367..f53f40d89ef2a 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalAggregationTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalAggregationTestCase.java @@ -523,9 +523,12 @@ protected

P parseAndAssert(final InternalAggregati * objects, they are used with "keyed" aggregations and contain * named bucket objects. Any new named object on this level should * also be a bucket and be parsed as such. + * + * we also exclude top_hits that contain SearchHits, as all unknown fields + * on a root level of SearchHit are interpreted as meta-fields and will be kept. */ Predicate basicExcludes = path -> path.isEmpty() || path.endsWith(Aggregation.CommonFields.META.getPreferredName()) - || path.endsWith(Aggregation.CommonFields.BUCKETS.getPreferredName()); + || path.endsWith(Aggregation.CommonFields.BUCKETS.getPreferredName()) || path.contains("top_hits"); Predicate excludes = basicExcludes.or(excludePathsFromXContentInsertion()); mutated = insertRandomFields(xContentType, originalBytes, excludes, random()); } else { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperUnitTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperUnitTests.java index d8ff71c5ad86a..e6b6ec01221be 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperUnitTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperUnitTests.java @@ -14,11 +14,11 @@ import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.index.Index; import org.elasticsearch.index.mapper.FieldNamesFieldMapper; -import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.SeqNoFieldMapper; import org.elasticsearch.index.mapper.SourceFieldMapper; import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.indices.IndicesModule; import org.elasticsearch.license.XPackLicenseState; import org.elasticsearch.license.XPackLicenseState.Feature; import org.elasticsearch.script.ScriptService; @@ -45,7 +45,7 @@ public class SecurityIndexReaderWrapperUnitTests extends ESTestCase { private static final Set META_FIELDS; static { - final Set metaFields = new HashSet<>(Arrays.asList(MapperService.getAllMetaFields())); + final Set metaFields = new HashSet<>(IndicesModule.getBuiltInMetadataFields()); metaFields.add(SourceFieldMapper.NAME); metaFields.add(FieldNamesFieldMapper.NAME); metaFields.add(SeqNoFieldMapper.NAME); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java index b3142b6963a28..de4c4eb53534e 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/datafeed/extractor/scroll/ScrollDataExtractorTests.java @@ -505,12 +505,11 @@ private SearchResponse createSearchResponse(List timestamps, List when(searchResponse.getScrollId()).thenReturn(randomAlphaOfLength(1000)); List hits = new ArrayList<>(); for (int i = 0; i < timestamps.size(); i++) { - SearchHit hit = new SearchHit(randomInt()); Map fields = new HashMap<>(); fields.put(extractedFields.timeField(), new DocumentField("time", Collections.singletonList(timestamps.get(i)))); fields.put("field_1", new DocumentField("field_1", Collections.singletonList(field1Values.get(i)))); fields.put("field_2", new DocumentField("field_2", Collections.singletonList(field2Values.get(i)))); - hit.fields(fields); + SearchHit hit = new SearchHit(randomInt(), null, null, fields, null); hits.add(hit); } SearchHits searchHits = new SearchHits(hits.toArray(new SearchHit[0]), diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/test/SearchHitBuilder.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/test/SearchHitBuilder.java index af4be50a13453..a112836baa953 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/test/SearchHitBuilder.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/test/SearchHitBuilder.java @@ -23,8 +23,8 @@ public class SearchHitBuilder { private final Map fields; public SearchHitBuilder(int docId) { - hit = new SearchHit(docId); fields = new HashMap<>(); + hit = new SearchHit(docId, null, null, fields, null); } public SearchHitBuilder addField(String name, Object value) { @@ -42,9 +42,6 @@ public SearchHitBuilder setSource(String sourceJson) { } public SearchHit build() { - if (!fields.isEmpty()) { - hit.fields(fields); - } return hit; } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java index d81b31db5c95e..dc75e1cebcc4d 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/ComputingExtractorTests.java @@ -80,10 +80,9 @@ public void testGet() { for (int i = 0; i < times; i++) { double value = randomDouble(); double expected = Math.log(value); - SearchHit hit = new SearchHit(1); DocumentField field = new DocumentField(fieldName, singletonList(value)); - hit.fields(singletonMap(fieldName, field)); + SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, field), null); assertEquals(expected, extractor.process(hit)); } } -} \ No newline at end of file +} diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java index e52ef992d547f..712676f4c98ff 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractorTests.java @@ -92,9 +92,8 @@ public void testGetDottedValueWithDocValues() { documentFieldValues.add(randomValue()); } - SearchHit hit = new SearchHit(1); DocumentField field = new DocumentField(fieldName, documentFieldValues); - hit.fields(singletonMap(fieldName, field)); + SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, field), null); Object result = documentFieldValues.isEmpty() ? null : documentFieldValues.get(0); assertEquals(result, extractor.extract(hit)); } @@ -153,9 +152,8 @@ public void testGetDocValue() { if (randomBoolean()) { documentFieldValues.add(randomValue()); } - SearchHit hit = new SearchHit(1); DocumentField field = new DocumentField(fieldName, documentFieldValues); - hit.fields(singletonMap(fieldName, field)); + SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, field), null); Object result = documentFieldValues.isEmpty() ? null : documentFieldValues.get(0); assertEquals(result, extractor.extract(hit)); } @@ -165,9 +163,8 @@ public void testGetDate() { ZoneId zoneId = randomZone(); long millis = 1526467911780L; List documentFieldValues = Collections.singletonList(Long.toString(millis)); - SearchHit hit = new SearchHit(1); DocumentField field = new DocumentField("my_date_field", documentFieldValues); - hit.fields(singletonMap("my_date_field", field)); + SearchHit hit = new SearchHit(1, null, null, singletonMap("my_date_field", field), null); FieldHitExtractor extractor = new FieldHitExtractor("my_date_field", DATETIME, zoneId, true); assertEquals(DateUtils.asDateTime(millis, zoneId), extractor.extract(hit)); } @@ -204,9 +201,8 @@ public void testToString() { public void testMultiValuedDocValue() { String fieldName = randomAlphaOfLength(5); FieldHitExtractor fe = getFieldHitExtractor(fieldName, true); - SearchHit hit = new SearchHit(1); DocumentField field = new DocumentField(fieldName, asList("a", "b")); - hit.fields(singletonMap(fieldName, field)); + SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, field), null); QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extract(hit)); assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported")); } @@ -563,9 +559,8 @@ public void testMultipleGeoPointExtractionFromSource() throws IOException { public void testGeoPointExtractionFromDocValues() { String fieldName = randomAlphaOfLength(5); FieldHitExtractor fe = new FieldHitExtractor(fieldName, GEO_POINT, UTC, true); - SearchHit hit = new SearchHit(1); DocumentField field = new DocumentField(fieldName, singletonList("2, 1")); - hit.fields(singletonMap(fieldName, field)); + SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, field), null); assertEquals(new GeoShape(1, 2), fe.extract(hit)); hit = new SearchHit(1); assertNull(fe.extract(hit)); @@ -573,10 +568,9 @@ public void testGeoPointExtractionFromDocValues() { public void testGeoPointExtractionFromMultipleDocValues() { String fieldName = randomAlphaOfLength(5); - SearchHit hit = new SearchHit(1); FieldHitExtractor fe = new FieldHitExtractor(fieldName, GEO_POINT, UTC, true); - - hit.fields(singletonMap(fieldName, new DocumentField(fieldName, Arrays.asList("2,1", "3,4")))); + SearchHit hit = new SearchHit(1, null, null, singletonMap(fieldName, + new DocumentField(fieldName, Arrays.asList("2,1", "3,4"))), null); QlIllegalArgumentException ex = expectThrows(QlIllegalArgumentException.class, () -> fe.extract(hit)); assertThat(ex.getMessage(), is("Arrays (returned by [" + fieldName + "]) are not supported"));