Consolidate all Plugin.createComponents arguments into a single accessor class

[thecoop]

Create a createComponents overload that only takes a single services argument, allowing new services to be added without breaking API compatibility as much.

Keep the old method for compatibility with existing plugins

[elasticsearchmachine]

Pinging @elastic/es-core-infra (Team:Core/Infra)

[thecoop]

This can just be left around for compatibility, but any new services would be added to PluginServices only

[rjernst]

We don't need to do this, or at least we can remove this once serverless is updated. There are no compatibility guarantees for non-stable plugins.

[thecoop]

I'll migrate all the existing implementations as a separate PR

[ldematte]

I think this is OK but a bit verbose; maybe a builder would be nicer to use

Plugin.ComponentsBuilder b = p.createComponents();
var componetns = b
   .withClient(client)
   .withClusterService(clusterService)
   ...
   .build();

or

var components = p.createComponents(b -> b.withClient(client).withClusterService(clusterService)...)

Just suggesting alternatives, it's OK either way

[thecoop]

A builder is good when you're building something up in stages, but this is all-or-nothing - you need all these things to have a PluginServices object, it makes no sense without them. It'll also add a lot of boilerplate to the builder implementation & verification.

[DaveCTurner]

👍 but in terms of boilerplate-reduction why not just use a record? Do we really need an interface and an anonymous class to implement it?

[thecoop]

I considered that...an interface gives us a bit more flexibility for the future, and this is a formal public class contract so should be done more 'properly'

[rjernst]

A record seems more suitable to me. What flexibility to you envision an interface gives? What makes it a better formal contract?

[thecoop]

I would say an interface is more usual in an API like this. It can be mocked out easily, and it allows flexibility for any future changes we may wish to add, such as, off the top of my head:

• Adding code to track which services are accessed by which plugins
• Exceptions can be thrown if a service is not available in a particular context
• Dynamically loading services into particular class contexts only when they are accessed
• Test implementations can add restrictions on how services are accessed, or check they are only accessed in certain threads
• We could migrate to a more listener/registration based API using this interface as a base, maybe moving some of the existing methods/plugin interfaces into here in the future
• We could split this interface up into different sub-interfaces for different types of plugins

There's a whole load of future modifications we could do by using an interface. A record doesn't allow these. And there's nothing additional a record gives us here that an interface doesn't.

[thecoop]

I've changed the implementing class to a record, so it's a bit more succinct in the definition, and not anonymous

[rjernst]

LGTM