From d2420f4644528508d0f7ba689b119e2a0201269f Mon Sep 17 00:00:00 2001 From: philippus Date: Sat, 1 Jun 2024 15:36:35 +0200 Subject: [PATCH 1/2] Don't generate a query body for NoopQuery --- .../queries/NoopQueryBodyFnTest.scala | 13 +++++++++ .../searches/queries/NoopQueryBuilderFn.scala | 7 +++++ .../searches/queries/QueryBuilderFn.scala | 2 +- .../search/queries/NoopQueryTest.scala | 28 +++++++++++++++++++ 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 elastic4s-core/src/test/scala/com/sksamuel/elastic4s/requests/searches/queries/NoopQueryBodyFnTest.scala create mode 100644 elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/NoopQueryBuilderFn.scala create mode 100644 elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/search/queries/NoopQueryTest.scala diff --git a/elastic4s-core/src/test/scala/com/sksamuel/elastic4s/requests/searches/queries/NoopQueryBodyFnTest.scala b/elastic4s-core/src/test/scala/com/sksamuel/elastic4s/requests/searches/queries/NoopQueryBodyFnTest.scala new file mode 100644 index 000000000..ed5add3ad --- /dev/null +++ b/elastic4s-core/src/test/scala/com/sksamuel/elastic4s/requests/searches/queries/NoopQueryBodyFnTest.scala @@ -0,0 +1,13 @@ +package com.sksamuel.elastic4s.requests.searches.queries + +import com.sksamuel.elastic4s.ElasticDsl._ +import com.sksamuel.elastic4s.handlers.searches.queries.QueryBuilderFn +import org.scalatest.funsuite.AnyFunSuite +import org.scalatest.matchers.should.Matchers + +class NoopQueryBodyFnTest extends AnyFunSuite with Matchers { + test("NoopQuery should not generate a query body") { + val q = boolQuery().should(Seq(NoopQuery)) + QueryBuilderFn(q).string shouldBe """{"bool":{"should":[]}}""" + } +} diff --git a/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/NoopQueryBuilderFn.scala b/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/NoopQueryBuilderFn.scala new file mode 100644 index 000000000..a5eb0e25a --- /dev/null +++ b/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/NoopQueryBuilderFn.scala @@ -0,0 +1,7 @@ +package com.sksamuel.elastic4s.handlers.searches.queries + +import com.sksamuel.elastic4s.json.{XContentBuilder, XContentFactory} + +object NoopQueryBuilderFn { + def apply(): XContentBuilder = XContentFactory.parse("") +} diff --git a/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/QueryBuilderFn.scala b/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/QueryBuilderFn.scala index 1a18474cc..65f01b3c6 100644 --- a/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/QueryBuilderFn.scala +++ b/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/QueryBuilderFn.scala @@ -44,7 +44,7 @@ object QueryBuilderFn { case q: MoreLikeThisQuery => MoreLikeThisQueryBuilderFn(q) case q: MultiMatchQuery => MultiMatchBodyFn(q) case q: NestedQuery => NestedQueryBodyFn(q) - case NoopQuery => XContentFactory.jsonBuilder() + case NoopQuery => NoopQueryBuilderFn() case q: ParentIdQuery => ParentIdQueryBodyFn(q) case q: PercolateQuery => PercolateQueryBodyFn(q) case q: PinnedQuery => PinnedQueryBuilderFn(q) diff --git a/elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/search/queries/NoopQueryTest.scala b/elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/search/queries/NoopQueryTest.scala new file mode 100644 index 000000000..4f01bd4d3 --- /dev/null +++ b/elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/search/queries/NoopQueryTest.scala @@ -0,0 +1,28 @@ +package com.sksamuel.elastic4s.search.queries + +import scala.util.Try + +import com.sksamuel.elastic4s.requests.searches.queries.NoopQuery +import com.sksamuel.elastic4s.testkit.DockerTests +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +class NoopQueryTest extends AnyFlatSpec with Matchers with DockerTests { + Try { + client.execute { + deleteIndex("noop") + }.await + } + + client.execute { + createIndex("noop") + }.await + + "noop query" should "not error out when executed" in { + client.execute { + search("noop").query { + boolQuery().should(Seq(NoopQuery)) + } + }.await.result + } +} From b70fec16014eb46f8f71cbd25baec981ff1b56e7 Mon Sep 17 00:00:00 2001 From: philippus Date: Sat, 1 Jun 2024 21:11:49 +0200 Subject: [PATCH 2/2] Exclude empty queries in sequences --- .../aggs/builders/FiltersAggregationBuilder.scala | 2 +- .../searches/queries/compound/BoolQueryBuilderFn.scala | 8 ++++---- .../searches/queries/compound/DisMaxQueryBodyFn.scala | 4 ++-- .../sksamuel/elastic4s/search/queries/NoopQueryTest.scala | 8 ++++++++ 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/elastic4s-core/src/main/scala/com/sksamuel/elastic4s/requests/searches/aggs/builders/FiltersAggregationBuilder.scala b/elastic4s-core/src/main/scala/com/sksamuel/elastic4s/requests/searches/aggs/builders/FiltersAggregationBuilder.scala index d27e0fa6c..4c1069bb5 100644 --- a/elastic4s-core/src/main/scala/com/sksamuel/elastic4s/requests/searches/aggs/builders/FiltersAggregationBuilder.scala +++ b/elastic4s-core/src/main/scala/com/sksamuel/elastic4s/requests/searches/aggs/builders/FiltersAggregationBuilder.scala @@ -11,7 +11,7 @@ object FiltersAggregationBuilder { val filters = { builder.startArray("filters") - val filters = agg.filters.map(QueryBuilderFn.apply).map(_.string).mkString(",") + val filters = agg.filters.map(QueryBuilderFn.apply).map(_.string).filter(_.nonEmpty).mkString(",") builder.rawValue(filters) builder.endArray() } diff --git a/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/compound/BoolQueryBuilderFn.scala b/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/compound/BoolQueryBuilderFn.scala index 912974d51..8cbf0e268 100644 --- a/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/compound/BoolQueryBuilderFn.scala +++ b/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/compound/BoolQueryBuilderFn.scala @@ -11,28 +11,28 @@ object BoolQueryBuilderFn { if (bool.must.nonEmpty) { builder.startArray("must") - val musts = bool.must.map(QueryBuilderFn.apply).map(_.string).mkString(",") + val musts = bool.must.map(QueryBuilderFn.apply).map(_.string).filter(_.nonEmpty).mkString(",") builder.rawValue(musts) builder.endArray() } if (bool.should.nonEmpty) { builder.startArray("should") - val should = bool.should.map(QueryBuilderFn.apply).map(_.string).mkString(",") + val should = bool.should.map(QueryBuilderFn.apply).map(_.string).filter(_.nonEmpty).mkString(",") builder.rawValue(should) builder.endArray() } if (bool.not.nonEmpty) { builder.startArray("must_not") - val nots = bool.not.map(QueryBuilderFn.apply).map(_.string).mkString(",") + val nots = bool.not.map(QueryBuilderFn.apply).map(_.string).filter(_.nonEmpty).mkString(",") builder.rawValue(nots) builder.endArray() } if (bool.filters.nonEmpty) { builder.startArray("filter") - val filters = bool.filters.map(QueryBuilderFn.apply).map(_.string).mkString(",") + val filters = bool.filters.map(QueryBuilderFn.apply).map(_.string).filter(_.nonEmpty).mkString(",") builder.rawValue(filters) builder.endArray() } diff --git a/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/compound/DisMaxQueryBodyFn.scala b/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/compound/DisMaxQueryBodyFn.scala index a1cb7cef9..56c8afb59 100644 --- a/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/compound/DisMaxQueryBodyFn.scala +++ b/elastic4s-handlers/src/main/scala/com/sksamuel/elastic4s/handlers/searches/queries/compound/DisMaxQueryBodyFn.scala @@ -2,7 +2,7 @@ package com.sksamuel.elastic4s.handlers.searches.queries.compound import com.sksamuel.elastic4s.handlers.searches.queries.QueryBuilderFn import com.sksamuel.elastic4s.json.{XContentBuilder, XContentFactory} -import com.sksamuel.elastic4s.requests.searches.queries.DisMaxQuery +import com.sksamuel.elastic4s.requests.searches.queries.{DisMaxQuery, NoopQuery} object DisMaxQueryBodyFn { def apply(q: DisMaxQuery): XContentBuilder = { @@ -15,7 +15,7 @@ object DisMaxQueryBodyFn { builder.startArray("queries") // Workaround for bug where separator is not added with rawValues - q.queries.map(QueryBuilderFn.apply).foreach { query => + q.queries.map(QueryBuilderFn.apply).filter(_.string.nonEmpty).foreach { query => builder.rawValue(query) } builder.endArray() diff --git a/elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/search/queries/NoopQueryTest.scala b/elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/search/queries/NoopQueryTest.scala index 4f01bd4d3..adb07ad5c 100644 --- a/elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/search/queries/NoopQueryTest.scala +++ b/elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/search/queries/NoopQueryTest.scala @@ -25,4 +25,12 @@ class NoopQueryTest extends AnyFlatSpec with Matchers with DockerTests { } }.await.result } + + "it" should "not error out when executed in a sequence" in { + client.execute { + search("noop").query { + boolQuery().should(Seq(NoopQuery, NoopQuery)) + } + }.await.result + } }