8000 Fix geo point definition to accept z value · arangodb/arangodb@90feb2f · GitHub
[go: up one dir, main page]
Skip to content

Commit 90feb2f

Browse files
jbajicjbajic
committed
Fix geo point definition to accept z value
1 parent 46ef058 commit 90feb2f

File tree

6 files changed

+339
-3
lines changed

6 files changed

+339
-3
lines changed

‎CHANGELOG‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
v3.11.4 (2023-10-04)
22
--------------------
33

4+
* Fix BTS-2139: GeoJSON coordinate validation now properly accepts both 2D and 3D
5+
coordinates for Point geometries.
6+
47
* Fix updating of collection properties (schema) when the collection has a view
58
on top.
69

‎arangod/Aql/AqlFunctionFeature.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ void AqlFunctionFeature::addGeometryConstructors() {
425425
FF::CanRunOnDBServerOneShard, FF::CanUseInAnalyzer);
426426

427427
// geometry types
428-
add({"GEO_POINT", ".,.", flags, &functions::GeoPoint});
428+
add({"GEO_POINT", ".,.|.", flags, &functions::GeoPoint});
429429
add({"GEO_MULTIPOINT", ".", flags, &functions::GeoMultiPoint});
430430
add({"GEO_POLYGON", ".", flags, &functions::GeoPolygon});
431431
add({"GEO_MULTIPOLYGON", ".", flags, &functions::GeoMultiPolygon});

