disallow creating geo_shape mappings with deprecated parameters

server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoFilterIT.java

@@ -200,8 +200,9 @@ public void testShapeRelations() throws Exception {
                 .endObject()
                 .endObject());
 
+        final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
         CreateIndexRequestBuilder mappingRequest = client().admin().indices().prepareCreate("shapes")
-            .setMapping(mapping);
+            .setMapping(mapping).setSettings(settings(version).build());
         mappingRequest.get();
         client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID).setWaitForGreenStatus().get();
 

server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java

@@ -7,6 +7,7 @@
  */
 package org.elasticsearch.search.geo;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.cluster.ClusterState;
@@ -22,6 +23,7 @@
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.indices.IndicesService;
 import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.test.VersionUtils;
 
 import static org.elasticsearch.index.query.QueryBuilders.geoShapeQuery;
 import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
@@ -32,6 +34,11 @@
 
 public class GeoShapeIntegrationIT extends ESIntegTestCase {
 
+    @Override
+    protected boolean forbidPrivateIndexSettings() {
+        return false;
+    }
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal) {
         return Settings.builder()
@@ -125,25 +132,25 @@ public void testIgnoreMalformed() throws Exception {
         assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
     }
 
-    public void testMappingUpdate() throws Exception {
+    public void testMappingUpdate() {
         // create index
-        assertAcked(client().admin().indices().prepareCreate("test")
-            .setMapping("shape", "type=geo_shape").get());
+        final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
+        assertAcked(client().admin().indices().prepareCreate("test").setSettings(settings(version).build())
+            .setMapping("shape", "type=geo_shape,strategy=recursive").get());
         ensureGreen();
 
         String update ="{\n" +
             "  \"properties\": {\n" +
             "    \"shape\": {\n" +
-            "      \"type\": \"geo_shape\",\n" +
-            "      \"strategy\": \"recursive\"\n" +
+            "      \"type\": \"geo_shape\"" +
             "    }\n" +
             "  }\n" +
             "}";
 
         IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().indices()
             .preparePutMapping("test")
             .setSource(update, XContentType.JSON).get());
-        assertThat(e.getMessage(), containsString("mapper [shape] of type [geo_shape] cannot change strategy from [BKD] to [recursive]"));
+        assertThat(e.getMessage(), containsString("mapper [shape] of type [geo_shape] cannot change strategy from [recursive] to [BKD]"));
     }
 
     /**
@@ -205,7 +212,9 @@ public void testIndexPolygonDateLine() throws Exception {
         assertAcked(client().admin().indices().prepareCreate("vector").setMapping(mappingVector).get());
         ensureGreen();
 
-        assertAcked(client().admin().indices().prepareCreate("quad").setMapping(mappingQuad).get());
+        final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
+        assertAcked(client().admin().indices().prepareCreate("quad")
+            .setSettings(settings(version).build()).setMapping(mappingQuad).get());
         ensureGreen();
 
         String source = "{\n" +

server/src/internalClusterTest/java/org/elasticsearch/search/geo/LegacyGeoShapeIntegrationIT.java

@@ -8,6 +8,7 @@
 package org.elasticsearch.search.geo;
 
 import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.Version;
 import org.elasticsearch.action.admin.cluster.settings.ClusterUpdateSettingsRequest;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.cluster.ClusterState;
@@ -24,6 +25,7 @@
 import org.elasticsearch.index.mapper.MappedFieldType;
 import org.elasticsearch.indices.IndicesService;
 import org.elasticsearch.test.ESIntegTestCase;
+import org.elasticsearch.test.VersionUtils;
 
 import java.io.IOException;
 
@@ -35,10 +37,16 @@
 
 public class LegacyGeoShapeIntegrationIT extends ESIntegTestCase {
 
+    @Override
+    protected boolean forbidPrivateIndexSettings() {
+        return false;
+    }
+
     /**
      * Test that orientation parameter correctly persists across cluster restart
      */
     public void testOrientationPersistence() throws Exception {
+        final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
         String idxName = "orientation";
         String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject()
                 .startObject("properties").startObject("location")
@@ -49,7 +57,7 @@ public void testOrientationPersistence() throws Exception {
                 .endObject().endObject());
 
         // create index
-        assertAcked(prepareCreate(idxName).setMapping(mapping));
+        assertAcked(prepareCreate(idxName).setMapping(mapping).setSettings(settings(version).build()));
 
         mapping = Strings.toString(XContentFactory.jsonBuilder().startObject()
                 .startObject("properties").startObject("location")
@@ -59,7 +67,7 @@ public void testOrientationPersistence() throws Exception {
                 .endObject()
                 .endObject().endObject());
 
-        assertAcked(prepareCreate(idxName+"2").setMapping(mapping));
+        assertAcked(prepareCreate(idxName+"2").setMapping(mapping).setSettings(settings(version).build()));
         ensureGreen(idxName, idxName+"2");
 
         internalCluster().fullRestart();
@@ -95,7 +103,8 @@ public void testOrientationPersistence() throws Exception {
      */
     public void testIgnoreMalformed() throws Exception {
         // create index
-        assertAcked(client().admin().indices().prepareCreate("test")
+        final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
+        assertAcked(prepareCreate("test").setSettings(settings(version).build())
             .setMapping("shape", "type=geo_shape,tree=quadtree,ignore_malformed=true").get());
         ensureGreen();
 
@@ -136,9 +145,9 @@ public void testIndexShapeRouting() throws Exception {
             "    }\n" +
             "  }}";
 
-
+        final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
         // create index
-        assertAcked(client().admin().indices().prepareCreate("test").setMapping(mapping).get());
+        assertAcked(prepareCreate("test").setSettings(settings(version).build()).setMapping(mapping).get());
         ensureGreen();
 
         String source = "{\n" +
@@ -162,7 +171,8 @@ public void testIndexShapeRouting() throws Exception {
      */
     public void testLegacyCircle() throws Exception {
         // create index
-        assertAcked(client().admin().indices().prepareCreate("test")
+        final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
+        assertAcked(prepareCreate("test").setSettings(settings(version).build())
             .setMapping("shape", "type=geo_shape,strategy=recursive,tree=geohash").get());
         ensureGreen();
 
@@ -181,9 +191,10 @@ public void testLegacyCircle() throws Exception {
     }
 
     public void testDisallowExpensiveQueries() throws InterruptedException, IOException {
+        final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
         try {
             // create index
-            assertAcked(client().admin().indices().prepareCreate("test")
+            assertAcked(client().admin().indices().prepareCreate("test").setSettings(settings(version).build())
                     .setMapping("shape", "type=geo_shape,strategy=recursive,tree=geohash").get());
             ensureGreen();
 

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

@@ -25,6 +25,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * FieldMapper for indexing {@link LatLonShape}s.
@@ -133,11 +134,13 @@ public Query geoShapeQuery(Geometry shape, String fieldName, ShapeRelation relat
         boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(parserContext.getSettings());
         boolean coerceByDefault = COERCE_SETTING.get(parserContext.getSettings());
         if (LegacyGeoShapeFieldMapper.containsDeprecatedParameter(node.keySet())) {
+            Set<String> deprecatedParams = LegacyGeoShapeFieldMapper.getDeprecatedParameters(node.keySet());
             builder = new LegacyGeoShapeFieldMapper.Builder(
                 name,
                 parserContext.indexVersionCreated(),
                 ignoreMalformedByDefault,
-                coerceByDefault);
+                coerceByDefault,
+                deprecatedParams);
         } else {
             builder = new Builder(name, ignoreMalformedByDefault, coerceByDefault);
         }

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

@@ -44,6 +44,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * FieldMapper for indexing {@link org.locationtech.spatial4j.shape.Shape}s.
@@ -79,6 +80,10 @@ public static boolean containsDeprecatedParameter(Set<String> paramKeys) {
         return DEPRECATED_PARAMETERS.stream().anyMatch(paramKeys::contains);
     }
 
+    public static Set<String> getDeprecatedParameters(Set<String> paramKeys) {
+        return DEPRECATED_PARAMETERS.stream().filter((p) -> paramKeys.contains(p)).collect(Collectors.toSet());
+    }
+
     public static class Defaults {
         public static final SpatialStrategy STRATEGY = SpatialStrategy.RECURSIVE;
         public static final String TREE = "quadtree";
@@ -158,15 +163,18 @@ public static class Builder extends FieldMapper.Builder {
         Parameter<Map<String, String>> meta = Parameter.metaParam();
 
         private final Version indexCreatedVersion;
+        private final Set<String> deprecatedParam;
 
-        public Builder(String name, Version version, boolean ignoreMalformedByDefault, boolean coerceByDefault) {
+        public Builder(String name, Version version, boolean ignoreMalformedByDefault, boolean coerceByDefault,
+                       Set<String> deprecatedParams) {
             super(name);
 
             if (ShapesAvailability.JTS_AVAILABLE == false || ShapesAvailability.SPATIAL4J_AVAILABLE == false) {
                 throw new ElasticsearchParseException("Non-BKD field parameters are not supported for [{}] field type", CONTENT_TYPE);
             }
 
             this.indexCreatedVersion = version;
+            this.deprecatedParam = deprecatedParams;
             this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault);
             this.coerce = coerceParam(m -> builder(m).coerce.get(), coerceByDefault);
 
@@ -443,6 +451,7 @@ public PrefixTreeStrategy resolvePrefixTreeStrategy(String strategyName) {
     }
 
     private final Version indexCreatedVersion;
+    private final Set<String> deprecatedParams;
     private final Builder builder;
 
     public LegacyGeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType,
@@ -452,7 +461,12 @@ public LegacyGeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldT
         super(simpleName, mappedFieldType, Collections.singletonMap(mappedFieldType.name(), Lucene.KEYWORD_ANALYZER),
             builder.ignoreMalformed.get(), builder.coerce.get(), builder.ignoreZValue.get(), builder.orientation.get(),
             multiFields, copyTo, indexer, parser);
+        if (builder.indexCreatedVersion.onOrAfter(Version.V_8_0_0)) {
+            throw new IllegalArgumentException("using deprecated parameters " + Arrays.toString(builder.deprecatedParam.toArray())
+                + " in mapper [" + name() + "] of type [geo_shape] is no longer allowed");
+        }
         this.indexCreatedVersion = builder.indexCreatedVersion;
+        this.deprecatedParams = builder.deprecatedParam;
         this.builder = builder;
     }
 
@@ -488,7 +502,7 @@ protected String contentType() {
     @Override
     public FieldMapper.Builder getMergeBuilder() {
         return new Builder(simpleName(), indexCreatedVersion,
-            builder.ignoreMalformed.getDefaultValue().value(), builder.coerce.getDefaultValue().value()).init(this);
+            builder.ignoreMalformed.getDefaultValue().value(), builder.coerce.getDefaultValue().value(), deprecatedParams).init(this);
     }
 
     @Override

server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java

@@ -292,7 +292,8 @@ public void testParse3DPolygon() throws IOException, ParseException {
         LinearRing shell = GEOMETRY_FACTORY.createLinearRing(shellCoordinates.toArray(new Coordinate[shellCoordinates.size()]));
         Polygon expected = GEOMETRY_FACTORY.createPolygon(shell, null);
         final LegacyGeoShapeFieldMapper mapperBuilder =
-            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).build(new ContentPath());
+            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true, Collections.singleton("strategy"))
+                .build(new ContentPath());
         try (XContentParser parser = createParser(polygonGeoJson)) {
             parser.nextToken();
             ElasticsearchGeoAssertions.assertEquals(jtsGeom(expected), ShapeParser.parse(parser, mapperBuilder).buildS4J());

server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java

@@ -326,7 +326,8 @@ public void testParseMixedDimensionPolyWithHoleStoredZ() throws IOException {
         parser.nextToken();
 
         final LegacyGeoShapeFieldMapper mapperBuilder =
-            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).build(new ContentPath());
+            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true, Collections.singleton("strategy"))
+                .build(new ContentPath());
 
         // test store z disabled
         ElasticsearchException e = expectThrows(ElasticsearchException.class,
@@ -349,7 +350,8 @@ public void testParsePolyWithStoredZ() throws IOException {
         parser.nextToken();
 
         final LegacyGeoShapeFieldMapper mapperBuilder =
-            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).build(new ContentPath());
+            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true, Collections.singleton("strategy"))
+                .build(new ContentPath());
 
         ShapeBuilder<?, ?, ?> shapeBuilder = ShapeParser.parse(parser, mapperBuilder);
         assertEquals(shapeBuilder.numDimensions(), 3);
@@ -363,13 +365,15 @@ public void testParseOpenPolygon() throws IOException {
         parser.nextToken();
 
         final LegacyGeoShapeFieldMapper defaultMapperBuilder =
-            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).coerce(false).build(new ContentPath());
+            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true, Collections.singleton("strategy"))
+                .coerce(false).build(new ContentPath());
         ElasticsearchParseException exception = expectThrows(ElasticsearchParseException.class,
             () -> ShapeParser.parse(parser, defaultMapperBuilder));
         assertEquals("invalid LinearRing found (coordinates are not closed)", exception.getMessage());
 
         final LegacyGeoShapeFieldMapper coercingMapperBuilder =
-            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).coerce(true).build(new ContentPath());
+            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true, Collections.singleton("strategy"))
+                .coerce(true).build(new ContentPath());
         ShapeBuilder<?, ?, ?> shapeBuilder = ShapeParser.parse(parser, coercingMapperBuilder);
         assertNotNull(shapeBuilder);
         assertEquals("polygon ((100.0 5.0, 100.0 10.0, 90.0 10.0, 90.0 5.0, 100.0 5.0))", shapeBuilder.toWKT());

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

@@ -297,9 +297,7 @@ public Function<String, Predicate<String>> getFieldFilter() {
             "              \"type\": \"geo_point\"\n" +
             "            },\n" +
             "            \"area_visible\": {\n" +
-            "              \"type\": \"geo_shape\",  \n" +
-            "              \"tree\": \"quadtree\",\n" +
-            "              \"precision\": \"1m\"\n" +
+            "              \"type\": \"geo_shape\"" +
             "            }\n" +
             "          }\n" +
             "        },\n" +

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

@@ -13,6 +13,7 @@
 import org.apache.lucene.spatial.prefix.tree.GeohashPrefixTree;
 import org.apache.lucene.spatial.prefix.tree.QuadPrefixTree;
 import org.elasticsearch.ElasticsearchException;
+import org.elasticsearch.Version;
 import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.geo.GeoUtils;
@@ -25,6 +26,7 @@
 import org.elasticsearch.index.query.SearchExecutionContext;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.TestGeoShapeFieldMapperPlugin;
+import org.elasticsearch.test.VersionUtils;
 
 import java.io.IOException;
 import java.util.Collection;
@@ -98,6 +100,24 @@ protected boolean supportsMeta() {
         return false;
     }
 
+    @Override
+    protected MapperService createMapperService(XContentBuilder mappings) throws IOException {
+        Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
+        return createMapperService(version, mappings);
+    }
+
+    public void testInvalidCurrentVersion() {
+        MapperParsingException e =
+            expectThrows(MapperParsingException.class,
+                () -> createMapperService(Version.CURRENT, fieldMapping((b) -> {
+                    b.field("type", "geo_shape").field("strategy", "recursive").field("points_only", true);
+                })));
+        assertThat(e.getMessage(),
+            containsString("using deprecated parameters [points_only, strategy] " +
+                "in mapper [field] of type [geo_shape] is no longer allowed"));
+        assertFieldWarnings(new String[] {"points_only", "strategy"});
+    }
+
     public void testLegacySwitches() throws IOException {
         // if one of the legacy parameters is added to a 'type':'geo_shape' config then
         // that will select the legacy field mapper

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

@@ -10,8 +10,10 @@
 import org.elasticsearch.Version;
 import org.elasticsearch.common.geo.SpatialStrategy;
 import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper.GeoShapeFieldType;
+import org.elasticsearch.test.VersionUtils;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -31,8 +33,8 @@ public void testSetStrategyName() {
     }
 
     public void testFetchSourceValue() throws IOException {
-
-        MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", Version.CURRENT, false, true)
+        Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
+        MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", version, false, true, Collections.singleton("strategy"))
             .build(new ContentPath()).fieldType();
 
         Map<String, Object> jsonLineString = Map.of("type", "LineString", "coordinates",