Continue to accept unused 'universal' params in <8.0 indexes (#59381)

We have a number of parameters which are universally parsed by almost all
mappers, whether or not they make sense. Migrating the binary and boolean
mappers to the new style of declaring their parameters explicitly has meant
that these universal parameters stopped being accepted, which would break
existing mappings.

This commit adds some extra logic to ParametrizedFieldMapper that checks
for the existence of these universal parameters, and issues a warning on
7x indexes if it finds them. Indexes created in 8.0 and beyond will throw an
error.

Fixes #59359

server/src/main/java/org/elasticsearch/index/mapper/ParametrizedFieldMapper.java

@@ -19,18 +19,24 @@
 
 package org.elasticsearch.index.mapper;
 
+import org.apache.logging.log4j.LogManager;
 import org.apache.lucene.document.FieldType;
+import org.elasticsearch.Version;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.support.XContentMapValues;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 import java.util.function.BiFunction;
 import java.util.function.Function;
 
@@ -46,6 +52,9 @@ import java.util.function.Function;
  */
 public abstract class ParametrizedFieldMapper extends FieldMapper {
 
+    private static final DeprecationLogger deprecationLogger
+        = new DeprecationLogger(LogManager.getLogger(ParametrizedFieldMapper.class));
+
     /**
      * Creates a new ParametrizedFieldMapper
      */
@@ -330,6 +339,12 @@ public abstract class ParametrizedFieldMapper extends FieldMapper {
                 }
                 Parameter<?> parameter = paramsMap.get(propName);
                 if (parameter == null) {
+                    if (isDeprecatedParameter(propName, parserContext.indexVersionCreated())) {
+                        deprecationLogger.deprecatedAndMaybeLog(propName,
+                            "Parameter [{}] has no effect on type [{}] and will be removed in future", propName, type);
+                        iterator.remove();
+                        continue;
+                    }
                     throw new MapperParsingException("unknown parameter [" + propName
                         + "] on mapper [" + name + "] of type [" + type + "]");
                 }
@@ -341,5 +356,15 @@ public abstract class ParametrizedFieldMapper extends FieldMapper {
                 iterator.remove();
             }
         }
+
+        // These parameters were previously *always* parsed by TypeParsers#parseField(), even if they
+        // made no sense; if we've got here, that means that they're not declared on a current mapper,
+        // and so we emit a deprecation warning rather than failing a previously working mapping.
+        private static final Set<String> DEPRECATED_PARAMS
+            = new HashSet<>(Arrays.asList("store", "meta", "index", "doc_values", "boost", "index_options", "similarity"));
+
+        private static boolean isDeprecatedParameter(String propName, Version indexCreatedVersion) {
+            return DEPRECATED_PARAMS.contains(propName);
+        }
     }
 }

server/src/test/java/org/elasticsearch/index/mapper/ParametrizedMapperTests.java

@@ -67,6 +67,7 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
             = Parameter.boolParam("fixed2", false, m -> toType(m).fixed2, false);
         final Parameter<String> variable
             = Parameter.stringParam("variable", true, m -> toType(m).variable, "default").acceptsNull();
+        final Parameter<Boolean> index = Parameter.boolParam("index", false, m -> toType(m).index, true);
 
         protected Builder(String name) {
             super(name);
@@ -74,7 +75,7 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
 
         @Override
         protected List<Parameter<?>> getParameters() {
-            return Arrays.asList(fixed, fixed2, variable);
+            return Arrays.asList(fixed, fixed2, variable, index);
         }
 
         @Override
@@ -99,6 +100,7 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
         private final boolean fixed;
         private final boolean fixed2;
         private final String variable;
+        private final boolean index;
 
         protected TestMapper(String simpleName, String fullName, MultiFields multiFields, CopyTo copyTo,
                              ParametrizedMapperTests.Builder builder) {
@@ -106,6 +108,7 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
             this.fixed = builder.fixed.getValue();
             this.fixed2 = builder.fixed2.getValue();
             this.variable = builder.variable.getValue();
+            this.index = builder.index.getValue();
         }
 
         @Override
@@ -124,7 +127,7 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
         }
     }
 
-    private static TestMapper fromMapping(String mapping) {
+    private static TestMapper fromMapping(String mapping, Version version) {
         Mapper.TypeParser.ParserContext pc = new Mapper.TypeParser.ParserContext(s -> null, null, s -> {
             if (Objects.equals("keyword", s)) {
                 return new KeywordFieldMapper.TypeParser();
@@ -133,12 +136,16 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
                 return new BinaryFieldMapper.TypeParser();
             }
             return null;
-        }, Version.CURRENT, () -> null);
+        }, version, () -> null);
         return (TestMapper) new TypeParser()
             .parse("field", XContentHelper.convertToMap(JsonXContent.jsonXContent, mapping, true), pc)
             .build(new Mapper.BuilderContext(Settings.EMPTY, new ContentPath(0)));
     }
 
+    private static TestMapper fromMapping(String mapping) {
+        return fromMapping(mapping, Version.CURRENT);
+    }
+
     // defaults - create empty builder config, and serialize with and without defaults
     public void testDefaults() throws IOException {
         String mapping = "{\"type\":\"test_mapper\"}";
@@ -154,7 +161,8 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
         builder.startObject();
         mapper.toXContent(builder, params);
         builder.endObject();
-        assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":true,\"fixed2\":false,\"variable\":\"default\"}}",
+        assertEquals("{\"field\":{\"type\":\"test_mapper\",\"fixed\":true," +
+                "\"fixed2\":false,\"variable\":\"default\",\"index\":true}}",
             Strings.toString(builder));
     }
 
@@ -245,8 +253,15 @@ public class ParametrizedMapperTests extends ESSingleNodeTestCase {
 
         indexService.mapperService().merge("_doc", new CompressedXContent(mapping), MapperService.MergeReason.MAPPING_UPDATE);
         assertEquals(mapping, Strings.toString(indexService.mapperService().documentMapper()));
+    }
 
-
+    public void testDeprecatedParameters() throws IOException {
+        // 'index' is declared explicitly, 'store' is not, but is one of the previously always-accepted params
+        String mapping = "{\"type\":\"test_mapper\",\"index\":false,\"store\":true}";
+        TestMapper mapper = fromMapping(mapping, Version.V_7_8_0);
+        assertWarnings("Parameter [store] has no effect on type [test_mapper] and will be removed in future");
+        assertFalse(mapper.index);
+        assertEquals("{\"field\":{\"type\":\"test_mapper\",\"index\":false}}", Strings.toString(mapper));
     }
 
 }