Skip to content

Commit

Permalink
Fixes #3184 - Make LifeCycle implement AutoCloseable.
Browse files Browse the repository at this point in the history
Rather than making LifeCycle AutoCloseable, just implement AutoCloseable in the client components.

Update tests to use try-with-resources accordingly.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
sbordet committed Aug 14, 2024
1 parent cc2cef1 commit e7f1512
Show file tree
Hide file tree
Showing 15 changed files with 111 additions and 178 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ public void testConscryptHTTP2Client() throws Exception
sslContextFactory.setProvider("Conscrypt");
Conscrypt.setDefaultHostnameVerifier((certs, hostname, session) -> true);

HTTP2Client client = new HTTP2Client();
try
try (HTTP2Client client = new HTTP2Client())
{
client.addBean(sslContextFactory);
client.start();
Expand Down Expand Up @@ -97,10 +96,6 @@ public void onDataAvailable(Stream stream)

assertTrue(latch.await(15, TimeUnit.SECONDS));
}
finally
{
client.stop();
}
}

private boolean canConnectTo(String host, int port)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,17 +132,12 @@ public void testSimpleRequest() throws Exception
ClientConnector clientConnector = new ClientConnector();
clientConnector.setSslContextFactory(newClientSslContextFactory());
HTTP2Client h2Client = new HTTP2Client(clientConnector);
HttpClient client = new HttpClient(new HttpClientTransportOverHTTP2(h2Client));
client.start();
try
try (HttpClient client = new HttpClient(new HttpClientTransportOverHTTP2(h2Client)))
{
client.start();
int port = ((ServerConnector)server.getConnectors()[0]).getLocalPort();
ContentResponse contentResponse = client.GET("https://localhost:" + port);
assertEquals(200, contentResponse.getStatus());
}
finally
{
client.stop();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ public void testJDK9HTTP2Client() throws Exception

Assumptions.assumeTrue(canConnectTo(host, port));

HTTP2Client client = new HTTP2Client();
try
try (HTTP2Client client = new HTTP2Client())
{
SslContextFactory.Client sslContextFactory = new SslContextFactory.Client();
client.addBean(sslContextFactory);
Expand Down Expand Up @@ -87,10 +86,6 @@ public void onDataAvailable(Stream stream)

latch.await(15, TimeUnit.SECONDS);
}
finally
{
client.stop();
}
}

