Issue #3730 - Cleaning up Scopes in WebSocketClient

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

@@ -52,6 +52,7 @@
 import org.eclipse.jetty.websocket.api.WebSocketPolicy;
 import org.eclipse.jetty.websocket.api.extensions.ExtensionFactory;
 import org.eclipse.jetty.websocket.client.ClientUpgradeRequest;
+import org.eclipse.jetty.websocket.client.HttpContainerScope;
 import org.eclipse.jetty.websocket.client.WebSocketClient;
 import org.eclipse.jetty.websocket.client.io.UpgradeListener;
 import org.eclipse.jetty.websocket.common.WebSocketSession;
@@ -123,7 +124,7 @@ public ClientContainer()
      */
     public ClientContainer(final HttpClient httpClient)
     {
-        this(new SimpleContainerScope(WebSocketPolicy.newClientPolicy()), httpClient);
+        this(new HttpContainerScope(httpClient));
     }
 
     /**

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

@@ -0,0 +1,119 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.websocket.client;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.Objects;
+import java.util.concurrent.Executor;
+
+import org.eclipse.jetty.client.HttpClient;
+import org.eclipse.jetty.io.ByteBufferPool;
+import org.eclipse.jetty.util.DecoratedObjectFactory;
+import org.eclipse.jetty.util.component.ContainerLifeCycle;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+import org.eclipse.jetty.websocket.api.WebSocketPolicy;
+import org.eclipse.jetty.websocket.common.WebSocketSessionListener;
+import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope;
+
+/**
+ * A simple Scope that is focused around a HttpClient, DecoratedObjectFactory, and Client WebSocketPolicy.
+ */
+public class HttpContainerScope extends ContainerLifeCycle implements WebSocketContainerScope
+{
+    private final HttpClient httpClient;
+    private final DecoratedObjectFactory decoratedObjectFactory;
+    private final WebSocketPolicy webSocketPolicy;
+    private List<WebSocketSessionListener> sessionListeners = new ArrayList<>();
+
+    public HttpContainerScope(HttpClient httpClient)
+    {
+        this(httpClient, new DecoratedObjectFactory());
+    }
+
+    public HttpContainerScope(HttpClient httpClient, DecoratedObjectFactory decoratedObjectFactory)
+    {
+        this.httpClient = Objects.requireNonNull(httpClient, "HttpClient");
+        this.decoratedObjectFactory = decoratedObjectFactory != null ? decoratedObjectFactory : new DecoratedObjectFactory();
+        this.webSocketPolicy = WebSocketPolicy.newClientPolicy();
+    }
+
+    public HttpContainerScope(SslContextFactory sslContextFactory, Executor executor, ByteBufferPool bufferPool, DecoratedObjectFactory decoratedObjectFactory)
+    {
+        this.httpClient = new HttpClient(sslContextFactory);
+        this.httpClient.setExecutor(executor);
+        this.httpClient.setByteBufferPool(bufferPool);
+        this.decoratedObjectFactory = decoratedObjectFactory != null ? decoratedObjectFactory : new DecoratedObjectFactory();
+        this.webSocketPolicy = WebSocketPolicy.newClientPolicy();
+    }
+
+    public HttpClient getHttpClient()
+    {
+        return httpClient;
+    }
+
+    @Override
+    public ByteBufferPool getBufferPool()
+    {
+        return httpClient.getByteBufferPool();
+    }
+
+    @Override
+    public Executor getExecutor()
+    {
+        return httpClient.getExecutor();
+    }
+
+    @Override
+    public DecoratedObjectFactory getObjectFactory()
+    {
+        return decoratedObjectFactory;
+    }
+
+    @Override
+    public WebSocketPolicy getPolicy()
+    {
+        return webSocketPolicy;
+    }
+
+    @Override
+    public SslContextFactory getSslContextFactory()
+    {
+        return httpClient.getSslContextFactory();
+    }
+
+    @Override
+    public void addSessionListener(WebSocketSessionListener listener)
+    {
+        this.sessionListeners.add(listener);
+    }
+
+    @Override
+    public void removeSessionListener(WebSocketSessionListener listener)
+    {
+        this.sessionListeners.remove(listener);
+    }
+
+    @Override
+    public Collection<WebSocketSessionListener> getSessionListeners()
+    {
+        return sessionListeners;
+    }
+}

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

