From edfd032384fa890d357d0890627891fea9358c81 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 25 Mar 2021 09:12:53 +0100 Subject: [PATCH 01/11] disallow creating geo_shape mappings with deprecated parameters --- .../elasticsearch/search/geo/GeoFilterIT.java | 3 +- .../search/geo/GeoShapeIntegrationIT.java | 23 ++-- .../geo/LegacyGeoShapeIntegrationIT.java | 25 ++-- .../mapper/LegacyGeoShapeFieldMapper.java | 4 + .../mapper/FieldFilterMapperPluginTests.java | 4 +- .../LegacyGeoShapeFieldMapperTests.java | 17 +++ .../mapper/LegacyGeoShapeFieldTypeTests.java | 15 ++- .../search/geo/GeoShapeQueryTests.java | 118 +++++++++++------- 8 files changed, 145 insertions(+), 64 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoFilterIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoFilterIT.java index 7572e4e3d90cc..f1c76403f9e4c 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoFilterIT.java +++ b/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(); diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java index d417973bf495c..3ef31833ffe0a 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoShapeIntegrationIT.java +++ b/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,17 +132,17 @@ 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" + "}"; @@ -143,7 +150,7 @@ public void testMappingUpdate() throws Exception { 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" + diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/LegacyGeoShapeIntegrationIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/LegacyGeoShapeIntegrationIT.java index 6acaa03349c9b..d0a9d7670ad72 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/LegacyGeoShapeIntegrationIT.java +++ b/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(); diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java index 8ec74e9b61065..2c140b4d85f50 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java @@ -452,6 +452,10 @@ 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("mapper [" + name() + + "] of type [geo_shape] with deprecated parameters is no longer allowed"); + } this.indexCreatedVersion = builder.indexCreatedVersion; this.builder = builder; } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java b/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java index 9863ad0f3dd6e..0156087d053f3 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java @@ -297,9 +297,7 @@ public Function> 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" + diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java index 552d8d81cc383..813c112f96d43 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java +++ b/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,21 @@ 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(this::minimalMapping))); + assertThat(e.getMessage(), + containsString("mapper [field] of type [geo_shape] with deprecated parameters is no longer allowed")); + assertFieldWarnings("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 diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java index 11159e27a3dfd..1e9bc4550f042 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java @@ -10,11 +10,14 @@ 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.List; import java.util.Map; +import static org.hamcrest.Matchers.containsString; + public class LegacyGeoShapeFieldTypeTests extends FieldTypeTestCase { /** @@ -30,9 +33,17 @@ public void testSetStrategyName() { assertTrue(fieldType.pointsOnly()); } - public void testFetchSourceValue() throws IOException { + public void testInvalidCurrentVersion() { + IllegalArgumentException e = + expectThrows(IllegalArgumentException.class, + () -> new LegacyGeoShapeFieldMapper.Builder("field", Version.CURRENT, false, true).build(new ContentPath())); + assertThat(e.getMessage(), + containsString("mapper [field] of type [geo_shape] with deprecated parameters is no longer allowed")); + } - MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", Version.CURRENT, false, true) + public void testFetchSourceValue() throws IOException { + Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); + MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", version, false, true) .build(new ContentPath()).fieldType(); Map jsonLineString = Map.of("type", "LineString", "coordinates", diff --git a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java index 64f77d7addb64..f73f74bef10cb 100644 --- a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java +++ b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java @@ -10,6 +10,7 @@ import com.carrotsearch.randomizedtesting.generators.RandomNumbers; import org.apache.lucene.geo.GeoTestUtil; +import org.elasticsearch.Version; import org.elasticsearch.action.index.IndexRequest; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.CheckedSupplier; @@ -35,6 +36,7 @@ import org.elasticsearch.index.query.ExistsQueryBuilder; import org.elasticsearch.index.query.GeoShapeQueryBuilder; import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.test.VersionUtils; import org.elasticsearch.test.geo.RandomShapeGenerator; import org.locationtech.jts.geom.Coordinate; import org.locationtech.spatial4j.shape.Rectangle; @@ -73,6 +75,11 @@ protected XContentBuilder createDefaultMapping() throws Exception { return xcb; } + @Override + protected boolean forbidPrivateIndexSettings() { + return false; + } + protected XContentBuilder createPrefixTreeMapping(String tree) throws Exception { XContentBuilder xcb = XContentFactory.jsonBuilder().startObject() .startObject("properties").startObject("geo") @@ -85,16 +92,23 @@ protected XContentBuilder createPrefixTreeMapping(String tree) throws Exception return xcb; } - protected XContentBuilder createRandomMapping() throws Exception { - XContentBuilder xcb = XContentFactory.jsonBuilder().startObject() - .startObject("properties").startObject("geo") - .field("type", "geo_shape"); - if (randomBoolean()) { - xcb = xcb.field("tree", randomFrom(PREFIX_TREES)); + protected void createRandomMapping(String indexName, Settings settings) throws Exception { + boolean legacy = randomBoolean(); + final XContentBuilder mapping = legacy ? createPrefixTreeMapping(randomFrom(PREFIX_TREES)) : createDefaultMapping(); + final Settings finalSetting; + if (legacy) { + MapperParsingException ex = + expectThrows(MapperParsingException.class, + () -> client().admin().indices().prepareCreate(indexName).setMapping(mapping).setSettings(settings).get()); + assertThat(ex.getMessage(), + containsString("mapper [geo] of type [geo_shape] is using deprecated parameters and cannot be created")); + Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); + finalSetting = settings(version).put(settings).build(); + } else { + finalSetting = settings; } - xcb = xcb.endObject().endObject().endObject(); - - return xcb; + client().admin().indices().prepareCreate(indexName).setMapping(mapping).setSettings(finalSetting).get(); + ensureGreen(); } public void testShapeFetchingPath() throws Exception { @@ -182,7 +196,6 @@ public void testShapeFetchingPath() throws Exception { public void testRandomGeoCollectionQuery() throws Exception { // Create a random geometry collection to index. GeometryCollectionBuilder gcb = RandomShapeGenerator.createGeometryCollection(random());; - org.apache.lucene.geo.Polygon randomPoly = GeoTestUtil.nextPolygon(); assumeTrue("Skipping the check for the polygon with a degenerated dimension", @@ -196,10 +209,7 @@ public void testRandomGeoCollectionQuery() throws Exception { logger.info("Created Random GeometryCollection containing {} shapes", gcb.numShapes()); - XContentBuilder mapping = createRandomMapping(); - Settings settings = Settings.builder().put("index.number_of_shards", 1).build(); - client().admin().indices().prepareCreate("test").setMapping(mapping).setSettings(settings).get(); - ensureGreen(); + createRandomMapping("test", Settings.builder().put("index.number_of_shards", 1).build()); XContentBuilder docSource = gcb.toXContent(jsonBuilder().startObject().field("geo"), null).endObject(); client().prepareIndex("test").setId("1").setSource(docSource).setRefreshPolicy(IMMEDIATE).get(); @@ -384,8 +394,7 @@ public void testEdgeCases() throws Exception { } public void testIndexedShapeReferenceSourceDisabled() throws Exception { - String mapping = Strings.toString(createRandomMapping()); - client().admin().indices().prepareCreate("test").setMapping(mapping).get(); + createRandomMapping("test", Settings.builder().put("index.number_of_shards", 1).build()); createIndex("shapes", Settings.EMPTY, "shape_type", "_source", "enabled=false"); ensureGreen(); @@ -427,9 +436,7 @@ public void testPointQuery() throws Exception { gcb.shape(pb); // create mapping - String mapping = Strings.toString(createRandomMapping()); - client().admin().indices().prepareCreate("test").setMapping(mapping).get(); - ensureGreen(); + createRandomMapping("test", Settings.EMPTY); XContentBuilder docSource = gcb.toXContent(jsonBuilder().startObject().field("geo"), null).endObject(); client().prepareIndex("test").setId("1").setSource(docSource).setRefreshPolicy(IMMEDIATE).get(); @@ -466,7 +473,18 @@ public void testContainsShapeQuery() throws Exception { usePrefixTrees ? createPrefixTreeMapping(LegacyGeoShapeFieldMapper.PrefixTrees.QUADTREE) : createDefaultMapping()); - client().admin().indices().prepareCreate("test").setMapping(mapping).get(); + + if (usePrefixTrees) { + MapperParsingException ex = + expectThrows(MapperParsingException.class, + () -> client().admin().indices().prepareCreate("test").setMapping(mapping).get()); + assertThat(ex.getMessage(), + containsString("mapper [geo] of type [geo_shape] is using deprecated parameters and cannot be created")); + } + + Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); + Settings settings = usePrefixTrees ? settings(version).build() : Settings.EMPTY; + client().admin().indices().prepareCreate("test").setMapping(mapping).setSettings(settings).get(); ensureGreen(); XContentBuilder docSource = gcb.toXContent(jsonBuilder().startObject().field("geo"), null).endObject(); @@ -493,10 +511,7 @@ public void testExistsQuery() throws Exception { GeometryCollectionBuilder gcb = RandomShapeGenerator.createGeometryCollection(random()); logger.info("Created Random GeometryCollection containing {} shapes", gcb.numShapes()); - String mapping = Strings.toString(createRandomMapping()); - - client().admin().indices().prepareCreate("test").setMapping(mapping).get(); - ensureGreen(); + createRandomMapping("test", Settings.EMPTY); XContentBuilder docSource = gcb.toXContent(jsonBuilder().startObject().field("geo"), null).endObject(); client().prepareIndex("test").setId("1").setSource(docSource).setRefreshPolicy(IMMEDIATE).get(); @@ -518,7 +533,15 @@ public void testPointsOnly() throws Exception { .endObject() .endObject().endObject()); - client().admin().indices().prepareCreate("geo_points_only").setMapping(mapping).get(); + MapperParsingException ex = + expectThrows(MapperParsingException.class, + () -> client().admin().indices().prepareCreate("geo_points_only").setMapping(mapping).get()); + assertThat(ex.getMessage(), + containsString("mapper [geo] of type [geo_shape] is using deprecated parameters and cannot be created")); + + Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); + Settings settings = settings(version).build(); + client().admin().indices().prepareCreate("geo_points_only").setMapping(mapping).setSettings(settings).get(); ensureGreen(); ShapeBuilder shape = RandomShapeGenerator.createShape(random()); @@ -551,7 +574,15 @@ public void testPointsOnlyExplicit() throws Exception { .endObject() .endObject().endObject()); - client().admin().indices().prepareCreate("geo_points_only").setMapping(mapping).get(); + MapperParsingException ex = + expectThrows(MapperParsingException.class, + () -> client().admin().indices().prepareCreate("geo_points_only").setMapping(mapping).get()); + assertThat(ex.getMessage(), + containsString("mapper [geo] of type [geo_shape] is using deprecated parameters and cannot be created")); + + Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); + Settings settings = settings(version).build(); + client().admin().indices().prepareCreate("geo_points_only").setMapping(mapping).setSettings(settings).get(); ensureGreen(); // MULTIPOINT @@ -575,10 +606,8 @@ public void testPointsOnlyExplicit() throws Exception { } public void testIndexedShapeReference() throws Exception { - String mapping = Strings.toString(createRandomMapping()); - client().admin().indices().prepareCreate("test").setMapping(mapping).get(); - createIndex("shapes"); - ensureGreen(); + + createRandomMapping("test", Settings.EMPTY); EnvelopeBuilder shape = new EnvelopeBuilder(new Coordinate(-45, 45), new Coordinate(45, -45)); @@ -625,7 +654,17 @@ public void testFieldAlias() throws IOException { .endObject() .endObject() .endObject()); - client().admin().indices().prepareCreate("test").setMapping(mapping).get(); + + + MapperParsingException ex = + expectThrows(MapperParsingException.class, + () -> client().admin().indices().prepareCreate("test").setMapping(mapping).get()); + assertThat(ex.getMessage(), + containsString("mapper [geo] of type [geo_shape] is using deprecated parameters and cannot be created")); + + Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); + Settings settings = settings(version).build(); + client().admin().indices().prepareCreate("test").setMapping(mapping).setSettings(settings).get(); ensureGreen(); ShapeBuilder shape = RandomShapeGenerator.createShape(random(), RandomShapeGenerator.ShapeType.MULTIPOINT); @@ -641,7 +680,6 @@ public void testFieldAlias() throws IOException { public void testQueryRandomGeoCollection() throws Exception { // Create a random geometry collection. - String mapping = Strings.toString(createRandomMapping()); GeometryCollectionBuilder gcb = RandomShapeGenerator.createGeometryCollection(random()); org.apache.lucene.geo.Polygon randomPoly = GeoTestUtil.nextPolygon(); CoordinatesBuilder cb = new CoordinatesBuilder(); @@ -652,8 +690,7 @@ public void testQueryRandomGeoCollection() throws Exception { logger.info("Created Random GeometryCollection containing {} shapes", gcb.numShapes()); - client().admin().indices().prepareCreate("test").setMapping(mapping).get(); - ensureGreen(); + createRandomMapping("test", Settings.EMPTY); XContentBuilder docSource = gcb.toXContent(jsonBuilder().startObject().field("geo"), null).endObject(); client().prepareIndex("test").setId("1").setSource(docSource).setRefreshPolicy(IMMEDIATE).get(); @@ -671,9 +708,7 @@ public void testQueryRandomGeoCollection() throws Exception { } public void testShapeFilterWithDefinedGeoCollection() throws Exception { - String mapping = Strings.toString(createRandomMapping()); - client().admin().indices().prepareCreate("test").setMapping(mapping).get(); - ensureGreen(); + createRandomMapping("test", Settings.EMPTY); XContentBuilder docSource = jsonBuilder().startObject().startObject("geo") .field("type", "geometrycollection") @@ -741,9 +776,7 @@ public void testShapeFilterWithDefinedGeoCollection() throws Exception { } public void testDistanceQuery() throws Exception { - String mapping = Strings.toString(createRandomMapping()); - client().admin().indices().prepareCreate("test_distance").setMapping(mapping).get(); - ensureGreen(); + createRandomMapping("test_distance", Settings.EMPTY); CircleBuilder circleBuilder = new CircleBuilder().center(new Coordinate(1, 0)).radius(350, DistanceUnit.KILOMETERS); @@ -779,10 +812,7 @@ public void testDistanceQuery() throws Exception { } public void testIndexRectangleSpanningDateLine() throws Exception { - String mapping = Strings.toString(createRandomMapping()); - - client().admin().indices().prepareCreate("test").setMapping(mapping).get(); - ensureGreen(); + createRandomMapping("test", Settings.EMPTY); EnvelopeBuilder envelopeBuilder = new EnvelopeBuilder(new Coordinate(178, 10), new Coordinate(-178, -10)); From 40ad9bcb27560607464aae87aa3553e4735d7af5 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 25 Mar 2021 11:29:04 +0100 Subject: [PATCH 02/11] better error message --- .../index/mapper/GeoShapeFieldMapper.java | 5 ++++- .../mapper/LegacyGeoShapeFieldMapper.java | 18 ++++++++++++++---- .../common/geo/GeoJsonShapeParserTests.java | 3 ++- .../common/geo/GeoWKTShapeParserTests.java | 12 ++++++++---- .../mapper/LegacyGeoShapeFieldMapperTests.java | 9 ++++++--- .../mapper/LegacyGeoShapeFieldTypeTests.java | 13 ++----------- .../GeoShapeWithDocValuesFieldMapper.java | 5 ++++- 7 files changed, 40 insertions(+), 25 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java index 210c4f85364ea..a695ed6062038 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java +++ b/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 deprecatedParams = LegacyGeoShapeFieldMapper.getDeprecatedParameters(node.keySet()); builder = new LegacyGeoShapeFieldMapper.Builder( name, parserContext.indexVersionCreated(), ignoreMalformedByDefault, - coerceByDefault); + coerceByDefault, + deprecatedParams); } else { builder = new Builder(name, ignoreMalformedByDefault, coerceByDefault); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java index 2c140b4d85f50..71d0212229149 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/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 paramKeys) { return DEPRECATED_PARAMETERS.stream().anyMatch(paramKeys::contains); } + public static Set getDeprecatedParameters(Set 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,8 +163,10 @@ public static class Builder extends FieldMapper.Builder { Parameter> meta = Parameter.metaParam(); private final Version indexCreatedVersion; + private final Set deprecatedParam; - public Builder(String name, Version version, boolean ignoreMalformedByDefault, boolean coerceByDefault) { + public Builder(String name, Version version, boolean ignoreMalformedByDefault, boolean coerceByDefault, + Set deprecatedParams) { super(name); if (ShapesAvailability.JTS_AVAILABLE == false || ShapesAvailability.SPATIAL4J_AVAILABLE == false) { @@ -167,6 +174,7 @@ public Builder(String name, Version version, boolean ignoreMalformedByDefault, b } 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 deprecatedParams; private final Builder builder; public LegacyGeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType, @@ -453,10 +462,11 @@ public LegacyGeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldT 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("mapper [" + name() - + "] of type [geo_shape] with deprecated parameters is no longer allowed"); + 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; } @@ -492,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 diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java index 5db1b0c501717..cf4f67fb7237a 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java +++ b/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()); diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java index e2b0b6bcde7a6..75887d1f26576 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java +++ b/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()); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java index 813c112f96d43..8b55d4991ee45 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java @@ -109,10 +109,13 @@ protected MapperService createMapperService(XContentBuilder mappings) throws IOE public void testInvalidCurrentVersion() { MapperParsingException e = expectThrows(MapperParsingException.class, - () -> createMapperService(Version.CURRENT, fieldMapping(this::minimalMapping))); + () -> createMapperService(Version.CURRENT, fieldMapping((b) -> { + b.field("type", "geo_shape").field("strategy", "recursive").field("points_only", true); + }))); assertThat(e.getMessage(), - containsString("mapper [field] of type [geo_shape] with deprecated parameters is no longer allowed")); - assertFieldWarnings("strategy"); + 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 { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java index 1e9bc4550f042..19fb4f65da3b1 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java @@ -13,11 +13,10 @@ import org.elasticsearch.test.VersionUtils; import java.io.IOException; +import java.util.Collections; import java.util.List; import java.util.Map; -import static org.hamcrest.Matchers.containsString; - public class LegacyGeoShapeFieldTypeTests extends FieldTypeTestCase { /** @@ -33,17 +32,9 @@ public void testSetStrategyName() { assertTrue(fieldType.pointsOnly()); } - public void testInvalidCurrentVersion() { - IllegalArgumentException e = - expectThrows(IllegalArgumentException.class, - () -> new LegacyGeoShapeFieldMapper.Builder("field", Version.CURRENT, false, true).build(new ContentPath())); - assertThat(e.getMessage(), - containsString("mapper [field] of type [geo_shape] with deprecated parameters is no longer allowed")); - } - public void testFetchSourceValue() throws IOException { Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); - MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", version, false, true) + MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", version, false, true, Collections.singleton("strategy")) .build(new ContentPath()).fieldType(); Map jsonLineString = Map.of("type", "LineString", "coordinates", diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java index 462cd6eafe37b..5cfc437dee9db 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java @@ -40,6 +40,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.Supplier; /** @@ -174,11 +175,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 deprecatedParams = LegacyGeoShapeFieldMapper.getDeprecatedParameters(node.keySet()); builder = new LegacyGeoShapeFieldMapper.Builder( name, parserContext.indexVersionCreated(), ignoreMalformedByDefault, - coerceByDefault); + coerceByDefault, + deprecatedParams); } else { builder = new GeoShapeWithDocValuesFieldMapper.Builder( name, From d707cf0609b7ddcc4826b1264dad243e3251f528 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 25 Mar 2021 12:10:37 +0100 Subject: [PATCH 03/11] iter --- .../mapping/types/geo-shape.asciidoc | 39 +------------------ .../common/geo/GeoJsonShapeParserTests.java | 3 +- .../common/geo/GeoWKTShapeParserTests.java | 10 +++-- .../search/geo/GeoShapeQueryTests.java | 12 +++--- 4 files changed, 17 insertions(+), 47 deletions(-) diff --git a/docs/reference/mapping/types/geo-shape.asciidoc b/docs/reference/mapping/types/geo-shape.asciidoc index d90ae2a2a736f..e7ca00762c1e1 100644 --- a/docs/reference/mapping/types/geo-shape.asciidoc +++ b/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 <> to approximate the circle as a <>. -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] diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java index cf4f67fb7237a..1c107bce7f3a0 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java @@ -291,8 +291,9 @@ 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, Collections.singleton("strategy")) + new LegacyGeoShapeFieldMapper.Builder("test", version, false, true, Collections.singleton("strategy")) .build(new ContentPath()); try (XContentParser parser = createParser(polygonGeoJson)) { parser.nextToken(); diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java index 75887d1f26576..0cb612e40d822 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java @@ -35,6 +35,7 @@ import org.elasticsearch.index.mapper.GeoShapeFieldMapper; import org.elasticsearch.index.mapper.GeoShapeIndexer; import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper; +import org.elasticsearch.test.VersionUtils; import org.elasticsearch.test.geo.RandomShapeGenerator; import org.locationtech.jts.geom.Coordinate; import org.locationtech.jts.geom.LineString; @@ -325,8 +326,9 @@ public void testParseMixedDimensionPolyWithHoleStoredZ() throws IOException { XContentParser parser = createParser(xContentBuilder); parser.nextToken(); + final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); final LegacyGeoShapeFieldMapper mapperBuilder = - new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true, Collections.singleton("strategy")) + new LegacyGeoShapeFieldMapper.Builder("test", version, false, true, Collections.singleton("strategy")) .build(new ContentPath()); // test store z disabled @@ -349,8 +351,9 @@ public void testParsePolyWithStoredZ() throws IOException { XContentParser parser = createParser(xContentBuilder); parser.nextToken(); + final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); final LegacyGeoShapeFieldMapper mapperBuilder = - new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true, Collections.singleton("strategy")) + new LegacyGeoShapeFieldMapper.Builder("test", version, false, true, Collections.singleton("strategy")) .build(new ContentPath()); ShapeBuilder shapeBuilder = ShapeParser.parse(parser, mapperBuilder); @@ -364,8 +367,9 @@ public void testParseOpenPolygon() throws IOException { XContentParser parser = createParser(xContentBuilder); parser.nextToken(); + final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); final LegacyGeoShapeFieldMapper defaultMapperBuilder = - new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true, Collections.singleton("strategy")) + new LegacyGeoShapeFieldMapper.Builder("test", version, false, true, Collections.singleton("strategy")) .coerce(false).build(new ContentPath()); ElasticsearchParseException exception = expectThrows(ElasticsearchParseException.class, () -> ShapeParser.parse(parser, defaultMapperBuilder)); diff --git a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java index f73f74bef10cb..774ad51fd50eb 100644 --- a/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java +++ b/server/src/test/java/org/elasticsearch/search/geo/GeoShapeQueryTests.java @@ -101,7 +101,7 @@ protected void createRandomMapping(String indexName, Settings settings) throws E expectThrows(MapperParsingException.class, () -> client().admin().indices().prepareCreate(indexName).setMapping(mapping).setSettings(settings).get()); assertThat(ex.getMessage(), - containsString("mapper [geo] of type [geo_shape] is using deprecated parameters and cannot be created")); + containsString("using deprecated parameters [tree] in mapper [geo] of type [geo_shape] is no longer allowed")); Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); finalSetting = settings(version).put(settings).build(); } else { @@ -479,7 +479,7 @@ public void testContainsShapeQuery() throws Exception { expectThrows(MapperParsingException.class, () -> client().admin().indices().prepareCreate("test").setMapping(mapping).get()); assertThat(ex.getMessage(), - containsString("mapper [geo] of type [geo_shape] is using deprecated parameters and cannot be created")); + containsString("using deprecated parameters [tree] in mapper [geo] of type [geo_shape] is no longer allowed")); } Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); @@ -537,7 +537,8 @@ public void testPointsOnly() throws Exception { expectThrows(MapperParsingException.class, () -> client().admin().indices().prepareCreate("geo_points_only").setMapping(mapping).get()); assertThat(ex.getMessage(), - containsString("mapper [geo] of type [geo_shape] is using deprecated parameters and cannot be created")); + containsString("using deprecated parameters [points_only, tree, distance_error_pct, tree_levels] " + + "in mapper [geo] of type [geo_shape] is no longer allowed")); Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); Settings settings = settings(version).build(); @@ -578,7 +579,8 @@ public void testPointsOnlyExplicit() throws Exception { expectThrows(MapperParsingException.class, () -> client().admin().indices().prepareCreate("geo_points_only").setMapping(mapping).get()); assertThat(ex.getMessage(), - containsString("mapper [geo] of type [geo_shape] is using deprecated parameters and cannot be created")); + containsString("using deprecated parameters [points_only, tree, distance_error_pct, tree_levels] " + + "in mapper [geo] of type [geo_shape] is no longer allowed")); Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); Settings settings = settings(version).build(); @@ -660,7 +662,7 @@ public void testFieldAlias() throws IOException { expectThrows(MapperParsingException.class, () -> client().admin().indices().prepareCreate("test").setMapping(mapping).get()); assertThat(ex.getMessage(), - containsString("mapper [geo] of type [geo_shape] is using deprecated parameters and cannot be created")); + containsString("using deprecated parameters [tree] in mapper [geo] of type [geo_shape] is no longer allowed")); Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); Settings settings = settings(version).build(); From 82995d1300d4c3c8a28af69d6da967700e981f00 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 29 Apr 2021 10:39:40 +0200 Subject: [PATCH 04/11] iter --- .../index/mapper/GeoShapeFieldMapper.java | 5 +- .../mapper/LegacyGeoShapeFieldMapper.java | 69 +++++++++++++------ .../common/geo/GeoJsonShapeParserTests.java | 2 +- .../common/geo/GeoWKTShapeParserTests.java | 8 +-- .../LegacyGeoShapeFieldMapperTests.java | 6 +- .../mapper/LegacyGeoShapeFieldTypeTests.java | 3 +- .../GeoShapeWithDocValuesFieldMapper.java | 5 +- 7 files changed, 58 insertions(+), 40 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java index 753352e7d2cf7..faadc06d102a5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java @@ -27,7 +27,6 @@ import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.Set; /** * FieldMapper for indexing {@link LatLonShape}s. @@ -145,13 +144,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())) { - Set deprecatedParams = LegacyGeoShapeFieldMapper.getDeprecatedParameters(node.keySet()); builder = new LegacyGeoShapeFieldMapper.Builder( name, parserContext.indexVersionCreated(), ignoreMalformedByDefault, - coerceByDefault, - deprecatedParams); + coerceByDefault); } else { builder = new Builder(name, ignoreMalformedByDefault, coerceByDefault); } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java index c8d8dc1b4f8ad..8a6ee0546fc69 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java @@ -82,10 +82,6 @@ public static boolean containsDeprecatedParameter(Set paramKeys) { return DEPRECATED_PARAMETERS.stream().anyMatch(paramKeys::contains); } - public static Set getDeprecatedParameters(Set 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"; @@ -130,6 +126,43 @@ private static Builder builder(FieldMapper in) { return ((LegacyGeoShapeFieldMapper)in).builder; } + private static void checkVersion(String name, Mapper.TypeParser.ParserContext context, String parameter) { + if (context.indexVersionCreated().onOrAfter(Version.V_8_0_0)) { + throw new IllegalArgumentException("using deprecated parameter [" + parameter + + "] in mapper [" + name + "] of type [geo_shape] is no longer allowed"); + } + } + + private static SpatialStrategy spatialStrategy(String name, Mapper.TypeParser.ParserContext context, Object o) { + checkVersion(name, context, "strategy"); + return o == null ? null : SpatialStrategy.fromString(o.toString()); + } + + private static String tree(String name, Mapper.TypeParser.ParserContext context, Object o) { + checkVersion(name, context, "tree"); + return o == null ? null : o.toString(); + } + + private static Integer treeLevels(String name, Mapper.TypeParser.ParserContext context, Object o) { + checkVersion(name, context, "tree_levels"); + return o == null ? null : XContentMapValues.nodeIntegerValue(o); + } + + private static DistanceUnit.Distance precision(String name, Mapper.TypeParser.ParserContext context, Object o) { + checkVersion(name, context, "precision"); + return o == null ? null : DistanceUnit.Distance.parseDistance(o.toString()); + } + + private static Double distanceErrorPct(String name, Mapper.TypeParser.ParserContext context, Object o) { + checkVersion(name, context, "distance_error_pct"); + return o == null ? null : XContentMapValues.nodeDoubleValue(o); + } + + private static Boolean pointsOnly(String name, Mapper.TypeParser.ParserContext context, Object o) { + checkVersion(name, context, "points_only"); + return o == null ? null : XContentMapValues.nodeBooleanValue(o); + } + public static class Builder extends FieldMapper.Builder { Parameter indexed = Parameter.indexParam(m -> builder(m).indexed.get(), true); @@ -140,35 +173,34 @@ public static class Builder extends FieldMapper.Builder { Parameter> orientation = orientationParam(m -> builder(m).orientation.get()); Parameter strategy = new Parameter<>("strategy", false, () -> SpatialStrategy.RECURSIVE, - (n, c, o) -> SpatialStrategy.fromString(o.toString()), m -> builder(m).strategy.get()) + (n, c, o) -> spatialStrategy(n, c, o), m -> builder(m).strategy.get()) .deprecated(); - Parameter tree = Parameter.stringParam("tree", false, m -> builder(m).tree.get(), Defaults.TREE) + Parameter tree = new Parameter<>("tree", false, () -> Defaults.TREE, + (n, c, o) -> tree(n, c, o), m -> builder(m).tree.get()) .deprecated(); Parameter treeLevels = new Parameter<>("tree_levels", false, () -> null, - (n, c, o) -> o == null ? null : XContentMapValues.nodeIntegerValue(o), + (n, c, o) -> treeLevels(n, c, o), m -> builder(m).treeLevels.get()) .deprecated(); Parameter precision = new Parameter<>("precision", false, () -> null, - (n, c, o) -> o == null ? null : DistanceUnit.Distance.parseDistance(o.toString()), + (n, c, o) -> precision(n, c, o), m -> builder(m).precision.get()) .deprecated(); Parameter distanceErrorPct = new Parameter<>("distance_error_pct", true, () -> null, - (n, c, o) -> o == null ? null : XContentMapValues.nodeDoubleValue(o), + (n, c, o) -> distanceErrorPct(n, c, o), m -> builder(m).distanceErrorPct.get()) .deprecated() .acceptsNull(); Parameter pointsOnly = new Parameter<>("points_only", false, () -> null, - (n, c, o) -> XContentMapValues.nodeBooleanValue(o), m -> builder(m).pointsOnly.get()) + (n, c, o) -> pointsOnly(n, c, o), m -> builder(m).pointsOnly.get()) .deprecated().acceptsNull(); Parameter> meta = Parameter.metaParam(); private final Version indexCreatedVersion; - private final Set deprecatedParam; - public Builder(String name, Version version, boolean ignoreMalformedByDefault, boolean coerceByDefault - Set deprecatedParams) { + public Builder(String name, Version version, boolean ignoreMalformedByDefault, boolean coerceByDefault) { super(name); if (ShapesAvailability.JTS_AVAILABLE == false || ShapesAvailability.SPATIAL4J_AVAILABLE == false) { @@ -176,7 +208,6 @@ public Builder(String name, Version version, boolean ignoreMalformedByDefault, b } this.indexCreatedVersion = version; - this.deprecatedParam = deprecatedParams; this.ignoreMalformed = ignoreMalformedParam(m -> builder(m).ignoreMalformed.get(), ignoreMalformedByDefault); this.coerce = coerceParam(m -> builder(m).coerce.get(), coerceByDefault); @@ -376,7 +407,7 @@ public Query geoShapeQuery(Geometry shape, String fieldName, ShapeRelation relat @Override public Query geoShapeQuery(Geometry shape, String fieldName, SpatialStrategy strategy, ShapeRelation relation, - SearchExecutionContext context) { + SearchExecutionContext context) { return queryProcessor.geoShapeQuery(shape, fieldName, strategy, relation, context); } @@ -459,7 +490,6 @@ public PrefixTreeStrategy resolvePrefixTreeStrategy(String strategyName) { } private final Version indexCreatedVersion; - private final Set deprecatedParams; private final Builder builder; public LegacyGeoShapeFieldMapper(String simpleName, MappedFieldType mappedFieldType, @@ -469,12 +499,7 @@ 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, 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; } @@ -520,7 +545,7 @@ protected String contentType() { @Override public FieldMapper.Builder getMergeBuilder() { return new Builder(simpleName(), indexCreatedVersion, - builder.ignoreMalformed.getDefaultValue().value(), builder.coerce.getDefaultValue().value(), deprecatedParams).init(this); + builder.ignoreMalformed.getDefaultValue().value(), builder.coerce.getDefaultValue().value()).init(this); } @Override diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java index 1c107bce7f3a0..999fd006f4382 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoJsonShapeParserTests.java @@ -293,7 +293,7 @@ public void testParse3DPolygon() throws IOException, ParseException { 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, false, true, Collections.singleton("strategy")) + new LegacyGeoShapeFieldMapper.Builder("test", version, false, true) .build(new ContentPath()); try (XContentParser parser = createParser(polygonGeoJson)) { parser.nextToken(); diff --git a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java b/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java index 0cb612e40d822..acea22651bd3c 100644 --- a/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java +++ b/server/src/test/java/org/elasticsearch/common/geo/GeoWKTShapeParserTests.java @@ -328,7 +328,7 @@ public void testParseMixedDimensionPolyWithHoleStoredZ() throws IOException { final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); final LegacyGeoShapeFieldMapper mapperBuilder = - new LegacyGeoShapeFieldMapper.Builder("test", version, false, true, Collections.singleton("strategy")) + new LegacyGeoShapeFieldMapper.Builder("test", version, false, true) .build(new ContentPath()); // test store z disabled @@ -353,7 +353,7 @@ public void testParsePolyWithStoredZ() throws IOException { final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); final LegacyGeoShapeFieldMapper mapperBuilder = - new LegacyGeoShapeFieldMapper.Builder("test", version, false, true, Collections.singleton("strategy")) + new LegacyGeoShapeFieldMapper.Builder("test", version, false, true) .build(new ContentPath()); ShapeBuilder shapeBuilder = ShapeParser.parse(parser, mapperBuilder); @@ -369,14 +369,14 @@ public void testParseOpenPolygon() throws IOException { final Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); final LegacyGeoShapeFieldMapper defaultMapperBuilder = - new LegacyGeoShapeFieldMapper.Builder("test", version, false, true, Collections.singleton("strategy")) + new LegacyGeoShapeFieldMapper.Builder("test", version, false, true) .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, Collections.singleton("strategy")) + new LegacyGeoShapeFieldMapper.Builder("test", Version.CURRENT, false, true) .coerce(true).build(new ContentPath()); ShapeBuilder shapeBuilder = ShapeParser.parse(parser, coercingMapperBuilder); assertNotNull(shapeBuilder); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java index 8b55d4991ee45..f0d15afd28c45 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java @@ -110,12 +110,12 @@ 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); + b.field("type", "geo_shape").field("strategy", "recursive"); }))); assertThat(e.getMessage(), - containsString("using deprecated parameters [points_only, strategy] " + + containsString("using deprecated parameter [strategy] " + "in mapper [field] of type [geo_shape] is no longer allowed")); - assertFieldWarnings(new String[] {"points_only", "strategy"}); + assertFieldWarnings(new String[] {"strategy"}); } public void testLegacySwitches() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java index 19fb4f65da3b1..cc4326e4e4fe6 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldTypeTests.java @@ -13,7 +13,6 @@ import org.elasticsearch.test.VersionUtils; import java.io.IOException; -import java.util.Collections; import java.util.List; import java.util.Map; @@ -34,7 +33,7 @@ public void testSetStrategyName() { public void testFetchSourceValue() throws IOException { Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); - MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", version, false, true, Collections.singleton("strategy")) + MappedFieldType mapper = new LegacyGeoShapeFieldMapper.Builder("field", version, false, true) .build(new ContentPath()).fieldType(); Map jsonLineString = Map.of("type", "LineString", "coordinates", diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java index 005bb77ac770f..71065a458b62c 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java @@ -44,7 +44,6 @@ import java.util.Arrays; import java.util.List; import java.util.Map; -import java.util.Set; import java.util.function.Supplier; /** @@ -177,13 +176,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())) { - Set deprecatedParams = LegacyGeoShapeFieldMapper.getDeprecatedParameters(node.keySet()); builder = new LegacyGeoShapeFieldMapper.Builder( name, parserContext.indexVersionCreated(), ignoreMalformedByDefault, - coerceByDefault, - deprecatedParams); + coerceByDefault); } else { builder = new GeoShapeWithDocValuesFieldMapper.Builder( name, From 4b4a5e34d03348b0b3f22afcb62ceb8cdb942950 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 29 Apr 2021 11:07:13 +0200 Subject: [PATCH 05/11] move check to n GeoShapeFieldMapper's type parser --- .../index/mapper/GeoShapeFieldMapper.java | 6 ++ .../mapper/LegacyGeoShapeFieldMapper.java | 55 ++++--------------- .../LegacyGeoShapeFieldMapperTests.java | 1 - 3 files changed, 17 insertions(+), 45 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java index faadc06d102a5..3ac1b13bd3f26 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java +++ b/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 deprecated = LegacyGeoShapeFieldMapper.getDeprecatedParameters(node.keySet()); + throw new IllegalArgumentException("using deprecated parameter " + Arrays.toString(deprecated.toArray()) + + " in mapper [" + name + "] of type [geo_shape] is no longer allowed"); + } builder = new LegacyGeoShapeFieldMapper.Builder( name, parserContext.indexVersionCreated(), diff --git a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java index 8a6ee0546fc69..0ff9b009f1ec0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/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 paramKeys) { return DEPRECATED_PARAMETERS.stream().anyMatch(paramKeys::contains); } + public static Set getDeprecatedParameters(Set 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"; @@ -126,43 +131,6 @@ private static Builder builder(FieldMapper in) { return ((LegacyGeoShapeFieldMapper)in).builder; } - private static void checkVersion(String name, Mapper.TypeParser.ParserContext context, String parameter) { - if (context.indexVersionCreated().onOrAfter(Version.V_8_0_0)) { - throw new IllegalArgumentException("using deprecated parameter [" + parameter - + "] in mapper [" + name + "] of type [geo_shape] is no longer allowed"); - } - } - - private static SpatialStrategy spatialStrategy(String name, Mapper.TypeParser.ParserContext context, Object o) { - checkVersion(name, context, "strategy"); - return o == null ? null : SpatialStrategy.fromString(o.toString()); - } - - private static String tree(String name, Mapper.TypeParser.ParserContext context, Object o) { - checkVersion(name, context, "tree"); - return o == null ? null : o.toString(); - } - - private static Integer treeLevels(String name, Mapper.TypeParser.ParserContext context, Object o) { - checkVersion(name, context, "tree_levels"); - return o == null ? null : XContentMapValues.nodeIntegerValue(o); - } - - private static DistanceUnit.Distance precision(String name, Mapper.TypeParser.ParserContext context, Object o) { - checkVersion(name, context, "precision"); - return o == null ? null : DistanceUnit.Distance.parseDistance(o.toString()); - } - - private static Double distanceErrorPct(String name, Mapper.TypeParser.ParserContext context, Object o) { - checkVersion(name, context, "distance_error_pct"); - return o == null ? null : XContentMapValues.nodeDoubleValue(o); - } - - private static Boolean pointsOnly(String name, Mapper.TypeParser.ParserContext context, Object o) { - checkVersion(name, context, "points_only"); - return o == null ? null : XContentMapValues.nodeBooleanValue(o); - } - public static class Builder extends FieldMapper.Builder { Parameter indexed = Parameter.indexParam(m -> builder(m).indexed.get(), true); @@ -173,27 +141,26 @@ public static class Builder extends FieldMapper.Builder { Parameter> orientation = orientationParam(m -> builder(m).orientation.get()); Parameter strategy = new Parameter<>("strategy", false, () -> SpatialStrategy.RECURSIVE, - (n, c, o) -> spatialStrategy(n, c, o), m -> builder(m).strategy.get()) + (n, c, o) -> SpatialStrategy.fromString(o.toString()), m -> builder(m).strategy.get()) .deprecated(); - Parameter tree = new Parameter<>("tree", false, () -> Defaults.TREE, - (n, c, o) -> tree(n, c, o), m -> builder(m).tree.get()) + Parameter tree = Parameter.stringParam("tree", false, m -> builder(m).tree.get(), Defaults.TREE) .deprecated(); Parameter treeLevels = new Parameter<>("tree_levels", false, () -> null, - (n, c, o) -> treeLevels(n, c, o), + (n, c, o) -> o == null ? null : XContentMapValues.nodeIntegerValue(o), m -> builder(m).treeLevels.get()) .deprecated(); Parameter precision = new Parameter<>("precision", false, () -> null, - (n, c, o) -> precision(n, c, o), + (n, c, o) -> o == null ? null : DistanceUnit.Distance.parseDistance(o.toString()), m -> builder(m).precision.get()) .deprecated(); Parameter distanceErrorPct = new Parameter<>("distance_error_pct", true, () -> null, - (n, c, o) -> distanceErrorPct(n, c, o), + (n, c, o) -> o == null ? null : XContentMapValues.nodeDoubleValue(o), m -> builder(m).distanceErrorPct.get()) .deprecated() .acceptsNull(); Parameter pointsOnly = new Parameter<>("points_only", false, () -> null, - (n, c, o) -> pointsOnly(n, c, o), m -> builder(m).pointsOnly.get()) + (n, c, o) -> XContentMapValues.nodeBooleanValue(o), m -> builder(m).pointsOnly.get()) .deprecated().acceptsNull(); Parameter> meta = Parameter.metaParam(); diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java index f0d15afd28c45..e116d45e03efa 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java @@ -115,7 +115,6 @@ public void testInvalidCurrentVersion() { assertThat(e.getMessage(), containsString("using deprecated parameter [strategy] " + "in mapper [field] of type [geo_shape] is no longer allowed")); - assertFieldWarnings(new String[] {"strategy"}); } public void testLegacySwitches() throws IOException { From 44edae5ae7d30e974285b2f8680e45d9b290ec97 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 29 Apr 2021 11:30:00 +0200 Subject: [PATCH 06/11] typo --- .../index/mapper/LegacyGeoShapeFieldMapperTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java index e116d45e03efa..32ef8a169d50b 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java @@ -113,7 +113,7 @@ public void testInvalidCurrentVersion() { b.field("type", "geo_shape").field("strategy", "recursive"); }))); assertThat(e.getMessage(), - containsString("using deprecated parameter [strategy] " + + containsString("using deprecated parameters [strategy] " + "in mapper [field] of type [geo_shape] is no longer allowed")); } From 1b5ecf8405c92b912c9c30a82c1f77f49ef2c4c4 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 29 Apr 2021 11:46:14 +0200 Subject: [PATCH 07/11] iter --- .../org/elasticsearch/index/mapper/GeoShapeFieldMapper.java | 4 ++-- .../java/org/elasticsearch/index/mapper/MapperTestCase.java | 2 ++ .../index/mapper/GeoShapeWithDocValuesFieldMapper.java | 6 ++++++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java index 3ac1b13bd3f26..9817ce0497f8f 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java @@ -146,8 +146,8 @@ public Query geoShapeQuery(Geometry shape, String fieldName, ShapeRelation relat boolean coerceByDefault = COERCE_SETTING.get(parserContext.getSettings()); if (LegacyGeoShapeFieldMapper.containsDeprecatedParameter(node.keySet())) { if (parserContext.indexVersionCreated().onOrAfter(Version.V_8_0_0)) { - Set deprecated = LegacyGeoShapeFieldMapper.getDeprecatedParameters(node.keySet()); - throw new IllegalArgumentException("using deprecated parameter " + Arrays.toString(deprecated.toArray()) + Set 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( diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index 2f9ab57f2292f..4cd93f3addb0f 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -655,6 +655,8 @@ public final void testIndexTimeStoredFieldsAccess() throws IOException { minimalMapping(b); b.field("store", true); })); + assumeTrue("Mapper service is too old", + mapperService.parserContext().indexVersionCreated().onOrAfter(Version.V_8_0_0)); assertParseMinimalWarnings(); } catch (MapperParsingException e) { assertParseMinimalWarnings(); diff --git a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java index 71065a458b62c..0a670cb54437e 100644 --- a/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java +++ b/x-pack/plugin/spatial/src/main/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapper.java @@ -44,6 +44,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.Supplier; /** @@ -176,6 +177,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 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(), From ff7fd85673d83fb02fc4785c4d66e423e5583020 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 29 Apr 2021 12:12:09 +0200 Subject: [PATCH 08/11] iter --- .../index/mapper/GeoShapeFieldMapperTests.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java index f86f78266a9ec..2b7051dda3641 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java @@ -7,12 +7,14 @@ */ package org.elasticsearch.index.mapper; +import org.elasticsearch.Version; import org.elasticsearch.common.Strings; import org.elasticsearch.common.geo.builders.ShapeBuilder; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.test.TestGeoShapeFieldMapperPlugin; +import org.elasticsearch.test.VersionUtils; import java.io.IOException; import java.util.Collection; @@ -176,7 +178,8 @@ public void testGeoShapeMapperMerge() throws Exception { } public void testGeoShapeLegacyMerge() throws Exception { - MapperService m = createMapperService(fieldMapping(b -> b.field("type", "geo_shape"))); + Version version = VersionUtils.randomPreviousCompatibleVersion(random(), Version.V_8_0_0); + MapperService m = createMapperService(version, fieldMapping(b -> b.field("type", "geo_shape"))); Exception e = expectThrows(IllegalArgumentException.class, () -> merge(m, fieldMapping(b -> b.field("type", "geo_shape").field("strategy", "recursive")))); @@ -184,7 +187,7 @@ public void testGeoShapeLegacyMerge() throws Exception { containsString("mapper [field] of type [geo_shape] cannot change strategy from [BKD] to [recursive]")); assertFieldWarnings("strategy"); - MapperService lm = createMapperService(fieldMapping(b -> b.field("type", "geo_shape").field("strategy", "recursive"))); + MapperService lm = createMapperService(version, fieldMapping(b -> b.field("type", "geo_shape").field("strategy", "recursive"))); e = expectThrows(IllegalArgumentException.class, () -> merge(lm, fieldMapping(b -> b.field("type", "geo_shape")))); assertThat(e.getMessage(), From 6d66d6a54ed0c1b007c2878a15cfaac2e4c90a42 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 29 Apr 2021 13:32:21 +0200 Subject: [PATCH 09/11] make possible to override creation of mapper service with specific version --- .../index/mapper/LegacyGeoShapeFieldMapperTests.java | 8 +++++++- .../elasticsearch/index/mapper/MapperServiceTestCase.java | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java index 32ef8a169d50b..e3fe274b2a5a3 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java @@ -106,10 +106,16 @@ protected MapperService createMapperService(XContentBuilder mappings) throws IOE return createMapperService(version, mappings); } + @Override + protected MapperService createMapperService(Version version, XContentBuilder mapping) throws IOException { + assumeFalse("Version is too new", version.onOrAfter(Version.V_8_0_0)); + return super.createMapperService(version, mapping); + } + public void testInvalidCurrentVersion() { MapperParsingException e = expectThrows(MapperParsingException.class, - () -> createMapperService(Version.CURRENT, fieldMapping((b) -> { + () -> super.createMapperService(Version.CURRENT, fieldMapping((b) -> { b.field("type", "geo_shape").field("strategy", "recursive"); }))); assertThat(e.getMessage(), diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java index 40f8da8076470..0a331cc3239a2 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java @@ -152,7 +152,7 @@ protected final MapperService createMapperService(Settings settings, String mapp return mapperService; } - protected final MapperService createMapperService(Version version, XContentBuilder mapping) throws IOException { + protected MapperService createMapperService(Version version, XContentBuilder mapping) throws IOException { return createMapperService(version, getIndexSettings(), () -> true, mapping); } From e93344b8be776b7c39d7625217c2b7ccae52ebf0 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 29 Apr 2021 14:19:20 +0200 Subject: [PATCH 10/11] Add test for GeoShapeWithDocValues --- .../GeoShapeWithDocValuesFieldMapperTests.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java index 72edb57324194..b10cf5824270a 100644 --- a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/mapper/GeoShapeWithDocValuesFieldMapperTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.index.mapper.DocumentMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.Mapper; +import org.elasticsearch.index.mapper.MapperParsingException; import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.mapper.MapperTestCase; import org.elasticsearch.index.mapper.ParsedDocument; @@ -38,6 +39,7 @@ import java.util.Collection; import java.util.Collections; +import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; @@ -265,6 +267,17 @@ public void testGeoShapeMapperMerge() throws Exception { assertThat(geoShapeFieldMapper.fieldType().orientation(), equalTo(ShapeBuilder.Orientation.CW)); } + public void testInvalidCurrentVersion() { + MapperParsingException e = + expectThrows(MapperParsingException.class, + () -> super.createMapperService(Version.CURRENT, fieldMapping((b) -> { + b.field("type", "geo_shape").field("strategy", "recursive"); + }))); + assertThat(e.getMessage(), + containsString("using deprecated parameters [strategy] " + + "in mapper [field] of type [geo_shape] is no longer allowed")); + } + public void testSerializeDefaults() throws Exception { DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(this::minimalMapping)); String serialized = toXContentString((GeoShapeWithDocValuesFieldMapper) defaultMapper.mappers().getMapper("field")); From 4f3a451a05587e12cf3d867b421ec3b1895b8f04 Mon Sep 17 00:00:00 2001 From: iverase Date: Thu, 29 Apr 2021 14:27:16 +0200 Subject: [PATCH 11/11] update comment --- .../index/mapper/LegacyGeoShapeFieldMapperTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java index e3fe274b2a5a3..83cc19e56c137 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java @@ -108,7 +108,7 @@ protected MapperService createMapperService(XContentBuilder mappings) throws IOE @Override protected MapperService createMapperService(Version version, XContentBuilder mapping) throws IOException { - assumeFalse("Version is too new", version.onOrAfter(Version.V_8_0_0)); + assumeFalse("LegacyGeoShapeFieldMapper can't be created in version " + version, version.onOrAfter(Version.V_8_0_0)); return super.createMapperService(version, mapping); }