Fixed a design mistake of ConnectionEndpoint having a direct dependency on HttpRequestExecutor class

This commit is contained in:
Oleg Kalnichevski 2024-01-17 12:34:33 +01:00
parent a8db310f2d
commit a1fa1739bf
5 changed files with 108 additions and 17 deletions

View File

@ -40,10 +40,12 @@
import org.apache.hc.client5.testing.sync.extension.TestClientResources;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.HttpHost;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.URIScheme;
import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
import org.apache.hc.core5.http.io.HttpClientConnection;
import org.apache.hc.core5.http.message.BasicClassicHttpRequest;
import org.apache.hc.core5.http.protocol.BasicHttpContext;
import org.apache.hc.core5.http.protocol.DefaultHttpProcessor;
@ -58,6 +60,7 @@
import org.apache.hc.core5.util.TimeValue;
import org.apache.hc.core5.util.Timeout;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
@ -72,6 +75,28 @@ public class TestConnectionManagement {
@RegisterExtension
private TestClientResources testResources = new TestClientResources(URIScheme.HTTP, TIMEOUT);
ConnectionEndpoint.RequestExecutor exec;
@BeforeEach
public void setup() {
exec = new ConnectionEndpoint.RequestExecutor() {
final HttpRequestExecutor requestExecutor = new HttpRequestExecutor();
final HttpProcessor httpProcessor = new DefaultHttpProcessor(
new RequestTargetHost(), new RequestContent(), new RequestConnControl());
@Override
public ClassicHttpResponse execute(final ClassicHttpRequest request,
final HttpClientConnection conn,
final HttpContext context) throws IOException, HttpException {
requestExecutor.preProcess(request, httpProcessor, context);
final ClassicHttpResponse response = requestExecutor.execute(request, conn, context);
requestExecutor.postProcess(response, httpProcessor, context);
return response;
}
};
}
public ClassicTestServer startServer() throws IOException {
return testResources.startServer(null, null, null);
}
@ -110,11 +135,6 @@ public void testReleaseConnection() throws Exception {
connManager.connect(endpoint1, null, context);
final HttpProcessor httpProcessor = new DefaultHttpProcessor(
new RequestTargetHost(), new RequestContent(), new RequestConnControl());
final HttpRequestExecutor exec = new HttpRequestExecutor();
exec.preProcess(request, httpProcessor, context);
try (final ClassicHttpResponse response1 = endpoint1.execute("id1", request, exec, context)) {
Assertions.assertEquals(HttpStatus.SC_OK, response1.getCode());
}
@ -178,11 +198,6 @@ public void testReleaseConnectionWithTimeLimits() throws Exception {
final ConnectionEndpoint endpoint1 = leaseRequest1.get(Timeout.ZERO_MILLISECONDS);
connManager.connect(endpoint1, null, context);
final HttpProcessor httpProcessor = new DefaultHttpProcessor(
new RequestTargetHost(), new RequestContent(), new RequestConnControl());
final HttpRequestExecutor exec = new HttpRequestExecutor();
exec.preProcess(request, httpProcessor, context);
try (final ClassicHttpResponse response1 = endpoint1.execute("id1", request, exec, context)) {
Assertions.assertEquals(HttpStatus.SC_OK, response1.getCode());
}

View File

@ -213,7 +213,11 @@ public ClassicHttpResponse execute(
if (log.isDebugEnabled()) {
log.debug("{} start execution {}", ConnPoolSupport.getId(endpoint), id);
}
return endpoint.execute(id, request, requestExecutor, context);
return endpoint.execute(
id,
request,
requestExecutor::execute,
context);
}
@Override

View File

@ -610,6 +610,10 @@ public void setSocketTimeout(final Timeout timeout) {
getValidatedConnection().setSocketTimeout(timeout);
}
/**
* @deprecated Use {@link #execute(String, ClassicHttpRequest, RequestExecutor, HttpContext)}
*/
@Deprecated
@Override
public ClassicHttpResponse execute(
final String exchangeId,
@ -624,6 +628,23 @@ public ClassicHttpResponse execute(
return requestExecutor.execute(request, getValidatedConnection(), context);
}
/**
* @since 5.4
*/
@Override
public ClassicHttpResponse execute(
final String exchangeId,
final ClassicHttpRequest request,
final RequestExecutor requestExecutor,
final HttpContext context) throws IOException, HttpException {
Args.notNull(request, "HTTP request");
Args.notNull(requestExecutor, "Request executor");
if (LOG.isDebugEnabled()) {
LOG.debug("{} Executing exchange {}", id, exchangeId);
}
return requestExecutor.execute(request, getValidatedConnection(), context);
}
}
/**

View File

@ -710,6 +710,10 @@ public void setSocketTimeout(final Timeout timeout) {
getValidatedPoolEntry().getConnection().setSocketTimeout(timeout);
}
/**
* @deprecated Use {@link #execute(String, ClassicHttpRequest, RequestExecutor, HttpContext)}
*/
@Deprecated
@Override
public ClassicHttpResponse execute(
final String exchangeId,
@ -725,5 +729,22 @@ public ClassicHttpResponse execute(
return requestExecutor.execute(request, connection, context);
}
/**
* @since 5.4
*/
public ClassicHttpResponse execute(
final String exchangeId,
final ClassicHttpRequest request,
final RequestExecutor requestExecutor,
final HttpContext context) throws IOException, HttpException {
Args.notNull(request, "HTTP request");
Args.notNull(requestExecutor, "Request executor");
final ManagedHttpClientConnection connection = getValidatedPoolEntry().getConnection();
if (LOG.isDebugEnabled()) {
LOG.debug("{} executing exchange {} over {}", id, exchangeId, ConnPoolSupport.getId(connection));
}
return requestExecutor.execute(request, connection, context);
}
}
}

View File

@ -30,11 +30,13 @@
import java.io.IOException;
import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.Internal;
import org.apache.hc.core5.annotation.ThreadingBehavior;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;
import org.apache.hc.core5.http.HttpException;
import org.apache.hc.core5.http.impl.io.HttpRequestExecutor;
import org.apache.hc.core5.http.io.HttpClientConnection;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.io.ModalCloseable;
import org.apache.hc.core5.util.Timeout;
@ -51,6 +53,16 @@
@Contract(threading = ThreadingBehavior.SAFE)
public abstract class ConnectionEndpoint implements ModalCloseable {
/**
* @deprecated Use {@link #execute(String, ClassicHttpRequest, RequestExecutor, HttpContext)}
*/
@Deprecated
public abstract ClassicHttpResponse execute(
String id,
ClassicHttpRequest request,
HttpRequestExecutor executor,
HttpContext context) throws IOException, HttpException;
/**
* Executes HTTP request using the provided request executor.
* <p>
@ -59,14 +71,19 @@ public abstract class ConnectionEndpoint implements ModalCloseable {
*
* @param id unique operation ID or {@code null}.
* @param request the request message.
* @param executor the request executor.
* @param requestExecutor the request executor.
* @param context the execution context.
*
* @since 5.4
*/
public abstract ClassicHttpResponse execute(
String id,
ClassicHttpRequest request,
HttpRequestExecutor executor,
HttpContext context) throws IOException, HttpException;
// In the next major release this method must be made abstract.
public ClassicHttpResponse execute(
final String id,
final ClassicHttpRequest request,
final RequestExecutor requestExecutor,
final HttpContext context) throws IOException, HttpException {
return requestExecutor.execute(request,null, context);
}
/**
* Determines if the connection to the remote endpoint is still open and valid.
@ -80,4 +97,17 @@ public abstract ClassicHttpResponse execute(
*/
public abstract void setSocketTimeout(Timeout timeout);
/**
* @since 5.4
*/
@Internal
public interface RequestExecutor {
ClassicHttpResponse execute(
ClassicHttpRequest request,
HttpClientConnection conn,
HttpContext context) throws IOException, HttpException;
}
}