Re-add support for some metadata field parameters (#118825)

cbuescher · web-flow · commit cc69e06974a8 · 2024-12-18T22:35:55.000+01:00
We removed support for type, fields, copy_to and boost in metadata field definitions with #116944 but with the move towards supporting N-2 read-only indices we need to add them back. This change reverts previous removal commits and adapts tests to also check we now throw errors for newly created indices.
diff --git a/docs/changelog/118825.yaml b/docs/changelog/118825.yaml
@@ -1,11 +1,11 @@
-pr: 116944
+pr: 118825
 summary: "Remove support for type, fields, `copy_to` and boost in metadata field definition"
 area: Mapping
 type: breaking
 issues: []
 breaking:
   title: "Remove support for type, fields, copy_to and boost in metadata field definition"
   area: Mapping
-  details: The type, fields, copy_to and boost parameters are no longer supported in metadata field definition
+  details: The type, fields, copy_to and boost parameters are no longer supported in metadata field definition starting with version 9.
   impact: Users providing type, fields, copy_to or boost as part of metadata field definition should remove them from their mappings.
   notable: false
diff --git a/server/src/main/java/org/elasticsearch/index/mapper/MetadataFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/MetadataFieldMapper.java
@@ -10,13 +10,17 @@
 package org.elasticsearch.index.mapper;
 
 import org.elasticsearch.common.Explicit;
+import org.elasticsearch.common.logging.DeprecationCategory;
 import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
+import org.elasticsearch.index.IndexVersion;
+import org.elasticsearch.index.IndexVersions;
 import org.elasticsearch.xcontent.XContentBuilder;
 
 import java.io.IOException;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.Set;
 import java.util.function.Function;
 
 /**
@@ -132,6 +136,8 @@ public final MetadataFieldMapper build(MapperBuilderContext context) {
             return build();
         }
 
+        private static final Set<String> UNSUPPORTED_PARAMETERS_8_6_0 = Set.of("type", "fields", "copy_to", "boost");
+
         public final void parseMetadataField(String name, MappingParserContext parserContext, Map<String, Object> fieldNode) {
             final Parameter<?>[] params = getParameters();
             Map<String, Parameter<?>> paramsMap = Maps.newHashMapWithExpectedSize(params.length);
@@ -144,6 +150,22 @@ public final void parseMetadataField(String name, MappingParserContext parserCon
                 final Object propNode = entry.getValue();
                 Parameter<?> parameter = paramsMap.get(propName);
                 if (parameter == null) {
+                    IndexVersion indexVersionCreated = parserContext.indexVersionCreated();
+                    if (indexVersionCreated.before(IndexVersions.UPGRADE_TO_LUCENE_10_0_0)
+                        && UNSUPPORTED_PARAMETERS_8_6_0.contains(propName)) {
+                        if (indexVersionCreated.onOrAfter(IndexVersions.V_8_6_0)) {
+                            // silently ignore type, and a few other parameters: sadly we've been doing this for a long time
+                            deprecationLogger.warn(
+                                DeprecationCategory.API,
+                                propName,
+                                "Parameter [{}] has no effect on metadata field [{}] and will be removed in future",
+                                propName,
+                                name
+                            );
+                        }
+                        iterator.remove();
+                        continue;
+                    }
                     throw new MapperParsingException("unknown parameter [" + propName + "] on metadata field [" + name + "]");
                 }
                 parameter.parse(name, parserContext, propNode);
diff --git a/test/framework/src/main/java/org/elasticsearch/index/KnownIndexVersions.java b/test/framework/src/main/java/org/elasticsearch/index/KnownIndexVersions.java
@@ -19,6 +19,7 @@ public class KnownIndexVersions {
      * A sorted list of all known index versions
      */
     public static final List<IndexVersion> ALL_VERSIONS = List.copyOf(IndexVersions.getAllVersions());
+
     /**
      * A sorted list of all known index versions that can be written to
      */
diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MetadataMapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MetadataMapperTestCase.java
@@ -12,6 +12,7 @@
 import org.elasticsearch.common.compress.CompressedXContent;
 import org.elasticsearch.core.CheckedConsumer;
 import org.elasticsearch.index.IndexVersion;
+import org.elasticsearch.index.IndexVersions;
 import org.elasticsearch.index.mapper.MapperService.MergeReason;
 import org.elasticsearch.test.index.IndexVersionUtils;
 import org.elasticsearch.xcontent.XContentBuilder;
@@ -142,4 +143,88 @@ public final void testFixedMetaFieldsAreNotConfigurable() throws IOException {
         );
         assertEquals("Failed to parse mapping: " + fieldName() + " is not configurable", exception.getMessage());
     }
