From eda94890c84dbdb12810957dab3e540c3ac8a167 Mon Sep 17 00:00:00 2001 From: Jorge Bescos Gascon Date: Fri, 24 Jun 2022 13:39:08 +0200 Subject: [PATCH] Review comments Signed-off-by: Jorge Bescos Gascon --- .../client/HttpUrlConnectorProvider.java | 22 ++++++++-- .../jersey/client/HttpUrlConnectorTest.java | 18 ++++----- .../ssl/SslHttpUrlConnectorTest.java | 16 ++++---- .../externalproperties/HttpProxyTest.java | 2 +- .../jersey4003/LostResponseTest.java | 40 +++++++++---------- 5 files changed, 57 insertions(+), 41 deletions(-) diff --git a/core-client/src/main/java/org/glassfish/jersey/client/HttpUrlConnectorProvider.java b/core-client/src/main/java/org/glassfish/jersey/client/HttpUrlConnectorProvider.java index 2fa7af2e4c7..25a6d78cf56 100644 --- a/core-client/src/main/java/org/glassfish/jersey/client/HttpUrlConnectorProvider.java +++ b/core-client/src/main/java/org/glassfish/jersey/client/HttpUrlConnectorProvider.java @@ -255,6 +255,20 @@ protected Connector createHttpUrlConnector(Client client, ConnectionFactory conn */ public interface ConnectionFactory { + /** + * Get a {@link java.net.HttpURLConnection} for a given URL. + *

+ * Implementation of the method MUST be thread-safe and MUST ensure that + * a dedicated {@link java.net.HttpURLConnection} instance is returned for concurrent + * requests. + *

+ * + * @param url the endpoint URL. + * @return the {@link java.net.HttpURLConnection}. + * @throws java.io.IOException in case the connection cannot be provided. + */ + public HttpURLConnection getConnection(URL url) throws IOException; + /** * Get a {@link java.net.HttpURLConnection} for a given URL. *

@@ -268,14 +282,16 @@ public interface ConnectionFactory { * @return the {@link java.net.HttpURLConnection}. * @throws java.io.IOException in case the connection cannot be provided. */ - public HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException; + default HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException { + return (proxy == null) ? getConnection(url) : (HttpURLConnection) url.openConnection(proxy); + } } private static class DefaultConnectionFactory implements ConnectionFactory { @Override - public HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException { - return (HttpURLConnection) (proxy != null ? url.openConnection(proxy) : url.openConnection()); + public HttpURLConnection getConnection(final URL url) throws IOException { + return (HttpURLConnection) url.openConnection(); } } diff --git a/core-client/src/test/java/org/glassfish/jersey/client/HttpUrlConnectorTest.java b/core-client/src/test/java/org/glassfish/jersey/client/HttpUrlConnectorTest.java index e45e038a103..5bb7a787809 100644 --- a/core-client/src/test/java/org/glassfish/jersey/client/HttpUrlConnectorTest.java +++ b/core-client/src/test/java/org/glassfish/jersey/client/HttpUrlConnectorTest.java @@ -16,14 +16,11 @@ package org.glassfish.jersey.client; -import static org.junit.Assert.assertEquals; - import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.net.HttpURLConnection; import java.net.ProtocolException; -import java.net.Proxy; import java.net.SocketTimeoutException; import java.net.URI; import java.net.URL; @@ -33,10 +30,6 @@ import java.util.List; import java.util.Map; -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.HttpsURLConnection; -import javax.net.ssl.SSLPeerUnverifiedException; -import javax.net.ssl.SSLSocketFactory; import javax.ws.rs.client.Client; import javax.ws.rs.client.ClientBuilder; import javax.ws.rs.client.Entity; @@ -46,10 +39,17 @@ import javax.ws.rs.core.Link; import javax.ws.rs.core.Response; +import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLSocketFactory; + import org.glassfish.jersey.client.internal.HttpUrlConnector; + import org.junit.Assert; import org.junit.Ignore; import org.junit.Test; +import static org.junit.Assert.assertEquals; /** * Various tests for the default client connector. @@ -91,7 +91,7 @@ public void testConnectionTimeoutNoEntity() { public void testResolvedRequestUri() { HttpUrlConnectorProvider.ConnectionFactory factory = new HttpUrlConnectorProvider.ConnectionFactory() { @Override - public HttpURLConnection getConnection(URL endpointUrl, Proxy proxy) throws IOException { + public HttpURLConnection getConnection(URL endpointUrl) throws IOException { HttpURLConnection result = (HttpURLConnection) endpointUrl.openConnection(); return wrapRedirectedHttp(result); } @@ -435,7 +435,7 @@ public void testSSLConnection() { ClientRequest request = client.target("https://localhost:8080").request().buildGet().request(); HttpUrlConnectorProvider.ConnectionFactory factory = new HttpUrlConnectorProvider.ConnectionFactory() { @Override - public HttpURLConnection getConnection(URL endpointUrl, Proxy proxy) throws IOException { + public HttpURLConnection getConnection(URL endpointUrl) throws IOException { HttpURLConnection result = (HttpURLConnection) endpointUrl.openConnection(); return wrapNoContentHttps(result); } diff --git a/tests/e2e-client/src/test/java/org/glassfish/jersey/tests/e2e/client/connector/ssl/SslHttpUrlConnectorTest.java b/tests/e2e-client/src/test/java/org/glassfish/jersey/tests/e2e/client/connector/ssl/SslHttpUrlConnectorTest.java index d81139588d6..df3f1dcd4e2 100644 --- a/tests/e2e-client/src/test/java/org/glassfish/jersey/tests/e2e/client/connector/ssl/SslHttpUrlConnectorTest.java +++ b/tests/e2e-client/src/test/java/org/glassfish/jersey/tests/e2e/client/connector/ssl/SslHttpUrlConnectorTest.java @@ -16,15 +16,11 @@ package org.glassfish.jersey.tests.e2e.client.connector.ssl; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; - import java.io.IOException; import java.io.PrintWriter; import java.io.StringWriter; import java.net.HttpURLConnection; import java.net.InetAddress; -import java.net.Proxy; import java.net.Socket; import java.net.URL; import java.util.List; @@ -34,20 +30,24 @@ import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import javax.net.ssl.HttpsURLConnection; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLSocketFactory; import javax.ws.rs.client.Client; import javax.ws.rs.client.ClientBuilder; import javax.ws.rs.core.Response; +import javax.net.ssl.HttpsURLConnection; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLSocketFactory; + import org.glassfish.jersey.apache.connector.ApacheConnectorProvider; import org.glassfish.jersey.apache5.connector.Apache5ConnectorProvider; import org.glassfish.jersey.client.ClientConfig; import org.glassfish.jersey.client.HttpUrlConnectorProvider; import org.glassfish.jersey.client.authentication.HttpAuthenticationFeature; import org.glassfish.jersey.logging.LoggingFeature; + import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * Test custom socket factory in HttpUrlConnection using SSL @@ -70,7 +70,7 @@ public void testSSLWithCustomSocketFactory() throws Exception { .connectorProvider(new HttpUrlConnectorProvider().connectionFactory( new HttpUrlConnectorProvider.ConnectionFactory() { @Override - public HttpURLConnection getConnection(URL url, Proxy proxy) throws IOException { + public HttpURLConnection getConnection(final URL url) throws IOException { HttpsURLConnection connection = (HttpsURLConnection) url.openConnection(); connection.setSSLSocketFactory(socketFactory); return connection; diff --git a/tests/integration/externalproperties/src/test/java/org/glassfish/jersey/tests/externalproperties/HttpProxyTest.java b/tests/integration/externalproperties/src/test/java/org/glassfish/jersey/tests/externalproperties/HttpProxyTest.java index 94100297f0a..f74a55da144 100644 --- a/tests/integration/externalproperties/src/test/java/org/glassfish/jersey/tests/externalproperties/HttpProxyTest.java +++ b/tests/integration/externalproperties/src/test/java/org/glassfish/jersey/tests/externalproperties/HttpProxyTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020, 2022 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at diff --git a/tests/integration/jersey-4003/src/test/java/org/glassfish/jersey/tests/integration/jersey4003/LostResponseTest.java b/tests/integration/jersey-4003/src/test/java/org/glassfish/jersey/tests/integration/jersey4003/LostResponseTest.java index 1780d05a8d1..2ba8042961c 100644 --- a/tests/integration/jersey-4003/src/test/java/org/glassfish/jersey/tests/integration/jersey4003/LostResponseTest.java +++ b/tests/integration/jersey-4003/src/test/java/org/glassfish/jersey/tests/integration/jersey4003/LostResponseTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2022 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2020 Oracle and/or its affiliates. All rights reserved. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0, which is available at @@ -16,11 +16,28 @@ package org.glassfish.jersey.tests.integration.jersey4003; +import org.glassfish.jersey.client.ClientConfig; +import org.glassfish.jersey.client.HttpUrlConnectorProvider; +import org.glassfish.jersey.client.JerseyClientBuilder; +import org.glassfish.jersey.client.JerseyCompletionStageRxInvoker; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import org.mockito.Mockito; + +import javax.ws.rs.client.Client; +import javax.ws.rs.client.Entity; +import javax.ws.rs.client.InvocationCallback; +import javax.ws.rs.client.ResponseProcessingException; +import javax.ws.rs.core.GenericType; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.OutputStream; import java.net.HttpURLConnection; -import java.net.Proxy; import java.net.URL; import java.util.concurrent.CompletionStage; import java.util.concurrent.CountDownLatch; @@ -29,23 +46,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; -import javax.ws.rs.client.Client; -import javax.ws.rs.client.Entity; -import javax.ws.rs.client.InvocationCallback; -import javax.ws.rs.client.ResponseProcessingException; -import javax.ws.rs.core.GenericType; -import javax.ws.rs.core.MediaType; -import javax.ws.rs.core.Response; - -import org.glassfish.jersey.client.ClientConfig; -import org.glassfish.jersey.client.HttpUrlConnectorProvider; -import org.glassfish.jersey.client.JerseyClientBuilder; -import org.glassfish.jersey.client.JerseyCompletionStageRxInvoker; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.mockito.Mockito; - public class LostResponseTest { private static final String DUMMY_URL = "http://foo"; @@ -59,7 +59,7 @@ public void setup() throws IOException { HttpUrlConnectorProvider.ConnectionFactory connectionFactory = Mockito.mock(HttpUrlConnectorProvider.ConnectionFactory.class); HttpURLConnection connection = Mockito.mock(HttpURLConnection.class); - Mockito.when(connectionFactory.getConnection(Mockito.any(URL.class), Mockito.any())).thenReturn(connection); + Mockito.when(connectionFactory.getConnection(Mockito.any(URL.class))).thenReturn(connection); OutputStream outputStream = Mockito.mock(OutputStream.class); Mockito.when(connection.getOutputStream()).thenReturn(outputStream);