@@ -58,7 +58,6 @@
 import org.eclipse.jetty.websocket.common.WebSocketSessionListener;
 import org.eclipse.jetty.websocket.common.events.EventDriverFactory;
 import org.eclipse.jetty.websocket.common.extensions.WebSocketExtensionFactory;
-import org.eclipse.jetty.websocket.common.scopes.SimpleContainerScope;
 import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope;
 
 /**
@@ -86,21 +85,24 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketCont
     private boolean stopAtShutdown = true;
 
     /**
-     * Instantiate a WebSocketClient with defaults
+     * Instantiate a WebSocketClient with defaults.
      */
     public WebSocketClient()
     {
-        this((HttpClient)null);
+        this(new HttpContainerScope(HttpClientProvider.get(null)));
+        // Add as bean, as HttpClient was created in this class
+        addBean(this.httpClient);
     }
 
     /**
-     * Instantiate a WebSocketClient using HttpClient for defaults
+     * Instantiate a WebSocketClient using provided HttpClient.
      *
-     * @param httpClient the HttpClient to base internal defaults off of
+     * @param httpClient the HttpClient to use for WebSocketClient.
      */
     public WebSocketClient(HttpClient httpClient)
     {
-        this(httpClient, null);
+        // Use external HttpClient
+        this(new HttpContainerScope(Objects.requireNonNull(httpClient)));
     }
 
     /**
@@ -111,48 +113,65 @@ public WebSocketClient(HttpClient httpClient)
      */
     public WebSocketClient(HttpClient httpClient, DecoratedObjectFactory objectFactory)
     {
-        this(new SimpleContainerScope(new WebSocketPolicy(WebSocketBehavior.CLIENT), null, null, null, objectFactory), null, null, httpClient);
+        // Use external HttpClient
+        this(new HttpContainerScope(Objects.requireNonNull(httpClient), objectFactory));
     }
 
     /**
      * Create a new WebSocketClient
      *
      * @param sslContextFactory ssl context factory to use on the internal {@link HttpClient}
+     * @deprecated use {@link #WebSocketClient(HttpClient)} instead
      */
+    @Deprecated
     public WebSocketClient(SslContextFactory sslContextFactory)
     {
-        this(sslContextFactory, null, null);
+        this(new HttpContainerScope(sslContextFactory, null, null, null));
+        // Add as bean, as HttpClient was created in this class
+        addBean(this.httpClient);
     }
 
     /**
      * Create a new WebSocketClient
      *
      * @param executor the executor to use on the internal {@link HttpClient}
+     * @deprecated use {@link #WebSocketClient(HttpClient)} instead
      */
+    @Deprecated
     public WebSocketClient(Executor executor)
     {
-        this(null, executor, null);
+        this(new HttpContainerScope(null, executor, null, null));
+        // Add as bean, as HttpClient was created in this class
+        addBean(this.httpClient);
     }
 
     /**
      * Create a new WebSocketClient
      *
      * @param bufferPool byte buffer pool to use on the internal {@link HttpClient}
+     * @deprecated use {@link #WebSocketClient(HttpClient)} instead
      */
+    @Deprecated
     public WebSocketClient(ByteBufferPool bufferPool)
     {
-        this(null, null, bufferPool);
+        this(new HttpContainerScope(null, null, bufferPool, null));
+        // Add as bean, as HttpClient was created in this class
+        addBean(this.httpClient);
     }
 
     /**
      * Create a new WebSocketClient
      *
      * @param sslContextFactory ssl context factory to use on the internal {@link HttpClient}
      * @param executor the executor to use on the internal {@link HttpClient}
+     * @deprecated use {@link #WebSocketClient(HttpClient)} instead
      */
+    @Deprecated
     public WebSocketClient(SslContextFactory sslContextFactory, Executor executor)
     {
-        this(sslContextFactory, executor, null);
+        this(new HttpContainerScope(sslContextFactory, executor, null, null));
+        // Add as bean, as HttpClient was created in this class
+        addBean(this.httpClient);
     }
 
     /**
@@ -173,10 +192,14 @@ public WebSocketClient(WebSocketContainerScope scope)
      * @param scope the Container Scope
      * @param sslContextFactory SSL ContextFactory to use in preference to one from
      * {@link WebSocketContainerScope#getSslContextFactory()}
+     * @deprecated use {@link #WebSocketClient(HttpClient)} instead
      */
