[Rest Api Compatibility] Make query registration easier #75722
Conversation
Refactoring to NamedXContentRegistry to make it easier to register new query builders. It removes the concept of separate compatibel namedXContentRegistry and adds a second dimension - restApiVersion - to registry in NamedXContentRegistry. This makes the design similar to the solution in ObjectParser where the field parser lookup map also needs has a restApiVersion relates elastic#51816
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
…tent_registry_for_queries
|
@elasticmachine run elasticsearch-ci/bwc |
| @@ -738,9 +738,9 @@ protected Node(final Environment initialEnvironment, | |||
| } | |||
|
|
|||
| // package scope for testing | |||
| List<NamedXContentRegistry.Entry> getCompatibleNamedXContents() { | |||
| Stream<NamedXContentRegistry.Entry> getCompatibleNamedXContents() { | |||
There was a problem hiding this comment.
I'm a bit suspicious about this switch from List to Stream -- can you walk it back or convince me that it's a great idea?
There was a problem hiding this comment.
the reason is that when NamedXContentRegistry is created here https://github.com/elastic/elasticsearch/pull/75722/files#diff-d01b960cab84fe2338f230b5e7535bc8efa9f9778ecf61c9f527067351dca895R413
it aggregates all streams before converting to a list. I was thinking this could avoid unnecessary list creation in getCompatibleNamedXContents
but.. It made me thinking if we even need this now. We can register both compatible and current NamedXContentEntries in the same registry..
I guess I should just remove that Node#getCompatibleNamedXContents and Plugin#getNamedXContentForCompatibility
There was a problem hiding this comment.
This is how usage of shared registry for compatible and current NamedXContentRegistry.Entry entries will look like
draft for type query:
https://github.com/elastic/elasticsearch/pull/75453/files#diff-57f10f7614a75787774b36d41833688a0474b84eb239e4c84499feec9634eefbR845
server/src/test/java/org/elasticsearch/search/SearchModuleTests.java
Outdated
Show resolved
Hide resolved
Refactoring to NamedXContentRegistry to make it easier to register new query builders. It removes the concept of separate compatibel namedXContentRegistry and adds a second dimension - restApiVersion - to registry in NamedXContentRegistry. This makes the design similar to the solution in ObjectParser where the field parser lookup map also needs has a restApiVersion relates elastic#51816
Refactoring to NamedXContentRegistry to make it easier to register new
query builders. It removes the concept of separate compatibel
namedXContentRegistry and adds a second dimension - restApiVersion - to
registry in NamedXContentRegistry.
This makes the design similar to the solution in ObjectParser where the
field parser lookup map also needs has a restApiVersion
relates #51816
gradle check?