diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoBoundingBoxQueryLegacyGeoShapeIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoBoundingBoxQueryLegacyGeoShapeIT.java index 01dab2c53072d..e633b53ac9541 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoBoundingBoxQueryLegacyGeoShapeIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/GeoBoundingBoxQueryLegacyGeoShapeIT.java @@ -10,11 +10,25 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.TestLegacyGeoShapeFieldMapperPlugin; import java.io.IOException; +import java.util.Collection; +import java.util.Collections; public class GeoBoundingBoxQueryLegacyGeoShapeIT extends GeoBoundingBoxQueryIntegTestCase { + @Override + protected boolean addMockGeoShapeFieldMapper() { + return false; + } + + @Override + protected Collection> nodePlugins() { + return Collections.singleton(TestLegacyGeoShapeFieldMapperPlugin.class); + } + @Override public XContentBuilder getMapping() throws IOException { return XContentFactory.jsonBuilder().startObject().startObject("type1") diff --git a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/LegacyGeoShapeIT.java b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/LegacyGeoShapeIT.java index d3f5e4fb55863..22ae7ba5615bc 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/search/geo/LegacyGeoShapeIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/search/geo/LegacyGeoShapeIT.java @@ -11,18 +11,30 @@ import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.geometry.Circle; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.TestLegacyGeoShapeFieldMapperPlugin; import java.io.IOException; +import java.util.Collection; +import java.util.Collections; import static org.elasticsearch.index.query.QueryBuilders.geoShapeQuery; import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; -import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; public class LegacyGeoShapeIT extends GeoShapeIntegTestCase { + @Override + protected boolean addMockGeoShapeFieldMapper() { + return false; + } + + @Override + protected Collection> nodePlugins() { + return Collections.singleton(TestLegacyGeoShapeFieldMapperPlugin.class); + } + @Override protected void getGeoShapeMapping(XContentBuilder b) throws IOException { b.field("type", "geo_shape"); @@ -34,26 +46,6 @@ protected boolean allowExpensiveQueries() { return false; } - public void testMappingUpdate() { - // create index - assertAcked(client().admin().indices().prepareCreate("test") - .addMapping("shape", "shape", "type=geo_shape,strategy=recursive").get()); - ensureGreen(); - - String update ="{\n" + - " \"properties\": {\n" + - " \"shape\": {\n" + - " \"type\": \"geo_shape\"" + - " }\n" + - " }\n" + - "}"; - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().indices() - .preparePutMapping("test").setType("shape") - .setSource(update, XContentType.JSON).get()); - assertThat(e.getMessage(), containsString("mapper [shape] of type [geo_shape] cannot change strategy from [recursive] to [BKD]")); - } - /** * Test that the circle is still supported for the legacy shapes */ 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 db23284ba7f86..3c930e030cac0 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/GeoShapeFieldMapper.java @@ -16,8 +16,8 @@ import org.elasticsearch.common.geo.GeoShapeUtils; import org.elasticsearch.common.geo.GeometryFormatterFactory; import org.elasticsearch.common.geo.GeometryParser; -import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.geo.Orientation; +import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.logging.DeprecationCategory; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.geometry.Geometry; @@ -145,21 +145,11 @@ protected Function, List> getFormatter(String format) { } } - @SuppressWarnings("deprecation") + @Deprecated public static Mapper.TypeParser PARSER = (name, node, parserContext) -> { - FieldMapper.Builder builder; boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(parserContext.getSettings()); boolean coerceByDefault = COERCE_SETTING.get(parserContext.getSettings()); - if (parserContext.indexVersionCreated().before(Version.V_6_6_0) - || LegacyGeoShapeFieldMapper.containsDeprecatedParameter(node.keySet())) { - builder = new LegacyGeoShapeFieldMapper.Builder( - name, - parserContext.indexVersionCreated(), - ignoreMalformedByDefault, - coerceByDefault); - } else { - builder = new Builder(name, ignoreMalformedByDefault, coerceByDefault); - } + FieldMapper.Builder builder = new Builder(name, ignoreMalformedByDefault, coerceByDefault); builder.parse(name, parserContext, node); return builder; }; @@ -193,17 +183,6 @@ public FieldMapper.Builder getMergeBuilder() { ).init(this); } - @Override - @SuppressWarnings("deprecation") - protected void checkIncomingMergeType(FieldMapper mergeWith) { - if (mergeWith instanceof LegacyGeoShapeFieldMapper) { - String strategy = ((LegacyGeoShapeFieldMapper)mergeWith).strategy(); - throw new IllegalArgumentException("mapper [" + name() - + "] of type [geo_shape] cannot change strategy from [BKD] to [" + strategy + "]"); - } - super.checkIncomingMergeType(mergeWith); - } - @Override protected void index(DocumentParserContext context, Geometry geometry) throws IOException { if (geometry == null) { 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 70f59a1ba2290..a3a4051d0dec5 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapper.java @@ -300,6 +300,19 @@ public LegacyGeoShapeFieldMapper build(MapperBuilderContext context) { } } + @Deprecated + public static Mapper.TypeParser PARSER = (name, node, parserContext) -> { + boolean ignoreMalformedByDefault = IGNORE_MALFORMED_SETTING.get(parserContext.getSettings()); + boolean coerceByDefault = COERCE_SETTING.get(parserContext.getSettings()); + FieldMapper.Builder builder = new LegacyGeoShapeFieldMapper.Builder( + name, + parserContext.indexVersionCreated(), + ignoreMalformedByDefault, + coerceByDefault); + builder.parse(name, parserContext, node); + return builder; + }; + private static class LegacyGeoShapeParser extends Parser> { private LegacyGeoShapeParser() { @@ -507,9 +520,10 @@ public FieldMapper.Builder getMergeBuilder() { @Override protected void checkIncomingMergeType(FieldMapper mergeWith) { - if (mergeWith instanceof GeoShapeFieldMapper) { + if (mergeWith instanceof AbstractShapeGeometryFieldMapper + && (mergeWith instanceof LegacyGeoShapeFieldMapper) == false) { throw new IllegalArgumentException("mapper [" + name() - + "] of type [geo_shape] cannot change strategy from [" + strategy() + "] to [BKD]"); + + "] of type [geo_shape] cannot change strategy from [recursive] to [BKD]"); } super.checkIncomingMergeType(mergeWith); } 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 63026080347a0..c2681aff08a11 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/FieldFilterMapperPluginTests.java @@ -294,9 +294,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" + " },\n" + 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 31da438ec2524..0300611d53f45 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/GeoShapeFieldMapperTests.java @@ -180,23 +180,6 @@ public void testGeoShapeMapperMerge() throws Exception { assertThat(geoShapeFieldMapper.fieldType().orientation(), equalTo(Orientation.CW)); } - public void testGeoShapeLegacyMerge() throws Exception { - MapperService m = createMapperService(fieldMapping(b -> b.field("type", "geo_shape"))); - Exception e = expectThrows(IllegalArgumentException.class, - () -> merge(m, fieldMapping(b -> b.field("type", "geo_shape").field("strategy", "recursive")))); - - assertThat(e.getMessage(), - 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"))); - e = expectThrows(IllegalArgumentException.class, - () -> merge(lm, fieldMapping(b -> b.field("type", "geo_shape")))); - assertThat(e.getMessage(), - containsString("mapper [field] of type [geo_shape] cannot change strategy from [recursive] to [BKD]")); - assertFieldWarnings("strategy"); - } - public void testSerializeDefaults() throws Exception { DocumentMapper mapper = createDocumentMapper(fieldMapping(this::minimalMapping)); assertThat( 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 52d400c0e9267..bf8c74b86fa68 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/LegacyGeoShapeFieldMapperTests.java @@ -25,7 +25,7 @@ import org.elasticsearch.geometry.Point; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.plugins.Plugin; -import org.elasticsearch.test.TestGeoShapeFieldMapperPlugin; +import org.elasticsearch.test.TestLegacyGeoShapeFieldMapperPlugin; import java.io.IOException; import java.util.Collection; @@ -95,7 +95,7 @@ protected void registerParameters(ParameterChecker checker) throws IOException { @Override protected Collection getPlugins() { - return List.of(new TestGeoShapeFieldMapperPlugin()); + return List.of(new TestLegacyGeoShapeFieldMapperPlugin()); } @Override @@ -595,10 +595,10 @@ public void testDisallowExpensiveQueries() throws IOException { LegacyGeoShapeFieldMapper geoShapeFieldMapper = (LegacyGeoShapeFieldMapper) fieldMapper; ElasticsearchException e = expectThrows(ElasticsearchException.class, - () -> geoShapeFieldMapper.fieldType().geoShapeQuery( - new Point(-10, 10), "location", SpatialStrategy.TERM, ShapeRelation.INTERSECTS, searchExecutionContext)); + () -> geoShapeFieldMapper.fieldType().geoShapeQuery( + new Point(-10, 10), "location", SpatialStrategy.TERM, ShapeRelation.INTERSECTS, searchExecutionContext)); assertEquals("[geo-shape] queries on [PrefixTree geo shapes] cannot be executed when " + - "'search.allow_expensive_queries' is set to false.", e.getMessage()); + "'search.allow_expensive_queries' is set to false.", e.getMessage()); assertFieldWarnings("tree", "strategy"); } diff --git a/server/src/test/java/org/elasticsearch/index/query/LegacyGeoShapeFieldQueryTests.java b/server/src/test/java/org/elasticsearch/index/query/LegacyGeoShapeFieldQueryTests.java index d876195933ba2..74539cdc56e4f 100644 --- a/server/src/test/java/org/elasticsearch/index/query/LegacyGeoShapeFieldQueryTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/LegacyGeoShapeFieldQueryTests.java @@ -16,12 +16,22 @@ import org.elasticsearch.geo.GeometryTestUtils; import org.elasticsearch.geometry.Geometry; import org.elasticsearch.geometry.ShapeType; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.TestLegacyGeoShapeFieldMapperPlugin; import org.elasticsearch.test.VersionUtils; import java.io.IOException; +import java.util.Collection; +import java.util.Collections; public class LegacyGeoShapeFieldQueryTests extends GeoShapeQueryBuilderTests { + @SuppressWarnings("deprecation") + @Override + protected Collection> getPlugins() { + return Collections.singletonList(TestLegacyGeoShapeFieldMapperPlugin.class); + } + @Override protected String fieldName() { return GEO_SHAPE_FIELD_NAME; diff --git a/server/src/test/java/org/elasticsearch/search/geo/LegacyGeoShapeQueryTests.java b/server/src/test/java/org/elasticsearch/search/geo/LegacyGeoShapeQueryTests.java index a9f68dc70d3d7..a514723886abe 100644 --- a/server/src/test/java/org/elasticsearch/search/geo/LegacyGeoShapeQueryTests.java +++ b/server/src/test/java/org/elasticsearch/search/geo/LegacyGeoShapeQueryTests.java @@ -21,8 +21,12 @@ import org.elasticsearch.geometry.Point; import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper; import org.elasticsearch.index.mapper.MapperParsingException; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.test.TestLegacyGeoShapeFieldMapperPlugin; import java.io.IOException; +import java.util.Collection; +import java.util.Collections; import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; @@ -38,6 +42,11 @@ public class LegacyGeoShapeQueryTests extends GeoShapeQueryTestCase { LegacyGeoShapeFieldMapper.PrefixTrees.QUADTREE }; + @Override + protected Collection> getPlugins() { + return Collections.singleton(TestLegacyGeoShapeFieldMapperPlugin.class); + } + @Override protected void createMapping(String indexName, String type, String fieldName, Settings settings) throws Exception { final XContentBuilder xcb = XContentFactory.jsonBuilder().startObject() diff --git a/test/framework/src/main/java/org/elasticsearch/test/TestLegacyGeoShapeFieldMapperPlugin.java b/test/framework/src/main/java/org/elasticsearch/test/TestLegacyGeoShapeFieldMapperPlugin.java new file mode 100644 index 0000000000000..d6f07f7e38d33 --- /dev/null +++ b/test/framework/src/main/java/org/elasticsearch/test/TestLegacyGeoShapeFieldMapperPlugin.java @@ -0,0 +1,34 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ +package org.elasticsearch.test; + +import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper; +import org.elasticsearch.index.mapper.Mapper; +import org.elasticsearch.plugins.MapperPlugin; +import org.elasticsearch.plugins.Plugin; + +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; + +/** + * Some tests depend on the {@link LegacyGeoShapeFieldMapper}. + * This mapper is registered in the spatial-extras module, but used in some integration + * tests in server code. The goal is to migrate all of the spatial/geo pieces to the spatial-extras + * module such that no tests in server depend on this test plugin + */ +@Deprecated +public class TestLegacyGeoShapeFieldMapperPlugin extends Plugin implements MapperPlugin { + + @Override + public Map getMappers() { + Map mappers = new LinkedHashMap<>(); + mappers.put(LegacyGeoShapeFieldMapper.CONTENT_TYPE, LegacyGeoShapeFieldMapper.PARSER); + return Collections.unmodifiableMap(mappers); + } +} diff --git a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoBoundingBoxQueryGeoShapeWithDocValuesIT.java b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoBoundingBoxQueryGeoShapeWithDocValuesIT.java new file mode 100644 index 0000000000000..b4481340b6a4d --- /dev/null +++ b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoBoundingBoxQueryGeoShapeWithDocValuesIT.java @@ -0,0 +1,40 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.spatial.search; + +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.search.geo.GeoBoundingBoxQueryIntegTestCase; +import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; + +public class GeoBoundingBoxQueryGeoShapeWithDocValuesIT extends GeoBoundingBoxQueryIntegTestCase { + + @Override + protected boolean addMockGeoShapeFieldMapper() { + return false; + } + + @Override + protected Collection> nodePlugins() { + return Collections.singleton(LocalStateSpatialPlugin.class); + } + + @Override + public XContentBuilder getMapping() throws IOException { + XContentBuilder xContentBuilder = XContentFactory.jsonBuilder().startObject().startObject("type1") + .startObject("properties").startObject("location").field("type", "geo_shape"); + xContentBuilder.endObject().endObject().endObject().endObject(); + return xContentBuilder; + } +} + diff --git a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoBoundingBoxQueryLegacyGeoShapeWithDocValuesIT.java b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoBoundingBoxQueryLegacyGeoShapeWithDocValuesIT.java new file mode 100644 index 0000000000000..dcea8efe43755 --- /dev/null +++ b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoBoundingBoxQueryLegacyGeoShapeWithDocValuesIT.java @@ -0,0 +1,39 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.spatial.search; + +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.search.geo.GeoBoundingBoxQueryIntegTestCase; +import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; + +public class GeoBoundingBoxQueryLegacyGeoShapeWithDocValuesIT extends GeoBoundingBoxQueryIntegTestCase { + + @Override + protected boolean addMockGeoShapeFieldMapper() { + return false; + } + + @Override + protected Collection> nodePlugins() { + return Collections.singleton(LocalStateSpatialPlugin.class); + } + + @Override + public XContentBuilder getMapping() throws IOException { + return XContentFactory.jsonBuilder().startObject().startObject("type1") + .startObject("properties").startObject("location").field("type", "geo_shape").field("strategy", "recursive") + .endObject().endObject().endObject().endObject(); + } +} + diff --git a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeWithDocValuesIT.java b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeWithDocValuesIT.java new file mode 100644 index 0000000000000..9a3a664bf017b --- /dev/null +++ b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/GeoShapeWithDocValuesIT.java @@ -0,0 +1,65 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.spatial.search; + +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.search.geo.GeoShapeIntegTestCase; +import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; + +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.hamcrest.Matchers.containsString; + +public class GeoShapeWithDocValuesIT extends GeoShapeIntegTestCase { + + @Override + protected boolean addMockGeoShapeFieldMapper() { + return false; + } + + @Override + protected Collection> nodePlugins() { + return Collections.singleton(LocalStateSpatialPlugin.class); + } + + @Override + protected void getGeoShapeMapping(XContentBuilder b) throws IOException { + b.field("type", "geo_shape"); + } + + @Override + protected boolean allowExpensiveQueries() { + return true; + } + + public void testMappingUpdate() { + // create index + assertAcked(client().admin().indices().prepareCreate("test") + .addMapping("shape", "shape", "type=geo_shape").get()); + ensureGreen(); + + String update ="{\n" + + " \"properties\": {\n" + + " \"shape\": {\n" + + " \"type\": \"geo_shape\"," + + " \"strategy\": \"recursive\"" + + " }\n" + + " }\n" + + "}"; + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().indices() + .preparePutMapping("test").setType("shape") + .setSource(update, XContentType.JSON).get()); + assertThat(e.getMessage(), containsString("mapper [shape] of type [geo_shape] cannot change strategy from [BKD] to [recursive]")); + } +} diff --git a/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/LegacyGeoShapeWithDocValuesIT.java b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/LegacyGeoShapeWithDocValuesIT.java new file mode 100644 index 0000000000000..fb3e6d828655e --- /dev/null +++ b/x-pack/plugin/spatial/src/internalClusterTest/java/org/elasticsearch/xpack/spatial/search/LegacyGeoShapeWithDocValuesIT.java @@ -0,0 +1,93 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.spatial.search; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.xcontent.ToXContent; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.geometry.Circle; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.search.geo.GeoShapeIntegTestCase; +import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; + +import static org.elasticsearch.index.query.QueryBuilders.geoShapeQuery; +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; + +public class LegacyGeoShapeWithDocValuesIT extends GeoShapeIntegTestCase { + + @Override + protected boolean addMockGeoShapeFieldMapper() { + return false; + } + + @Override + protected Collection> nodePlugins() { + return Collections.singleton(LocalStateSpatialPlugin.class); + } + + @Override + protected void getGeoShapeMapping(XContentBuilder b) throws IOException { + b.field("type", "geo_shape"); + b.field("strategy", "recursive"); + } + + @Override + protected boolean allowExpensiveQueries() { + return false; + } + + public void testMappingUpdate() { + // create index + assertAcked(client().admin().indices().prepareCreate("test") + .addMapping("shape", "shape", "type=geo_shape,strategy=recursive").get()); + ensureGreen(); + + String update ="{\n" + + " \"properties\": {\n" + + " \"shape\": {\n" + + " \"type\": \"geo_shape\"" + + " }\n" + + " }\n" + + "}"; + + IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> client().admin().indices() + .preparePutMapping("test").setType("shape") + .setSource(update, XContentType.JSON).get()); + assertThat(e.getMessage(), containsString("mapper [shape] of type [geo_shape] cannot change strategy from [recursive] to [BKD]")); + } + + /** + * Test that the circle is still supported for the legacy shapes + */ + public void testLegacyCircle() throws Exception { + // create index + assertAcked(prepareCreate("test") + .addMapping("shape", "shape", "type=geo_shape,strategy=recursive,tree=geohash").get()); + ensureGreen(); + + indexRandom(true, client().prepareIndex("test", "shape").setId("0").setSource("shape", (ToXContent) (builder, params) -> { + builder.startObject().field("type", "circle") + .startArray("coordinates").value(30).value(50).endArray() + .field("radius","77km") + .endObject(); + return builder; + })); + + // test self crossing of circles + SearchResponse searchResponse = client().prepareSearch("test").setQuery(geoShapeQuery("shape", + new Circle(30, 50, 77000))).get(); + assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); + } +} 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 1dcfc621fb5e6..5e25f1656f15a 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 @@ -277,4 +277,12 @@ public GeoShapeWithDocValuesFieldType fieldType() { return (GeoShapeWithDocValuesFieldType) super.fieldType(); } + @Override + protected void checkIncomingMergeType(FieldMapper mergeWith) { + if (mergeWith instanceof LegacyGeoShapeFieldMapper) { + throw new IllegalArgumentException("mapper [" + name() + + "] of type [geo_shape] cannot change strategy from [BKD] to [recursive]"); + } + super.checkIncomingMergeType(mergeWith); + } } 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 b86c76203a67d..5cb563d117004 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 @@ -173,7 +173,6 @@ public void testCoerceParsing() throws IOException { } - /** * Test that accept_z_value parameter correctly parses */ @@ -310,6 +309,32 @@ public void testGeoShapeMapperMerge() throws Exception { assertThat(geoShapeFieldMapper.fieldType().orientation(), equalTo(Orientation.CW)); } + public void testGeoShapeLegacyMerge() throws Exception { + MapperService m = createMapperService(fieldMapping(b -> b.field("type", "geo_shape"))); + Exception e = expectThrows(IllegalArgumentException.class, + () -> merge(m, fieldMapping(b -> b.field("type", "geo_shape").field("strategy", "recursive")))); + + assertThat(e.getMessage(), + 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"))); + e = expectThrows(IllegalArgumentException.class, + () -> merge(lm, fieldMapping(b -> b.field("type", "geo_shape")))); + assertThat(e.getMessage(), + containsString("mapper [field] of type [geo_shape] cannot change strategy from [recursive] to [BKD]")); + assertFieldWarnings("strategy"); + } + + private void assertFieldWarnings(String... fieldNames) { + String[] warnings = new String[fieldNames.length]; + for (int i = 0; i < fieldNames.length; ++i) { + warnings[i] = "Parameter [" + fieldNames[i] + "] " + + "is deprecated and will be removed in a future version"; + } + assertWarnings(warnings); + } + public void testSerializeDefaults() throws Exception { DocumentMapper defaultMapper = createDocumentMapper(fieldMapping(this::minimalMapping)); String serialized = toXContentString((GeoShapeWithDocValuesFieldMapper) defaultMapper.mappers().getMapper("field")); diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/query/GeoShapeWithDocValuesQueryTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/query/GeoShapeWithDocValuesQueryTests.java new file mode 100644 index 0000000000000..65af3b9f105d9 --- /dev/null +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/query/GeoShapeWithDocValuesQueryTests.java @@ -0,0 +1,74 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.spatial.index.query; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.geo.GeoJson; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.geo.GeometryTestUtils; +import org.elasticsearch.geometry.MultiPoint; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.search.geo.GeoShapeQueryTestCase; +import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; + +import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.index.query.QueryBuilders.geoShapeQuery; + +public class GeoShapeWithDocValuesQueryTests extends GeoShapeQueryTestCase { + + @Override + protected Collection> getPlugins() { + return Collections.singleton(LocalStateSpatialPlugin.class); + } + + @Override + protected void createMapping(String indexName, String type, String fieldName, Settings settings) throws Exception { + XContentBuilder xcb = XContentFactory.jsonBuilder().startObject() + .startObject("properties").startObject(fieldName) + .field("type", "geo_shape") + .endObject().endObject().endObject(); + client().admin().indices().prepareCreate(defaultIndexName).addMapping(type, xcb).get(); + } + + public void testFieldAlias() throws IOException { + String mapping = Strings.toString(XContentFactory.jsonBuilder() + .startObject() + .startObject("properties") + .startObject(defaultGeoFieldName) + .field("type", "geo_shape") + .endObject() + .startObject("alias") + .field("type", "alias") + .field("path", defaultGeoFieldName) + .endObject() + .endObject() + .endObject()); + + client().admin().indices().prepareCreate(defaultIndexName).addMapping(defaultType, mapping, XContentType.JSON).get(); + ensureGreen(); + + MultiPoint multiPoint = GeometryTestUtils.randomMultiPoint(false); + client().prepareIndex(defaultIndexName, defaultType).setId("1") + .setSource(GeoJson.toXContent(multiPoint, jsonBuilder().startObject().field(defaultGeoFieldName), null).endObject()) + .setRefreshPolicy(IMMEDIATE).get(); + + SearchResponse response = client().prepareSearch(defaultIndexName) + .setQuery(geoShapeQuery("alias", multiPoint)) + .get(); + assertEquals(1, response.getHits().getTotalHits().value); + } +} diff --git a/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/query/LegacyGeoShapeWithDocValuesQueryTests.java b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/query/LegacyGeoShapeWithDocValuesQueryTests.java new file mode 100644 index 0000000000000..76d80cd3376c3 --- /dev/null +++ b/x-pack/plugin/spatial/src/test/java/org/elasticsearch/xpack/spatial/index/query/LegacyGeoShapeWithDocValuesQueryTests.java @@ -0,0 +1,162 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +package org.elasticsearch.xpack.spatial.index.query; + +import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.common.Strings; +import org.elasticsearch.common.geo.GeoJson; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.xcontent.XContentBuilder; +import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentType; +import org.elasticsearch.geo.GeometryTestUtils; +import org.elasticsearch.geometry.Geometry; +import org.elasticsearch.geometry.MultiPoint; +import org.elasticsearch.geometry.Point; +import org.elasticsearch.index.mapper.LegacyGeoShapeFieldMapper; +import org.elasticsearch.index.mapper.MapperParsingException; +import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.search.geo.GeoShapeQueryTestCase; +import org.elasticsearch.xpack.spatial.LocalStateSpatialPlugin; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; + +import static org.elasticsearch.action.support.WriteRequest.RefreshPolicy.IMMEDIATE; +import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; +import static org.elasticsearch.index.query.QueryBuilders.geoIntersectionQuery; +import static org.elasticsearch.index.query.QueryBuilders.geoShapeQuery; +import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; +import static org.hamcrest.Matchers.containsString; + +public class LegacyGeoShapeWithDocValuesQueryTests extends GeoShapeQueryTestCase { + + @SuppressWarnings( "deprecation" ) + private static final String[] PREFIX_TREES = new String[] { + LegacyGeoShapeFieldMapper.PrefixTrees.GEOHASH, + LegacyGeoShapeFieldMapper.PrefixTrees.QUADTREE + }; + + @Override + protected Collection> getPlugins() { + return Collections.singleton(LocalStateSpatialPlugin.class); + } + + @Override + protected void createMapping(String indexName, String type, String fieldName, Settings settings) throws Exception { + final XContentBuilder xcb = XContentFactory.jsonBuilder().startObject() + .startObject("properties").startObject(fieldName) + .field("type", "geo_shape") + .field("tree", randomFrom(PREFIX_TREES)) + .endObject() + .endObject() + .endObject(); + client().admin().indices().prepareCreate(indexName).addMapping(type, xcb).setSettings(settings).get(); + } + + @Override + protected boolean forbidPrivateIndexSettings() { + return false; + } + + public void testPointsOnlyExplicit() throws Exception { + String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("properties").startObject(defaultGeoFieldName) + .field("type", "geo_shape") + .field("tree", randomBoolean() ? "quadtree" : "geohash") + .field("tree_levels", "6") + .field("distance_error_pct", "0.01") + .field("points_only", true) + .endObject() + .endObject().endObject()); + + + client().admin().indices().prepareCreate("geo_points_only").addMapping(defaultType, mapping, XContentType.JSON).get(); + ensureGreen(); + + // MULTIPOINT + MultiPoint multiPoint = GeometryTestUtils.randomMultiPoint(false); + client().prepareIndex("geo_points_only", defaultType).setId("1") + .setSource(GeoJson.toXContent(multiPoint, jsonBuilder().startObject().field(defaultGeoFieldName), null).endObject()) + .setRefreshPolicy(IMMEDIATE).get(); + + // POINT + Point point = GeometryTestUtils.randomPoint(false); + client().prepareIndex("geo_points_only", defaultType).setId("2") + .setSource(GeoJson.toXContent(point, jsonBuilder().startObject().field(defaultGeoFieldName), null).endObject()) + .setRefreshPolicy(IMMEDIATE).get(); + + // test that point was inserted + SearchResponse response = client().prepareSearch("geo_points_only") + .setQuery(matchAllQuery()) + .get(); + + assertEquals(2, response.getHits().getTotalHits().value); + } + + public void testPointsOnly() throws Exception { + String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject() + .startObject("properties").startObject(defaultGeoFieldName) + .field("type", "geo_shape") + .field("tree", randomBoolean() ? "quadtree" : "geohash") + .field("tree_levels", "6") + .field("distance_error_pct", "0.01") + .field("points_only", true) + .endObject() + .endObject().endObject()); + + client().admin().indices().prepareCreate("geo_points_only").addMapping(defaultType, mapping, XContentType.JSON).get(); + ensureGreen(); + + Geometry geometry = GeometryTestUtils.randomGeometry(false); + try { + client().prepareIndex("geo_points_only", defaultType).setId("1") + .setSource(GeoJson.toXContent(geometry, jsonBuilder().startObject().field(defaultGeoFieldName), null).endObject()) + .setRefreshPolicy(IMMEDIATE).get(); + } catch (MapperParsingException e) { + // Random geometry generator created something other than a POINT type, verify the correct exception is thrown + assertThat(e.getMessage(), containsString("is configured for points only")); + return; + } + + // test that point was inserted + SearchResponse response = + client().prepareSearch("geo_points_only").setQuery(geoIntersectionQuery(defaultGeoFieldName, geometry)).get(); + assertEquals(1, response.getHits().getTotalHits().value); + } + + public void testFieldAlias() throws IOException { + String mapping = Strings.toString(XContentFactory.jsonBuilder() + .startObject() + .startObject("properties") + .startObject(defaultGeoFieldName) + .field("type", "geo_shape") + .field("tree", randomBoolean() ? "quadtree" : "geohash") + .endObject() + .startObject("alias") + .field("type", "alias") + .field("path", defaultGeoFieldName) + .endObject() + .endObject() + .endObject()); + + client().admin().indices().prepareCreate(defaultIndexName).addMapping(defaultType, mapping, XContentType.JSON).get(); + ensureGreen(); + + MultiPoint multiPoint = GeometryTestUtils.randomMultiPoint(false); + client().prepareIndex(defaultIndexName, defaultType).setId("1") + .setSource(GeoJson.toXContent(multiPoint, jsonBuilder().startObject().field(defaultGeoFieldName), null).endObject()) + .setRefreshPolicy(IMMEDIATE).get(); + + SearchResponse response = client().prepareSearch(defaultIndexName) + .setQuery(geoShapeQuery("alias", multiPoint)) + .get(); + assertEquals(1, response.getHits().getTotalHits().value); + } +}