Issue #4407 - clean up usage of ClientEndpointConfig and testing

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java

@@ -41,7 +41,6 @@ import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.websocket.core.WebSocketComponents;
 import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient;
 import org.eclipse.jetty.websocket.javax.common.ConfiguredEndpoint;
-import org.eclipse.jetty.websocket.javax.common.InvalidWebSocketException;
 import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketContainer;
 import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketExtensionConfig;
 import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandler;
@@ -200,40 +199,33 @@ public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer imple
     }
 
     @Override
-    public Session connectToServer(final Class<? extends Endpoint> endpointClass, final ClientEndpointConfig config, URI path) throws IOException
+    public Session connectToServer(final Class<? extends Endpoint> endpointClass, final ClientEndpointConfig providedConfig, URI path) throws DeploymentException, IOException
     {
-        ClientEndpointConfig clientEndpointConfig = config;
+        return connectToServer(newEndpoint(endpointClass), providedConfig, path);
-        if (clientEndpointConfig == null)
-        {
-            clientEndpointConfig = new EmptyClientEndpointConfig();
-        }
-        ConfiguredEndpoint instance = newConfiguredEndpoint(endpointClass, clientEndpointConfig);
-        return connect(instance, path);
     }
 
     @Override
-    public Session connectToServer(final Class<?> annotatedEndpointClass, final URI path) throws IOException
+    public Session connectToServer(final Class<?> annotatedEndpointClass, final URI path) throws DeploymentException, IOException
     {
-        ConfiguredEndpoint instance = newConfiguredEndpoint(annotatedEndpointClass, new EmptyClientEndpointConfig());
+        return connectToServer(newEndpoint(annotatedEndpointClass), path);
-        return connect(instance, path);
     }
 
     @Override
-    public Session connectToServer(final Endpoint endpoint, final ClientEndpointConfig config, final URI path) throws DeploymentException, IOException
+    public Session connectToServer(final Endpoint endpoint, final ClientEndpointConfig providedConfig, final URI path) throws DeploymentException, IOException
     {
-        ClientEndpointConfig clientEndpointConfig = config;
+        ClientEndpointConfig config = providedConfig;
-        if (clientEndpointConfig == null)
+        if (config != null)
-        {
+            config = new EmptyClientEndpointConfig();
-            clientEndpointConfig = new EmptyClientEndpointConfig();
+
-        }
+        ConfiguredEndpoint instance = new ConfiguredEndpoint(endpoint, config);
-        ConfiguredEndpoint instance = newConfiguredEndpoint(endpoint, clientEndpointConfig);
         return connect(instance, path);
     }
 
     @Override
     public Session connectToServer(Object endpoint, URI path) throws DeploymentException, IOException
     {
-        ConfiguredEndpoint instance = newConfiguredEndpoint(endpoint, new EmptyClientEndpointConfig());
+        ClientEndpointConfig config = getAnnotatedConfig(endpoint);
+        ConfiguredEndpoint instance = new ConfiguredEndpoint(endpoint, config);
         return connect(instance, path);
     }
 
@@ -249,46 +241,24 @@ public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer imple
         return getHttpClient().getExecutor();
     }
 
-    private ConfiguredEndpoint newConfiguredEndpoint(Class<?> endpointClass, EndpointConfig config)
+    private <T> T newEndpoint(Class<T> endpointClass) throws DeploymentException
     {
         try
         {
-            return newConfiguredEndpoint(endpointClass.getConstructor().newInstance(), config);
+            return endpointClass.getConstructor().newInstance();
         }
         catch (Throwable e)
         {
-            throw new InvalidWebSocketException("Unable to instantiate websocket: " + endpointClass.getName());
+            throw new DeploymentException("Unable to instantiate websocket: " + endpointClass.getName());
         }
     }
 
-    public ConfiguredEndpoint newConfiguredEndpoint(Object endpoint, EndpointConfig providedConfig) throws DeploymentException
+    private ClientEndpointConfig getAnnotatedConfig(Object endpoint) throws DeploymentException
-    {
-        EndpointConfig config = providedConfig;
-
-        if (config == null)
-        {
-            config = newEmptyConfig(endpoint);
-        }
-
-        config = readAnnotatedConfig(endpoint, config);
-
-        return new ConfiguredEndpoint(endpoint, config);
-    }
-
-    protected EndpointConfig newEmptyConfig(Object endpoint)
-    {
-        return new EmptyClientEndpointConfig();
-    }
-
-    private EndpointConfig readAnnotatedConfig(Object endpoint, EndpointConfig config) throws DeploymentException
     {
         ClientEndpoint anno = endpoint.getClass().getAnnotation(ClientEndpoint.class);
-        if (anno != null)
+        if (anno == null)
-        {
+            throw new DeploymentException("Could not get ClientEndpoint annotation for " + endpoint.getClass().getName());
-            // Overwrite Config from Annotation
+
-            // TODO: should we merge with provided config?
+        return new AnnotatedClientEndpointConfig(anno);
-            return new AnnotatedClientEndpointConfig(anno);
-        }
-        return config;
     }
 }

jetty-websocket/javax-websocket-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/client/AnnotatedClientEndpointTest.java

@@ -18,15 +18,13 @@
 
 package org.eclipse.jetty.websocket.javax.tests.client;
 
+import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.Collections;
 import java.util.Date;
-import java.util.List;
 import javax.websocket.ClientEndpoint;
 import javax.websocket.ClientEndpointConfig;
 import javax.websocket.ContainerProvider;
