(dsl): Support 'geoDistance' query #211

markaya · 2023-05-09T13:46:44Z

Summary:

Add geo-distance query
Add unit tests for geo-distance query methods
Add integration tests for geo-distance query

Resolves geoDistanceQuery in #91

drmarjanovic · 2023-05-10T10:05:47Z

modules/library/src/main/scala/zio/elasticsearch/ElasticQuery.scala

@@ -108,6 +108,74 @@ object ElasticQuery {
  final def filter(queries: ElasticQuery[Any]*): BoolQuery[Any] =
    Bool[Any](filter = queries.toList, must = Nil, mustNot = Nil, should = Nil, boost = None, minimumShouldMatch = None)

+  /**
+   * Constructs an instance of [[zio.elasticsearch.query.GeoDistanceQuery]] using the specified parameters.


Should we use Constructs a type-safe instance of...?

drmarjanovic · 2023-05-10T10:07:20Z

modules/library/src/main/scala/zio/elasticsearch/ElasticQuery.scala

+   *
+   * @param field
+   *   the type-safe field for which query is specified for
+   * @param longitudeAndLatitude


I'm not a fan of this name.

drmarjanovic · 2023-05-10T10:07:35Z

modules/library/src/main/scala/zio/elasticsearch/ElasticQuery.scala

+    GeoDistance(field = field, point = Left((longitude, latitude)))
+
+  /**
+   * Constructs an instance of [[zio.elasticsearch.query.GeoDistanceQuery]] using the specified parameters.


Should we use type-safe here as well?

drmarjanovic · 2023-05-10T10:09:33Z

modules/library/src/main/scala/zio/elasticsearch/ElasticQuery.scala

+   * @param field
+   *   the type-safe field for which query is specified for
+   * @param longitudeAndLatitude
+   *   longitude and latitude of desired point written as string(e.g. "40,31") or geo hash (e.g. "drm3btev3e86")


Missing " " between as string and (e.g. ...).

drmarjanovic · 2023-05-10T10:09:33Z

modules/library/src/main/scala/zio/elasticsearch/ElasticQuery.scala

+   * @param field
+   *   the field for which query is specified for
+   * @param longitudeAndLatitude
+   *   longitude and latitude of desired point written as string(e.g. "40,31") or geo hash (e.g. "drm3btev3e86")


Missing " " between as string and (e.g. ...).

drmarjanovic · 2023-05-10T10:17:28Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+  /**
+   * sets the `distanceType` parameter for the [[zio.elasticsearch.query.GeoDistanceQuery]]
+   *
+   * Defines how to compute the distance. Can either be [[zio.elasticsearch.query.DistanceType.Arc]] (default), or


Check this and you can use a similar format.

drmarjanovic · 2023-05-10T10:18:52Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+
+  def distanceType(value: DistanceType): GeoDistanceQuery[S] = self.copy(distanceType = Some(value))
+
+  private def getPointJson: (String, Json) = point match {


I would move this to paramsToJson if it's used only once. Also, no need for a method.

drmarjanovic · 2023-05-10T10:21:14Z

modules/library/src/test/scala/zio/elasticsearch/ElasticQuerySpec.scala

@@ -833,6 +836,48 @@ object ElasticQuerySpec extends ZIOSpecDefault {
              )
            )
          )
+        },
+        test("geoDistance") {


Let's keep this sorted.

test("bool") ... test("exists") ... test("geoDistance") ... test("match") test("matchAll") ...

drmarjanovic · 2023-05-10T10:21:52Z

modules/library/src/test/scala/zio/elasticsearch/ElasticQuerySpec.scala

@@ -2324,6 +2369,112 @@ object ElasticQuerySpec extends ZIOSpecDefault {
          assert(queryWithBoost.toJson)(equalTo(expectedWithBoost.toJson)) &&
          assert(queryWithCaseInsensitive.toJson)(equalTo(expectedWithCaseInsensitive.toJson)) &&
          assert(queryWithAllParams.toJson)(equalTo(expectedWithAllParams.toJson))
+        },
+        test("geoDistance") {


drmarjanovic · 2023-05-10T10:23:45Z

modules/library/src/test/scala/zio/elasticsearch/ElasticQuerySpec.scala

+              |}
+              |""".stripMargin
+
+          assert(query.toJson)(equalTo(expected.toJson))


You are missing && here.

dbulaja98 · 2023-05-10T12:24:08Z

modules/library/src/main/scala/zio/elasticsearch/ElasticQuery.scala

+   * @param field
+   *   the type-safe field for which query is specified for
+   * @param longitude
+   *   longitude of desired point


...of the desired point.

dbulaja98 · 2023-05-10T12:24:15Z

modules/library/src/main/scala/zio/elasticsearch/ElasticQuery.scala

+   * @param longitude
+   *   longitude of desired point
+   * @param latitude
+   *   latitude of desired point


of the desired point.

dbulaja98 · 2023-05-10T12:24:40Z

modules/library/src/main/scala/zio/elasticsearch/ElasticQuery.scala

+   * @param field
+   *   the field for which query is specified for
+   * @param longitude
+   *   longitude of desired point


Same for every place in this file.

dbulaja98 · 2023-05-10T12:27:10Z

modules/library/src/main/scala/zio/elasticsearch/query/Distance.scala

+
+package zio.elasticsearch.query
+
+final case class Distance(distanceValue: Double, distanceUnit: DistanceUnit) {


Should this be private[elasticsearch]?

dbulaja98 · 2023-05-10T13:05:44Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+   * [[zio.elasticsearch.query.DistanceUnit]].
+   *
+   * @param value
+   *   the [[Double]] value for a non-negative real number used for distance


Maybe just: a non-negative number...

dbulaja98 · 2023-05-10T13:08:28Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+
+  /**
+   * Sets the `distanceType` parameter for the [[zio.elasticsearch.query.GeoDistanceQuery]].
+   *


Remove empty line.

dbulaja98 · 2023-05-10T13:09:32Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+   * @param unit
+   *   the [[zio.elasticsearch.query.DistanceUnit]] in which we want to represent the distance
+   * @return
+   *   a new instance of the [[zio.elasticsearch.query.GeoDistanceQuery]] with the score value set.


...with the distance..??

dbulaja98 · 2023-05-10T13:10:23Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+   *   - [[zio.elasticsearch.query.DistanceType.Arc]]: Default algorithm
+   *   - [[zio.elasticsearch.query.DistanceType.Plane]]: Faster, but inaccurate on long distances and close to the poles
+   * @return
+   *   a new instance of the [[zio.elasticsearch.query.GeoDistanceQuery]] with the score value set.


score value or distanceType value?

dbulaja98 · 2023-05-10T13:10:37Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+   * @param value
+   *   the [[String]] value to represent the name field
+   * @return
+   *   a new instance of the [[zio.elasticsearch.query.GeoDistanceQuery]] with the score value set.


Maybe apply everywhere "...enriched with the paramName param."

dbulaja98 · 2023-05-10T13:14:10Z

modules/library/src/main/scala/zio/elasticsearch/query/Queries.scala

+  distance: Option[Distance] = None,
+  distanceType: Option[DistanceType] = None,
+  queryName: Option[String] = None,
+  validationMethod: Option[ValidationMethod] = None


Why do we have predefined values here?

drmarjanovic · 2023-05-10T11:48:10Z

modules/library/src/test/scala/zio/elasticsearch/domain/Location.scala

+import zio.elasticsearch.FieldAccessorBuilder
+import zio.schema.{DeriveSchema, Schema}
+
+case class Location(lat: Double, lon: Double)


Let's use final here.

drmarjanovic · 2023-05-10T11:49:44Z

modules/library/src/test/scala/zio/elasticsearch/ElasticQuerySpec.scala

+          val queryWithName =
+            geoDistance(TestDocument.stringField, 20.0, 21.1).name("name")
+          val queryWithValidationMethod =
+            geoDistance(TestDocument.stringField, 20.0, 21.1).validationMethod(IgnoreMalformed)


Let's add queryWithAllParams.

modules/library/src/test/scala/zio/elasticsearch/ElasticQuerySpec.scala

drmarjanovic requested changes May 10, 2023

View reviewed changes

markaya added 3 commits May 10, 2023 13:09

Add geodistance query

8f1c702

Fix linter

939f0a1

Add integration tests and fix code remarks

c919276

markaya force-pushed the task-support-geodistance-query branch from a2ad8ab to c919276 Compare May 10, 2023 11:10

drmarjanovic changed the title ~~Add Geo-distance query~~ (dsl): Support 'geoDistance' query May 10, 2023

dbulaja98 requested changes May 10, 2023

View reviewed changes

Fix code remarks

244857c

drmarjanovic requested changes May 10, 2023

View reviewed changes

markaya added 2 commits May 11, 2023 08:43

fix code remarks and change location to String

a5b00bc

Fix linter

c09b236

drmarjanovic approved these changes May 11, 2023

View reviewed changes

dbulaja98 approved these changes May 11, 2023

View reviewed changes

dbulaja98 merged commit 1a2cc3b into main May 11, 2023

dbulaja98 deleted the task-support-geodistance-query branch May 11, 2023 11:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(dsl): Support 'geoDistance' query #211

(dsl): Support 'geoDistance' query #211

markaya commented May 9, 2023 •

edited

Loading

drmarjanovic May 10, 2023

drmarjanovic May 10, 2023

drmarjanovic May 10, 2023

drmarjanovic May 10, 2023

drmarjanovic May 10, 2023

drmarjanovic May 10, 2023

drmarjanovic May 10, 2023

drmarjanovic May 10, 2023

drmarjanovic May 10, 2023

drmarjanovic May 10, 2023

dbulaja98 May 10, 2023

dbulaja98 May 10, 2023

dbulaja98 May 10, 2023

dbulaja98 May 10, 2023

dbulaja98 May 10, 2023

dbulaja98 May 10, 2023

dbulaja98 May 10, 2023

dbulaja98 May 10, 2023

dbulaja98 May 10, 2023

dbulaja98 May 10, 2023

dbulaja98 May 10, 2023

drmarjanovic May 10, 2023

drmarjanovic May 10, 2023


		def distanceType(value: DistanceType): GeoDistanceQuery[S] = self.copy(distanceType = Some(value))

		private def getPointJson: (String, Json) = point match {


		package zio.elasticsearch.query

		final case class Distance(distanceValue: Double, distanceUnit: DistanceUnit) {

(dsl): Support 'geoDistance' query #211

(dsl): Support 'geoDistance' query #211

Conversation

markaya commented May 9, 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

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

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

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

markaya commented May 9, 2023 •

edited

Loading