‎arangod/Aql/Functions.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6303,20 +6303,31 @@ AqlValue functions::GeoPoint(ExpressionContext* expressionContext,
63036303

63046304
AqlValue lon1 = extractFunctionParameterValue(parameters, 0);
63056305
AqlValue lat1 = extractFunctionParameterValue(parameters, 1);
6306+
AqlValue z1 = extractFunctionParameterValue(parameters, 2);
63066307

63076308
// non-numeric input
63086309
if (!lat1.isNumber() || !lon1.isNumber()) {
63096310
registerWarning(expressionContext, "GEO_POINT",
63106311
TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH);
63116312
return AqlValue(AqlValueHintNull());
63126313
}
6314+
if (!z1.isEmpty() && !z1.isNumber()) {
6315+
registerWarning(expressionContext, "GEO_POINT",
6316+
TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH);
6317+
return AqlValue(AqlValueHintNull());
6318+
}
63136319

63146320
bool failed;
63156321
bool error = false;
63166322
double lon1Value = lon1.toDouble(failed);
63176323
error |= failed;
63186324
double lat1Value = lat1.toDouble(failed);
63196325
error |= failed;
6326+
std::optional<double> z1Value;
6327+
if (!z1.isEmpty()) {
6328+
z1Value = z1.toDouble(failed);
6329+
error |= failed;
6330+
}
63206331

63216332
if (error) {
63226333
registerWarning(expressionContext, "GEO_POINT",
@@ -6330,6 +6341,9 @@ AqlValue functions::GeoPoint(ExpressionContext* expressionContext,
63306341
builder->add("coordinates", VPackValue(VPackValueType::Array));
63316342
builder->add(VPackValue(lon1Value));
63326343
builder->add(VPackValue(lat1Value));
6344+
if (z1Value.has_value()) {
6345+
builder->add(VPackValue(*z1Value));
6346+
}
63336347
builder->close();
63346348
builder->close();
63356349

‎lib/Geo/GeoJson.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,8 @@ template<bool Validation, bool GeoJson>
201201
bool parseImpl(velocypack::Slice vpack, S2LatLng& vertex) {
202202
TRI_ASSERT(vpack.isArray());
203203
velocypack::ArrayIterator jt{vpack};
204-
if (Validation && ADB_UNLIKELY(jt.size() != 2)) {
204+
// We ignore Z coordinate for now since it's not supported by S2
205+
if (Validation && (jt.size() < 2 || jt.size() > 3)) [[unlikely]] {
205206
return false;
206207
}
207208
auto const first = *jt;

‎tests/Geo/GeoJsonTest.cpp‎

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
#include <velocypack/Builder.h>
3232

3333
#include "Aql/VelocyPackHelper.h"
34-
#include "Basics/Common.h"
3534
#include "Basics/voc-errors.h"
3635
#include "Geo/GeoJson.h"
3736
#include "Geo/ShapeContainer.h"
@@ -183,6 +182,36 @@ TEST_F(InvalidGeoJSONInputTest, bad_point_too_many_coordinates) {
183182
coords->add(VPackValue(0.0));
184183
coords->add(VPackValue(0.0));
185184
coords->add(VPackValue(0.0));
185+
coords->add(VPackValue(0.0)); // 4 coordinates - this should be invalid
186+
}
187+
VPackSlice vpack = builder.slice();
188+
189+
ASSERT_EQ(geo::json::Type::POINT, geo::json::type(vpack));
190+
ASSERT_TRUE(geo::json::parsePoint(vpack, point).is(TRI_ERROR_BAD_PARAMETER));
191+
}
192+
193+
TEST_F(InvalidGeoJSONInputTest, bad_point_invalid_coordinate_count) {
194+
{
195+
velocypack::ObjectBuilder object(&builder);
196+
object->add("type", VPackValue("Point"));
197+
velocypack::ArrayBuilder coords(&builder, "coordinates");
198+
coords->add(VPackValue(0.0));
199+
}
200+
VPackSlice vpack = builder.slice();
201+
202+
ASSERT_EQ(geo::json::Type::POINT, geo::json::type(vpack));
203+
ASSERT_TRUE(geo::json::parsePoint(vpack, point).is(TRI_ERROR_BAD_PARAMETER));
204+
}
205+
206+
TEST_F(InvalidGeoJSONInputTest, bad_point_four_coordinates) {
207+
{
208+
velocypack::ObjectBuilder object(&builder);
209+
object->add("type", VPackValue("Point"));
210+
velocypack::ArrayBuilder coords(&builder, "coordinates");
211+
coords->add(VPackValue(0.0));
212+
coords->add(VPackValue(0.0));
213+
coords->add(VPackValue(0.0));
214+
coords->add(VPackValue(0.0));
186215
}
187216
VPackSlice vpack = builder.slice();
188217

@@ -262,12 +291,14 @@ TEST_F(InvalidGeoJSONInputTest, bad_multipoint_extra_numbers_in_bad_points) {
262291
point->add(VPackValue(0.0));
263292
point->add(VPackValue(0.0));
264293
point->add(VPackValue(0.0));
294+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
265295
}
266296
{
267297
velocypack::ArrayBuilder point(&builder);
268298
point->add(VPackValue(0.0));
269299
point->add(VPackValue(0.0));
270300
point->add(VPackValue(0.0));
301+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
271302
}
272303
}
273304
VPackSlice vpack = builder.slice();
@@ -327,12 +358,14 @@ TEST_F(InvalidGeoJSONInputTest, bad_linestring_extra_numbers_in_bad_points) {
327358
point->add(VPackValue(0.0));
328359
point->add(VPackValue(0.0));
329360
point->add(VPackValue(0.0));
361+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
330362
}
331363
{
332364
velocypack::ArrayBuilder point(&builder);
333365
point->add(VPackValue(0.0));
334366
point->add(VPackValue(0.0));
335367
point->add(VPackValue(0.0));
368+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
336369
}
337370
}
338371
VPackSlice vpack = builder.slice();
@@ -412,12 +445,14 @@ TEST_F(InvalidGeoJSONInputTest,
412445
point->add(VPackValue(0.0));
413446
point->add(VPackValue(0.0));
414447
point->add(VPackValue(0.0));
448+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
415449
}
416450
{
417451
velocypack::ArrayBuilder point(&builder);
418452
point->add(VPackValue(0.0));
419453
point->add(VPackValue(0.0));
420454
point->add(VPackValue(0.0));
455+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
421456
}
422457
}
423458
VPackSlice vpack = builder.slice();
@@ -487,12 +522,14 @@ TEST_F(InvalidGeoJSONInputTest, bad_loop_extra_numbers_in_bad_points) {
487522
point->add(VPackValue(0.0));
488523
point->add(VPackValue(0.0));
489524
point->add(VPackValue(0.0));
525+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
490526
}
491527
{
492528
velocypack::ArrayBuilder point(&builder);
493529
point->add(VPackValue(0.0));
494530
point->add(VPackValue(0.0));
495531
point->add(VPackValue(0.0));
532+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
496533
}
497534
}
498535
VPackSlice vpack = builder.slice();
@@ -627,24 +664,28 @@ TEST_F(InvalidGeoJSONInputTest, bad_polygon_extra_numbers_in_bad_points) {
627664
point->add(VPackValue(0.0));
628665
point->add(VPackValue(0.0));
629666
point->add(VPackValue(0.0));
667+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
630668
}
631669
{
632670
velocypack::ArrayBuilder point(&builder);
633671
point->add(VPackValue(1.0));
634672
point->add(VPackValue(0.0));
635673
point->add(VPackValue(0.0));
674+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
636675
}
637676
{
638677
velocypack::ArrayBuilder point(&builder);
639678
point->add(VPackValue(0.0));
640679
point->add(VPackValue(1.0));
641680
point->add(VPackValue(0.0));
681+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
642682
}
643683
{
644684
velocypack::ArrayBuilder point(&builder);
645685
point->add(VPackValue(0.0));
646686
point->add(VPackValue(0.0));
647687
point->add(VPackValue(0.0));
688+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
648689
}
649690
}
650691
}
@@ -974,24 +1015,28 @@ TEST_F(InvalidGeoJSONInputTest, bad_multipolygon_extra_numbers_in_bad_points) {
9741015
point->add(VPackValue(0.0));
9751016
point->add(VPackValue(0.0));
9761017
point->add(VPackValue(0.0));
1018+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
9771019
}
9781020
{
9791021
velocypack::ArrayBuilder point(&builder);
9801022
point->add(VPackValue(1.0));
9811023
point->add(VPackValue(0.0));
9821024
point->add(VPackValue(0.0));
1025+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
9831026
}
9841027
{
9851028
velocypack::ArrayBuilder point(&builder);
9861029
point->add(VPackValue(0.0));
9871030
point->add(VPackValue(1.0));
9881031
point->add(VPackValue(0.0));
1032+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
9891033
}
9901034
{
9911035
velocypack::ArrayBuilder point(&builder);
9921036
point->add(VPackValue(0.0));
9931037
point->add(VPackValue(0.0));
9941038
point->add(VPackValue(0.0));
1039+
point->add(VPackValue(0.0)); // 4 coordinates - invalid
9951040
}
9961041
}
9971042
}
@@ -1238,6 +1283,40 @@ TEST_F(ValidGeoJSONInputTest, valid_point) {
12381283
ASSERT_EQ(1.0, point.lat().degrees());
12391284
}
12401285

