Remove support for chained multi-fields. (#42333)

Follow-up to #41926, where we deprecated support for multi-fields within
multi-fields.

Addresses #41267.

Author: jtibshirani

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.

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);

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<String, Mapper.TypeParser> 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> 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.");
     }
 }

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));

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<String, Object> 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<String, Object> 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) {