Skip to content

Commit f58d785

Browse files
iveraseiverase
authored
Deprecate GeoPolygon query in favour of GeoShape query. (#64227)
1 parent e97f233 commit f58d785

File tree

6 files changed

+115
-62
lines changed

6 files changed

+115
-62
lines changed

‎docs/reference/query-dsl/geo-polygon-query.asciidoc‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
<titleabbrev>Geo-polygon</titleabbrev>
55
++++
66

7+
deprecated[7.12.0,"Use <<query-dsl-geo-shape-query>> instead,
8+
where polygons are defined in geojson or wkt."]
9+
710
A query returning hits that only fall within a polygon of
811
points. Here is an example:
912

@@ -31,6 +34,7 @@ GET /_search
3134
}
3235
}
3336
--------------------------------------------------
37+
// TEST[warning:Deprecated field [geo_polygon] used, replaced by [[geo_shape] query where polygons are defined in geojson or wkt]]
3438

3539
[discrete]
3640
==== Query Options
@@ -80,6 +84,7 @@ GET /_search
8084
}
8185
}
8286
--------------------------------------------------
87+
// TEST[warning:Deprecated field [geo_polygon] used, replaced by [[geo_shape] query where polygons are defined in geojson or wkt]]
8388

8489
[discrete]
8590
===== Lat Lon as String
@@ -110,6 +115,7 @@ GET /_search
110115
}
111116
}
112117
--------------------------------------------------
118+
// TEST[warning:Deprecated field [geo_polygon] used, replaced by [[geo_shape] query where polygons are defined in geojson or wkt]]
113119

114120
[discrete]
115121
===== Geohash
@@ -138,6 +144,7 @@ GET /_search
138144
}
139145
}
140146
--------------------------------------------------
147+
// TEST[warning:Deprecated field [geo_polygon] used, replaced by [[geo_shape] query where polygons are defined in geojson or wkt]]
141148

142149
[discrete]
143150
==== geo_point Type

‎server/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,16 @@
3232
import java.util.List;
3333
import java.util.Objects;
3434

