Issue #3730 - Further updates from PR Review

Signed-off-by: Joakim Erdfelt <[email protected]>

jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/ClientContainer.java

@@ -57,7 +57,6 @@
 import org.eclipse.jetty.websocket.common.WebSocketSession;
 import org.eclipse.jetty.websocket.common.WebSocketSessionListener;
 import org.eclipse.jetty.websocket.common.scopes.DelegatedContainerScope;
-import org.eclipse.jetty.websocket.common.scopes.SimpleContainerScope;
 import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope;
 import org.eclipse.jetty.websocket.jsr356.annotations.AnnotatedEndpointScanner;
 import org.eclipse.jetty.websocket.jsr356.client.AnnotatedClientEndpointMetadata;
@@ -109,9 +108,7 @@ public class ClientContainer extends ContainerLifeCycle implements WebSocketCont
     public ClientContainer()
     {
         // This constructor is used with Standalone JSR Client usage.
-        this(new SimpleContainerScope(WebSocketPolicy.newClientPolicy()));
-        client.setDaemon(true);
-        client.addManaged(client.getHttpClient());
+        this(new WebSocketClient());
     }
 
     /**
@@ -134,7 +131,6 @@ public ClientContainer(final HttpClient httpClient)
     public ClientContainer(final WebSocketContainerScope scope)
     {
         this(scope, null);
-        client.addManaged(client.getHttpClient());
     }
 
     /**

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

@@ -90,8 +90,6 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketCont
     public WebSocketClient()
     {
         this(HttpClientProvider.get(null), null);
-        // Add as bean, as HttpClient was created in this constructor
-        addBean(this.httpClient);
     }
 
     /**
@@ -101,8 +99,7 @@ public WebSocketClient()
      */
     public WebSocketClient(HttpClient httpClient)
     {
-        // Use external HttpClient
-        this(Objects.requireNonNull(httpClient), null);
+        this(httpClient, null);
     }
 
     /**
@@ -113,8 +110,9 @@ public WebSocketClient(HttpClient httpClient)
      */
     public WebSocketClient(HttpClient httpClient, DecoratedObjectFactory decoratedObjectFactory)
     {
-        this.httpClient = httpClient;
+        this.httpClient = Objects.requireNonNull(httpClient, "HttpClient");
 
+        addBean(httpClient);
         addBean(sessionTracker);
         addSessionListener(sessionTracker);
 
@@ -141,8 +139,6 @@ public WebSocketClient(HttpClient httpClient, DecoratedObjectFactory decoratedOb
     public WebSocketClient(SslContextFactory sslContextFactory)
     {
         this(newHttpClient(sslContextFactory, null, null), null);
-        // Add as bean, as HttpClient was created in this constructor
-        addBean(this.httpClient);
     }
 
     /**
@@ -155,8 +151,6 @@ public WebSocketClient(SslContextFactory sslContextFactory)
     public WebSocketClient(Executor executor)
     {
         this(newHttpClient(null, executor, null), null);
-        // Add as bean, as HttpClient was created in this constructor
-        addBean(this.httpClient);
     }
 
     /**
@@ -169,8 +163,6 @@ public WebSocketClient(Executor executor)
     public WebSocketClient(ByteBufferPool bufferPool)
     {
         this(newHttpClient(null, null, bufferPool), null);
-        // Add as bean, as HttpClient was created in this constructor
-        addBean(this.httpClient);
     }
 
     /**
@@ -184,21 +176,19 @@ public WebSocketClient(ByteBufferPool bufferPool)
     public WebSocketClient(SslContextFactory sslContextFactory, Executor executor)
     {
         this(newHttpClient(sslContextFactory, executor, null), null);
-        // Add as bean, as HttpClient was created in this constructor
-        addBean(this.httpClient);
     }
 
     /**
      * Create WebSocketClient other Container Scope, to allow sharing of
      * internal features like Executor, ByteBufferPool, SSLContextFactory, etc.
      *
      * @param scope the Container Scope
+     * @deprecated use {@link #WebSocketClient(HttpClient)} instead
      */
+    @Deprecated
     public WebSocketClient(WebSocketContainerScope scope)
     {
         this(newHttpClient(scope.getSslContextFactory(), scope.getExecutor(), scope.getBufferPool()), scope.getObjectFactory());
-        // Add as bean, as HttpClient was created in this constructor
-        addBean(this.httpClient);
     }
 
     /**
@@ -224,8 +214,6 @@ public WebSocketClient(WebSocketContainerScope scope, SslContextFactory sslConte
          * SslContextFactory for the underlying HttpClient instance.
          */
         this(newHttpClient(sslContextFactory, scope.getExecutor(), scope.getBufferPool()), scope.getObjectFactory());
-        // Add as bean, as HttpClient was created in this constructor
-        addBean(this.httpClient);
     }
 
     /**
@@ -241,8 +229,6 @@ public WebSocketClient(WebSocketContainerScope scope, SslContextFactory sslConte
     public WebSocketClient(SslContextFactory sslContextFactory, Executor executor, ByteBufferPool bufferPool)
     {
         this(newHttpClient(sslContextFactory, executor, bufferPool), null);
-        // Add as bean, as HttpClient was created in this class
-        addBean(this.httpClient);
     }
 
     /**
@@ -261,8 +247,6 @@ public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory e
          * It's left in for backwards compat reasons.
          */
         this(scope, eventDriverFactory, sessionFactory, HttpClientProvider.get(scope));
-        // Add as bean, as HttpClient was created in this constructor
-        addBean(this.httpClient);
     }
 
     /**
@@ -276,24 +260,17 @@ public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory e
      */
     public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory eventDriverFactory, SessionFactory sessionFactory, HttpClient httpClient)
     {
-        if (httpClient == null)
-        {
-            this.httpClient = HttpClientProvider.get(scope);
-            // Add as bean, as HttpClient was created in this constructor
-            addBean(this.httpClient);
-        }
-        else
-        {
-            this.httpClient = httpClient;
-        }
+        this.httpClient = httpClient == null ? HttpClientProvider.get(scope) : httpClient;
+        addBean(this.httpClient);
 
         addBean(sessionTracker);
         addSessionListener(sessionTracker);
 
         // Ensure we get a Client version of the policy.
         this.policy = scope.getPolicy().delegateAs(WebSocketBehavior.CLIENT);
+
         // Support Late Binding of DecoratedObjectFactory (that CDI establishes in its own servlet context listeners)
-        this.objectFactorySupplier = () -> scope.getObjectFactory();
+        this.objectFactorySupplier = scope::getObjectFactory;
 
         this.extensionRegistry = new WebSocketExtensionFactory(this);
         addBean(extensionRegistry);
@@ -302,7 +279,7 @@ public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory e
         this.sessionFactory = sessionFactory == null ? new WebSocketSessionFactory(this) : sessionFactory;
     }
 
-    public static HttpClient newHttpClient(SslContextFactory sslContextFactory, Executor executor, ByteBufferPool bufferPool)
+    private static HttpClient newHttpClient(SslContextFactory sslContextFactory, Executor executor, ByteBufferPool bufferPool)
     {
         HttpClient httpClient = new HttpClient(sslContextFactory);
         httpClient.setExecutor(executor);