[
https://issues.apache.org/jira/browse/HTTPCLIENT-2216?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17539949#comment-17539949
]
Oleg Kalnichevski commented on HTTPCLIENT-2216:
-----------------------------------------------
[~dkrompet] I do not understand the issue. You should be mocking / injecting
`HttpAsyncClient` not `CloseableHttpAsyncClient`.
Client services should be using `HttpAsyncClient` not
`CloseableHttpAsyncClient`.
Oleg
> 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: dev-unsubscr...@hc.apache.org
For additional commands, e-mail: dev-h...@hc.apache.org