private boolean canConnectTo(String host, int port)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
* }</pre>
*/
@ManagedObject("The HTTP client")
public class HttpClient extends ContainerLifeCycle
public class HttpClient extends ContainerLifeCycle implements AutoCloseable
{
public static final String USER_AGENT = "Jetty/" + Jetty.VERSION;
private static final Logger LOG = LoggerFactory.getLogger(HttpClient.class);
Expand Down Expand Up @@ -1141,4 +1141,10 @@ public ClientConnectionFactory newSslClientConnectionFactory(SslContextFactory.C
sslContextFactory = getSslContextFactory();
return new SslClientConnectionFactory(sslContextFactory, getByteBufferPool(), getExecutor(), connectionFactory);
}

@Override
public void close() throws Exception
{
stop();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,11 @@ public class HttpClientJMXTest
@Test
public void testHttpClientName() throws Exception
{
String name = "foo";
HttpClient httpClient = new HttpClient();
httpClient.setName(name);

try
try (HttpClient httpClient = new HttpClient())
{
String name = "foo";
httpClient.setName(name);

MBeanServer mbeanServer = ManagementFactory.getPlatformMBeanServer();
MBeanContainer mbeanContainer = new MBeanContainer(mbeanServer);
// Adding MBeanContainer as a bean will trigger the registration of MBeans.
Expand All @@ -59,9 +58,5 @@ public void testHttpClientName() throws Exception
assertEquals(name, oName.getKeyProperty("context"));
}
}
finally
{
httpClient.stop();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@
*} </pre>
*/
@ManagedObject
public class HTTP2Client extends ContainerLifeCycle
public class HTTP2Client extends ContainerLifeCycle implements AutoCloseable
{
private final ClientConnector connector;
private int inputBufferSize = 8192;
Expand Down Expand Up @@ -492,4 +492,10 @@ private ClientConnectionFactory newClientConnectionFactory(SslContextFactory.Cli
}
return factory;
}

@Override
public void close() throws Exception
{
stop();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,27 +106,26 @@ public class HttpClientTransportOverHTTP2Test extends AbstractTest
public void testPropertiesAreForwarded() throws Exception
{
HTTP2Client http2Client = new HTTP2Client();
HttpClient httpClient = new HttpClient(new HttpClientTransportOverHTTP2(http2Client));
Executor executor = new QueuedThreadPool();
httpClient.setExecutor(executor);
httpClient.setConnectTimeout(13);
httpClient.setIdleTimeout(17);
httpClient.setUseInputDirectByteBuffers(false);
httpClient.setUseOutputDirectByteBuffers(false);

httpClient.start();

assertTrue(http2Client.isStarted());
assertSame(httpClient.getExecutor(), http2Client.getExecutor());
assertSame(httpClient.getScheduler(), http2Client.getScheduler());
assertSame(httpClient.getByteBufferPool(), http2Client.getByteBufferPool());
assertEquals(httpClient.getConnectTimeout(), http2Client.getConnectTimeout());
assertEquals(httpClient.getIdleTimeout(), http2Client.getIdleTimeout());
assertEquals(httpClient.isUseInputDirectByteBuffers(), http2Client.isUseInputDirectByteBuffers());
assertEquals(httpClient.isUseOutputDirectByteBuffers(), http2Client.isUseOutputDirectByteBuffers());

httpClient.stop();

try (HttpClient httpClient = new HttpClient(new HttpClientTransportOverHTTP2(http2Client)))
{
Executor executor = new QueuedThreadPool();
httpClient.setExecutor(executor);
httpClient.setConnectTimeout(13);
httpClient.setIdleTimeout(17);
httpClient.setUseInputDirectByteBuffers(false);
httpClient.setUseOutputDirectByteBuffers(false);

httpClient.start();

assertTrue(http2Client.isStarted());
assertSame(httpClient.getExecutor(), http2Client.getExecutor());
assertSame(httpClient.getScheduler(), http2Client.getScheduler());
assertSame(httpClient.getByteBufferPool(), http2Client.getByteBufferPool());
assertEquals(httpClient.getConnectTimeout(), http2Client.getConnectTimeout());
assertEquals(httpClient.getIdleTimeout(), http2Client.getIdleTimeout());
assertEquals(httpClient.isUseInputDirectByteBuffers(), http2Client.isUseInputDirectByteBuffers());
assertEquals(httpClient.isUseOutputDirectByteBuffers(), http2Client.isUseOutputDirectByteBuffers());
}
assertTrue(http2Client.isStopped());
}

Expand Down Expand Up @@ -833,16 +832,16 @@ public void testExternalServer() throws Exception
{
ClientConnector clientConnector = new ClientConnector();
HTTP2Client http2Client = new HTTP2Client(clientConnector);
SslContextFactory.Client sslContextFactory = new SslContextFactory.Client();
clientConnector.setSslContextFactory(sslContextFactory);
HttpClient httpClient = new HttpClient(new HttpClientTransportOverHTTP2(http2Client));
Executor executor = new QueuedThreadPool();
clientConnector.setExecutor(executor);
httpClient.start();

ContentResponse response = httpClient.GET("https://webtide.com/");
assertEquals(HttpStatus.OK_200, response.getStatus());

httpClient.stop();
try (HttpClient httpClient = new HttpClient(new HttpClientTransportOverHTTP2(http2Client)))
{
Executor executor = new QueuedThreadPool();
clientConnector.setExecutor(executor);
SslContextFactory.Client sslContextFactory = new SslContextFactory.Client();
clientConnector.setSslContextFactory(sslContextFactory);
httpClient.start();

ContentResponse response = httpClient.GET("https://webtide.com/");
assertEquals(HttpStatus.OK_200, response.getStatus());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,9 @@ public boolean handle(Request request, Response response, Callback callback)
}
});

HTTP2Client http2Client = new HTTP2Client();
http2Client.start();
try
try (HTTP2Client http2Client = new HTTP2Client())
{
http2Client.start();
String host = "localhost";
int port = connector.getLocalPort();
InetSocketAddress address = new InetSocketAddress(host, port);
Expand Down Expand Up @@ -116,9 +115,5 @@ public void onDataAvailable(Stream stream)
assertNotNull(frame);
assertTrue(frame.getMetaData().isResponse());
}
finally
{
http2Client.stop();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@
* <p>HTTP/3+QUIC support is experimental and not suited for production use.
* APIs may change incompatibly between releases.</p>
*/
public class HTTP3Client extends ContainerLifeCycle
public class HTTP3Client extends ContainerLifeCycle implements AutoCloseable
{
public static final String CLIENT_CONTEXT_KEY = HTTP3Client.class.getName();
public static final String SESSION_LISTENER_CONTEXT_KEY = CLIENT_CONTEXT_KEY + ".listener";
Expand Down Expand Up @@ -217,4 +217,10 @@ public CompletableFuture<Void> shutdown()
{
return container.shutdown();
}

@Override
public void close() throws Exception
{
stop();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,19 +54,13 @@ public void testExternalServerWithHttpClient() throws Exception
SslContextFactory.Client sslClient = new SslContextFactory.Client();
ClientQuicConfiguration quicConfig = new ClientQuicConfiguration(sslClient, null);
HTTP3Client client = new HTTP3Client(quicConfig);
HttpClientTransportOverHTTP3 transport = new HttpClientTransportOverHTTP3(client);
HttpClient httpClient = new HttpClient(transport);
httpClient.start();
try
try (HttpClient httpClient = new HttpClient(new HttpClientTransportOverHTTP3(client)))
{
httpClient.start();
URI uri = URI.create("https://maven-central-eu.storage-download.googleapis.com/maven2/org/apache/maven/maven-parent/38/maven-parent-38.pom");
ContentResponse response = httpClient.newRequest(uri).send();
assertThat(response.getContentAsString(), containsString("<artifactId>maven-parent</artifactId>"));
}
finally
{
httpClient.stop();
}
}

@Test
Expand All @@ -75,10 +69,9 @@ public void testExternalServerWithHTTP3Client() throws Exception
{
SslContextFactory.Client sslClient = new SslContextFactory.Client();
ClientQuicConfiguration quicConfig = new ClientQuicConfiguration(sslClient, null);
HTTP3Client client = new HTTP3Client(quicConfig);
client.start();
try
try (HTTP3Client client = new HTTP3Client(quicConfig))
{
client.start();
// Well-known HTTP/3 servers to try.
// HostPort hostPort = new HostPort("maven-central-eu.storage-download.googleapis.com:443");
HostPort hostPort = new HostPort("google.com:443");
Expand Down Expand Up @@ -137,9 +130,5 @@ public void onTrailer(Stream.Client stream, HeadersFrame frame)

assertTrue(requestLatch.await(5, TimeUnit.SECONDS));
}
finally
{
client.stop();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -118,20 +118,15 @@ public boolean handle(Request request, Response response, Callback callback)

// Use the deprecated APIs for backwards compatibility testing.
ClientConnector clientConnector = ClientConnector.forUnixDomain(unixDomainPath);
HttpClient httpClient = new HttpClient(new HttpClientTransportDynamic(clientConnector));
httpClient.start();
try
try (HttpClient httpClient = new HttpClient(new HttpClientTransportDynamic(clientConnector)))
{
httpClient.start();
ContentResponse response = httpClient.newRequest(uri)
.timeout(5, TimeUnit.SECONDS)
.send();

assertEquals(HttpStatus.OK_200, response.getStatus());
}
finally
{
httpClient.stop();
}
}

@Test
Expand All @@ -153,29 +148,24 @@ public boolean handle(Request request, Response response, Callback callback)
}
});

HttpClient httpClient = new HttpClient(new HttpClientTransportDynamic());
Origin proxyOrigin = new Origin(
"http",
new Origin.Address("localhost", fakeProxyPort),
null,
new Origin.Protocol(List.of("http/1.1"), false),
new Transport.TCPUnix(unixDomainPath)
);
httpClient.getProxyConfiguration().addProxy(new HttpProxy(proxyOrigin, null));
httpClient.start();
try
try (HttpClient httpClient = new HttpClient(new HttpClientTransportDynamic()))
{
Origin proxyOrigin = new Origin(
"http",
new Origin.Address("localhost", fakeProxyPort),
null,
new Origin.Protocol(List.of("http/1.1"), false),
new Transport.TCPUnix(unixDomainPath)
);
httpClient.getProxyConfiguration().addProxy(new HttpProxy(proxyOrigin, null));
httpClient.start();
ContentResponse response = httpClient.newRequest("localhost", fakeServerPort)
.transport(new Transport.TCPUnix(unixDomainPath))
.timeout(5, TimeUnit.SECONDS)
.send();

assertEquals(HttpStatus.OK_200, response.getStatus());
}
finally
{
httpClient.stop();
}
}

@Test
Expand Down Expand Up @@ -215,10 +205,9 @@ else if ("/v2".equals(target))
}
});

HttpClient httpClient = new HttpClient(new HttpClientTransportDynamic());
httpClient.start();
try
try (HttpClient httpClient = new HttpClient(new HttpClientTransportDynamic()))
{
httpClient.start();
// Try PROXYv1 with the PROXY information retrieved from the EndPoint.
// PROXYv1 does not support the UNIX family.
ContentResponse response1 = httpClient.newRequest("localhost", 0)
Expand All @@ -241,10 +230,6 @@ else if ("/v2".equals(target))

assertEquals(HttpStatus.OK_200, response2.getStatus());
}
finally
{
httpClient.stop();
}
}

@Test
Expand Down
Loading

0 comments on commit e7f1512

Please sign in to comment.