From 149d9d486227a9f18cba19e8d307ff0e5197ed9c Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 26 Aug 2019 13:40:32 -0500 Subject: [PATCH 1/5] Fixes #4020 - Revert ExtensionFactory change to interface. + The change in commit 30dc103a129d08304e1f1a99a84c045049de305d was done to allow the InflaterPool and DeflaterPool to be managed by the Jetty lifecycle. + This restore the original abstract class ExtensionFactory. + Had to break the traditional LifeCycle usage for a more non-traditional one in order to both, not break this existing API, and not introduce jetty-util to the webapp classloader. + This will restore API / binary compatibility for other projects, like spring-boot. Signed-off-by: Joakim Erdfelt --- .../api/extensions/ExtensionFactory.java | 58 ++++++++++++--- .../extensions/WebSocketExtensionFactory.java | 71 ++++++++++++++++++- 2 files changed, 118 insertions(+), 11 deletions(-) diff --git a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java index eae3dd3fbe6..8d46d60a5eb 100644 --- a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java +++ b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java @@ -18,22 +18,64 @@ package org.eclipse.jetty.websocket.api.extensions; +import java.util.HashMap; +import java.util.Iterator; import java.util.Map; +import java.util.ServiceLoader; import java.util.Set; -public interface ExtensionFactory extends Iterable> +public abstract class ExtensionFactory implements Iterable> { - Map> getAvailableExtensions(); + private ServiceLoader extensionLoader = ServiceLoader.load(Extension.class); + private Map> availableExtensions; - Class getExtension(String name); + public ExtensionFactory() + { + availableExtensions = new HashMap<>(); + for (Extension ext : extensionLoader) + { + if (ext != null) + { + availableExtensions.put(ext.getName(), ext.getClass()); + } + } + } - Set getExtensionNames(); + public Map> getAvailableExtensions() + { + return availableExtensions; + } - boolean isAvailable(String name); + public Class getExtension(String name) + { + return availableExtensions.get(name); + } - Extension newInstance(ExtensionConfig config); + public Set getExtensionNames() + { + return availableExtensions.keySet(); + } - void register(String name, Class extension); + public boolean isAvailable(String name) + { + return availableExtensions.containsKey(name); + } - void unregister(String name); + @Override + public Iterator> iterator() + { + return availableExtensions.values().iterator(); + } + + public abstract Extension newInstance(ExtensionConfig config); + + public void register(String name, Class extension) + { + availableExtensions.put(name, extension); + } + + public void unregister(String name) + { + availableExtensions.remove(name); + } } diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java index f382d687afa..706429567b8 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java @@ -27,6 +27,7 @@ import java.util.zip.Deflater; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.compression.CompressionPool; import org.eclipse.jetty.util.compression.DeflaterPool; import org.eclipse.jetty.util.compression.InflaterPool; @@ -37,8 +38,9 @@ import org.eclipse.jetty.websocket.api.extensions.ExtensionFactory; import org.eclipse.jetty.websocket.common.extensions.compress.CompressExtension; import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope; -public class WebSocketExtensionFactory extends ContainerLifeCycle implements ExtensionFactory +public class WebSocketExtensionFactory extends ExtensionFactory implements LifeCycle { + private ContainerLifeCycle lifecycle; private WebSocketContainerScope container; private ServiceLoader extensionLoader = ServiceLoader.load(Extension.class); private Map> availableExtensions; @@ -47,6 +49,7 @@ public class WebSocketExtensionFactory extends ContainerLifeCycle implements Ext public WebSocketExtensionFactory(WebSocketContainerScope container) { + lifecycle = new ContainerLifeCycle(); availableExtensions = new HashMap<>(); for (Extension ext : extensionLoader) { @@ -55,8 +58,8 @@ public class WebSocketExtensionFactory extends ContainerLifeCycle implements Ext } this.container = container; - addBean(inflaterPool); - addBean(deflaterPool); + lifecycle.addBean(inflaterPool); + lifecycle.addBean(deflaterPool); } @Override @@ -144,4 +147,66 @@ public class WebSocketExtensionFactory extends ContainerLifeCycle implements Ext { return availableExtensions.values().iterator(); } + + /* --- All of the below ugliness due to not being able to break API compatibility with ExtensionFactory --- */ + + @Override + public void start() throws Exception + { + lifecycle.start(); + } + + @Override + public void stop() throws Exception + { + lifecycle.stop(); + } + + @Override + public boolean isRunning() + { + return lifecycle.isRunning(); + } + + @Override + public boolean isStarted() + { + return lifecycle.isStarted(); + } + + @Override + public boolean isStarting() + { + return lifecycle.isStarting(); + } + + @Override + public boolean isStopping() + { + return lifecycle.isStopping(); + } + + @Override + public boolean isStopped() + { + return lifecycle.isStopped(); + } + + @Override + public boolean isFailed() + { + return lifecycle.isFailed(); + } + + @Override + public void addLifeCycleListener(Listener listener) + { + lifecycle.addLifeCycleListener(listener); + } + + @Override + public void removeLifeCycleListener(Listener listener) + { + lifecycle.removeLifeCycleListener(listener); + } } From 6bcfa2dc6ef409ce0ef55b81d7765f63e4cd5b86 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 27 Aug 2019 13:25:08 -0500 Subject: [PATCH 2/5] Fixes #4020 - Deprecate ExtensionFactory + This class is removed in Jetty 10 anyway. + If all you want is to access available extension names then use the Factory.getAvailableExtensionNames() method (which exists in Jetty 10.0.0 as well) Signed-off-by: Joakim Erdfelt --- .../api/extensions/ExtensionFactory.java | 6 ++++++ .../server/WebSocketServerFactory.java | 8 ++++++++ .../servlet/WebSocketServletFactory.java | 17 +++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java index 8d46d60a5eb..94a27769457 100644 --- a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java +++ b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/ExtensionFactory.java @@ -24,6 +24,12 @@ import java.util.Map; import java.util.ServiceLoader; import java.util.Set; +/** + * The Factory for Extensions. + * + * @deprecated this class is removed from Jetty 10.0.0+ + */ +@Deprecated public abstract class ExtensionFactory implements Iterable> { private ServiceLoader extensionLoader = ServiceLoader.load(Extension.class); diff --git a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java index 520302d95f5..2b5e116bd08 100644 --- a/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java +++ b/jetty-websocket/websocket-server/src/main/java/org/eclipse/jetty/websocket/server/WebSocketServerFactory.java @@ -29,6 +29,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.concurrent.Executor; import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; @@ -450,6 +451,13 @@ public class WebSocketServerFactory extends ContainerLifeCycle implements WebSoc return eventDriverFactory; } + @Override + public Set getAvailableExtensionNames() + { + return Collections.unmodifiableSet(extensionFactory.getExtensionNames()); + } + + @Deprecated @Override public ExtensionFactory getExtensionFactory() { diff --git a/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketServletFactory.java b/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketServletFactory.java index fec0084cd30..b2d98b18ff6 100644 --- a/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketServletFactory.java +++ b/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/WebSocketServletFactory.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.websocket.servlet; import java.io.IOException; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.util.Set; import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -66,8 +67,24 @@ public interface WebSocketServletFactory void stop() throws Exception; + /** + * Get the set of available Extensions by registered name. + * + * @return the set of available extensions by registered name. + */ + Set getAvailableExtensionNames(); + WebSocketCreator getCreator(); + /** + * Get the registered extensions for this WebSocket factory. + * + * @return the ExtensionFactory + * @see #getAvailableExtensionNames() + * @deprecated this class is removed from Jetty 10.0.0+. To remove specific extensions + * from negotiation use {@link WebSocketCreator} to remove then during handshake. + */ + @Deprecated ExtensionFactory getExtensionFactory(); /** From 2979ed5046eaa6705df937c9b80cc5497adfe9c9 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 27 Aug 2019 20:25:10 -0500 Subject: [PATCH 3/5] Fixes #4020 - Satisfy Container LifeCycle dumpable behaviors Signed-off-by: Joakim Erdfelt --- .../extensions/WebSocketExtensionFactory.java | 56 ++++++++++++++----- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java index 706429567b8..f1ca84d635f 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java @@ -18,6 +18,7 @@ package org.eclipse.jetty.websocket.common.extensions; +import java.io.IOException; import java.util.HashMap; import java.util.Iterator; import java.util.Map; @@ -27,6 +28,7 @@ import java.util.zip.Deflater; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.component.ContainerLifeCycle; +import org.eclipse.jetty.util.component.Dumpable; import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.util.compression.CompressionPool; import org.eclipse.jetty.util.compression.DeflaterPool; @@ -38,9 +40,9 @@ import org.eclipse.jetty.websocket.api.extensions.ExtensionFactory; import org.eclipse.jetty.websocket.common.extensions.compress.CompressExtension; import org.eclipse.jetty.websocket.common.scopes.WebSocketContainerScope; -public class WebSocketExtensionFactory extends ExtensionFactory implements LifeCycle +public class WebSocketExtensionFactory extends ExtensionFactory implements LifeCycle, Dumpable { - private ContainerLifeCycle lifecycle; + private ContainerLifeCycle containerLifeCycle; private WebSocketContainerScope container; private ServiceLoader extensionLoader = ServiceLoader.load(Extension.class); private Map> availableExtensions; @@ -49,7 +51,7 @@ public class WebSocketExtensionFactory extends ExtensionFactory implements LifeC public WebSocketExtensionFactory(WebSocketContainerScope container) { - lifecycle = new ContainerLifeCycle(); + containerLifeCycle = new ContainerLifeCycle(); availableExtensions = new HashMap<>(); for (Extension ext : extensionLoader) { @@ -58,8 +60,8 @@ public class WebSocketExtensionFactory extends ExtensionFactory implements LifeC } this.container = container; - lifecycle.addBean(inflaterPool); - lifecycle.addBean(deflaterPool); + containerLifeCycle.addBean(inflaterPool); + containerLifeCycle.addBean(deflaterPool); } @Override @@ -153,60 +155,84 @@ public class WebSocketExtensionFactory extends ExtensionFactory implements LifeC @Override public void start() throws Exception { - lifecycle.start(); + containerLifeCycle.start(); } @Override public void stop() throws Exception { - lifecycle.stop(); + containerLifeCycle.stop(); } @Override public boolean isRunning() { - return lifecycle.isRunning(); + return containerLifeCycle.isRunning(); } @Override public boolean isStarted() { - return lifecycle.isStarted(); + return containerLifeCycle.isStarted(); } @Override public boolean isStarting() { - return lifecycle.isStarting(); + return containerLifeCycle.isStarting(); } @Override public boolean isStopping() { - return lifecycle.isStopping(); + return containerLifeCycle.isStopping(); } @Override public boolean isStopped() { - return lifecycle.isStopped(); + return containerLifeCycle.isStopped(); } @Override public boolean isFailed() { - return lifecycle.isFailed(); + return containerLifeCycle.isFailed(); } @Override public void addLifeCycleListener(Listener listener) { - lifecycle.addLifeCycleListener(listener); + containerLifeCycle.addLifeCycleListener(listener); } @Override public void removeLifeCycleListener(Listener listener) { - lifecycle.removeLifeCycleListener(listener); + containerLifeCycle.removeLifeCycleListener(listener); + } + + @Override + public String dump() + { + return containerLifeCycle.dump(); + } + + @Override + public String dumpSelf() + { + return containerLifeCycle.dumpSelf(); + } + + @Override + public void dump(Appendable out, String indent) throws IOException + { + containerLifeCycle.dump(out, indent); + } + + @Override + public String toString() + { + return String.format("%s@%x{%s}", WebSocketExtensionFactory.class.getName(), hashCode(), containerLifeCycle.getState()); } } From e56d91196d6a669b04dc499cfc56329cbccd6297 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 28 Aug 2019 12:31:33 -0500 Subject: [PATCH 4/5] Issue #4020 - Adding JMX to BrowserDebugTool to test dump Signed-off-by: Joakim Erdfelt --- jetty-websocket/websocket-server/pom.xml | 6 ++++++ .../jetty/websocket/server/browser/BrowserDebugTool.java | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/jetty-websocket/websocket-server/pom.xml b/jetty-websocket/websocket-server/pom.xml index e406f52595c..c3e50f78364 100644 --- a/jetty-websocket/websocket-server/pom.xml +++ b/jetty-websocket/websocket-server/pom.xml @@ -107,6 +107,12 @@ tests test + + org.eclipse.jetty + jetty-jmx + ${project.version} + test + org.eclipse.jetty.toolchain jetty-test-helper diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/browser/BrowserDebugTool.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/browser/BrowserDebugTool.java index 751e5b5479e..2cd9a7fb9f6 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/browser/BrowserDebugTool.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/browser/BrowserDebugTool.java @@ -20,12 +20,14 @@ package org.eclipse.jetty.websocket.server.browser; import java.io.FileNotFoundException; import java.io.IOException; +import java.lang.management.ManagementFactory; import java.net.MalformedURLException; import java.net.URISyntaxException; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; +import org.eclipse.jetty.jmx.MBeanContainer; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.server.handler.ResourceHandler; @@ -131,10 +133,17 @@ public class BrowserDebugTool implements WebSocketCreator public void prepare(int port) throws IOException, URISyntaxException { server = new Server(); + + // Setup JMX + MBeanContainer mbContainer = new MBeanContainer(ManagementFactory.getPlatformMBeanServer()); + server.addBean(mbContainer, true); + + // Setup Connector connector = new ServerConnector(server); connector.setPort(port); server.addConnector(connector); + // Setup WebSocket WebSocketHandler wsHandler = new WebSocketHandler() { @Override From 37e7884382dadf9feb7092ed928e2134b5aa3df2 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 28 Aug 2019 12:36:07 -0500 Subject: [PATCH 5/5] Issue #4020 - Applying change requested from PR review Signed-off-by: Joakim Erdfelt --- .../extensions/WebSocketExtensionFactory.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java index f1ca84d635f..118f7a8a4cd 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/extensions/WebSocketExtensionFactory.java @@ -51,12 +51,19 @@ public class WebSocketExtensionFactory extends ExtensionFactory implements LifeC public WebSocketExtensionFactory(WebSocketContainerScope container) { - containerLifeCycle = new ContainerLifeCycle(); + containerLifeCycle = new ContainerLifeCycle() + { + @Override + public String toString() + { + return String.format("%s@%x{%s}", WebSocketExtensionFactory.class.getSimpleName(), hashCode(), containerLifeCycle.getState()); + } + }; availableExtensions = new HashMap<>(); for (Extension ext : extensionLoader) { if (ext != null) - availableExtensions.put(ext.getName(),ext.getClass()); + availableExtensions.put(ext.getName(), ext.getClass()); } this.container = container; @@ -135,7 +142,7 @@ public class WebSocketExtensionFactory extends ExtensionFactory implements LifeC @Override public void register(String name, Class extension) { - availableExtensions.put(name,extension); + availableExtensions.put(name, extension); } @Override @@ -233,6 +240,6 @@ public class WebSocketExtensionFactory extends ExtensionFactory implements LifeC @Override public String toString() { - return String.format("%s@%x{%s}", WebSocketExtensionFactory.class.getName(), hashCode(), containerLifeCycle.getState()); + return containerLifeCycle.toString(); } }