Issue #5832 - changes from review

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

jetty-websocket/websocket-javax-client/src/main/java/module-info.java

@@ -20,6 +20,7 @@
     exports org.eclipse.jetty.websocket.javax.client;
     exports org.eclipse.jetty.websocket.javax.client.internal to org.eclipse.jetty.websocket.javax.server;
 
+    requires org.slf4j;
     requires org.eclipse.jetty.client;
     requires org.eclipse.jetty.websocket.core.client;
     requires org.eclipse.jetty.websocket.javax.common;

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

@@ -58,14 +58,7 @@ protected WebSocketContainer getContainer()
         // TODO: do we want to provide a non-standard way to configure to always return the same clientContainer based on a config somewhere? (system.property?)
 
         JavaxWebSocketClientContainer clientContainer = new JavaxWebSocketClientContainer();
-        try
-        {
-            clientContainer.start();
-        }
-        catch (Exception e)
-        {
-            throw new RuntimeException("Unable to start Client Container", e);
-        }
+        LifeCycle.start(clientContainer);
         return clientContainer;
     }
 }

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

@@ -46,6 +46,8 @@
 import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketExtensionConfig;
 import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandler;
 import org.eclipse.jetty.websocket.javax.common.JavaxWebSocketFrameHandlerFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Container for Client use of the javax.websocket API.
@@ -55,6 +57,8 @@
 @ManagedObject("JSR356 Client Container")
 public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer implements javax.websocket.WebSocketContainer
 {
+    private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketClientContainer.class);
+
     protected WebSocketCoreClient coreClient;
     protected Function<WebSocketComponents, WebSocketCoreClient> coreClientFactory;
     private final JavaxWebSocketClientFrameHandlerFactory frameHandlerFactory;
@@ -295,12 +299,25 @@ protected void doClientStart()
                 .getMethod("getContextHandler")
                 .invoke(context);
 
+            contextHandler.getClass()
+                .getMethod("addManaged", LifeCycle.class)
+                .invoke(contextHandler, this);
+
             AbstractLifeCycleListener shutdownListener = new AbstractLifeCycleListener()
             {
                 @Override
                 public void lifeCycleStopping(LifeCycle event)
                 {
-                    LifeCycle.stop(JavaxWebSocketClientContainer.this);
+                    try
+                    {
+                        contextHandler.getClass()
+                            .getMethod("removeBean", Object.class)
+                            .invoke(contextHandler, JavaxWebSocketClientContainer.this);
+                    }
+                    catch (Throwable t)
+                    {
+                        LOG.warn("could not remove client WebSocketContainer bean from {}", contextHandler);
+                    }
                 }
             };
 

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

@@ -15,6 +15,7 @@
 
 import java.io.IOException;
 import java.net.URI;
+import java.util.Collection;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServlet;
 import javax.servlet.http.HttpServletRequest;
@@ -43,11 +44,12 @@
 public class ClientInWebappTest
 {
     private Server server;
+    private ServletContextHandler contextHandler;
     private URI serverUri;
     private HttpClient httpClient;
     private volatile WebSocketContainer container;
 
-    public class ServerSocket extends HttpServlet
+    public class WebSocketClientInServlet extends HttpServlet
     {
         @Override
         protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
@@ -64,9 +66,9 @@ public void before() throws Exception
         connector.setPort(8080);
         server.addConnector(connector);
 
-        ServletContextHandler contextHandler = new ServletContextHandler();
+        contextHandler = new ServletContextHandler();
         contextHandler.setContextPath("/");
-        contextHandler.addServlet(new ServletHolder(new ServerSocket()), "/");
+        contextHandler.addServlet(new ServletHolder(new WebSocketClientInServlet()), "/");
         server.setHandler(contextHandler);
         server.start();
         serverUri = WSURI.toWebsocket(server.getURI());
@@ -93,6 +95,11 @@ public void testWebSocketClientContainerInWebapp() throws Exception
         JavaxWebSocketClientContainer clientContainer = (JavaxWebSocketClientContainer)container;
         assertThat(clientContainer.isRunning(), is(true));
 
+        // The container should be a bean on the ContextHandler.
+        Collection<WebSocketContainer> containedBeans = contextHandler.getBeans(WebSocketContainer.class);
+        assertThat(containedBeans.size(), is(1));
+        assertThat(containedBeans.toArray()[0], is(container));
+
         // The client should be attached to the servers LifeCycle and should stop with it.
         server.stop();
         assertThat(clientContainer.isRunning(), is(false));