From f07b90f3c351fb352d2058229b50fbf94442c07d Mon Sep 17 00:00:00 2001 From: Julie Tibshirani Date: Tue, 28 May 2019 09:49:40 -0700 Subject: [PATCH] Remove support for chained multi-fields. (#42333) Follow-up to #41926, where we deprecated support for multi-fields within multi-fields. Addresses #41267. --- .../migration/migrate_8_0/mappings.asciidoc | 9 ++ .../index/mapper/TypeParsers.java | 18 ++-- .../mapper/ExternalFieldMapperTests.java | 89 ------------------- .../ExternalValuesMapperIntegrationIT.java | 10 +-- .../index/mapper/TypeParsersTests.java | 38 +++++--- 5 files changed, 52 insertions(+), 112 deletions(-) diff --git a/docs/reference/migration/migrate_8_0/mappings.asciidoc b/docs/reference/migration/migrate_8_0/mappings.asciidoc index 371e9fc44c415..16e75473885c6 100644 --- a/docs/reference/migration/migrate_8_0/mappings.asciidoc +++ b/docs/reference/migration/migrate_8_0/mappings.asciidoc @@ -14,3 +14,12 @@ The number of completion contexts within a single completion field has been limited to 10. + +[float] +==== Defining multi-fields within multi-fields + +Previously, it was possible to define a multi-field within a multi-field. +Defining chained multi-fields was deprecated in 7.3 and is now no longer +supported. To migrate the mappings, all instances of `fields` that occur within +a `fields` block should be removed, either by flattening the chained `fields` +blocks into a single level, or by switching to `copy_to` if appropriate. \ No newline at end of file diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java b/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java index 9848a23cac11b..12c80361a855c 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java @@ -22,6 +22,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.lucene.index.IndexOptions; import org.elasticsearch.ElasticsearchParseException; +import org.elasticsearch.Version; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.time.DateFormatter; import org.elasticsearch.common.xcontent.support.XContentMapValues; @@ -219,11 +220,18 @@ public static boolean parseMultiField(FieldMapper.Builder builder, String name, String propName, Object propNode) { if (propName.equals("fields")) { if (parserContext.isWithinMultiField()) { - deprecationLogger.deprecatedAndMaybeLog("multifield_within_multifield", "At least one multi-field, [" + name + "], was " + - "encountered that itself contains a multi-field. Defining multi-fields within a multi-field is deprecated and will " + - "no longer be supported in 8.0. To resolve the issue, all instances of [fields] that occur within a [fields] block " + - "should be removed from the mappings, either by flattening the chained [fields] blocks into a single level, or " + - "switching to [copy_to] if appropriate."); + // For indices created prior to 8.0, we only emit a deprecation warning and do not fail type parsing. This is to + // maintain the backwards-compatibility guarantee that we can always load indexes from the previous major version. + if (parserContext.indexVersionCreated().before(Version.V_8_0_0)) { + deprecationLogger.deprecatedAndMaybeLog("multifield_within_multifield", "At least one multi-field, [" + name + "], " + + "was encountered that itself contains a multi-field. Defining multi-fields within a multi-field is deprecated " + + "and is not supported for indices created in 8.0 and later. To migrate the mappings, all instances of [fields] " + + "that occur within a [fields] block should be removed from the mappings, either by flattening the chained " + + "[fields] blocks into a single level, or switching to [copy_to] if appropriate."); + } else { + throw new IllegalArgumentException("Encountered a multi-field [" + name + "] which itself contains a multi-field. " + + "Defining chained multi-fields is not supported."); + } } parserContext = parserContext.createMultiFieldContext(parserContext); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java index e5d3040f7a3bc..5515603db3476 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java @@ -20,7 +20,6 @@ package org.elasticsearch.index.mapper; import org.apache.lucene.index.IndexableField; -import org.apache.lucene.util.BytesRef; import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.Strings; @@ -132,12 +131,6 @@ public void testExternalValuesWithMultifield() throws Exception { .startObject("field") .field("type", "text") .field("store", true) - .startObject("fields") - .startObject("raw") - .field("type", "keyword") - .field("store", true) - .endObject() - .endObject() .endObject() .endObject() .endObject() @@ -164,87 +157,5 @@ public void testExternalValuesWithMultifield() throws Exception { IndexableField field = doc.rootDoc().getField("field.field"); assertThat(field, notNullValue()); assertThat(field.stringValue(), is("foo")); - - IndexableField raw = doc.rootDoc().getField("field.field.raw"); - - assertThat(raw, notNullValue()); - assertThat(raw.binaryValue(), is(new BytesRef("foo"))); - - assertWarnings("At least one multi-field, [field], was " + - "encountered that itself contains a multi-field. Defining multi-fields within a multi-field is deprecated and will " + - "no longer be supported in 8.0. To resolve the issue, all instances of [fields] that occur within a [fields] block " + - "should be removed from the mappings, either by flattening the chained [fields] blocks into a single level, or " + - "switching to [copy_to] if appropriate."); - } - - public void testExternalValuesWithMultifieldTwoLevels() throws Exception { - IndexService indexService = createIndex("test"); - Map mapperParsers = new HashMap<>(); - mapperParsers.put(ExternalMapperPlugin.EXTERNAL, new ExternalMapper.TypeParser(ExternalMapperPlugin.EXTERNAL, "foo")); - mapperParsers.put(ExternalMapperPlugin.EXTERNAL_BIS, new ExternalMapper.TypeParser(ExternalMapperPlugin.EXTERNAL, "bar")); - mapperParsers.put(TextFieldMapper.CONTENT_TYPE, new TextFieldMapper.TypeParser()); - MapperRegistry mapperRegistry = new MapperRegistry(mapperParsers, Collections.emptyMap(), MapperPlugin.NOOP_FIELD_FILTER); - - Supplier queryShardContext = () -> { - return indexService.newQueryShardContext(0, null, () -> { throw new UnsupportedOperationException(); }, null); - }; - DocumentMapperParser parser = new DocumentMapperParser(indexService.getIndexSettings(), indexService.mapperService(), - indexService.xContentRegistry(), indexService.similarityService(), mapperRegistry, queryShardContext); - - DocumentMapper documentMapper = parser.parse("type", new CompressedXContent( - Strings - .toString(XContentFactory.jsonBuilder().startObject().startObject("type").startObject("properties") - .startObject("field") - .field("type", ExternalMapperPlugin.EXTERNAL) - .startObject("fields") - .startObject("field") - .field("type", "text") - .startObject("fields") - .startObject("generated") - .field("type", ExternalMapperPlugin.EXTERNAL_BIS) - .endObject() - .startObject("raw") - .field("type", "text") - .endObject() - .endObject() - .endObject() - .startObject("raw") - .field("type", "text") - .endObject() - .endObject() - .endObject() - .endObject().endObject().endObject()))); - - ParsedDocument doc = documentMapper.parse(new SourceToParse("test", "type", "1", BytesReference - .bytes(XContentFactory.jsonBuilder() - .startObject() - .field("field", "1234") - .endObject()), - XContentType.JSON)); - - assertThat(doc.rootDoc().getField("field.bool"), notNullValue()); - assertThat(doc.rootDoc().getField("field.bool").stringValue(), is("T")); - - assertThat(doc.rootDoc().getField("field.point"), notNullValue()); - - assertThat(doc.rootDoc().getField("field.shape"), notNullValue()); - - assertThat(doc.rootDoc().getField("field.field"), notNullValue()); - assertThat(doc.rootDoc().getField("field.field").stringValue(), is("foo")); - - assertThat(doc.rootDoc().getField("field.field.generated.generated"), notNullValue()); - assertThat(doc.rootDoc().getField("field.field.generated.generated").stringValue(), is("bar")); - - assertThat(doc.rootDoc().getField("field.field.raw"), notNullValue()); - assertThat(doc.rootDoc().getField("field.field.raw").stringValue(), is("foo")); - - assertThat(doc.rootDoc().getField("field.raw"), notNullValue()); - assertThat(doc.rootDoc().getField("field.raw").stringValue(), is("foo")); - - assertWarnings("At least one multi-field, [field], was " + - "encountered that itself contains a multi-field. Defining multi-fields within a multi-field is deprecated and will " + - "no longer be supported in 8.0. To resolve the issue, all instances of [fields] that occur within a [fields] block " + - "should be removed from the mappings, either by flattening the chained [fields] blocks into a single level, or " + - "switching to [copy_to] if appropriate."); } } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ExternalValuesMapperIntegrationIT.java b/server/src/test/java/org/elasticsearch/index/mapper/ExternalValuesMapperIntegrationIT.java index 6d47e4a784e06..7e7764d9514fe 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/ExternalValuesMapperIntegrationIT.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/ExternalValuesMapperIntegrationIT.java @@ -139,14 +139,8 @@ public void testExternalValuesWithMultifield() throws Exception { .field("type", ExternalMapperPlugin.EXTERNAL_UPPER) .startObject("fields") .startObject("g") - .field("type", "text") + .field("type", "keyword") .field("store", true) - .startObject("fields") - .startObject("raw") - .field("type", "keyword") - .field("store", true) - .endObject() - .endObject() .endObject() .endObject() .endObject() @@ -156,7 +150,7 @@ public void testExternalValuesWithMultifield() throws Exception { refresh(); SearchResponse response = client().prepareSearch("test-idx") - .setQuery(QueryBuilders.termQuery("f.g.raw", "FOO BAR")) + .setQuery(QueryBuilders.termQuery("f.g", "FOO BAR")) .execute().actionGet(); assertThat(response.getHits().getTotalHits().value, equalTo((long) 1)); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/TypeParsersTests.java b/server/src/test/java/org/elasticsearch/index/mapper/TypeParsersTests.java index 70f469b96370c..b0fbc3618ed2f 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/TypeParsersTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/TypeParsersTests.java @@ -39,6 +39,7 @@ import org.elasticsearch.index.analysis.NamedAnalyzer; import org.elasticsearch.index.analysis.TokenFilterFactory; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.VersionUtils; import java.io.IOException; import java.util.Collections; @@ -48,6 +49,7 @@ import static org.elasticsearch.index.analysis.AnalysisRegistry.DEFAULT_ANALYZER_NAME; import static org.elasticsearch.index.analysis.AnalysisRegistry.DEFAULT_SEARCH_ANALYZER_NAME; import static org.elasticsearch.index.analysis.AnalysisRegistry.DEFAULT_SEARCH_QUOTED_ANALYZER_NAME; +import static org.hamcrest.core.IsEqual.equalTo; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -179,19 +181,35 @@ public void testMultiFieldWithinMultiField() throws IOException { .endObject() .endObject(); + Mapper.TypeParser typeParser = new KeywordFieldMapper.TypeParser(); + + // For indices created prior to 8.0, we should only emit a warning and not fail parsing. Map fieldNode = XContentHelper.convertToMap( BytesReference.bytes(mapping), true, mapping.contentType()).v2(); - Mapper.TypeParser typeParser = new KeywordFieldMapper.TypeParser(); - Mapper.TypeParser.ParserContext parserContext = new Mapper.TypeParser.ParserContext("type", - null, null, type -> typeParser, Version.CURRENT, null); - - TypeParsers.parseField(builder, "some-field", fieldNode, parserContext); - assertWarnings("At least one multi-field, [sub-field], was " + - "encountered that itself contains a multi-field. Defining multi-fields within a multi-field is deprecated and will " + - "no longer be supported in 8.0. To resolve the issue, all instances of [fields] that occur within a [fields] block " + - "should be removed from the mappings, either by flattening the chained [fields] blocks into a single level, or " + - "switching to [copy_to] if appropriate."); + Version olderVersion = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); + Mapper.TypeParser.ParserContext olderContext = new Mapper.TypeParser.ParserContext("type", + null, null, type -> typeParser, olderVersion, null); + + TypeParsers.parseField(builder, "some-field", fieldNode, olderContext); + assertWarnings("At least one multi-field, [sub-field], " + + "was encountered that itself contains a multi-field. Defining multi-fields within a multi-field is deprecated " + + "and is not supported for indices created in 8.0 and later. To migrate the mappings, all instances of [fields] " + + "that occur within a [fields] block should be removed from the mappings, either by flattening the chained " + + "[fields] blocks into a single level, or switching to [copy_to] if appropriate."); + + // For indices created in 8.0 or later, we should throw an error. + Map fieldNodeCopy = XContentHelper.convertToMap( + BytesReference.bytes(mapping), true, mapping.contentType()).v2(); + + Version version = VersionUtils.randomVersionBetween(random(), Version.V_8_0_0, Version.CURRENT); + Mapper.TypeParser.ParserContext context = new Mapper.TypeParser.ParserContext("type", + null, null, type -> typeParser, version, null); + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, + () -> TypeParsers.parseField(builder, "some-field", fieldNodeCopy, context)); + assertThat(e.getMessage(), equalTo("Encountered a multi-field [sub-field] which itself contains a " + + "multi-field. Defining chained multi-fields is not supported.")); } private Analyzer createAnalyzerWithMode(String name, AnalysisMode mode) {