Add CompletionStage variants to methods in the Client Interface and default to ActionListener impl

[cwperks]

Description

This is a quality of life PR that I think can help reduce a lot of code around the core and plugins using the methods that accept an ActionListener.

Below is an example:

client.index(indexRequest, new ActionListener<IndexResponse>() {
    @Override
    public void onResponse(IndexResponse response) {
        // handle success
        System.out.println("Got: " + response);
    }
    
    @Override
    public void onFailure(Exception e) {
        // handle error
        logger.error("Failed", e);
    }
});

The same pattern can be reduced to the following

client.indexAsync(indexRequest).thenAccept(response -> {
    // handle success
    System.out.println("Got: " + response);
}).exceptionally(error -> {
    System.out.println("Received error: " + error);
    return null;
});

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[andrross]

I like this idea. What's the downside? Why hasn't it been done before? The only reason I can think of to not do this is that it complicates the interface by adding multiple ways to do the same thing. But the new way to do the thing is much better than the old way so that seems like a worthwhile tradeoff.

[cwperks]

What I like about it is that it isn't a breaking change bc it provides default implementations that call the ActionListener versions.

[github-actions]

✅ Gradle check result for 1fa7b0b: SUCCESS

[codecov]

Codecov Report

❌ Patch coverage is 0% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.97%. Comparing base (740b34a) to head (b4a3f04).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...n/java/org/opensearch/transport/client/Client.java	0.00%	48 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #18998      +/-   ##
============================================
+ Coverage     72.93%   72.97%   +0.03%     
+ Complexity    69469    69458      -11     
============================================
  Files          5647     5647              
  Lines        319013   319061      +48     
  Branches      46151    46151              
============================================
+ Hits         232663   232822     +159     
+ Misses        67561    67403     -158     
- Partials      18789    18836      +47     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[cwperks]

@reta any concerns adding CompletableFuture variants to the Client interface?

[reta]

@reta any concerns adding CompletableFuture variants to the Client interface?

Thanks @cwperks , the idea is sound but we need to factor that CompletableFuture offers (much) richer semantics, in particular cancellation (using cancel() method). We won't be interrupting any ongoing communications, would we?

[cwperks]

We won't be interrupting any ongoing communications, would we?

No, I would just be interested in introducing this as syntactic sugar as an alternative for using the methods that accept ActionListener.

[reta]

We won't be interrupting any ongoing communications, would we?

No, I would just be interested in introducing this as syntactic sugar as an alternative for using the methods that accept ActionListener.

In this case I would recommend to use CompletionStage instead: it has all the APIs that you would like to benefit from.

[cwperks]

@reta Updated the new methods to return CompletionStage instead of CompletableFuture to discourage calling methods like cancel().

[github-actions]

❕ Gradle check result for b4a3f04: UNSTABLE

Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.