From c6fd7a616fb1eb7afbdf5aa79565875034a9e1ef Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Wed, 13 Feb 2019 15:03:15 +1100 Subject: [PATCH 1/3] Issue #3341 - Use HttpClientProvider in WebSocketCoreClient Signed-off-by: Lachlan Roberts --- jetty-websocket/jetty-websocket-client/pom.xml | 7 ------- jetty-websocket/websocket-core/pom.xml | 6 ++++++ .../websocket-core/src/main/java/module-info.java | 1 + .../core/client}/DefaultHttpClientProvider.java | 11 ++++++----- .../websocket/core/client}/HttpClientProvider.java | 8 ++++---- .../websocket/core/client/WebSocketCoreClient.java | 10 +++------- .../core/client}/XmlBasedHttpClientProvider.java | 8 ++++---- 7 files changed, 24 insertions(+), 27 deletions(-) rename jetty-websocket/{jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl => websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client}/DefaultHttpClientProvider.java (81%) rename jetty-websocket/{jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl => websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client}/HttpClientProvider.java (93%) rename jetty-websocket/{jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl => websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client}/XmlBasedHttpClientProvider.java (97%) diff --git a/jetty-websocket/jetty-websocket-client/pom.xml b/jetty-websocket/jetty-websocket-client/pom.xml index 3b922fbb0be..10ff69001a5 100644 --- a/jetty-websocket/jetty-websocket-client/pom.xml +++ b/jetty-websocket/jetty-websocket-client/pom.xml @@ -32,13 +32,6 @@ jetty-client ${project.version} - - org.eclipse.jetty - jetty-xml - ${project.version} - true - - org.eclipse.jetty jetty-server diff --git a/jetty-websocket/websocket-core/pom.xml b/jetty-websocket/websocket-core/pom.xml index 5a0ab6af971..cdbc67958d0 100644 --- a/jetty-websocket/websocket-core/pom.xml +++ b/jetty-websocket/websocket-core/pom.xml @@ -31,6 +31,12 @@ jetty-http ${project.version} + + org.eclipse.jetty + jetty-xml + ${project.version} + true + org.eclipse.jetty jetty-client diff --git a/jetty-websocket/websocket-core/src/main/java/module-info.java b/jetty-websocket/websocket-core/src/main/java/module-info.java index ffcaed7e690..93358f5eb3a 100644 --- a/jetty-websocket/websocket-core/src/main/java/module-info.java +++ b/jetty-websocket/websocket-core/src/main/java/module-info.java @@ -37,6 +37,7 @@ module org.eclipse.jetty.websocket.core requires org.eclipse.jetty.io; requires org.eclipse.jetty.http; requires org.eclipse.jetty.server; + requires static org.eclipse.jetty.xml; requires org.eclipse.jetty.util; uses Extension; diff --git a/jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl/DefaultHttpClientProvider.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/DefaultHttpClientProvider.java similarity index 81% rename from jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl/DefaultHttpClientProvider.java rename to jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/DefaultHttpClientProvider.java index 0a92db92be8..cad3935ef9e 100644 --- a/jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl/DefaultHttpClientProvider.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/DefaultHttpClientProvider.java @@ -16,7 +16,7 @@ // ======================================================================== // -package org.eclipse.jetty.websocket.client.impl; +package org.eclipse.jetty.websocket.core.client; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.util.ssl.SslContextFactory; @@ -26,13 +26,14 @@ class DefaultHttpClientProvider { public static HttpClient newHttpClient() { - SslContextFactory sslContextFactory = new SslContextFactory(); - HttpClient client = new HttpClient(sslContextFactory); + HttpClient client = new HttpClient(new SslContextFactory()); + client.getSslContextFactory().setEndpointIdentificationAlgorithm("HTTPS"); + QueuedThreadPool threadPool = new QueuedThreadPool(); - String name = "WebSocketClient@" + client.hashCode(); - threadPool.setName(name); + threadPool.setName("WebSocketClient@" + client.hashCode()); threadPool.setDaemon(true); client.setExecutor(threadPool); + return client; } } diff --git a/jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl/HttpClientProvider.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java similarity index 93% rename from jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl/HttpClientProvider.java rename to jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java index 4fc3e70b640..bf7089b1ea8 100644 --- a/jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl/HttpClientProvider.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java @@ -16,13 +16,13 @@ // ======================================================================== // -package org.eclipse.jetty.websocket.client.impl; +package org.eclipse.jetty.websocket.core.client; + +import java.lang.reflect.Method; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.util.log.Log; -import java.lang.reflect.Method; - public final class HttpClientProvider { public static HttpClient get() @@ -31,7 +31,7 @@ public final class HttpClientProvider { if (Class.forName("org.eclipse.jetty.xml.XmlConfiguration") != null) { - Class xmlClazz = Class.forName("org.eclipse.jetty.websocket.client.XmlBasedHttpClientProvider"); + Class xmlClazz = Class.forName("org.eclipse.jetty.websocket.core.client.XmlBasedHttpClientProvider"); Method getMethod = xmlClazz.getMethod("get"); Object ret = getMethod.invoke(null); if ((ret != null) && (ret instanceof HttpClient)) diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java index 3d247f3414f..0fbe0aa7199 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java @@ -27,7 +27,6 @@ import org.eclipse.jetty.util.DecoratedObjectFactory; import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.thread.ShutdownThread; import org.eclipse.jetty.websocket.core.ExtensionConfig; import org.eclipse.jetty.websocket.core.FrameHandler; @@ -61,12 +60,9 @@ public class WebSocketCoreClient extends ContainerLifeCycle implements FrameHand public WebSocketCoreClient(HttpClient httpClient, FrameHandler.Customizer customizer) { - if (httpClient==null) - { - httpClient = new HttpClient(new SslContextFactory()); - httpClient.getSslContextFactory().setEndpointIdentificationAlgorithm("HTTPS"); - httpClient.setName(String.format("%s@%x",getClass().getSimpleName(),hashCode())); - } + if (httpClient == null) + httpClient = HttpClientProvider.get(); + this.httpClient = httpClient; this.extensionRegistry = new WebSocketExtensionRegistry(); this.objectFactory = new DecoratedObjectFactory(); diff --git a/jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl/XmlBasedHttpClientProvider.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/XmlBasedHttpClientProvider.java similarity index 97% rename from jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl/XmlBasedHttpClientProvider.java rename to jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/XmlBasedHttpClientProvider.java index d168eb6d2d3..2be1cd253f4 100644 --- a/jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/impl/XmlBasedHttpClientProvider.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/XmlBasedHttpClientProvider.java @@ -16,15 +16,15 @@ // ======================================================================== // -package org.eclipse.jetty.websocket.client.impl; +package org.eclipse.jetty.websocket.core.client; + +import java.io.InputStream; +import java.net.URL; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.xml.XmlConfiguration; -import java.io.InputStream; -import java.net.URL; - class XmlBasedHttpClientProvider { public static HttpClient get() From 149620598d24aae5d42dea42094b8246df389ebc Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 14 Feb 2019 13:37:18 +1100 Subject: [PATCH 2/3] Issue #3341 - change HttpClientProvider to interface - HttpClientProvider is now an interface which defines a default method newHttpClient, its static get() method get will attempt to use the XmlHttpClientProvider to create a client, and if this fails to give a non null client it will be created with the default newHttpClient method Signed-off-by: Lachlan Roberts --- .../src/main/java/module-info.java | 3 -- .../websocket/client/HttpClientInitTest.java | 2 +- .../src/main/java/module-info.java | 4 +- .../client/DefaultHttpClientProvider.java | 39 ------------------- .../core/client/HttpClientProvider.java | 33 +++++++++------- .../core/client/WebSocketCoreClient.java | 3 +- ...ovider.java => XmlHttpClientProvider.java} | 7 ++-- 7 files changed, 28 insertions(+), 63 deletions(-) delete mode 100644 jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/DefaultHttpClientProvider.java rename jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/{XmlBasedHttpClientProvider.java => XmlHttpClientProvider.java} (88%) diff --git a/jetty-websocket/jetty-websocket-client/src/main/java/module-info.java b/jetty-websocket/jetty-websocket-client/src/main/java/module-info.java index 558c1770146..e414e1fd7b0 100644 --- a/jetty-websocket/jetty-websocket-client/src/main/java/module-info.java +++ b/jetty-websocket/jetty-websocket-client/src/main/java/module-info.java @@ -27,7 +27,4 @@ module org.eclipse.jetty.websocket.jetty.client requires org.eclipse.jetty.websocket.jetty.api; requires org.eclipse.jetty.websocket.core; requires org.eclipse.jetty.websocket.jetty.common; - - // Only required if using XmlBasedHttpClientProvider. - requires static org.eclipse.jetty.xml; } diff --git a/jetty-websocket/jetty-websocket-client/src/test/java/org/eclipse/jetty/websocket/client/HttpClientInitTest.java b/jetty-websocket/jetty-websocket-client/src/test/java/org/eclipse/jetty/websocket/client/HttpClientInitTest.java index 14b609e982f..221f582a3ec 100644 --- a/jetty-websocket/jetty-websocket-client/src/test/java/org/eclipse/jetty/websocket/client/HttpClientInitTest.java +++ b/jetty-websocket/jetty-websocket-client/src/test/java/org/eclipse/jetty/websocket/client/HttpClientInitTest.java @@ -46,7 +46,7 @@ public class HttpClientInitTest assertThat("Executor exists", executor, notNullValue()); assertThat("Executor instanceof", executor, instanceOf(QueuedThreadPool.class)); QueuedThreadPool threadPool = (QueuedThreadPool)executor; - assertThat("QueuedThreadPool.name", threadPool.getName(), startsWith("Jetty-WebSocketClient@")); + assertThat("QueuedThreadPool.name", threadPool.getName(), startsWith("WebSocketClient@")); } finally { diff --git a/jetty-websocket/websocket-core/src/main/java/module-info.java b/jetty-websocket/websocket-core/src/main/java/module-info.java index 93358f5eb3a..31e6af6f45d 100644 --- a/jetty-websocket/websocket-core/src/main/java/module-info.java +++ b/jetty-websocket/websocket-core/src/main/java/module-info.java @@ -37,9 +37,11 @@ module org.eclipse.jetty.websocket.core requires org.eclipse.jetty.io; requires org.eclipse.jetty.http; requires org.eclipse.jetty.server; - requires static org.eclipse.jetty.xml; requires org.eclipse.jetty.util; + // Only required if using XmlBasedHttpClientProvider. + requires static org.eclipse.jetty.xml; + uses Extension; provides Extension with diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/DefaultHttpClientProvider.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/DefaultHttpClientProvider.java deleted file mode 100644 index cad3935ef9e..00000000000 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/DefaultHttpClientProvider.java +++ /dev/null @@ -1,39 +0,0 @@ -// -// ======================================================================== -// 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.core.client; - -import org.eclipse.jetty.client.HttpClient; -import org.eclipse.jetty.util.ssl.SslContextFactory; -import org.eclipse.jetty.util.thread.QueuedThreadPool; - -class DefaultHttpClientProvider -{ - public static HttpClient newHttpClient() - { - HttpClient client = new HttpClient(new SslContextFactory()); - client.getSslContextFactory().setEndpointIdentificationAlgorithm("HTTPS"); - - QueuedThreadPool threadPool = new QueuedThreadPool(); - threadPool.setName("WebSocketClient@" + client.hashCode()); - threadPool.setDaemon(true); - client.setExecutor(threadPool); - - return client; - } -} diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java index bf7089b1ea8..4f3a11bd2b9 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java @@ -18,33 +18,36 @@ package org.eclipse.jetty.websocket.core.client; -import java.lang.reflect.Method; - import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.ssl.SslContextFactory; +import org.eclipse.jetty.util.thread.QueuedThreadPool; -public final class HttpClientProvider +public interface HttpClientProvider { - public static HttpClient get() + static HttpClient get() { try { - if (Class.forName("org.eclipse.jetty.xml.XmlConfiguration") != null) - { - Class xmlClazz = Class.forName("org.eclipse.jetty.websocket.core.client.XmlBasedHttpClientProvider"); - Method getMethod = xmlClazz.getMethod("get"); - Object ret = getMethod.invoke(null); - if ((ret != null) && (ret instanceof HttpClient)) - { - return (HttpClient)ret; - } - } + HttpClientProvider xmlProvider = new XmlHttpClientProvider(); + HttpClient client = xmlProvider.newHttpClient(); + if (client != null) + return client; } catch (Throwable ignore) { Log.getLogger(HttpClientProvider.class).ignore(ignore); } - return DefaultHttpClientProvider.newHttpClient(); + return new HttpClientProvider(){}.newHttpClient(); + } + + default HttpClient newHttpClient() + { + HttpClient client = new HttpClient(new SslContextFactory()); + QueuedThreadPool threadPool = new QueuedThreadPool(); + threadPool.setName("WebSocketClient@" + client.hashCode()); + client.setExecutor(threadPool); + return client; } } diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java index 0fbe0aa7199..759232bf107 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/WebSocketCoreClient.java @@ -20,6 +20,7 @@ package org.eclipse.jetty.websocket.core.client; import java.io.IOException; import java.net.URI; +import java.util.Objects; import java.util.concurrent.CompletableFuture; import org.eclipse.jetty.client.HttpClient; @@ -61,7 +62,7 @@ public class WebSocketCoreClient extends ContainerLifeCycle implements FrameHand public WebSocketCoreClient(HttpClient httpClient, FrameHandler.Customizer customizer) { if (httpClient == null) - httpClient = HttpClientProvider.get(); + httpClient = Objects.requireNonNull(HttpClientProvider.get()); this.httpClient = httpClient; this.extensionRegistry = new WebSocketExtensionRegistry(); diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/XmlBasedHttpClientProvider.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/XmlHttpClientProvider.java similarity index 88% rename from jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/XmlBasedHttpClientProvider.java rename to jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/XmlHttpClientProvider.java index 2be1cd253f4..dafd14d5bd5 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/XmlBasedHttpClientProvider.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/XmlHttpClientProvider.java @@ -25,9 +25,10 @@ import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.xml.XmlConfiguration; -class XmlBasedHttpClientProvider +class XmlHttpClientProvider implements HttpClientProvider { - public static HttpClient get() + @Override + public HttpClient newHttpClient() { URL resource = Thread.currentThread().getContextClassLoader().getResource("jetty-websocket-httpclient.xml"); if (resource == null) @@ -42,7 +43,7 @@ class XmlBasedHttpClientProvider } catch (Throwable t) { - Log.getLogger(XmlBasedHttpClientProvider.class).warn("Unable to load: " + resource, t); + Log.getLogger(XmlHttpClientProvider.class).warn("Unable to load: " + resource, t); } return null; From d8b5f0e1b3f75004c46e2b6166cc96954601a2d7 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 15 Feb 2019 18:31:53 +1100 Subject: [PATCH 3/3] Issue #3341 - HttpClientProvider changes from review Signed-off-by: Lachlan Roberts --- .../jetty/websocket/core/client/HttpClientProvider.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java index 4f3a11bd2b9..a9f7e965404 100644 --- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java +++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/client/HttpClientProvider.java @@ -39,10 +39,10 @@ public interface HttpClientProvider Log.getLogger(HttpClientProvider.class).ignore(ignore); } - return new HttpClientProvider(){}.newHttpClient(); + return HttpClientProvider.newDefaultHttpClient(); } - default HttpClient newHttpClient() + private static HttpClient newDefaultHttpClient() { HttpClient client = new HttpClient(new SslContextFactory()); QueuedThreadPool threadPool = new QueuedThreadPool(); @@ -50,4 +50,9 @@ public interface HttpClientProvider client.setExecutor(threadPool); return client; } + + default HttpClient newHttpClient() + { + return newDefaultHttpClient(); + } }