Merge pull request #13231 from jetty/jetty-12.1.x-13213-wsClientAPI

Issue #13213 - review of WebSocketClient.connect API

Author: lachlan-roberts

documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/client/websocket/WebSocketClientDocs.java

@@ -151,7 +151,7 @@ public void customHTTPRequest() throws Exception
         URI serverURI = URI.create("ws://domain.com/path");
 
         // Create a custom HTTP request.
-        ClientUpgradeRequest customRequest = new ClientUpgradeRequest();
+        ClientUpgradeRequest customRequest = new ClientUpgradeRequest(serverURI);
         // Specify a cookie.
         customRequest.getCookies().add(new HttpCookie("name", "value"));
         // Specify a custom header.
@@ -160,7 +160,7 @@ public void customHTTPRequest() throws Exception
         customRequest.setSubProtocols("chat");
 
         // Connect the client EndPoint to the server with a custom HTTP request.
-        CompletableFuture<Session> clientSessionPromise = webSocketClient.connect(clientEndPoint, serverURI, customRequest);
+        CompletableFuture<Session> clientSessionPromise = webSocketClient.connect(clientEndPoint, customRequest);
         // end::customHTTPRequest[]
     }
 
@@ -184,7 +184,7 @@ public void onHandshakeResponse(Request request, Response response)
         };
 
         // Connect the client EndPoint to the server with a custom HTTP request.
-        CompletableFuture<Session> clientSessionPromise = webSocketClient.connect(clientEndPoint, serverURI, null, listener);
+        CompletableFuture<Session> clientSessionPromise = webSocketClient.connect(clientEndPoint, serverURI, listener);
         // end::inspectHTTPResponse[]
     }
 

jetty-core/jetty-websocket/jetty-websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java

@@ -43,16 +43,16 @@ public final class ClientUpgradeRequest implements UpgradeRequest
     private long timeout;
     private String httpVersion;
 
+    /**
+     * @deprecated use {@link #ClientUpgradeRequest(URI)} instead.
+     */
+    @Deprecated(forRemoval = true, since = "12.1.0")
     public ClientUpgradeRequest()
     {
         /* anonymous, no requestURI, upgrade request */
         this.requestURI = null;
     }
 
-    /**
-     * @deprecated use {@link #ClientUpgradeRequest()} instead.
-     */
-    @Deprecated
     public ClientUpgradeRequest(URI uri)
     {
         this.requestURI = uri;
@@ -84,22 +84,14 @@ public String getHeader(String name)
     public int getHeaderInt(String name)
     {
         List<String> values = headers.get(name);
-        // no value list
         if (values == null)
-        {
             return -1;
-        }
+
         int size = values.size();
-        // empty value list
-        if (size <= 0)
-        {
+        if (size == 0)
             return -1;
-        }
-        // simple return
         if (size == 1)
-        {
             return Integer.parseInt(values.get(0));
-        }
         throw new NumberFormatException("Cannot convert multi-value header into int");
     }
 

jetty-core/jetty-websocket/jetty-websocket-jetty-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java

@@ -19,6 +19,7 @@
 import java.time.Duration;
 import java.util.Collection;
 import java.util.List;
+import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.Executor;
@@ -84,9 +85,56 @@ public WebSocketClient(HttpClient httpClient)
         installBean(sessionTracker);
     }
 