1286+
TEST_F(ValidGeoJSONInputTest, valid_point_with_z_coordinate) {
1287+
{
1288+
velocypack::ObjectBuilder object(&builder);
1289+
object->add("type", VPackValue("Point"));
1290+
velocypack::ArrayBuilder coords(&builder, "coordinates");
1291+
coords->add(VPackValue(0.0));
1292+
coords->add(VPackValue(1.0));
1293+
coords->add(VPackValue(100.0)); // Z coordinate (elevation)
1294+
}
1295+
VPackSlice vpack = builder.slice();
1296+
1297+
ASSERT_EQ(geo::json::Type::POINT, geo::json::type(vpack));
1298+
ASSERT_TRUE(geo::json::parsePoint(vpack, point).ok());
1299+
ASSERT_EQ(0.0, point.lng().degrees());
1300+
ASSERT_EQ(1.0, point.lat().degrees());
1301+
}
1302+
1303+
TEST_F(ValidGeoJSONInputTest, valid_point_with_z_coordinate_negative) {
1304+
{
1305+
velocypack::ObjectBuilder object(&builder);
1306+
object->add("type", VPackValue("Point"));
1307+
velocypack::ArrayBuilder coords(&builder, "coordinates");
1308+
coords->add(VPackValue(0.0));
1309+
coords->add(VPackValue(1.0));
1310+
coords->add(VPackValue(-100.0)); // Negative Z coordinate
1311+
}
1312+
VPackSlice vpack = builder.slice();
1313+
1314+
ASSERT_EQ(geo::json::Type::POINT, geo::json::type(vpack));
1315+
ASSERT_TRUE(geo::json::parsePoint(vpack, point).ok());
1316+
ASSERT_EQ(0.0, point.lng().degrees());
1317+
ASSERT_EQ(1.0, point.lat().degrees());
1318+
}
1319+
12411320
TEST_F(ValidGeoJSONInputTest, valid_point_as_region) {
12421321
{
12431322
velocypack::ObjectBuilder object(&builder);
@@ -2199,4 +2278,46 @@ TEST_F(ValidGeoJSONInputTest, NestedHoles) {
21992278
ASSERT_TRUE(r.ok()) << r.errorMessage();
22002279
}
22012280

2281+
TEST_F(ValidGeoJSONInputTest, valid_linestring_with_z_coordinates) {
2282+
{
2283+
velocypack::ObjectBuilder object(&builder);
2284+
object->add("type", VPackValue("Linestring"));
2285+
velocypack::ArrayBuilder points(&builder, "coordinates");
2286+
{
2287+
velocypack::ArrayBuilder point(&builder);
2288+
point->add(VPackValue(0.0));
2289+
point->add(VPackValue(0.0));
2290+
point->add(VPackValue(100.0)); // Z coordinate
2291+
}
2292+
{
2293+
velocypack::ArrayBuilder point(&builder);
2294+
point->add(VPackValue(1.0));
2295+
point->add(VPackValue(0.0));
2296+
point->add(VPackValue(200.0)); // Z coordinate
2297+
}
2298+
{
2299+
velocypack::ArrayBuilder point(&builder);
2300+
point->add(VPackValue(1.0));
2301+
point->add(VPackValue(1.0));
2302+
point->add(VPackValue(300.0)); // Z coordinate
2303+
}
2304+
{
2305+
velocypack::ArrayBuilder point(&builder);
2306+
point->add(VPackValue(0.0));
2307+
point->add(VPackValue(1.0));
2308+
point->add(VPackValue(400.0)); // Z coordinate
2309+
}
2310+
}
2311+
VPackSlice vpack = builder.slice();
2312+
2313+
ASSERT_EQ(geo::json::Type::LINESTRING, geo::json::type(vpack));
2314+
ASSERT_TRUE(geo::json::parseLinestring(vpack, line).ok());
2315+
2316+
ASSERT_EQ(4, line.num_vertices());
2317+
ASSERT_EQ(S2LatLng::FromDegrees(0.0, 0.0).ToPoint(), line.vertex(0));
2318+
ASSERT_EQ(S2LatLng::FromDegrees(0.0, 1.0).ToPoint(), line.vertex(1));
2319+
ASSERT_EQ(S2LatLng::FromDegrees(1.0, 1.0).ToPoint(), line.vertex(2));
2320+
ASSERT_EQ(S2LatLng::FromDegrees(1.0, 0.0).ToPoint(), line.vertex(3));
2321+
}
2322+
22022323
} // namespace arangodb

0 commit comments

Comments
 (0)
0