-import javax.websocket.Decoder;
-import javax.websocket.Encoder;
 import javax.websocket.EndpointConfig;
 import javax.websocket.HandshakeResponse;
 import javax.websocket.OnMessage;
@@ -41,14 +39,13 @@ import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
 
+import static java.util.stream.Collectors.joining;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.notNullValue;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 
-public class AnnotatedEndpointConfigTest
+public class AnnotatedClientEndpointTest
 {
     @ClientEndpoint(
         subprotocols = {"chat", "echo-whole"},
@@ -92,9 +89,7 @@ public class AnnotatedEndpointConfigTest
     }
 
     private static CoreServer server;
-    private static ClientEndpointConfig ceconfig;
+    private static ClientEndpointConfig config;
-    private static EndpointConfig config;
-    private static Session session;
     private static AnnotatedEndpointClient clientEndpoint;
 
     @BeforeAll
@@ -106,36 +101,33 @@ public class AnnotatedEndpointConfigTest
         // Start Server
         server.start();
 
-        // Connect client
+        // Create Client
         WebSocketContainer container = ContainerProvider.getWebSocketContainer();
         server.addBean(container); // allow to shutdown with server
+
+        // Connect to Server
         clientEndpoint = new AnnotatedEndpointClient();
-
+        assertNotNull(container.connectToServer(clientEndpoint, server.getWsUri()));
-        session = container.connectToServer(clientEndpoint, server.getWsUri());
+        assertNotNull(clientEndpoint.config);
-        assertThat("Session", session, notNullValue());
+        assertThat(clientEndpoint.config, instanceOf(ClientEndpointConfig.class));
-
+        config = (ClientEndpointConfig)clientEndpoint.config;
-        config = clientEndpoint.config;
-        assertThat("EndpointConfig", config, notNullValue());
-        assertThat("EndpointConfig", config, instanceOf(ClientEndpointConfig.class));
-
-        ceconfig = (ClientEndpointConfig)config;
-        assertThat("EndpointConfig", ceconfig, notNullValue());
     }
 
     @AfterAll
     public static void stopEnv()
     {
-        // Disconnect client
+        // Close Session
         try
         {
-            session.close();
+            if (clientEndpoint.session != null)
+                clientEndpoint.session.close();
         }
-        catch (Exception e)
+        catch (IOException e)
         {
-            e.printStackTrace(System.err);
+            e.printStackTrace();
         }
 
-        // Stop server
+        // Stop Server
         try
         {
             server.stop();
@@ -149,69 +141,39 @@ public class AnnotatedEndpointConfigTest
     @Test
     public void testTextMax() throws Exception
     {
-        assertThat("Client Text Max",
+        assertThat(clientEndpoint.session.getMaxTextMessageBufferSize(), is(111222));
-            clientEndpoint.session.getMaxTextMessageBufferSize(),
-            is(111222));
     }
 
     @Test
     public void testBinaryMax() throws Exception
     {
-        assertThat("Client Binary Max",
+        assertThat(clientEndpoint.session.getMaxBinaryMessageBufferSize(), is(333444));
-            clientEndpoint.session.getMaxBinaryMessageBufferSize(),
-            is(333444));
     }
 
     @Test
     public void testSubProtocols() throws Exception
     {
-        List<String> subprotocols = ceconfig.getPreferredSubprotocols();
+        String subprotocols = String.join(", ", config.getPreferredSubprotocols());
-        assertThat("Client Preferred SubProtocols", subprotocols, contains("chat", "echo-whole"));
+        assertThat(subprotocols, is("chat, echo-whole"));
     }
 
     @Test
     public void testDecoders() throws Exception
     {
-        List<Class<? extends Decoder>> decoders = config.getDecoders();
+        String decoders = config.getDecoders().stream().map(Class::getName).collect(joining(", "));
-        assertThat("Decoders", decoders, notNullValue());
+        assertThat(decoders, is(DateDecoder.class.getName()));
-
-        Class<?> expectedClass = DateDecoder.class;
-        boolean hasExpectedDecoder = false;
-        for (Class<? extends Decoder> decoder : decoders)
-        {
-            if (expectedClass.isAssignableFrom(decoder))
-            {
-                hasExpectedDecoder = true;
-            }
-        }
-
-        assertTrue(hasExpectedDecoder, "Client Decoders has " + expectedClass.getName());
     }
 
     @Test
     public void testEncoders() throws Exception
     {
-        List<Class<? extends Encoder>> encoders = config.getEncoders();
+        String encoders = config.getEncoders().stream().map(Class::getName).collect(joining(", "));
-        assertThat("AvailableEncoders", encoders, notNullValue());
+        assertThat(encoders, is(TimeEncoder.class.getName()));
-
-        Class<?> expectedClass = TimeEncoder.class;
-        boolean hasExpectedEncoder = false;
-        for (Class<? extends Encoder> encoder : encoders)
-        {
-            if (expectedClass.isAssignableFrom(encoder))
-            {
-                hasExpectedEncoder = true;
-            }
-        }
-
-        assertTrue(hasExpectedEncoder, "Client AvailableEncoders has " + expectedClass.getName());
     }
 
     @Test
     public void testConfigurator() throws Exception
     {
-        ClientEndpointConfig ceconfig = (ClientEndpointConfig)config;
+        assertThat(config.getConfigurator(), instanceOf(AnnotatedEndpointConfigurator.class));
-
-        assertThat("Client Configurator", ceconfig.getConfigurator(), instanceOf(AnnotatedEndpointConfigurator.class));
     }
 }