Skip to content

Commit

Permalink
Hide internal details of ActionListener impls (#94320)
Browse files Browse the repository at this point in the history
The `ActionListener` interface implements its static methods using
various inner classes. These inner classes are public because interfaces
do not support package-private inner classes, although they are `final`
and only have `protected` constructors which prevent anyone else from
instantiating them. It's certainly a mistake for code in other packages
to access these things.

This commit moves the implementation details into a separate class to
hide them from the rest of the world. It also tightens up the calls to
`onFailure()` in a few places to better handle the possibility of this
method throwing an exception. Finally, it moves
`DelegatingActionListener` to a top level public class since this
utility is used in quite a number of places.
  • Loading branch information
DaveCTurner committed Mar 7, 2023
1 parent 0bd7373 commit e1ad46b
Show file tree
Hide file tree
Showing 34 changed files with 391 additions and 314 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
package org.elasticsearch.index.rankeval;

import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.DelegatingActionListener;
import org.elasticsearch.action.search.MultiSearchRequest;
import org.elasticsearch.action.search.MultiSearchResponse;
import org.elasticsearch.action.search.MultiSearchResponse.Item;
Expand Down Expand Up @@ -143,7 +144,7 @@ protected void doExecute(Task task, RankEvalRequest request, ActionListener<Rank
);
}

static class RankEvalActionListener extends ActionListener.Delegating<MultiSearchResponse, RankEvalResponse> {
static class RankEvalActionListener extends DelegatingActionListener<MultiSearchResponse, RankEvalResponse> {

private final RatedRequest[] specifications;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchScrollAction;
import org.elasticsearch.action.search.SearchScrollRequest;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.client.internal.ParentTaskAssigningClient;
import org.elasticsearch.client.internal.support.AbstractClient;
import org.elasticsearch.common.bytes.BytesArray;
Expand Down Expand Up @@ -48,7 +49,6 @@

import static org.apache.lucene.tests.util.TestUtil.randomSimpleString;
import static org.elasticsearch.core.TimeValue.timeValueSeconds;
import static org.hamcrest.Matchers.instanceOf;

public class ClientScrollableHitSourceTests extends ESTestCase {

Expand All @@ -74,19 +74,12 @@ public void testRetrySuccess() throws InterruptedException {
dotestBasicsWithRetry(retries, 0, retries, e -> fail());
}

private static class ExpectedException extends RuntimeException {
ExpectedException(Throwable cause) {
super(cause);
}
}

public void testRetryFail() {
int retries = randomInt(10);
ExpectedException ex = expectThrows(
ExpectedException.class,
() -> { dotestBasicsWithRetry(retries, retries + 1, retries + 1, e -> { throw new ExpectedException(e); }); }
expectThrows(
EsRejectedExecutionException.class,
() -> PlainActionFuture.get(f -> dotestBasicsWithRetry(retries, retries + 1, retries + 1, f::onFailure), 0, TimeUnit.SECONDS)
);
assertThat(ex.getCause(), instanceOf(EsRejectedExecutionException.class));
}

private void dotestBasicsWithRetry(int retries, int minFailures, int maxFailures, Consumer<Exception> failureHandler)
Expand All @@ -110,6 +103,9 @@ private void dotestBasicsWithRetry(int retries, int minFailures, int maxFailures
hitSource.start();
for (int retry = 0; retry < randomIntBetween(minFailures, maxFailures); ++retry) {
client.fail(SearchAction.INSTANCE, new EsRejectedExecutionException());
if (retry >= retries) {
return;
}
client.awaitOperation();
++expectedSearchRetries;
}
Expand Down

0 comments on commit e1ad46b

Please sign in to comment.