Deprecate support for chained multi-fields. (elastic#41926)

jtibshirani · web-flow · commit f5e54b495dba · 2019-05-21T12:48:13.000-07:00
We now issue a deprecation warning if a multi-field definition contains a `[fields]` entry. This PR also simplifies the definition of `MultiFieldParserContext`. Addresses elastic#41267.
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java b/server/src/main/java/org/elasticsearch/index/mapper/Mapper.java
@@ -136,17 +136,17 @@ public Supplier<QueryShardContext> queryShardContextSupplier() {
             protected Function<String, SimilarityProvider> similarityLookupService() { return similarityLookupService; }
 
             public ParserContext createMultiFieldContext(ParserContext in) {
-                return new MultiFieldParserContext(in) {
-                    @Override
-                    public boolean isWithinMultiField() { return true; }
-                };
+                return new MultiFieldParserContext(in);
             }
 
             static class MultiFieldParserContext extends ParserContext {
                 MultiFieldParserContext(ParserContext in) {
                     super(in.type(), in.similarityLookupService(), in.mapperService(), in.typeParsers(),
                             in.indexVersionCreated(), in.queryShardContextSupplier());
                 }
+
+                @Override
+                public boolean isWithinMultiField() { return true; }
             }
 
         }
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java b/server/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java
@@ -19,8 +19,10 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.apache.logging.log4j.LogManager;
 import org.apache.lucene.index.IndexOptions;
 import org.elasticsearch.ElasticsearchParseException;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.time.DateFormatter;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 import org.elasticsearch.index.analysis.AnalysisMode;
@@ -37,6 +39,7 @@
 import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeStringValue;
 
 public class TypeParsers {
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(LogManager.getLogger(TypeParsers.class));
 
     public static final String DOC_VALUES = "doc_values";
     public static final String INDEX_OPTIONS_DOCS = "docs";
@@ -214,11 +217,18 @@ public static void parseField(FieldMapper.Builder builder, String name, Map<Stri
 
     public static boolean parseMultiField(FieldMapper.Builder builder, String name, Mapper.TypeParser.ParserContext parserContext,
                                           String propName, Object propNode) {
-        parserContext = parserContext.createMultiFieldContext(parserContext);
         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.");
+            }
 
-            final Map<String, Object> multiFieldsPropNodes;
+            parserContext = parserContext.createMultiFieldContext(parserContext);
 
+            final Map<String, Object> multiFieldsPropNodes;
             if (propNode instanceof List && ((List<?>) propNode).isEmpty()) {
                 multiFieldsPropNodes = Collections.emptyMap();
             } else if (propNode instanceof Map) {
diff --git a/server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/ExternalFieldMapperTests.java
@@ -169,6 +169,12 @@ public void testExternalValuesWithMultifield() throws Exception {
 
         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 {
@@ -234,5 +240,11 @@ public void testExternalValuesWithMultifieldTwoLevels() throws Exception {
 
         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/TypeParsersTests.java b/server/src/test/java/org/elasticsearch/index/mapper/TypeParsersTests.java
@@ -24,7 +24,11 @@
 import org.apache.lucene.analysis.standard.StandardAnalyzer;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
+import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentFactory;
+import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.analysis.AbstractTokenFilterFactory;
 import org.elasticsearch.index.analysis.AnalysisMode;
@@ -36,6 +40,7 @@
 import org.elasticsearch.index.analysis.TokenFilterFactory;
 import org.elasticsearch.test.ESTestCase;
 
+import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
@@ -157,6 +162,38 @@ public void testParseTextFieldCheckAnalyzerWithSearchAnalyzerAnalysisMode() {
         TypeParsers.parseTextField(builder, "name", new HashMap<>(fieldNode), parserContext);
     }
 
+    public void testMultiFieldWithinMultiField() throws IOException {
+        TextFieldMapper.Builder builder = new TextFieldMapper.Builder("textField");
+
+        XContentBuilder mapping = XContentFactory.jsonBuilder().startObject()
+            .field("type", "keyword")
+            .startObject("fields")
+                .startObject("sub-field")
+                    .field("type", "keyword")
+                    .startObject("fields")
+                        .startObject("sub-sub-field")
+                            .field("type", "keyword")
+                        .endObject()
+                    .endObject()
+                .endObject()
+            .endObject()
+        .endObject();
+
+        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.");
+    }
+
     private Analyzer createAnalyzerWithMode(String name, AnalysisMode mode) {
         TokenFilterFactory tokenFilter = new AbstractTokenFilterFactory(indexSettings, name, Settings.EMPTY) {
             @Override

Original file line number	Diff line number	Diff line change
`@@ -136,17 +136,17 @@ public Supplier<QueryShardContext> queryShardContextSupplier() {`
`136`	`136`	`protected Function<String, SimilarityProvider> similarityLookupService() { return similarityLookupService; }`
`137`	`137`
`138`	`138`	`public ParserContext createMultiFieldContext(ParserContext in) {`
`139`		`- return new MultiFieldParserContext(in) {`
`140`		`- @Override`
`141`		`- public boolean isWithinMultiField() { return true; }`
`142`		`- };`
	`139`	`+ return new MultiFieldParserContext(in);`
`143`	`140`	`}`
`144`	`141`
`145`	`142`	`static class MultiFieldParserContext extends ParserContext {`
`146`	`143`	`MultiFieldParserContext(ParserContext in) {`
`147`	`144`	`super(in.type(), in.similarityLookupService(), in.mapperService(), in.typeParsers(),`
`148`	`145`	`in.indexVersionCreated(), in.queryShardContextSupplier());`
`149`	`146`	`}`
	`147`	`+`
	`148`	`+ @Override`
	`149`	`+ public boolean isWithinMultiField() { return true; }`
`150`	`150`	`}`
`151`	`151`
`152`	`152`	`}`