+    /**
+     * Connect to remote websocket endpoint
+     *
+     * @param websocket the websocket object
+     * @param toUri the websocket uri to connect to
+     * @return the future for the session, available on success of connect
+     * @throws IOException if unable to connect
+     */
     public CompletableFuture<Session> connect(Object websocket, URI toUri) throws IOException
     {
-        return connect(websocket, toUri, null);
+        return connect(websocket, toUri, (JettyUpgradeListener)null);
+    }
+
+    /**
+     * Connect to remote websocket endpoint
+     *
+     * @param websocket the websocket object
+     * @param toUri the websocket uri to connect to
+     * @return the future for the session, available on success of connect
+     * @throws IOException if unable to connect
+     */
+    public CompletableFuture<Session> connect(Object websocket, URI toUri, JettyUpgradeListener upgradeListener) throws IOException
+    {
+        return connect(websocket, toUri, null, upgradeListener);
+    }
+
+    /**
+     * Connect to remote websocket endpoint
+     *
+     * @param websocket the websocket object
+     * @param request the upgrade request information
+     * @return the future for the session, available on success of connect
+     * @throws IOException if unable to connect
+     */
+    public CompletableFuture<Session> connect(Object websocket, ClientUpgradeRequest request) throws IOException
+    {
+        return connect(websocket, request, null);
+    }
+
+    /**
+     * Connect to remote websocket endpoint
+     *
+     * @param websocket the websocket object
+     * @param request the upgrade request information
+     * @return the future for the session, available on success of connect
+     * @throws IOException if unable to connect
+     */
+    public CompletableFuture<Session> connect(Object websocket, ClientUpgradeRequest request, JettyUpgradeListener upgradeListener) throws IOException
+    {
+        return connect(websocket, Objects.requireNonNull(request).getRequestURI(), request, upgradeListener);
     }
 
     /**
@@ -98,6 +146,7 @@ public CompletableFuture<Session> connect(Object websocket, URI toUri) throws IO
      * @return the future for the session, available on success of connect
      * @throws IOException if unable to connect
      */
