Review comments

Signed-off-by: Jorge Bescos Gascon <[email protected]>
eclipse-ee4j · Jun 24, 2022 · eda9489 · eda9489
1 parent f738329
commit eda9489
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 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
@@ -255,6 +255,20 @@ protected Connector createHttpUrlConnector(Client client, ConnectionFactory conn
      */
     public interface ConnectionFactory {
 
+        /**
+         * Get a {@link java.net.HttpURLConnection} for a given URL.
+         * <p>
+         * Implementation of the method MUST be thread-safe and MUST ensure that
+         * a dedicated {@link java.net.HttpURLConnection} instance is returned for concurrent
+         * requests.
+         * </p>
+         *
+         * @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.
          * <p>
@@ -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
@@ -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/...est/java/org/glassfish/jersey/tests/e2e/client/connector/ssl/SslHttpUrlConnectorTest.java b/...est/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/...properties/src/test/java/org/glassfish/jersey/tests/externalproperties/HttpProxyTest.java b/...properties/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/...003/src/test/java/org/glassfish/jersey/tests/integration/jersey4003/LostResponseTest.java b/...003/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);