From 86228a39d789489e71251c954f1d0a894b145091 Mon Sep 17 00:00:00 2001 From: Philippus Baalman Date: Fri, 7 Jun 2024 15:17:32 +0200 Subject: [PATCH] Don't generate a query body for NoopQuery (#3078) * Don't generate a query body for NoopQuery * Exclude empty queries in sequences --- .../builders/FiltersAggregationBuilder.scala | 2 +- .../queries/NoopQueryBodyFnTest.scala | 13 +++++++ .../searches/queries/NoopQueryBuilderFn.scala | 7 ++++ .../searches/queries/QueryBuilderFn.scala | 2 +- .../queries/compound/BoolQueryBuilderFn.scala | 8 ++--- .../queries/compound/DisMaxQueryBodyFn.scala | 4 +-- .../search/queries/NoopQueryTest.scala | 36 +++++++++++++++++++ 7 files changed, 64 insertions(+), 8 deletions(-) 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/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-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-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 new file mode 100644 index 000000000..adb07ad5c --- /dev/null +++ b/elastic4s-tests/src/test/scala/com/sksamuel/elastic4s/search/queries/NoopQueryTest.scala @@ -0,0 +1,36 @@ +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 + } + + "it" should "not error out when executed in a sequence" in { + client.execute { + search("noop").query { + boolQuery().should(Seq(NoopQuery, NoopQuery)) + } + }.await.result + } +}