Skip to content

Commit

Permalink
Don't generate a query body for NoopQuery (#3078)
Browse files Browse the repository at this point in the history
* Don't generate a query body for NoopQuery

* Exclude empty queries in sequences
  • Loading branch information
Philippus committed Jun 7, 2024
1 parent 86b192f commit 86228a3
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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":[]}}"""
}
}
Original file line number Diff line number Diff line change
@@ -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("")
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}

0 comments on commit 86228a3

Please sign in to comment.