Continue to accept unused 'universal' params in <8.0 indexes

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

@@ -20,6 +20,8 @@
 package org.elasticsearch.index.mapper;
 
 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;
@@ -31,6 +33,7 @@
 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 +49,8 @@
  */
 public abstract class ParametrizedFieldMapper extends FieldMapper {
 
+    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(ParametrizedFieldMapper.class);
+
     /**
      * Creates a new ParametrizedFieldMapper
      */
@@ -330,6 +335,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.deprecate(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 +352,18 @@ 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
+            = Set.of("store", "meta", "index", "doc_values", "boost", "index_options", "similarity");
+
+        private static boolean isDeprecatedParameter(String propName, Version indexCreatedVersion) {
+            if (indexCreatedVersion.onOrAfter(Version.V_8_0_0)) {
+                return false;
+            }
+            return DEPRECATED_PARAMS.contains(propName);
+        }
     }
 }

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

@@ -65,14 +65,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 List.of(fixed, fixed2, variable);
+            return List.of(fixed, fixed2, variable, index);
         }
 
         @Override
@@ -97,13 +98,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
@@ -122,7 +125,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();
@@ -131,12 +134,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\"}";
@@ -152,7 +159,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));
     }
 
@@ -243,8 +251,18 @@ 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));
 
+        MapperParsingException e = expectThrows(MapperParsingException.class, () -> fromMapping(mapping, Version.V_8_0_0));
+        assertEquals("unknown parameter [store] on mapper [field] of type [test_mapper]", e.getMessage());
     }
 
 }