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

Rudimentary geospatial support #1389

Merged
merged 18 commits into from
Nov 1, 2023
Merged

Rudimentary geospatial support #1389

merged 18 commits into from
Nov 1, 2023

Conversation

nielsenko
Copy link
Contributor

@nielsenko nielsenko commented Aug 28, 2023

Add rudimentary geospatial support. Example:

@RealmModel(ObjectType.embeddedObject)
class _Location {
  final String type = 'Point';
  List<double> coordinates = const [0, 0];
}

@RealmModel()
class _Restaurant {
  @PrimaryKey()
  late String name;
  _Location? location;
}

realm.query<Restaurant>('location geoWithin $shape');

Supported shapes are GeoBox, GeoCircle, and GeoPolygon

realm.query<Restaurant>('location geoWithin \$0', [shape]);

is also supported, thanks to realm/realm-core#6934, despite C-api not supporting geo shapes.

It works by allowing string arguments where geo shapes are required and trying to parse such from any given string, so is still implicitly dependent on inlining and hence not as efficient, but it will not force users to inline on the dart side, and hence they will automatically benefit once the C-api is updated, or we have switched to the new binding generator.

NOTE: This PR depends on a core-upgrade newer than v13.20.0, and hence a refactoring of the way errors are handled. It is currently based on top op #1398.

Fixes: #1155

@cla-bot cla-bot bot added the cla: yes label Aug 28, 2023
@nielsenko nielsenko force-pushed the kn/geospatial-phase-1 branch 2 times, most recently from f3ec45d to ce4a384 Compare August 28, 2023 13:36
@coveralls
Copy link

coveralls commented Aug 28, 2023

Pull Request Test Coverage Report for Build 6718946398

  • 72 of 72 (100.0%) changed or added relevant lines in 2 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 88.506%

Files with Coverage Reduction New Missed Lines %
lib/src/list.dart 2 85.93%
Totals Coverage Status
Change from base Build 6718494458: -0.2%
Covered Lines: 3419
Relevant Lines: 3863

💛 - Coveralls

@nielsenko nielsenko marked this pull request as ready for review August 30, 2023 10:20
@nielsenko nielsenko changed the base branch from main to ds/core13.19_sync_errors_changes August 30, 2023 10:22
Copy link
Member

@nirinchev nirinchev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable - I only have nitty comments.

Comment on lines 239 to 242
/// class _Location {
/// final String type = 'Point';
/// List<double> coordinates = const [0, 0];
/// }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the dart generator support playing around with the accessibility of fields? If so, we could suggest a more user-friendly API, similar to what we do in .NET: https://github.com/realm/realm-dotnet/blob/641f2dc5581776b80979c69abc4c29777313bd0b/CHANGELOG.md?plain=1#L140-L162

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added getters and setters for latitude and longitude, as well conversion methods between GeoPoint and Location in the doc comments (similar to how I did in geospatial_test.dart)

common/lib/src/realm_types.dart Show resolved Hide resolved
common/lib/src/realm_types.dart Show resolved Hide resolved
common/lib/src/realm_types.dart Outdated Show resolved Hide resolved
test/geospatial_test.dart Show resolved Hide resolved
@nielsenko nielsenko changed the base branch from ds/core13.19_sync_errors_changes to kn/upgrade-core September 12, 2023 09:48
@nielsenko nielsenko force-pushed the kn/geospatial-phase-1 branch 2 times, most recently from bd351a6 to da6b32a Compare September 12, 2023 13:04
Base automatically changed from kn/upgrade-core to ds/core13.19_sync_errors_changes September 14, 2023 13:26
test/geospatial_test.dart Outdated Show resolved Hide resolved
common/lib/src/realm_types.dart Show resolved Hide resolved
common/lib/src/realm_types.dart Show resolved Hide resolved
lib/src/native/realm_core.dart Show resolved Hide resolved
lib/src/native/realm_core.dart Show resolved Hide resolved
Base automatically changed from ds/core13.19_sync_errors_changes to main November 1, 2023 10:23
@nielsenko
Copy link
Contributor Author

Thx @nirinchev .. just need to rerun generator 👍

@nirinchev
Copy link
Member

Oh, yeah, needs to add the ignore lint comments that went in a bit earlier. Will do that.

@nirinchev nirinchev merged commit ddd5ed3 into main Nov 1, 2023
38 of 39 checks passed
@nirinchev nirinchev deleted the kn/geospatial-phase-1 branch November 1, 2023 11:47
nirinchev added a commit that referenced this pull request Nov 30, 2023
* main:
  Don't add the ISRG X1 Root on Android (#1434)
  Wait for initial sync to complete before starting the tests (#1435)
  kn/update analyzer (#1365)
  Add vNext Changelog header (#1429)
  [Release 1.6.0] (#1428)
  Upgrade to Core 13.23.4 (#1427)
  Flexible sync subscribe/unsubscribe API (#1354)
  Use default context rather than creating a new one (#1426)
  Handle duplicate certificates (#1425)
  kn/add certificate (#1378)
  asymmetric object allow non embedded links (#1402)
  Rudimentary geospatial support (#1389)
  Core 13.20.1 sync errors changes (#1387)
  Fix RealmObject.hashCode (#1420)
  add 'ignore_for_file: type=lint' to *.g.dart files (#1413)
  kn/fix skip then iterate bug (#1410)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Geospatial Queries for Points
4 participants