+    @Deprecated
     public WebSocketClient(WebSocketContainerScope scope, SslContextFactory sslContextFactory)
     {
-        this(sslContextFactory, scope.getExecutor(), scope.getBufferPool(), scope.getObjectFactory());
+        this(new HttpContainerScope(sslContextFactory, scope.getExecutor(), scope.getBufferPool(), scope.getObjectFactory()));
+        // Add as bean, as HttpClient was created in this class
+        addBean(this.httpClient);
     }
 
     /**
@@ -186,24 +209,13 @@ public WebSocketClient(WebSocketContainerScope scope, SslContextFactory sslConte
      * @param sslContextFactory shared SSL ContextFactory
      * @param executor shared Executor
      * @param bufferPool shared ByteBufferPool
+     * @deprecated use {@link #WebSocketClient(HttpClient)} instead
      */
+    @Deprecated
     public WebSocketClient(SslContextFactory sslContextFactory, Executor executor, ByteBufferPool bufferPool)
     {
-        this(sslContextFactory, executor, bufferPool, null);
-    }
-
-    /**
-     * Create WebSocketClient using sharing instances of SSLContextFactory
-     * Executor, and ByteBufferPool
-     *
-     * @param sslContextFactory shared SSL ContextFactory
-     * @param executor shared Executor
-     * @param bufferPool shared ByteBufferPool
-     * @param objectFactory shared DecoratedObjectFactory
-     */
-    private WebSocketClient(SslContextFactory sslContextFactory, Executor executor, ByteBufferPool bufferPool, DecoratedObjectFactory objectFactory)
-    {
-        this(new SimpleContainerScope(new WebSocketPolicy(WebSocketBehavior.CLIENT), bufferPool, executor, sslContextFactory, objectFactory));
+        this(new HttpContainerScope(sslContextFactory, executor, bufferPool, null));
+        // Add as bean, as HttpClient was created in this class
         addBean(this.httpClient);
     }
 
@@ -214,10 +226,14 @@ private WebSocketClient(SslContextFactory sslContextFactory, Executor executor,
      * @param scope the Container Scope
      * @param eventDriverFactory the EventDriver Factory to use
      * @param sessionFactory the SessionFactory to use
+     * @deprecated use {@link WebSocketClient#WebSocketClient(WebSocketContainerScope, EventDriverFactory, SessionFactory, HttpClient)} instead
      */
+    @Deprecated
     public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory eventDriverFactory, SessionFactory sessionFactory)
     {
-        this(scope, eventDriverFactory, sessionFactory, null);
+        this(scope, eventDriverFactory, sessionFactory, HttpClientProvider.get(scope));
+        // Add as bean, as HttpClient was created in this class
+        addBean(this.httpClient);
     }
 
     /**
@@ -234,6 +250,7 @@ public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory e
         if (httpClient == null)
         {
             this.httpClient = HttpClientProvider.get(scope);
+            // Add as bean, as HttpClient was created in this class
             addBean(this.httpClient);
         }
         else
@@ -255,6 +272,24 @@ public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory e
         this.sessionFactory = sessionFactory == null ? new WebSocketSessionFactory(this) : sessionFactory;
     }
 
+    WebSocketClient(final HttpContainerScope httpContainerScope)
+    {
+        this.httpClient = httpContainerScope.getHttpClient();
+
+        addBean(sessionTracker);
+        addSessionListener(sessionTracker);
+
+        // Ensure we get a Client version of the policy.
+        this.policy = httpContainerScope.getPolicy();
+        // Support Late Binding of Object Factory (for CDI)
+        this.objectFactorySupplier = httpContainerScope::getObjectFactory;
+        this.extensionRegistry = new WebSocketExtensionFactory(this);
+        addBean(extensionRegistry);
+
+        this.eventDriverFactory = new EventDriverFactory(this);
+        this.sessionFactory = new WebSocketSessionFactory(this);
+    }
+
     public Future<Session> connect(Object websocket, URI toUri) throws IOException
     {
         ClientUpgradeRequest request = new ClientUpgradeRequest(toUri);