Retrofit improvements (#1615)

* 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.

* 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.

Author: bfg

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);
       }

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<AsyncHttpClient> 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."));
+  }
+}

extras/retrofit2/src/test/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallFactoryTest.java

@@ -109,12 +109,12 @@ void shouldApplyAllConsumersToCallBeingConstructed() {
     };
 
     Consumer<AsyncHttpClientCall.AsyncHttpClientCallBuilder> 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);
+  }
 }