mbuhot / eskotlin

Elasticsearch Query DSL for Kotlin
MIT License
136 stars 28 forks source link

Adding the ability to accept multiple must/should clauses for bool query. #18

Closed vincentlauvlwj closed 6 years ago

vincentlauvlwj commented 6 years ago

We wrote a bool query with multiple clauses like this before:

val query = bool {
    should = listOf(
        term { "tag" to "wow" },
        term { "tag" to "elasticsearch" }
    )
}

I implemented a better syntax:

val query = bool {
    should {
        term { "tag" to "wow" }
        term { "tag" to "elasticsearch" }
    }
}

The new syntax is compatible with the previous one which only accepts a single clause in source code level.

Please check ListQueriesBlock.kt, Bool.kt and BoolTest.kt.

mbuhot commented 6 years ago

@vincentlauvlwj thanks for the PR. At this time I'd prefer not to change from listOf to the builder style DSL.

One of the goals of es_kotlin is to make translating JSON to kotlin as direct as possible, so where there are objects in JSON, we use builder functions {}, and where there are arrays in JSON we use listOf.

ListQueriesBlock must expose all the DSL functions, making it less extensible by users compare to directly constructing a list.

Hopefully Kotlin will get collection literals in a future release, making the syntax eventually closer to JSON with

  must = [
    term { "tag" to "wow" },
    term { "tag" to "elasticsearch" }
  ]
vincentlauvlwj commented 6 years ago

Yeah, I agree that ListQueriesBlock is not extensible enough. I just wrote it without careful consideration.

listOf is a little annoying, but it can be better with collection literals in the future.