(dsl): Support source filtering in search requests #196
Conversation
| def aggregate(aggregation: ElasticAggregation): SearchAndAggregateRequest | ||
|
|
||
| def excludes(fields: String*): SearchRequest |
There was a problem hiding this comment.
Do we want to support excludes(field: String, fields: String*)?
There was a problem hiding this comment.
I think we'll have to choose one or the other as having both will probably cause ambiguity as excludes(oneValue) satisfies both. I think the decision should come down to whether we think the list will be used in a dynamic manner, in which case the stricter form can be less convenient.
| def highlights(value: Highlights): SearchRequest | ||
|
|
||
| def includes(fields: String*): SearchRequest |
There was a problem hiding this comment.
Do we want to support includes(field: String, fields: String*)?
|
|
||
| def includes(fields: String*): R | ||
|
|
||
| final def includes(schema: Schema.Record[_]): R = { |
There was a problem hiding this comment.
Can we annotate this with @tailrec?
There was a problem hiding this comment.
With this implementation it's not possible due to the flatMap call on the list, and I'm not sure it is possible with a different implementation either, because each element might call the recursive function.
| def searchAfter(value: Json): SearchRequest | ||
| } | ||
|
|
||
| private[elasticsearch] final case class Search( | ||
| index: IndexName, | ||
| query: ElasticQuery[_], | ||
| sortBy: Set[Sort], | ||
| excluded: Option[List[String]], |
There was a problem hiding this comment.
I think a Chunk would be more appropriate here, especially since we rely on concatenation in def excludes, even though most likely excludes will not be used multiple times.
| included merge excluded.fold(Obj())(excluded => Obj("excludes" -> Arr(excluded.map(_.toJson): _*))) | ||
| } | ||
| .orElse(excluded.map(excluded => Obj("excludes" -> Arr(excluded.map(_.toJson): _*)))) | ||
| .fold(Obj())(filters => Obj("_source" -> filters)) |
There was a problem hiding this comment.
I wonder if something like this could be a bit easier to read:
val sourceJson: Json =
(included, excluded) match {
case (None, None) => Obj()
case (in, ex) =>
Obj( "_source" -> {
val includes = included.fold(Obj())(included => Obj("includes" -> Arr(included.map(_.toJson): _*)))
val excludes = excluded.fold(Obj())(excluded => Obj("excludes" -> Arr(excluded.map(_.toJson): _*))
includes merge excludes
}
}
Would this work?
|
|
||
| def includes(fields: String*): R | ||
|
|
||
| final def includes(schema: Schema.Record[_]): R = { |
There was a problem hiding this comment.
Would it make sense (or be possible) to have something like
includeOnly[A](implicit schema: Schema.Record[A])
?
| @@ -671,13 +707,23 @@ object ElasticRequest { | |||
| val sortJson: Json = | |||
| if (self.sortBy.nonEmpty) Obj("sort" -> Arr(self.sortBy.toList.map(_.paramsToJson): _*)) else Obj() | |||
|
|
|||
| val sourceJson: Json = | |||
There was a problem hiding this comment.
Same as above. (Arnold's comment)
|
|
||
| import zio.schema.Schema | ||
|
|
||
| trait HasSourceFiltering[R <: HasSourceFiltering[R]] { |
There was a problem hiding this comment.
I think we can make this trait private[elasticsearch]?
There was a problem hiding this comment.
Also we should write scaladoc for methods in this trait.
| .includes() | ||
| .excludes() match { | ||
| case r: ElasticRequest.Search => r.toJson | ||
| } | ||
| val expected = | ||
| """ | ||
| |{ | ||
| | "_source" : { | ||
| | "includes" : [], | ||
| | "excludes" : [] | ||
| | }, |
mvelimir
force-pushed
the
feature/include-and-exclude-fields
branch
from
2fe2f74
to
340ac40
Compare
No description provided.