[
https://issues.apache.org/jira/browse/HTTPCLIENT-2216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17540080#comment-17540080
]
Dillon Krompetz commented on HTTPCLIENT-2216:
---------------------------------------------
Hi [~olegk] , the issue I had was that `start()` and `close()` were not being
defined on the HttpAsyncClient interface alone.
My first pass did do as you mentioned and used the HttpAsyncClient interface in
my service. I see now that my pattern for calling start() and close() did not
match the intended usage.
Instead the intended pattern seems to have been to have the dependency provider
call start() on construction of the CloseableHttpAsyncClient provide the
dependency then close() on shutdown. This pattern was not immediately obvious
to me :/
> ClosableHttpAsyncClient interface and testing
> ---------------------------------------------
>
> Key: HTTPCLIENT-2216
> URL: https://issues.apache.org/jira/browse/HTTPCLIENT-2216
> Project: HttpComponents HttpClient
> Issue Type: Improvement
> Components: HttpClient (async)
> Affects Versions: 5.0
> Reporter: Dillon Krompetz
> Priority: Minor
>
> TLDR: Please insert interface that contains functionality of
> CloseableHttpAsyncClient so that this can be tested more easily.
> ===========
> The current CloseableHttpAsyncClient implementation currently has a problem
> in its interface and intended usage such that testing becomes more difficult
> than it needs to be.
>
> Class in question
> {code:java}
> public abstract class CloseableHttpAsyncClient implements HttpAsyncClient,
> ModalCloseable {code}
>
> Methods in question
> {code:java}
> public abstract void start(); {code}
> defined in CloseableHttpAsyncClient class
>
> {code:java}
> <T> Future<T> execute(
> AsyncRequestProducer requestProducer,
> AsyncResponseConsumer<T> responseConsumer,
> HandlerFactory<AsyncPushConsumer> pushHandlerFactory,
> HttpContext context,
> FutureCallback<T> callback
> ); {code}
> defined in HttpAsyncClient interface
>
> {code:java}
> void close(CloseMode closeMode); {code}
> defined in ModalCloseable interface
>
> How is this an issue?
> When a developer goes to make use of the Http5 async client they may follow
> the guides provided:
> [https://hc.apache.org/httpcomponents-client-5.1.x/examples-async.html] and
> see that they need access to the above three methods.
>
> They may introduce an adapter around this interface to take thier business
> logic and convert it into an Http request compatable with the client.
>
> {code:java}
> class MyHttp5AsyncClient {
> private CloseableHttpAsyncClient client;
>
> public MyHttp5AsyncClient(CloseableHttpAsyncClient httpAsyncClient) {
> this.client = httpAsyncClient;
> }
>
> public start() {
> this.client.start();
> }
>
> public stop() {
> this.client.close();
> }
>
> public execute(MyRequest request) {
> AsyncRequestProducer requestProducer = toRequestProducer(request);
> Future<SimpelHttpResponse> result = client.execute(
> requestProdcuer,
> SimpleResponseConsumer.create(),
> null,
> null,
> null
> );
> return result.get();
> }
> } {code}
>
> Note that in the CloseableHttpAsyncClient implementation that the execute
> method is final:
> {code:java}
> public final <T> Future<T> execute(
> final HttpHost target,
> final AsyncRequestProducer requestProducer,
> final AsyncResponseConsumer<T> responseConsumer,
> final HandlerFactory<AsyncPushConsumer> pushHandlerFactory,
> final HttpContext context,
> final FutureCallback<T> callback) {
> Args.notNull(requestProducer, "Request producer");
> Args.notNull(responseConsumer, "Response consumer");
> return doExecute(target, requestProducer, responseConsumer,
> pushHandlerFactory, context, callback);
> } {code}
>
> Testing
> Popular testing patterns include injecting the dependency into the class to
> verify that methods were called with certain arguments or to mock the result
> of calling a method to control the code flow in isolation. A very popular
> Java mocking and verifying library might be Mockito where a developer may
> write something like:
> {code:java}
> class MyHttp5AsyncClientTest {
>
> @Mock
> private CloseableHttpAsyncClient client;
>
> private MyHttp5AsyncClient toTest;
>
> @BeforeEach
> public void setup() {
> toTest = new MyHttp5AsyncClient(client);
> }
>
> @Test
> public void execute_whenCalledWithSomeInput_thenWillDoTheThing() {
> when(client.execute(
> any(AsyncRequestProducer.class),
> any(AsyncResponseConsumer.class),
> nullable(HandlerFactory.class),
> nullable(HttpContext.class),
> nullable(FutureCallback.class)
> )).thenReturn(
> CompletableFuture.completedFuture(null)
> );
>
> var result = toTest.execute(null);
> // ...
> // some expectations on the result that are interesting.
> }
> } {code}
> This test will fail because the hard implementation of
> CloseableHttpAsyncClient execute is final and when mocking it will call the
> actual method with the result of calls to Mockito's any() which returns
> nulls. This will then be caught by the guard statements in the actual method
> call to execute.
>
> How can this be made better?
> How I got around the issue:
> Inserted a wrapper around CloseableHttpAsyncClient that had the public
> methods I needed to use that could be mocked.
>
> How might this be better resolved:
> 1. Remove final modifier from the CloseableHttpAsyncClient methods that are
> likely to be mocked.
> There is likely a reason these methods were intended to not be open for
> modification :)
>
> 2. Insert an interface that CloseableHttpAsyncClient implements that captures
> its intended functionality.
> This new interface would include the public methods of
> CloseableHttpAsyncClient and extend the interfaces for HttpAsyncClient and
> ModalCloseable
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
