diff --git a/extras/retrofit2/pom.xml b/extras/retrofit2/pom.xml index 2271afe0ea..317e96b28a 100644 --- a/extras/retrofit2/pom.xml +++ b/extras/retrofit2/pom.xml @@ -13,7 +13,7 @@ 2.5.0 - 1.16.20 + 1.18.6 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 9197351233..39107ce02a 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 @@ -30,6 +30,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.function.Supplier; /** * {@link AsyncHttpClient} Retrofit2 {@link okhttp3.Call} @@ -48,6 +49,7 @@ class AsyncHttpClientCall implements Cloneable, okhttp3.Call { public static final long DEFAULT_EXECUTE_TIMEOUT_MILLIS = 30_000; private static final ResponseBody EMPTY_BODY = ResponseBody.create(null, ""); + /** * Tells whether call has been executed. * @@ -55,37 +57,44 @@ class AsyncHttpClientCall implements Cloneable, okhttp3.Call { * @see #isCanceled() */ private final AtomicReference> futureRef = new AtomicReference<>(); + /** - * HttpClient instance. + * {@link AsyncHttpClient} supplier */ @NonNull - AsyncHttpClient httpClient; + Supplier httpClientSupplier; + /** * {@link #execute()} response timeout in milliseconds. */ @Builder.Default long executeTimeoutMillis = DEFAULT_EXECUTE_TIMEOUT_MILLIS; + /** * Retrofit request. */ @NonNull @Getter(AccessLevel.NONE) Request request; + /** * List of consumers that get called just before actual async-http-client request is being built. */ @Singular("requestCustomizer") List> requestCustomizers; + /** * List of consumers that get called just before actual HTTP request is being fired. */ @Singular("onRequestStart") List> onRequestStart; + /** * List of consumers that get called when HTTP request finishes with an exception. */ @Singular("onRequestFailure") List> onRequestFailure; + /** * List of consumers that get called when HTTP request finishes successfully. */ @@ -236,6 +245,20 @@ public Response onCompleted(org.asynchttpclient.Response response) { return future; } + /** + * Returns HTTP client. + * + * @return http client + * @throws IllegalArgumentException if {@link #httpClientSupplier} returned {@code null}. + */ + protected AsyncHttpClient getHttpClient() { + val httpClient = httpClientSupplier.get(); + if (httpClient == null) { + throw new IllegalStateException("Async HTTP client instance supplier " + httpClientSupplier + " returned null."); + } + return httpClient; + } + /** * Converts async-http-client response to okhttp response. * 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 85139718d5..0077cd32e3 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,29 +18,22 @@ 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; /** - * {@link AsyncHttpClient} implementation of Retrofit2 {@link Call.Factory} + * {@link AsyncHttpClient} implementation of Retrofit2 + * {@link Call.Factory}. */ @Value @Builder(toBuilder = true) public class AsyncHttpClientCallFactory implements Call.Factory { /** - * {@link AsyncHttpClient} in use. - * - * @see #httpClientSupplier - */ - @Getter(AccessLevel.NONE) - AsyncHttpClient httpClient; - - /** - * Supplier of {@link AsyncHttpClient}, takes precedence over {@link #httpClient}. + * Supplier of {@link AsyncHttpClient}. */ + @NonNull @Getter(AccessLevel.NONE) Supplier httpClientSupplier; @@ -48,12 +41,13 @@ public class AsyncHttpClientCallFactory implements Call.Factory { * List of {@link Call} builder customizers that are invoked just before creating it. */ @Singular("callCustomizer") + @Getter(AccessLevel.PACKAGE) List> callCustomizers; @Override public Call newCall(Request request) { val callBuilder = AsyncHttpClientCall.builder() - .httpClient(httpClient) + .httpClientSupplier(httpClientSupplier) .request(request); // customize builder before creating a call @@ -64,15 +58,33 @@ public Call newCall(Request request) { } /** - * {@link AsyncHttpClient} in use by this factory. + * Returns {@link AsyncHttpClient} from {@link #httpClientSupplier}. * - * @return + * @return http client. */ - 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.")); + AsyncHttpClient getHttpClient() { + return httpClientSupplier.get(); + } + + /** + * Builder for {@link AsyncHttpClientCallFactory}. + */ + public static class AsyncHttpClientCallFactoryBuilder { + /** + * {@link AsyncHttpClient} supplier that returns http client to be used to execute HTTP requests. + */ + private Supplier httpClientSupplier; + + /** + * Sets concrete http client to be used by the factory to execute HTTP requests. Invocation of this method + * overrides any previous http client supplier set by {@link #httpClientSupplier(Supplier)}! + * + * @param httpClient http client + * @return reference to itself. + * @see #httpClientSupplier(Supplier) + */ + public AsyncHttpClientCallFactoryBuilder httpClient(@NonNull AsyncHttpClient httpClient) { + return httpClientSupplier(() -> httpClient); + } } } \ 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 cec3ece12b..864931a583 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 @@ -14,23 +14,34 @@ import lombok.extern.slf4j.Slf4j; import lombok.val; +import okhttp3.MediaType; import okhttp3.Request; +import okhttp3.RequestBody; import okhttp3.Response; import org.asynchttpclient.AsyncHttpClient; import org.asynchttpclient.RequestBuilder; import org.testng.annotations.Test; +import java.util.Objects; import java.util.UUID; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; -import static org.asynchttpclient.extras.retrofit.AsyncHttpClientCallTest.REQUEST; import static org.asynchttpclient.extras.retrofit.AsyncHttpClientCallTest.createConsumer; import static org.mockito.Mockito.mock; import static org.testng.Assert.*; @Slf4j public class AsyncHttpClientCallFactoryTest { + private static final MediaType MEDIA_TYPE = MediaType.parse("application/json"); + private static final String JSON_BODY = "{\"foo\": \"bar\"}"; + private static final RequestBody BODY = RequestBody.create(MEDIA_TYPE, JSON_BODY); + private static final String URL = "http://localhost:11000/foo/bar?a=b&c=d"; + private static final Request REQUEST = new Request.Builder() + .post(BODY) + .addHeader("X-Foo", "Bar") + .url(URL) + .build(); @Test void newCallShouldProduceExpectedResult() { // given @@ -152,7 +163,8 @@ void shouldApplyAllConsumersToCallBeingConstructed() { assertTrue(call.getRequestCustomizers().size() == 2); } - @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = "HTTP client is not set.") + @Test(expectedExceptions = NullPointerException.class, + expectedExceptionsMessageRegExp = "httpClientSupplier is marked @NonNull but is null") void shouldThrowISEIfHttpClientIsNotDefined() { // given val factory = AsyncHttpClientCallFactory.builder() @@ -168,17 +180,23 @@ void shouldThrowISEIfHttpClientIsNotDefined() { @Test void shouldUseHttpClientInstanceIfSupplierIsNotAvailable() { // given - val httpClientA = mock(AsyncHttpClient.class); + val httpClient = mock(AsyncHttpClient.class); val factory = AsyncHttpClientCallFactory.builder() - .httpClient(httpClientA) + .httpClient(httpClient) .build(); // when val usedHttpClient = factory.getHttpClient(); // then - assertTrue(usedHttpClient == httpClientA); + assertTrue(usedHttpClient == httpClient); + + // when + val call = (AsyncHttpClientCall) factory.newCall(REQUEST); + + // then: call should contain correct http client + assertTrue(call.getHttpClient()== httpClient); } @Test @@ -197,5 +215,12 @@ void shouldPreferHttpClientSupplierOverHttpClient() { // then assertTrue(usedHttpClient == httpClientB); + + // when: try to create new call + val call = (AsyncHttpClientCall) factory.newCall(REQUEST); + + // then: call should contain correct http client + assertNotNull(call); + assertTrue(call.getHttpClient() == httpClientB); } } diff --git a/extras/retrofit2/src/test/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallTest.java b/extras/retrofit2/src/test/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallTest.java index 84ed4ddf47..ab908730af 100644 --- a/extras/retrofit2/src/test/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallTest.java +++ b/extras/retrofit2/src/test/java/org/asynchttpclient/extras/retrofit/AsyncHttpClientCallTest.java @@ -24,6 +24,7 @@ import org.asynchttpclient.Response; import org.mockito.ArgumentCaptor; import org.testng.Assert; +import org.testng.annotations.BeforeMethod; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; @@ -35,10 +36,11 @@ import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; +import java.util.function.Supplier; import static org.asynchttpclient.extras.retrofit.AsyncHttpClientCall.runConsumer; import static org.asynchttpclient.extras.retrofit.AsyncHttpClientCall.runConsumers; -import static org.mockito.Matchers.any; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotEquals; @@ -47,6 +49,14 @@ public class AsyncHttpClientCallTest { static final Request REQUEST = new Request.Builder().url("http://www.google.com/").build(); + private AsyncHttpClient httpClient; + private Supplier httpClientSupplier = () -> httpClient; + + @BeforeMethod + void setup() { + this.httpClient = mock(AsyncHttpClient.class); + } + @Test(expectedExceptions = NullPointerException.class, dataProvider = "first") void builderShouldThrowInCaseOfMissingProperties(AsyncHttpClientCall.AsyncHttpClientCallBuilder builder) { builder.build(); @@ -54,12 +64,10 @@ void builderShouldThrowInCaseOfMissingProperties(AsyncHttpClientCall.AsyncHttpCl @DataProvider(name = "first") Object[][] dataProviderFirst() { - val httpClient = mock(AsyncHttpClient.class); - return new Object[][]{ {AsyncHttpClientCall.builder()}, {AsyncHttpClientCall.builder().request(REQUEST)}, - {AsyncHttpClientCall.builder().httpClient(httpClient)} + {AsyncHttpClientCall.builder().httpClientSupplier(httpClientSupplier)} }; } @@ -77,7 +85,7 @@ void shouldInvokeConsumersOnEachExecution(Consumer> ha val numRequestCustomizer = new AtomicInteger(); // prepare http client mock - val httpClient = mock(AsyncHttpClient.class); + this.httpClient = mock(AsyncHttpClient.class); val mockRequest = mock(org.asynchttpclient.Request.class); when(mockRequest.getHeaders()).thenReturn(EmptyHttpHeaders.INSTANCE); @@ -94,7 +102,7 @@ void shouldInvokeConsumersOnEachExecution(Consumer> ha // create call instance val call = AsyncHttpClientCall.builder() - .httpClient(httpClient) + .httpClientSupplier(httpClientSupplier) .request(REQUEST) .onRequestStart(e -> numStarted.incrementAndGet()) .onRequestFailure(t -> numFailed.incrementAndGet()) @@ -163,7 +171,7 @@ Object[][] dataProviderSecond() { void toIOExceptionShouldProduceExpectedResult(Throwable exception) { // given val call = AsyncHttpClientCall.builder() - .httpClient(mock(AsyncHttpClient.class)) + .httpClientSupplier(httpClientSupplier) .request(REQUEST) .build(); @@ -295,6 +303,18 @@ public void bodyIsNotNullInResponse() throws Exception { assertNotEquals(response.body(), null); } + @Test(expectedExceptions = IllegalStateException.class, expectedExceptionsMessageRegExp = ".*returned null.") + void getHttpClientShouldThrowISEIfSupplierReturnsNull() { + // given: + val call = AsyncHttpClientCall.builder() + .httpClientSupplier(() -> null) + .request(requestWithBody()) + .build(); + + // when: should throw ISE + call.getHttpClient(); + } + private void givenResponseIsProduced(AsyncHttpClient client, Response response) { when(client.executeRequest(any(org.asynchttpclient.Request.class), any())).thenAnswer(invocation -> { AsyncCompletionHandler handler = invocation.getArgument(1); @@ -304,9 +324,11 @@ private void givenResponseIsProduced(AsyncHttpClient client, Response response) } private okhttp3.Response whenRequestIsMade(AsyncHttpClient client, Request request) throws IOException { - AsyncHttpClientCall call = AsyncHttpClientCall.builder().httpClient(client).request(request).build(); - - return call.execute(); + return AsyncHttpClientCall.builder() + .httpClientSupplier(() -> client) + .request(request) + .build() + .execute(); } private Request requestWithBody() {