diff --git a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/ClientContainer.java b/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/ClientContainer.java index 0e1bb7f1e72e..46d1f4878a60 100644 --- a/jetty-websocket/javax-websocket-client-impl/src/main/java/org/eclipse/jetty/websocket/jsr356/ClientContainer.java +++ b/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()); } /** @@ -123,7 +120,7 @@ public ClientContainer() */ public ClientContainer(final HttpClient httpClient) { - this(new SimpleContainerScope(WebSocketPolicy.newClientPolicy()), httpClient); + this(new WebSocketClient(httpClient)); } /** @@ -134,7 +131,6 @@ public ClientContainer(final HttpClient httpClient) public ClientContainer(final WebSocketContainerScope scope) { this(scope, null); - client.addManaged(client.getHttpClient()); } /** @@ -187,8 +183,12 @@ protected ClientContainer(final WebSocketContainerScope scope, final HttpClient */ public ClientContainer(WebSocketClient client) { + Objects.requireNonNull(client, "WebSocketClient"); this.scopeDelegate = client; this.client = client; + addBean(this.client); + this.client.setEventDriverFactory(new JsrEventDriverFactory(scopeDelegate)); + this.client.setSessionFactory(new JsrSessionFactory(this)); this.internalClient = false; this.endpointClientMetadataCache = new ConcurrentHashMap<>(); diff --git a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java b/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java index 2d2e8e518c01..28e9cb58f2c4 100644 --- a/jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java +++ b/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; /** @@ -77,26 +76,26 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketCont // WebSocket Specifics private final WebSocketPolicy policy; private final WebSocketExtensionFactory extensionRegistry; - private final EventDriverFactory eventDriverFactory; - private final SessionFactory sessionFactory; private final SessionTracker sessionTracker = new SessionTracker(); private final List sessionListeners = new ArrayList<>(); + private EventDriverFactory eventDriverFactory; + private SessionFactory sessionFactory; // defaults to true for backwards compatibility private boolean stopAtShutdown = true; /** - * Instantiate a WebSocketClient with defaults + * Instantiate a WebSocketClient with defaults. */ public WebSocketClient() { - this((HttpClient)null); + this(HttpClientProvider.get(null), null); } /** - * 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) { @@ -106,42 +105,64 @@ public WebSocketClient(HttpClient httpClient) /** * Instantiate a WebSocketClient using HttpClient for defaults * - * @param httpClient the HttpClient to base internal defaults off of - * @param objectFactory the DecoratedObjectFactory for all client instantiated classes + * @param httpClient the HttpClient that underlying WebSocket client uses + * @param decoratedObjectFactory the DecoratedObjectFactory for all client instantiated classes */ - public WebSocketClient(HttpClient httpClient, DecoratedObjectFactory objectFactory) + public WebSocketClient(HttpClient httpClient, DecoratedObjectFactory decoratedObjectFactory) { - this(new SimpleContainerScope(new WebSocketPolicy(WebSocketBehavior.CLIENT), null, null, null, objectFactory), null, null, httpClient); + this.httpClient = Objects.requireNonNull(httpClient, "HttpClient"); + + addBean(httpClient); + addBean(sessionTracker); + addSessionListener(sessionTracker); + + // Always a pristine Client policy + this.policy = WebSocketPolicy.newClientPolicy(); + // We do not support late binding of DecoratedObjectFactory in this WebSocketClient + DecoratedObjectFactory objectFactory = decoratedObjectFactory == null ? new DecoratedObjectFactory() : decoratedObjectFactory; + this.objectFactorySupplier = () -> objectFactory; + + this.extensionRegistry = new WebSocketExtensionFactory(this); + addBean(extensionRegistry); + + this.eventDriverFactory = new EventDriverFactory(this); + this.sessionFactory = new WebSocketSessionFactory(this); } /** * 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(newHttpClient(sslContextFactory, null, null), null); } /** * 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(newHttpClient(null, executor, null), null); } /** * 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(newHttpClient(null, null, bufferPool), null); } /** @@ -149,10 +170,12 @@ public WebSocketClient(ByteBufferPool bufferPool) * * @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(newHttpClient(sslContextFactory, executor, null), null); } /** @@ -160,10 +183,12 @@ public WebSocketClient(SslContextFactory sslContextFactory, Executor executor) * internal features like Executor, ByteBufferPool, SSLContextFactory, etc. * * @param scope the Container Scope + * @deprecated use {@link #WebSocketClient(HttpClient)} instead */ + @Deprecated public WebSocketClient(WebSocketContainerScope scope) { - this(scope, null, null, null); + this(newHttpClient(scope.getSslContextFactory(), scope.getExecutor(), scope.getBufferPool()), scope.getObjectFactory()); } /** @@ -173,10 +198,22 @@ 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 is constructor is in an awful place, it's got a signature that has a scope, + * a concept that javax.websocket ServerContainer uses to share its buffer pools / executors / etc + * with the underlying HttpClient. + * This means that the constructor should go through the HttpClientProvider.get(scope) behaviors. + * but it also has an arbitrary SslContextFactory parameter, which isn't in the scope that + * HttpClientProvider uses. + * Since this isn't used by Jetty's implementation of the javax.websocket ServerContainer + * this behavior has been changed to be non-scoped so as to be able to use the provided + * SslContextFactory for the underlying HttpClient instance. + */ + this(newHttpClient(sslContextFactory, scope.getExecutor(), scope.getBufferPool()), scope.getObjectFactory()); } /** @@ -186,25 +223,12 @@ 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)); - addBean(this.httpClient); + this(newHttpClient(sslContextFactory, executor, bufferPool), null); } /** @@ -214,10 +238,15 @@ 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); + /* Nothing in Jetty uses this constructor anymore. + * It's left in for backwards compat reasons. + */ + this(scope, eventDriverFactory, sessionFactory, HttpClientProvider.get(scope)); } /** @@ -231,23 +260,18 @@ 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); - 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 Object Factory (for CDI) - this.objectFactorySupplier = () -> scope.getObjectFactory(); + + // Support Late Binding of DecoratedObjectFactory (that CDI establishes in its own servlet context listeners) + this.objectFactorySupplier = scope::getObjectFactory; + this.extensionRegistry = new WebSocketExtensionFactory(this); addBean(extensionRegistry); @@ -255,6 +279,14 @@ public WebSocketClient(final WebSocketContainerScope scope, EventDriverFactory e this.sessionFactory = sessionFactory == null ? new WebSocketSessionFactory(this) : sessionFactory; } + private static HttpClient newHttpClient(SslContextFactory sslContextFactory, Executor executor, ByteBufferPool bufferPool) + { + HttpClient httpClient = new HttpClient(sslContextFactory); + httpClient.setExecutor(executor); + httpClient.setByteBufferPool(bufferPool); + return httpClient; + } + public Future connect(Object websocket, URI toUri) throws IOException { ClientUpgradeRequest request = new ClientUpgradeRequest(toUri); @@ -347,6 +379,16 @@ public Future connect(Object websocket, URI toUri, ClientUpgradeRequest return wsReq.sendAsync(); } + public void setEventDriverFactory(EventDriverFactory eventDriverFactory) + { + this.eventDriverFactory = eventDriverFactory; + } + + public void setSessionFactory(SessionFactory sessionFactory) + { + this.sessionFactory = sessionFactory; + } + @Override protected void doStart() throws Exception { diff --git a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/WebSocketClientInitTest.java b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/WebSocketClientInitTest.java index 82b4c677417e..3f66da51f767 100644 --- a/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/WebSocketClientInitTest.java +++ b/jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/WebSocketClientInitTest.java @@ -55,7 +55,6 @@ public void testInit_HttpClient_StartedOutside() throws Exception assertThat("HttpClient started", http.isStarted(), is(true)); HttpClient httpBean = ws.getBean(HttpClient.class); - assertThat("HttpClient should not be found in WebSocketClient", httpBean, nullValue()); assertThat("HttpClient bean is managed", ws.isManaged(httpBean), is(false)); assertThat("WebSocketClient should not be found in HttpClient", http.getBean(WebSocketClient.class), nullValue()); }