From 805f246925ab1bf8c3e719e5f1fedceb5a908d9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brane=20F=2E=20Gra=C4=8Dnar?= Date: Mon, 25 Feb 2019 00:19:22 +0100 Subject: [PATCH 1/2] Don't try to cancel already completed future. If AHC is used concurrently with rxjava retrofit call adapter using `flatMap(Function, maxConcurrency)` warning messages are being logged about future not being able to be cancelled, because it's already complete. This patch remedies situation by first checking whether future has been already completed. --- .../asynchttpclient/extras/retrofit/AsyncHttpClientCall.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extras/retrofit2/src/main/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCall.java b/extras/retrofit2/src/main/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCall.java index 4c701738e5..9197351233 100644 --- a/extras/retrofit2/src/main/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCall.java +++ b/extras/retrofit2/src/main/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCall.java @@ -149,7 +149,7 @@ public void enqueue(Callback responseCallback) { @Override public void cancel() { val future = futureRef.get(); - if (future != null) { + if (future != null && !future.isDone()) { if (!future.cancel(true)) { log.warn("Cannot cancel future: {}", future); } From c72bf1c925fad665fcdf95fe48b34957f81759d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brane=20F=2E=20Gra=C4=8Dnar?= Date: Mon, 25 Feb 2019 01:01:18 +0100 Subject: [PATCH 2/2] Added ability to fetch async http client instance from a supplier. Sometimes it's handy not to tie call factory to particular http client instance, but to fetch it from a supplier. This patch adds ability to fetch http client from a supplier while retaining ability to use particular, fixed http client instance. --- .../retrofit/AsyncHttpClientCallFactory.java | 27 ++++++++- .../AsyncHttpClientCallFactoryTest.java | 59 +++++++++++++++++-- 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/extras/retrofit2/src/main/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallFactory.java b/extras/retrofit2/src/main/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallFactory.java index b7c087fd35..85139718d5 100644 --- a/extras/retrofit2/src/main/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallFactory.java +++ b/extras/retrofit2/src/main/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallFactory.java @@ -18,7 +18,9 @@ import org.asynchttpclient.AsyncHttpClient; import java.util.List; +import java.util.Optional; import java.util.function.Consumer; +import java.util.function.Supplier; import static org.asynchttpclient.extras.retrofit.AsyncHttpClientCall.runConsumers; @@ -30,10 +32,18 @@ public class AsyncHttpClientCallFactory implements Call.Factory { /** * {@link AsyncHttpClient} in use. + * + * @see #httpClientSupplier */ - @NonNull + @Getter(AccessLevel.NONE) AsyncHttpClient httpClient; + /** + * Supplier of {@link AsyncHttpClient}, takes precedence over {@link #httpClient}. + */ + @Getter(AccessLevel.NONE) + Supplier httpClientSupplier; + /** * List of {@link Call} builder customizers that are invoked just before creating it. */ @@ -52,4 +62,17 @@ public Call newCall(Request request) { // create a call return callBuilder.build(); } -} + + /** + * {@link AsyncHttpClient} in use by this factory. + * + * @return + */ + public AsyncHttpClient getHttpClient() { + return Optional.ofNullable(httpClientSupplier) + .map(Supplier::get) + .map(Optional::of) + .orElseGet(() -> Optional.ofNullable(httpClient)) + .orElseThrow(() -> new IllegalStateException("HTTP client is not set.")); + } +} \ No newline at end of file diff --git a/extras/retrofit2/src/test/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallFactoryTest.java b/extras/retrofit2/src/test/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallFactoryTest.java index 730ab5d007..cec3ece12b 100644 --- a/extras/retrofit2/src/test/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallFactoryTest.java +++ b/extras/retrofit2/src/test/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallFactoryTest.java @@ -109,12 +109,12 @@ void shouldApplyAllConsumersToCallBeingConstructed() { }; Consumer callCustomizer = callBuilder -> - callBuilder - .requestCustomizer(requestCustomizer) - .requestCustomizer(rb -> log.warn("I'm customizing: {}", rb)) - .onRequestSuccess(createConsumer(numRequestSuccess)) - .onRequestFailure(createConsumer(numRequestFailure)) - .onRequestStart(createConsumer(numRequestStart)); + callBuilder + .requestCustomizer(requestCustomizer) + .requestCustomizer(rb -> log.warn("I'm customizing: {}", rb)) + .onRequestSuccess(createConsumer(numRequestSuccess)) + .onRequestFailure(createConsumer(numRequestFailure)) + .onRequestStart(createConsumer(numRequestStart)); // create factory val factory = AsyncHttpClientCallFactory.builder() @@ -151,4 +151,51 @@ void shouldApplyAllConsumersToCallBeingConstructed() { assertNotNull(call.getRequestCustomizers()); assertTrue(call.getRequestCustomizers().size() == 2); } + + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "HTTP client is not set.") + void shouldThrowISEIfHttpClientIsNotDefined() { + // given + val factory = AsyncHttpClientCallFactory.builder() + .build(); + + // when + val httpClient = factory.getHttpClient(); + + // then + assertNull(httpClient); + } + + @Test + void shouldUseHttpClientInstanceIfSupplierIsNotAvailable() { + // given + val httpClientA = mock(AsyncHttpClient.class); + + val factory = AsyncHttpClientCallFactory.builder() + .httpClient(httpClientA) + .build(); + + // when + val usedHttpClient = factory.getHttpClient(); + + // then + assertTrue(usedHttpClient == httpClientA); + } + + @Test + void shouldPreferHttpClientSupplierOverHttpClient() { + // given + val httpClientA = mock(AsyncHttpClient.class); + val httpClientB = mock(AsyncHttpClient.class); + + val factory = AsyncHttpClientCallFactory.builder() + .httpClient(httpClientA) + .httpClientSupplier(() -> httpClientB) + .build(); + + // when + val usedHttpClient = factory.getHttpClient(); + + // then + assertTrue(usedHttpClient == httpClientB); + } }