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 @@
  */
 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 final void parse(String name, TypeParser.ParserContext parserContext, Map
                 }
                 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 final void parse(String name, TypeParser.ParserContext parserContext, Map
                 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,14 +67,15 @@ public static class Builder extends ParametrizedFieldMapper.Builder {
             = 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);
         }
 
         @Override
         protected List<Parameter<?>> getParameters() {
-            return Arrays.asList(fixed, fixed2, variable);
+            return Arrays.asList(fixed, fixed2, variable, index);
         }
 
         @Override
@@ -99,13 +100,15 @@ public static class TestMapper extends ParametrizedFieldMapper {
         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) {
             super(simpleName, new KeywordFieldMapper.KeywordFieldType(fullName), multiFields, copyTo);
             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 @@ protected String contentType() {
         }
     }
 
-    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 @@ private static TestMapper fromMapping(String mapping) {
                 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 void testDefaults() throws IOException {
         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 void testObjectSerialization() throws IOException {
 
         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));
     }
 
 }