Query Refactoring: Adding getBuilderPrototype() method to all QueryParsers#11344
Conversation
There was a problem hiding this comment.
I was wondering if we should use some convention on naming, like INSTANCE or PROTOTYPE, I have no strong preference though
|
looks good, left a few comments |
f876767 to
2a28587
Compare
|
@javanna had another round, moved the constants to the builders and reverted most of the package private constructors, only introduced private constructors where needed. |
|
LGTM thanks @cbuescher |
…rsers Currently there is a registry for all QueryParsers accessible via the IndicesQueriesModule. For deserializing nested queries e.g. for the BoolQueryBuilder (elastic#11121) we need to look up query builders by their name to be able to deserialize using a prototype builder of the concrete class. This PR adds a getBuilderPrototype() method to each query parser so we can re-use the parser registry to get the corresponding builder using the query name. Closes elastic#11344
e054e23 to
8a37cc2
Compare
|
Thanks, rebased and will merge then. |
…nkParsers Query Refactoring: Adding getBuilderPrototype() method to all QueryParsers
|
I am marking this as breaking, as it breaks plugins that introduce custom queries by requiring their |
… queries `QueryBuilder`s can be serialized being them `Writeable`. Given that we support different types of queries, which can be nested into one another, we also have to make sure that we read and write the name of the query before the actual query fields so that we know which instance of query we have to create when de-serializing. That is why serializing a query builder will be done by calling `QueryBuilderSerializer#write`, which will serialize the name of the query before the query itself, and reading only by calling `QueryBuilderSerializer#read`, which will lookup the query in the parsers registry, retrieve its corresponding empty builder from it (`getPrototypeBuilder`) and call `readFrom` against it, which will return a new query builder containing all the de-serialized fields. Compound queries will then have to use these methods to read and write their inner queries. The registration of custom queries via plugin doesn't change, only the parser needs to be registered still, but it has now to implement the `getPrototypeBuilder` method too introduced with elastic#11344, which returns an empy builder instance that can be used to de-serialize the query.
…rsers Currently there is a registry for all QueryParsers accessible via the IndicesQueriesModule. For deserializing nested queries e.g. for the BoolQueryBuilder (elastic#11121) we need to look up query builders by their name to be able to deserialize using a prototype builder of the concrete class. This PR adds a getBuilderPrototype() method to each query parser so we can re-use the parser registry to get the corresponding builder using the query name. Closes elastic#11344
…ring-linkParsers Query Refactoring: Adding getBuilderPrototype() method to all QueryParsers
Currently there is a registry for all QueryParsers accessible via the IndicesQueriesModule. For deserializing nested queries e.g. for the BoolQueryBuilder (#11121) we need to look up query builders by their name to be able to deserialize using a prototype builder of the concrete class. This PR adds a getBuilderPrototype() method to each query parser so we can re-use the parser registry to get the corresponding builder using the query name. This might change in the future when we decide to register also builders or merger parsers and builders in a later stage of the refactoring. (see #11121 (comment)) for past discussion.