Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix queries over ill-formatted geospatial data #6989

Merged
merged 2 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
([PR #6837](https://github.com/realm/realm-core/pull/6837), since v10.0.0)
* Reading existing logged-in users on app startup from the sync metadata Realm performed three no-op writes per user on the metadata Realm ([PR #6837](https://github.com/realm/realm-core/pull/6837), since v10.0.0).
* If a user was logged out while an access token refresh was in progress, the refresh completing would mark the user as logged in again and the user would be in an inconsistent state ([PR #6837](https://github.com/realm/realm-core/pull/6837), since v10.0.0).
* If querying over a geospatial dataset that had some objects with a type property set to something other than 'Point' (case insensitive) an exception would have been thrown. Instead of disrupting the query, those objects are now just ignored. ([PR 6989](https://github.com/realm/realm-core/issues/6989), since the introduction of geospatial)
* The Swift package failed to link required libraries when building for macCatalyst.

### Breaking changes
Expand Down
6 changes: 4 additions & 2 deletions src/realm/geospatial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ std::optional<GeoPoint> Geospatial::point_from_obj(const Obj& obj, ColKey type_c
}

if (!type_is_valid(obj.get<StringData>(type_col))) {
throw IllegalOperation("The only Geospatial type currently supported is 'point'");
return {}; // the only geospatial type currently supported is 'Point'
}

Lst<double> coords = obj.get_list<double>(coords_col);
Expand Down Expand Up @@ -211,7 +211,9 @@ void Geospatial::assign_to(Obj& link) const
return;
}
if (type != Type::Point) {
throw IllegalOperation("The only Geospatial type currently supported for storage is 'point'");
throw IllegalOperation(util::format("Attempting to store a '%1' in class '%2' but the only Geospatial type "
"currently supported for storage is 'Point'",
get_type_string(), link.get_table()->get_class_name()));
}
auto&& point = get<GeoPoint>();
link.set(type_col, get_type_string());
Expand Down
34 changes: 29 additions & 5 deletions test/test_query_geo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,13 @@ TEST(Geospatial_Assignment)

Geospatial geo_box(GeoBox{GeoPoint{1.1, 2.2}, GeoPoint{3.3, 4.4}});
CHECK(*GeoBox::from_polygon(geo_box.get<GeoBox>().to_polygon()) == geo_box.get<GeoBox>());
std::string_view err_msg = "The only Geospatial type currently supported for storage is 'point'";
CHECK_THROW_CONTAINING_MESSAGE(obj.set(location_column_key, geo_box), err_msg);
std::string_view err_msg_box = "Attempting to store a 'Box' in class 'Location' but the only Geospatial type "
"currently supported for storage is 'Point'";
CHECK_THROW_CONTAINING_MESSAGE(obj.set(location_column_key, geo_box), err_msg_box);
Geospatial geo_circle(GeoCircle{10, GeoPoint{1.1, 2.2}});
CHECK_THROW_CONTAINING_MESSAGE(obj.set(location_column_key, geo_circle), err_msg);
std::string_view err_msg_circle = "Attempting to store a 'Circle' in class 'Location' but the only Geospatial "
"type currently supported for storage is 'Point'";
CHECK_THROW_CONTAINING_MESSAGE(obj.set(location_column_key, geo_circle), err_msg_circle);
}

TEST(Geospatial_invalid_format)
Expand All @@ -137,10 +140,31 @@ TEST(Geospatial_invalid_format)
TEST(Query_GeoWithinBasics)
{
Group g;
std::vector<Geospatial> data = {GeoPoint{-2, -1}, GeoPoint{-1, -2}, GeoPoint{0, 0},
GeoPoint{0.5, 0.5}, GeoPoint{1, 1}, GeoPoint{2, 2}};
std::vector<Geospatial> data = {GeoPoint{-2, -1}, GeoPoint{-1, -2}, GeoPoint{0, 0}, GeoPoint{0.5, 0.5},
GeoPoint{1, 1}, GeoPoint{2, 2, 2}, GeoPoint()};
TableRef table = setup_with_points(g, data);
ColKey location_column_key = table->get_column_key("location");
// an object with null link location
table->create_object_with_primary_key(-42);
// an object with a location that doesn't have properties set on the point
Obj invalid_point = table->create_object_with_primary_key(-43);
invalid_point.create_and_set_linked_object(location_column_key);
// an object with the correct 'Point' but invalid coordinates
Obj invalid_coords = table->create_object_with_primary_key(-44);
Obj embedded_invalid = invalid_coords.create_and_set_linked_object(location_column_key);
embedded_invalid.set(embedded_invalid.get_table()->get_column_key("type"), "Point");
// an object with 4 elements in the coordinate list
Obj excess_coords = table->create_object_with_primary_key(-44);
Obj embedded_excess = excess_coords.create_and_set_linked_object(location_column_key);
embedded_excess.set(embedded_excess.get_table()->get_column_key("type"), "Point");
auto list = embedded_excess.get_list<double>(embedded_excess.get_table()->get_column_key("coordinates"));
list.add(2);
list.add(2);
list.add(2);
list.add(2);
Geospatial geo_excess = excess_coords.get<Geospatial>(location_column_key);
CHECK(geo_excess.is_valid() == Status::OK());

for (size_t i = 0; i < data.size(); ++i) {
Obj obj = table->get_object_with_primary_key(int64_t(i));
CHECK(obj);
Expand Down