+
+    public void testTypeAndFriendsAreAcceptedBefore_8_6_0() throws IOException {
+        assumeTrue("Metadata field " + fieldName() + " isn't configurable", isConfigurable());
+        IndexVersion previousVersion = IndexVersionUtils.getPreviousVersion(IndexVersions.V_8_6_0);
+        // we randomly also pick read-only versions to test that we can still parse the parameters for them
+        IndexVersion version = IndexVersionUtils.randomVersionBetween(
+            random(),
+            IndexVersionUtils.getLowestReadCompatibleVersion(),
+            previousVersion
+        );
+        assumeTrue("Metadata field " + fieldName() + " is not supported on version " + version, isSupportedOn(version));
+        MapperService mapperService = createMapperService(version, mapping(b -> {}));
+        // these parameters were previously silently ignored, they will still be ignored in existing indices
+        String[] unsupportedParameters = new String[] { "fields", "copy_to", "boost", "type" };
+        for (String param : unsupportedParameters) {
+            String mappingAsString = "{\n"
+                + "    \"_doc\" : {\n"
+                + "      \""
+                + fieldName()
+                + "\" : {\n"
+                + "        \""
+                + param
+                + "\" : \"any\"\n"
+                + "      }\n"
+                + "    }\n"
+                + "}";
+            assertNotNull(mapperService.parseMapping("_doc", MergeReason.MAPPING_UPDATE, new CompressedXContent(mappingAsString)));
+        }
+    }
+
+    public void testTypeAndFriendsAreDeprecatedFrom_8_6_0_TO_9_0_0() throws IOException {
+        assumeTrue("Metadata field " + fieldName() + " isn't configurable", isConfigurable());
+        IndexVersion previousVersion = IndexVersionUtils.getPreviousVersion(IndexVersions.UPGRADE_TO_LUCENE_10_0_0);
+        IndexVersion version = IndexVersionUtils.randomVersionBetween(random(), IndexVersions.V_8_6_0, previousVersion);
+        assumeTrue("Metadata field " + fieldName() + " is not supported on version " + version, isSupportedOn(version));
+        MapperService mapperService = createMapperService(version, mapping(b -> {}));
+        // these parameters were deprecated, they now should throw an error in new indices
+        String[] unsupportedParameters = new String[] { "fields", "copy_to", "boost", "type" };
+        for (String param : unsupportedParameters) {
+            String mappingAsString = "{\n"
+                + "    \"_doc\" : {\n"
+                + "      \""
+                + fieldName()
+                + "\" : {\n"
+                + "        \""
+                + param
+                + "\" : \"any\"\n"
+                + "      }\n"
+                + "    }\n"
+                + "}";
+            assertNotNull(mapperService.parseMapping("_doc", MergeReason.MAPPING_UPDATE, new CompressedXContent(mappingAsString)));
+            assertWarnings("Parameter [" + param + "] has no effect on metadata field [" + fieldName() + "] and will be removed in future");
+        }
+    }
+
+    public void testTypeAndFriendsThrow_After_9_0_0() throws IOException {
+        assumeTrue("Metadata field " + fieldName() + " isn't configurable", isConfigurable());
+        IndexVersion version = IndexVersionUtils.randomVersionBetween(
+            random(),
+            IndexVersions.UPGRADE_TO_LUCENE_10_0_0,
+            IndexVersion.current()
+        );
+        assumeTrue("Metadata field " + fieldName() + " is not supported on version " + version, isSupportedOn(version));
+        MapperService mapperService = createMapperService(version, mapping(b -> {}));
+        // these parameters were previously silently ignored, they are now deprecated in new indices
+        String[] unsupportedParameters = new String[] { "fields", "copy_to", "boost", "type" };
+        for (String param : unsupportedParameters) {
+            String mappingAsString = "{\n"
+                + "    \"_doc\" : {\n"
+                + "      \""
+                + fieldName()
+                + "\" : {\n"
+                + "        \""
+                + param
+                + "\" : \"any\"\n"
+                + "      }\n"
+                + "    }\n"
+                + "}";
+            expectThrows(
+                MapperParsingException.class,
+                () -> mapperService.parseMapping("_doc", MergeReason.MAPPING_UPDATE, new CompressedXContent(mappingAsString))
+            );
+        }
+    }
 }
