Bring back IBulkResponse for easier mocking

[mszczepaniakebsco]

Describe the feature:
Since 7.0.0, IBulkResponse interface was removed and Bulk methods now return BulkResponse. This causes mocking of all Bulk operations in consuming applications a little more difficult.
I think it was beneficial to be able to mock the response and test the application behavior and usage of Items, Errors, ItemsWithErrors objects.
I have a work around, but it seems more elegant to operate on interfaces for the sake of consumers.

[Mpdreamz]

Hi @mszczepaniakebsco thanks for raising this.

We've added interfaces for responses at a time when Elasticsearch had say 30 API's, since then Elasticsearch has grown to nearly 230+ API's the cognitive and literal byte size of these interfaces were weighing on us during development.

We've kept the interfaces for SearchResponse<> and are committed to keep these around since
we feel 80% of the people that want to mock the client want to do so for that API.

I am a strong believer though that you shouldn't mock what you don't own and should instead mock/stub/fake your layer that calls Elasticsearch.

In the cases where you call the client directly (e.g from your controller) and have no layer of indirection in between (which is also totally okay of course) we feel that injecting an IElasticClient with an InMemoryConnection is a viable alternative. We are working on releasing a package to aid with creating an in memory client that behaves a certain way see #3908

[mszczepaniakebsco]

Hi @Mpdreamz , I totally understand the burden of maintaining 230+ API with interfaces, base classes etc.
I disagree with your statement that I shouldn't mock what I don't own. It's fair to say: I shouldn't test the code that I don't own, and IMHO mocking is meant exactly for that. I have my layer that calls ElasticSearch and part of that code needs to be tested. I need to test how my code reacts to various characteristics of ElasticSearch response objects, for example: what my code supposed to do if BulkResponse.IsValid is false. I need to write code that does what is expected and I want to test it.

Right now, I am mocking the response by parsing properly crafted JSON object using RequestResponseSerializer, but that seems unnecessarily cumbersome.

With all that said, another idea that would help the consumers test the responses is to mark all/most/critical properties as "virtual". This way the Response object can be mocked without much hassle on the consumer end and also should not add any additional weight on your end.
Thoughts?

[sundalova]

I have a similar problem, but in my case I'm using Cat, Indices and Snapshot endpoints so I'm having troubles now with mocking both endpoints and responses.

I would like to be able to set up responses for the calls and simulate when server returned an error.

Could you also look into setting up responses not via deserializing json-string but actually via object initialization. Right now some of the models have internal setters - for example IndexStats and all it's members has internal setters and you need to actually find IElasticsearchSerializer in order to properly get objects and maybe I'm blind but it took me a while reading/decompiling to figure out how to do it.

I also found testing section in documentation for nest and elasticsearch.client but it contained only references to Java but it would be really nice to have it if you will go with introducing testing library.

[gabbersepp]

@Mpdreamz what about making those stuff "virtual" as suggested from @mszczepaniakebsco

[sundalova]

@Mpdreamz what about making those stuff "virtual" as suggested from @mszczepaniakebsco

I will agree as it will allow us to use mock libraries