Disallow creating geo_shape mappings with deprecated parameters (#70850)

With the introduction of BKD-based geo shape indexing in #32039, the prefix tree indexing method has 
been deprecated. From 8.0.0, it will not be allowed to create new mappings using deprecated parameters.

docs/reference/mapping/types/geo-shape.asciidoc

@@ -79,7 +79,7 @@ greater false positives. Note: This parameter is only relevant for `term` and
 `recursive` strategies.
 | `0.025`
 
-|`orientation` 
+|`orientation`
 a|Optional. Vertex order for the shape's coordinates list.
 
 This parameter sets and returns only a `RIGHT` (counterclockwise) or `LEFT`
@@ -649,43 +649,6 @@ IMPORTANT: You cannot index the `circle` type using the default
 <<ingest-circle-processor,circle ingest processor>> to approximate the circle as
 a <<geo-polygon,`polygon`>>.
 
-The `circle` type requires a `geo_shape` field mapping with the deprecated
-`recursive` Prefix Tree strategy.
-
-[source,console]
-----
-PUT /circle-example
-{
-  "mappings": {
-    "properties": {
-      "location": {
-        "type": "geo_shape",
-        "strategy": "recursive"
-      }
-    }
-  }
-}
-----
-// TEST[warning:Parameter [strategy] is deprecated and will be removed in a future version]
-
-The following request indexes a `circle` geo-shape.
-
-[source,console]
-----
-POST /circle-example/_doc
-{
-  "location" : {
-    "type" : "circle",
-    "coordinates" : [101.0, 1.0],
-    "radius" : "100m"
-  }
-}
-----
-// TEST[continued]
-
-Note: The inner `radius` field is required. If not specified, then
-the units of the `radius` will default to `METERS`.
-
 *NOTE:* Neither GeoJSON or WKT support a point-radius circle type.
 
 [discrete]

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,14 +34,19 @@
 
 public class GeoShapeIntegrationIT extends ESIntegTestCase {
 
+    @Override
+    protected boolean forbidPrivateIndexSettings() {
+        return false;
+    }
+
     @Override
     protected Settings nodeSettings(int nodeOrdinal, Settings otherSettings) {
         return Settings.builder()
-                // Check that only geo-shape queries on legacy PrefixTree based
-                // geo shapes are disallowed.
-                .put("search.allow_expensive_queries", false)
-                .put(super.nodeSettings(nodeOrdinal, otherSettings))
-                .build();
+            // Check that only geo-shape queries on legacy PrefixTree based
+            // geo shapes are disallowed.
+            .put("search.allow_expensive_queries", false)
+            .put(super.nodeSettings(nodeOrdinal, otherSettings))
+            .build();
     }
 
     /**
@@ -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]"));
     }
 
     /**
@@ -184,33 +191,35 @@ public void testIndexShapeRouting() throws Exception {
 
     public void testIndexPolygonDateLine() throws Exception {
         String mappingVector = "{\n" +
-                "    \"properties\": {\n" +
-                "      \"shape\": {\n" +
-                "        \"type\": \"geo_shape\"\n" +
-                "      }\n" +
-                "    }\n" +
-                "  }";
+            "    \"properties\": {\n" +
+            "      \"shape\": {\n" +
+            "        \"type\": \"geo_shape\"\n" +
+            "      }\n" +
+            "    }\n" +
+            "  }";
 
         String mappingQuad = "{\n" +
-                "    \"properties\": {\n" +
-                "      \"shape\": {\n" +
-                "        \"type\": \"geo_shape\",\n" +
-                "        \"tree\": \"quadtree\"\n" +
-                "      }\n" +
-                "    }\n" +
-                "  }";
+            "    \"properties\": {\n" +
+            "      \"shape\": {\n" +
+            "        \"type\": \"geo_shape\",\n" +
+            "        \"tree\": \"quadtree\"\n" +
+            "      }\n" +
+            "    }\n" +
+            "  }";
 
 
         // create index
         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" +
-                "    \"shape\" : \"POLYGON((179 0, -179 0, -179 2, 179 2, 179 0))\""+
-                "}";
+            "    \"shape\" : \"POLYGON((179 0, -179 0, -179 2, 179 2, 179 0))\""+
+            "}";
 
         indexRandom(true, client().prepareIndex("quad").setId("0").setSource(source, XContentType.JSON));
         indexRandom(true, client().prepareIndex("vector").setId("0").setSource(source, XContentType.JSON));
@@ -221,25 +230,25 @@ public void testIndexPolygonDateLine() throws Exception {
             assertAcked(client().admin().cluster().updateSettings(updateSettingsRequest).actionGet());
 
             SearchResponse searchResponse = client().prepareSearch("quad").setQuery(
-                    geoShapeQuery("shape", new PointBuilder(-179.75, 1))
+                geoShapeQuery("shape", new PointBuilder(-179.75, 1))
             ).get();
 
             assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
 
             searchResponse = client().prepareSearch("quad").setQuery(
-                    geoShapeQuery("shape", new PointBuilder(90, 1))
+                geoShapeQuery("shape", new PointBuilder(90, 1))
             ).get();
 
             assertThat(searchResponse.getHits().getTotalHits().value, equalTo(0L));
 
             searchResponse = client().prepareSearch("quad").setQuery(
-                    geoShapeQuery("shape", new PointBuilder(-180, 1))
+                geoShapeQuery("shape", new PointBuilder(-180, 1))
             ).get();
 
             assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));
 
             searchResponse = client().prepareSearch("quad").setQuery(
-                    geoShapeQuery("shape", new PointBuilder(180, 1))
+                geoShapeQuery("shape", new PointBuilder(180, 1))
             ).get();
 
             assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L));

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

@@ -27,6 +27,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * FieldMapper for indexing {@link LatLonShape}s.
@@ -144,6 +145,11 @@ 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())) {
+            if (parserContext.indexVersionCreated().onOrAfter(Version.V_8_0_0)) {
+                Set<String> deprecatedParams = LegacyGeoShapeFieldMapper.getDeprecatedParameters(node.keySet());
+                throw new IllegalArgumentException("using deprecated parameters " + Arrays.toString(deprecatedParams.toArray())
+                    + " in mapper [" + name + "] of type [geo_shape] is no longer allowed");
+            }
             builder = new LegacyGeoShapeFieldMapper.Builder(
                 name,
                 parserContext.indexVersionCreated(),

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

@@ -47,6 +47,7 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
 
 /**
  * FieldMapper for indexing {@link org.locationtech.spatial4j.shape.Shape}s.
@@ -82,6 +83,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";

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

@@ -291,8 +291,10 @@ 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 Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0);
         final LegacyGeoShapeFieldMapper mapperBuilder =
-            new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true).build(new ContentPath());
+            new LegacyGeoShapeFieldMapper.Builder("test", version, false, true)
+                .build(new ContentPath());
         try (XContentParser parser = createParser(polygonGeoJson)) {
             parser.nextToken();
             ElasticsearchGeoAssertions.assertEquals(jtsGeom(expected), ShapeParser.parse(parser, mapperBuilder).buildS4J());