+    @Deprecated(forRemoval = true, since = "12.1.0")
     public CompletableFuture<Session> connect(Object websocket, URI toUri, ClientUpgradeRequest request) throws IOException
     {
         return connect(websocket, toUri, request, null);
@@ -112,7 +161,9 @@ public CompletableFuture<Session> connect(Object websocket, URI toUri, ClientUpg
      * @param upgradeListener the upgrade listener
      * @return the future for the session, available on success of connect
      * @throws IOException if unable to connect
+     * @deprecated use {@link #connect(Object, ClientUpgradeRequest, JettyUpgradeListener)} or {@link #connect(Object, URI, JettyUpgradeListener)}.
      */
+    @Deprecated(forRemoval = true, since = "12.1.0")
     public CompletableFuture<Session> connect(Object websocket, URI toUri, ClientUpgradeRequest request, JettyUpgradeListener upgradeListener) throws IOException
     {
         for (Connection.Listener listener : getBeans(Connection.Listener.class))

jetty-core/jetty-websocket/jetty-websocket-jetty-client/src/test/java/examples/ClientDemo.java

@@ -271,11 +271,11 @@ private void disconnect()
     private void open() throws Exception
     {
         client.setIdleTimeout(Duration.ofMillis(timeout));
-        ClientUpgradeRequest request = new ClientUpgradeRequest();
+        URI wsUri = new URI("ws://" + host + ":" + port + "/");
+        ClientUpgradeRequest request = new ClientUpgradeRequest(wsUri);
         request.setSubProtocols(protocol);
         socket = new TestSocket();
-        URI wsUri = new URI("ws://" + host + ":" + port + "/");
-        client.connect(socket, wsUri, request).get(10, TimeUnit.SECONDS);
+        client.connect(socket, request).get(10, TimeUnit.SECONDS);
     }
 
     private void send(byte op, byte[] data, int fragment)

jetty-core/jetty-websocket/jetty-websocket-jetty-client/src/test/java/examples/SimpleEchoClient.java

@@ -39,8 +39,8 @@ public static void main(String[] args)
             client.start();
 
             URI echoUri = new URI(destUri);
-            ClientUpgradeRequest request = new ClientUpgradeRequest();
-            client.connect(socket, echoUri, request);
+            ClientUpgradeRequest request = new ClientUpgradeRequest(echoUri);
+            client.connect(socket, request);
             System.out.printf("Connecting to : %s%n", echoUri);
 
             // wait for closed socket connection.

jetty-core/jetty-websocket/jetty-websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/ConnectionHeaderTest.java

@@ -84,7 +84,7 @@ public void onHandshakeRequest(Request request)
         };
 
         EventSocket clientEndpoint = new EventSocket();
-        try (Session session = client.connect(clientEndpoint, uri, null, upgradeListener).get(5, TimeUnit.SECONDS))
+        try (Session session = client.connect(clientEndpoint, uri, upgradeListener).get(5, TimeUnit.SECONDS))
         {
             // Generate text frame
             String msg = "this is an echo ... cho ... ho ... o";

jetty-core/jetty-websocket/jetty-websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketExtensionConfigTest.java

@@ -95,7 +95,7 @@ public void testJettyExtensionConfig() throws Exception
         URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/filterPath");
         EventSocket socket = new EventSocket();
 
-        ClientUpgradeRequest request = new ClientUpgradeRequest();
+        ClientUpgradeRequest request = new ClientUpgradeRequest(uri);
         request.addExtensions(ExtensionConfig.parse("permessage-deflate"));
 
         CountDownLatch correctResponseExtensions = new CountDownLatch(1);
@@ -113,7 +113,7 @@ public void onHandshakeResponse(Request request, Response response)
             }
         };
 
-        CompletableFuture<Session> connect = client.connect(socket, uri, request, listener);
+        CompletableFuture<Session> connect = client.connect(socket, request, listener);
         try (Session session = connect.get(5, TimeUnit.SECONDS))
         {
             session.sendText("hello world", Callback.NOOP);

jetty-core/jetty-websocket/jetty-websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/JettyWebSocketNegotiationTest.java

@@ -77,10 +77,10 @@ public void testBadRequest() throws Exception
         URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/filterPath");
         EventSocket socket = new EventSocket();
 
-        ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest();
+        ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest(uri);
         upgradeRequest.addExtensions("permessage-deflate;invalidParameter");
 
-        CompletableFuture<Session> connect = client.connect(socket, uri, upgradeRequest);
+        CompletableFuture<Session> connect = client.connect(socket, upgradeRequest);
         Throwable t = assertThrows(ExecutionException.class, () -> connect.get(5, TimeUnit.SECONDS));
         assertThat(t.getMessage(), containsString("Failed to upgrade to websocket:"));
         assertThat(t.getMessage(), containsString("400 Bad Request"));
@@ -126,7 +126,7 @@ public void testManualNegotiationInCreator() throws Exception
         URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/filterPath");
         EventSocket socket = new EventSocket();
         AtomicReference<Response> responseReference = new AtomicReference<>();
-        ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest();
+        ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest(uri);
         upgradeRequest.addExtensions("permessage-deflate;client_no_context_takeover");
         JettyUpgradeListener upgradeListener = new JettyUpgradeListener()
         {
@@ -137,7 +137,7 @@ public void onHandshakeResponse(Request request, Response response)
             }
         };
 
-        client.connect(socket, uri, upgradeRequest, upgradeListener).get(5, TimeUnit.SECONDS);
+        client.connect(socket, upgradeRequest, upgradeListener).get(5, TimeUnit.SECONDS);
         Response response = responseReference.get();
         String extensions = response.getHeaders().get("Sec-WebSocket-Extensions");
         assertThat(extensions, is("permessage-deflate"));

jetty-core/jetty-websocket/jetty-websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/LargeDeflateTest.java

@@ -75,11 +75,11 @@ public void after() throws Exception
     @Test
     public void testDeflate() throws Exception
     {
-        ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest();
+        ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest(URI.create("ws://localhost:" + _connector.getLocalPort() + "/ws"));
         upgradeRequest.addExtensions("permessage-deflate");
 
         EventSocket clientSocket = new EventSocket();
-        Session session = _client.connect(clientSocket, URI.create("ws://localhost:" + _connector.getLocalPort() + "/ws"), upgradeRequest).get();
+        Session session = _client.connect(clientSocket, upgradeRequest).get();
         ByteBuffer sentMessage = largePayloads();
         session.sendBinary(sentMessage, Callback.NOOP);
         session.close(StatusCode.NORMAL, "close from test", Callback.NOOP);

jetty-core/jetty-websocket/jetty-websocket-jetty-tests/src/test/java/org/eclipse/jetty/websocket/tests/MaxOutgoingFramesTest.java

@@ -137,9 +137,9 @@ public void testMaxOutgoingFrames() throws Exception
 
         URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/");
         EventSocket socket = new EventSocket();
-        ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest();
+        ClientUpgradeRequest upgradeRequest = new ClientUpgradeRequest(uri);
         upgradeRequest.addExtensions(BlockingOutgoingExtension.class.getName());
-        client.connect(socket, uri, upgradeRequest).get(5, TimeUnit.SECONDS);
+        client.connect(socket, upgradeRequest).get(5, TimeUnit.SECONDS);
         assertTrue(socket.openLatch.await(5, TimeUnit.SECONDS));
 
         int numFrames = 30;