35+
/**
36+
* @deprecated use {@link GeoShapeQueryBuilder}
37+
*/
38+
@Deprecated
3539
public class GeoPolygonQueryBuilder extends AbstractQueryBuilder<GeoPolygonQueryBuilder> {
3640
public static final String NAME = "geo_polygon";
3741

42+
public static final String GEO_POLYGON_DEPRECATION_MSG = "[" + GeoShapeQueryBuilder.NAME + "] query " +
43+
"where polygons are defined in geojson or wkt";
44+
3845
/**
3946
* The default value for ignore_unmapped.
4047
*/
@@ -51,6 +58,7 @@ public class GeoPolygonQueryBuilder extends AbstractQueryBuilder<GeoPolygonQuery
5158

5259
private boolean ignoreUnmapped = DEFAULT_IGNORE_UNMAPPED;
5360

61+
@Deprecated
5462
public GeoPolygonQueryBuilder(String fieldName, List<GeoPoint> points) {
5563
if (Strings.isEmpty(fieldName)) {
5664
throw new IllegalArgumentException("fieldName must not be null");

‎server/src/main/java/org/elasticsearch/index/query/QueryBuilders.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,9 @@ public static GeoBoundingBoxQueryBuilder geoBoundingBoxQuery(String name) {
613613
* A filter to filter based on a polygon defined by a set of locations / points.
614614
*
615615
* @param name The location field name.
616+
* @deprecated use {@link #geoIntersectionQuery(String, Geometry)} instead
616617
*/
618+
@Deprecated
617619
public static GeoPolygonQueryBuilder geoPolygonQuery(String name, List<GeoPoint> points) {
618620
return new GeoPolygonQueryBuilder(name, points);
619621
}

‎server/src/main/java/org/elasticsearch/search/SearchModule.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,9 @@ private void registerQueryParsers(List<SearchPlugin> plugins) {
807807
registerQuery(new QuerySpec<>(GeoDistanceQueryBuilder.NAME, GeoDistanceQueryBuilder::new, GeoDistanceQueryBuilder::fromXContent));
808808
registerQuery(new QuerySpec<>(GeoBoundingBoxQueryBuilder.NAME, GeoBoundingBoxQueryBuilder::new,
809809
GeoBoundingBoxQueryBuilder::fromXContent));
810-
registerQuery(new QuerySpec<>(GeoPolygonQueryBuilder.NAME, GeoPolygonQueryBuilder::new, GeoPolygonQueryBuilder::fromXContent));
810+
registerQuery(new QuerySpec<>(
811+
(new ParseField(GeoPolygonQueryBuilder.NAME).withAllDeprecated(GeoPolygonQueryBuilder.GEO_POLYGON_DEPRECATION_MSG)),
812+
GeoPolygonQueryBuilder::new, GeoPolygonQueryBuilder::fromXContent));
811813
registerQuery(new QuerySpec<>(ExistsQueryBuilder.NAME, ExistsQueryBuilder::new, ExistsQueryBuilder::fromXContent));
812814
registerQuery(new QuerySpec<>(MatchNoneQueryBuilder.NAME, MatchNoneQueryBuilder::new, MatchNoneQueryBuilder::fromXContent));
813815
registerQuery(new QuerySpec<>(TermsSetQueryBuilder.NAME, TermsSetQueryBuilder::new, TermsSetQueryBuilder::fromXContent));

‎server/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java‎

Lines changed: 94 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ protected void doAssertLuceneQuery(GeoPolygonQueryBuilder queryBuilder, Query qu
6262
List<GeoPoint> points = queryBuilder.points();
6363
double[] lats = new double[points.size()];
6464
double[] lons = new double[points.size()];
65-
for (int i =0; i < points.size(); i++) {
65+
for (int i = 0; i < points.size(); i++) {
6666
lats[i] = points.get(i).getLat();
6767
lons[i] = points.get(i).getLon();
6868
}
@@ -73,6 +73,34 @@ protected void doAssertLuceneQuery(GeoPolygonQueryBuilder queryBuilder, Query qu
7373
}
7474
}
7575

76+
@Override
77+
public void testUnknownField() throws IOException {
78+
super.testUnknownField();
79+
assertDeprecationWarning();
80+
}
81+
82+
@Override
83+
public void testUnknownObjectException() throws IOException {
84+
super.testUnknownObjectException();
85+
assertDeprecationWarning();
86+
}
87+
88+
@Override
89+
public void testFromXContent() throws IOException {
90+
super.testFromXContent();
91+
assertDeprecationWarning();
92+
}
93+
94+
@Override
95+
public void testValidOutput() throws IOException {
96+
super.testValidOutput();
97+
assertDeprecationWarning();
98+
}
99+
100+
private void assertDeprecationWarning() {
101+
assertWarnings("Deprecated field [geo_polygon] used, replaced by [[geo_shape] query where polygons are defined in geojson or wkt]");
102+
}
103+
76104
private static List<GeoPoint> randomPolygon() {
77105
ShapeBuilder<?, ?, ?> shapeBuilder = null;
78106
// This is a temporary fix because sometimes the RandomShapeGenerator
@@ -97,7 +125,7 @@ public void testNullFieldName() {
97125

98126
public void testEmptyPolygon() {
99127
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
100-
() -> new GeoPolygonQueryBuilder(GEO_POINT_FIELD_NAME, Collections.emptyList()));
128+
() -> new GeoPolygonQueryBuilder(GEO_POINT_FIELD_NAME, Collections.emptyList()));
101129
assertEquals("polygon must not be null or empty", e.getMessage());
102130

103131
e = expectThrows(IllegalArgumentException.class, () -> new GeoPolygonQueryBuilder(GEO_POINT_FIELD_NAME, null));
@@ -110,7 +138,7 @@ public void testInvalidClosedPolygon() {
110138
points.add(new GeoPoint(90, 90));
111139
points.add(new GeoPoint(0, 90));
112140
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
113-
() -> new GeoPolygonQueryBuilder(GEO_POINT_FIELD_NAME, points));
141+
() -> new GeoPolygonQueryBuilder(GEO_POINT_FIELD_NAME, points));
114142
assertEquals("too few points defined for geo_polygon query", e.getMessage());
115143
}
116144

@@ -119,91 +147,96 @@ public void testInvalidOpenPolygon() {
119147
points.add(new GeoPoint(0, 90));
120148
points.add(new GeoPoint(90, 90));
121149
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
122-
() -> new GeoPolygonQueryBuilder(GEO_POINT_FIELD_NAME, points));
150+
() -> new GeoPolygonQueryBuilder(GEO_POINT_FIELD_NAME, points));
123151
assertEquals("too few points defined for geo_polygon query", e.getMessage());
124152
}
125153

126154
public void testParsingAndToQueryParsingExceptions() throws IOException {
127155
String[] brokenFiles = new String[]{
128-
"/org/elasticsearch/index/query/geo_polygon_exception_1.json",
129-
"/org/elasticsearch/index/query/geo_polygon_exception_2.json",
130-
"/org/elasticsearch/index/query/geo_polygon_exception_3.json",
131-
"/org/elasticsearch/index/query/geo_polygon_exception_4.json",
132-
"/org/elasticsearch/index/query/geo_polygon_exception_5.json"
156+
"/org/elasticsearch/index/query/geo_polygon_exception_1.json",
157+
"/org/elasticsearch/index/query/geo_polygon_exception_2.json",
158+
"/org/elasticsearch/index/query/geo_polygon_exception_3.json",
159+
"/org/elasticsearch/index/query/geo_polygon_exception_4.json",
160+
"/org/elasticsearch/index/query/geo_polygon_exception_5.json"
133161
};
134162
for (String brokenFile : brokenFiles) {
135163
String query = copyToStringFromClasspath(brokenFile);
136164
expectThrows(ParsingException.class, () -> parseQuery(query));
137165
}
166+
assertDeprecationWarning();
138167
}
139168

140169
public void testParsingAndToQuery1() throws IOException {
141170
String query = "{\n" +
142-
" \"geo_polygon\":{\n" +
143-
" \"" + GEO_POINT_FIELD_NAME + "\":{\n" +
144-
" \"points\":[\n" +
145-
" [-70, 40],\n" +
146-
" [-80, 30],\n" +
147-
" [-90, 20]\n" +
148-
" ]\n" +
149-
" }\n" +
150-
" }\n" +
151-
"}\n";
171+
" \"geo_polygon\":{\n" +
172+
" \"" + GEO_POINT_FIELD_NAME + "\":{\n" +
173+
" \"points\":[\n" +
174+
" [-70, 40],\n" +
175+
" [-80, 30],\n" +
176+
" [-90, 20]\n" +
177+
" ]\n" +
178+
" }\n" +
179+
" }\n" +
180+
"}\n";
152181
assertGeoPolygonQuery(query);
182+
assertDeprecationWarning();
153183
}
154184

155185
public void testParsingAndToQuery2() throws IOException {
156186
String query = "{\n" +
157-
" \"geo_polygon\":{\n" +
158-
" \"" + GEO_POINT_FIELD_NAME + "\":{\n" +
159-
" \"points\":[\n" +
160-
" {\n" +
161-
" \"lat\":40,\n" +
162-
" \"lon\":-70\n" +
163-
" },\n" +
164-
" {\n" +
165-
" \"lat\":30,\n" +
166-
" \"lon\":-80\n" +
167-
" },\n" +
168-
" {\n" +
169-
" \"lat\":20,\n" +
170-
" \"lon\":-90\n" +
171-
" }\n" +
172-
" ]\n" +
173-
" }\n" +
174-
" }\n" +
175-
"}\n";
187+
" \"geo_polygon\":{\n" +
188+
" \"" + GEO_POINT_FIELD_NAME + "\":{\n" +
189+
" \"points\":[\n" +
190+
" {\n" +
191+
" \"lat\":40,\n" +
192+
" \"lon\":-70\n" +
193+
" },\n" +
194+
" {\n" +
195+
" \"lat\":30,\n" +
196+
" \"lon\":-80\n" +
197+
" },\n" +
198+
" {\n" +
199+
" \"lat\":20,\n" +
200+
" \"lon\":-90\n" +
201+
" }\n" +
202+
" ]\n" +
203+
" }\n" +
204+
" }\n" +
205+
"}\n";
176206
assertGeoPolygonQuery(query);
207+
assertDeprecationWarning();
177208
}
178209

179210
public void testParsingAndToQuery3() throws IOException {
180211
String query = "{\n" +
181-
" \"geo_polygon\":{\n" +
182-
" \"" + GEO_POINT_FIELD_NAME + "\":{\n" +
183-
" \"points\":[\n" +
184-
" \"40, -70\",\n" +
185-
" \"30, -80\",\n" +
186-
" \"20, -90\"\n" +
187-
" ]\n" +
188-
" }\n" +
189-
" }\n" +
190-
"}\n";
212+
" \"geo_polygon\":{\n" +
213+
" \"" + GEO_POINT_FIELD_NAME + "\":{\n" +
214+
" \"points\":[\n" +
215+
" \"40, -70\",\n" +
216+
" \"30, -80\",\n" +
217+
" \"20, -90\"\n" +
218+
" ]\n" +
219+
" }\n" +
220+
" }\n" +
221+
"}\n";
191222
assertGeoPolygonQuery(query);
223+
assertDeprecationWarning();
192224
}
193225

194226
public void testParsingAndToQuery4() throws IOException {
195227
String query = "{\n" +
196-
" \"geo_polygon\":{\n" +
197-
" \"" + GEO_POINT_FIELD_NAME + "\":{\n" +
198-
" \"points\":[\n" +
199-
" \"drn5x1g8cu2y\",\n" +
200-
" \"30, -80\",\n" +
201-
" \"20, -90\"\n" +
202-
" ]\n" +
203-
" }\n" +
204-
" }\n" +
205-
"}\n";
228+
" \"geo_polygon\":{\n" +
229+
" \"" + GEO_POINT_FIELD_NAME + "\":{\n" +
230+
" \"points\":[\n" +
231+
" \"drn5x1g8cu2y\",\n" +
232+
" \"30, -80\",\n" +
233+
" \"20, -90\"\n" +
234+
" ]\n" +
235+
" }\n" +
236+
" }\n" +
237+
"}\n";
206238
assertGeoPolygonQuery(query);
239+
assertDeprecationWarning();
207240
}
208241

209242
private void assertGeoPolygonQuery(String query) throws IOException {
@@ -214,7 +247,7 @@ private void assertGeoPolygonQuery(String query) throws IOException {
214247

215248
public void testFromJson() throws IOException {
216249
String json =
217-
"{\n" +
250+
"{\n" +
218251
" \"geo_polygon\" : {\n" +
219252
" \"person.location\" : {\n" +
220253
" \"points\" : [ [ -70.0, 40.0 ], [ -80.0, 30.0 ], [ -90.0, 20.0 ], [ -70.0, 40.0 ] ]\n" +
@@ -227,6 +260,7 @@ public void testFromJson() throws IOException {
227260
GeoPolygonQueryBuilder parsed = (GeoPolygonQueryBuilder) parseQuery(json);
228261
checkGeneratedJson(json, parsed);
229262
assertEquals(json, 4, parsed.points().size());
263+
assertDeprecationWarning();
230264
}
231265

232266
public void testIgnoreUnmapped() throws IOException {
@@ -274,5 +308,6 @@ public void testPointValidation() throws IOException {
274308

275309
QueryShardException e2 = expectThrows(QueryShardException.class, () -> parseQuery(queryInvalidLon).toQuery(context));
276310
assertThat(e2.getMessage(), containsString("illegal longitude value [-190.0] for [geo_polygon]"));
311+
assertDeprecationWarning();
277312
}
278313
}

‎server/src/test/java/org/elasticsearch/search/SearchModuleTests.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,6 @@ public List<RescorerSpec<?>> getRescorers() {
307307
"fuzzy",
308308
"geo_bounding_box",
309309
"geo_distance",
310-
"geo_polygon",
311310
"geo_shape",
312311
"ids",
313312
"intervals",
@@ -345,7 +344,7 @@ public List<RescorerSpec<?>> getRescorers() {
345344
};
346345

347346
//add here deprecated queries to make sure we log a deprecation warnings when they are used
348-
private static final String[] DEPRECATED_QUERIES = new String[] {};
347+
private static final String[] DEPRECATED_QUERIES = new String[] {"geo_polygon"};
349348

350349
/**
351350
* Dummy test {@link AggregationBuilder} used to test registering aggregation builders.

0 commit comments

Comments
 (0)