Unable to effectively create decorator class for RestHighLevelClient due to final modifiers

[mveitas]

Now that all methods on the RestHighLevelClient are marked as final, it is not possible to be able to effectively decorate the RestHighLevelClient class. The use case I have for decorating this class is to provide some high level metrics around the usage of the client from our application code.

One possibility would be to introduce some interface that the RestHighLevelClient implements would allow the class to be wrapped and ensure that all methods are accounted for if there is a addition or removal of a method.

This issue comes from a conversation from #27238 (comment) that changed all of the modifiers to be final.

[elasticmachine]

Pinging @elastic/es-core-infra

[Edvinas01]

This would also be very useful when creating mocks / writing tests. As now in order to validate RestHighLevelClient calls, I have to write my own non-final wrapper which could be substituted and validated against in tests.

[nik9000]

About a dozen contributors got together to talk about open issues and this came up. We decided against making this change. Here goes our reasoning:

• These methods really weren't designed to be extension points and we feel like folks @Overrideing them would be super confusing. final feels like the right choice for them.
• But folks mocking them is totally reasonable! We could make an interface and folks could code to that and mock that.
• But then we're not really sure which methods we should put in the interface. What about getLowLevelClient, close, indices?

So we came to conclusion that folks should make their own delegates and mock them if they need mocking, that way folks don't have a huge interface with weird stuff on it that they never use floating around.

I admit a good deal of personal bias here: I've never liked mocking storage systems in tests. The tests just end up as brittle rituals that don't actually prove anything. But I've found mocking a reasonably high level construct around the storage system to be quite useful in testing the rest of my application. My experience with this kind of thing is fairly out of date but the other Elasticsearch contributors agreed so I have that going for me.

[purple52]

The original reporter of this issue essentially needs to be able to proxy RestHighLevelClient for gathering metrics. This is the same need that brought me here. The discussion referred to above seems to completely miss the point, focusing on mocking. If it's so difficult to decide what should go in an interface, then maybe the purpose of the class needs a bit of a rethink? Having to write a delegate class yourself makes using the client in reusable way in library or framework code impossible.

[Edvinas01]

@purple52 indeed mocking is only a tiny portion of what an interface would offer to its users and maintainers of this utility.

For example, if an interface were used and some factory or builder which would return instance of this interface, the maintainers would have the freedom to do what ever they want with RestHighLevelClient class (change its naming, constructor signature, return types - assuming that they would be interfaces as well). Now if you try to do any of those, quite a few people will be quite unhappy when upgrading.

[clakech]

* These methods really weren't designed to be extension points and we feel like folks `@Override`ing them would be super confusing.

Hi, here are my 2 cents after a discussion on twitter https://twitter.com/cyril_lakech/status/1219356360766316545

You should not try to avoid confusion for your users by blocking them. Document your recommandations/warnings and if users want to extend the client, it is their problems, not yours.

Unit tests are not Integration tests. Unit tests provide quick feedbacks, integration tests are slower.

This is not easy to mock final in Java (performance are so bad on mockito on this). Yes this is a Java problem. Still you provide a Java driver and should take care of ecosystem capabilities from the trenches and not only take care of the "good" design. A perfect design is not the good one here.

I am ok to create a delegate for the client, but how to be able to use final SearchHit in unit tests ? I can't.