[
https://issues.apache.org/jira/browse/HTTPCLIENT-2216?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Dillon Krompetz updated HTTPCLIENT-2216:
----------------------------------------
Description:
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
was:
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
> 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
>
> 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]
