Add support for Geo-spatial queries #1403

cmelchior · 2023-05-31T12:12:17Z

This PR is adding support for Geo-spatial queries. Core doesn't have a specific geo-type (yet), so this is just the first version which relies on duck-typing.

This means that geo queries can be used on any embedded objects that has a type and coordinates property.

This also has the implication that it is slightly awkward to to pass in geo-parameters. The current implementation overrides the toString() method which will format the data structures so they can be used from within a query.

TODO

Documentation
Figure out how many tests we need?
There is a chance that Core allow String arguments to be converted to geo-spatial arguments. Check with them about this.

# Conflicts: # packages/test-base/src/commonTest/kotlin/io/realm/kotlin/test/common/GeoSpatialTests.kt

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/GeoBox.kt

clementetb · 2023-06-27T13:52:22Z

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/Distance.kt

+ * This class represents an equatorial distance that can be used in geospatial queries like those
+ * represented by a [GeoCircle].
+ */
+public data class Distance private constructor(


This distance is the distance on the surface of the earth, isn't it? Could we use a different name like EarthSurfaceDistance or GeoDistance.

Yes, it is the distance on the earth. I guess we could have also gone with GeoDistance, but Distance was the name used in the technical scope: https://docs.google.com/document/d/1Z7CK79CqebEtVllVHTNEq8neKhoMWeFCDjKBrn_pq1Y/ and I don't think that is particular confusing? And it is shorter than GeoDistance.

It confuses me because it mixed linear and angular distances.

We implicitly use the earth's radius to correlate between them, but we are not explicitly stating in the documentation that we are using it. Instead, we could be explicit:

val distance: Distance = 1.2.earthRadians val distance: Distance = 1.2.earthDegrees val distance: Distance = 10.4.km val distance: Distance = 10.5.miles

Maybe I'm missing something obvious here, but how could radians be interpreted in any other way than Earth radians? Like what can it potentially be confused with?

I also had the same feeling like Clemente. It isn't at all obvious that radians are "earth radians" as a distance (if such a term even exists!). But once you start interpreting this as "equatorial distance in radians" it starts to be ok to work with. I think it is just something people will need to grasp and then it is ok. I tried to suggest updates to some docs by adding explicit "equatorial distance" here and there to speed up the "learning curve" to make it more clear. Anyway the km should make it possible to work with it without bothering with the internals.

The whole Geo-Spatial solution is generic, it can work on any sphere. It is this Distance implementation that tights it to the Earth.

The thing is that this solution works for other spheres too, planets or simulations. If these KM/Miles are used on a different sphere than the earth it would not be correct. Adding a comment about it would be fine.

Here I tried to convert values Distance as a linear distance, that's why I wrote "earthRadians" (radians*earth radius = linear distance ). But we actually require angular distance so forget about it.

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/Distance.kt

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/GeoBox.kt

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/GeoPoint.kt

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/GeoPolygon.kt

packages/test-base/src/commonTest/kotlin/io/realm/kotlin/test/common/GeoSpatialTests.kt

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/Distance.kt

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/GeoBox.kt

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/GeoCircle.kt

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/Distance.kt

packages/test-base/src/commonTest/kotlin/io/realm/kotlin/test/common/GeoSpatialTests.kt

Co-authored-by: Claus Rørbech <claus.rorbech@mongodb.com>

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/GeoCircle.kt

clementetb · 2023-06-27T20:35:06Z

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/Distance.kt

+ * This class represents an equatorial distance that can be used in geospatial queries like those
+ * represented by a [GeoCircle].
+ */
+public data class Distance private constructor(


It confuses me because it mixed linear and angular distances.

We implicitly use the earth's radius to correlate between them, but we are not explicitly stating in the documentation that we are using it. Instead, we could be explicit:

val distance: Distance = 1.2.earthRadians val distance: Distance = 1.2.earthDegrees val distance: Distance = 10.4.km val distance: Distance = 10.5.miles

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/GeoPolygon.kt

cmelchior · 2023-06-30T07:53:59Z

I reworked the API based on our discussions. It should be ready for another round of review.

rorbech

Cool. Only some minor comments here and there.

rorbech · 2023-06-30T08:25:40Z

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/geo/GeoPolygon.kt

+         * @throws IllegalArgumentException if the input polygon break some of the rules outlined
+         * above.
+         */
+        public fun create(outerRing: List<GeoPoint>, holes: List<List<GeoPoint>>): GeoPolygon {


Would it make sense to be able to GeoBox, GeoPolygon and GeoCircles as holes? ... But I guess we can always add a subtype to the regional types and an overload method, so nothing for this PR.

rorbech · 2023-06-30T08:39:31Z

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/geo/Distance.kt

+    public companion object {
+
+        private const val DEGREE_TO_RADIAN: Double = PI / 180.0
+        private const val RADIAN_TO_DEGREE: Double = 180.0 / PI


Feels a bit funny to define both just to be able use multiplication in both conversion. Maybe only define one and then divide in one of the ways.

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/geo/GeoBox.kt

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/geo/GeoPoint.kt

packages/test-base/src/commonTest/kotlin/io/realm/kotlin/test/common/GeoSpatialTests.kt

rorbech · 2023-06-30T09:11:24Z

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/internal/geo/UnmanagedGeoPolygon.kt

+        if (outerRing.size < MIN_RING_SIZE) {
+            throw IllegalArgumentException("The outer ring requires at least $MIN_RING_SIZE points: ${outerRing.size}")
+        }
+        if (outerRing.first() != outerRing.last()) {


Hmm. Maybe I was not right in suggesting this check. I am a bit confused, though, so don't know what is up and down 🙃. Seems like the general GeoPolygon class documentation state that there must be four https://github.com/realm/realm-kotlin/pull/1403/files#diff-bb869c11647feb614648b9cdafb62fb3aa3b95c2b18f6d81f3cd714f90cce286R29-R31, but then it is also mentioned that the first and last point is implicitly connected in https://github.com/realm/realm-kotlin/pull/1403/files#diff-bb869c11647feb614648b9cdafb62fb3aa3b95c2b18f6d81f3cd714f90cce286R102-R103. The reason why I suggested the check was just to catch errors before performing the actual queries, but if there is an option to leave out the duplicate first/last point and consider it being implicit I would vote for allowing that.

This check is totally valid as it is required that the first and last points to be the same, otherwise core would throw an exception.

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/geo/GeoPolygon.kt

clementetb · 2023-07-10T11:20:05Z

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/geo/GeoPoint.kt

+ * The recommended approach is encapsulating this inside its own [EmbeddedRealmObject], like this
+ *
+ * ```
+ * public class Location: EmbeddedRealmObject {


Why not provide the implementation for this type? We have types for querying but not for persistence. Also, this type could implement GeoPoint for better integration with the Geo APIs.

It is some temporal code, but having to copy/paste this code makes it hard to discover, and leaves us with no control over it, we cannot deprecate it, or notify the users to move to another "final" type.

This is because we can't force the type name from the client. E.g. if the server json schema defines the location type as Person_Address_Location (i.e. automatically generated name for an unnamed embedded object), we will end up with schema mismatch if the user attempts to use the SDK-specified Location. This is covered in the "Design Alternnatives" section of the Design doc.

In phase 2 we will have a dedicated type both on the client and the server, but this work was deemed quite involved and we didn't want to delay phase 1 because of it.

## Pull Request Info - Document geospatial queries: realm/realm-kotlin#1403 - Update to v1.11.0 > NOTE: API ref links won't work until v1.11.0 refs are published ### Jira - https://jira.mongodb.org/browse/DOCSP-30997 ### Staged Changes - [Geospatial Data - Kotlin SDK](https://docs-mongodbcom-staging.corp.mongodb.com/realm/docsworker-xlarge/docsp-30997-geospatial/sdk/kotlin/realm-database/schemas/data-types/geospatials/) - new page ### Reminder Checklist If your PR modifies the docs, you might need to also update some corresponding pages. Check if completed or N/A. - [x] Create Jira ticket for corresponding docs-app-services update(s), if any - [x] Checked/updated Admin API - [x] Checked/updated CLI reference ### Review Guidelines [REVIEWING.md](https://github.com/mongodb/docs-realm/blob/master/REVIEWING.md)

Initial code dump

6ca8846

cla-bot bot added the cla: yes label May 31, 2023

github-actions bot assigned cmelchior May 31, 2023

Christian Melchior added 5 commits June 23, 2023 09:43

Merge branch 'main' into cm/geo-support

054d9e3

# Conflicts: # packages/test-base/src/commonTest/kotlin/io/realm/kotlin/test/common/GeoSpatialTests.kt

More docs

b9b8c61

Merge branch 'main' into cm/geo-support

8e01eac

Tests and docs

a997890

Add entry to CHANGELOG

df78a0f

cmelchior marked this pull request as ready for review June 27, 2023 13:48

cmelchior requested review from rorbech and clementetb June 27, 2023 13:48

Doc updates

45ccaae

clementetb reviewed Jun 27, 2023

View reviewed changes

Christian Melchior added 4 commits June 27, 2023 17:53

PR feedback

7b54abc

Fix test

3d0121e

Mirror Duration API

e9267de

Fix build issue

227335a

rorbech suggested changes Jun 27, 2023

View reviewed changes

Apply suggestions from code review

0552702

Co-authored-by: Claus Rørbech <claus.rorbech@mongodb.com>

clementetb reviewed Jun 27, 2023

View reviewed changes

rorbech reviewed Jun 28, 2023

View reviewed changes

packages/library-base/src/commonMain/kotlin/io/realm/kotlin/types/GeoPolygon.kt Outdated Show resolved Hide resolved

Christian Melchior added 3 commits June 29, 2023 19:36

PR review feedback

ee1df6c

Cleanup

ebe02dc

Fix tests

f01019e

cmelchior requested review from clementetb and rorbech June 30, 2023 07:54

rorbech approved these changes Jun 30, 2023

View reviewed changes

Doc feedback

dafb863

clementetb reviewed Jul 11, 2023

View reviewed changes

Update changelog

eec1481

clementetb merged commit 1821ecc into main Jul 21, 2023
1 check passed

clementetb deleted the cm/geo-support branch July 21, 2023 11:08

cbullinger mentioned this pull request Jul 27, 2023

DOCSP-30997: Kotlin Geospatial Data page mongodb/docs-realm#2919

Merged

3 tasks

github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for Geo-spatial queries #1403

Add support for Geo-spatial queries #1403

cmelchior commented May 31, 2023 •

edited

Loading

clementetb Jun 27, 2023

cmelchior Jun 27, 2023

clementetb Jun 27, 2023

cmelchior Jun 28, 2023

rorbech Jun 28, 2023 •

edited

Loading

clementetb Jun 28, 2023

clementetb Jun 27, 2023

cmelchior commented Jun 30, 2023

rorbech left a comment

rorbech Jun 30, 2023

rorbech Jun 30, 2023

rorbech Jun 30, 2023

clementetb Jul 10, 2023

clementetb Jul 10, 2023

nirinchev Jul 20, 2023

Add support for Geo-spatial queries #1403

Add support for Geo-spatial queries #1403

Conversation

cmelchior commented May 31, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rorbech Jun 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmelchior commented Jun 30, 2023

rorbech left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmelchior commented May 31, 2023 •

edited

Loading

rorbech Jun 28, 2023 •

edited

Loading