Issue #3446 - changes from review

use the same FrameHandlerFactory for WebSocketServlet and
WebSocketUpgradeFilter

removed addMapping with PathSpec and only support addMapping
with String

JettyWebSocketServerContainer now implements WebSocketPolicy which
configures the FrameHandler.Customizer

Signed-off-by: lachan-roberts <lachlan@webtide.com>
This commit is contained in:
lachan-roberts 2019-03-11 18:02:14 +11:00 committed by Lachlan Roberts
parent e355ec31b8
commit 8f727bef40
8 changed files with 100 additions and 40 deletions

View File

@ -38,22 +38,6 @@ public class JettyServerFrameHandlerFactory
extends JettyWebSocketFrameHandlerFactory
implements FrameHandlerFactory, LifeCycle.Listener
{
public static JettyServerFrameHandlerFactory ensureFactory(ServletContext servletContext)
{
ContextHandler contextHandler = ServletContextHandler.getServletContextHandler(servletContext, "Jetty Websocket");
JettyServerFrameHandlerFactory factory = contextHandler.getBean(JettyServerFrameHandlerFactory.class);
if (factory == null)
{
JettyWebSocketServerContainer container = JettyWebSocketServerContainer.ensureContainer(servletContext);
servletContext.setAttribute(WebSocketContainer.class.getName(), container);
factory = new JettyServerFrameHandlerFactory(container);
contextHandler.addManaged(factory);
contextHandler.addLifeCycleListener(factory);
}
return factory;
}
public JettyServerFrameHandlerFactory(WebSocketContainer container)
{
super(container);

View File

@ -18,6 +18,7 @@
package org.eclipse.jetty.websocket.server;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@ -34,6 +35,8 @@ import org.eclipse.jetty.util.component.LifeCycle;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.eclipse.jetty.websocket.api.Session;
import org.eclipse.jetty.websocket.api.WebSocketBehavior;
import org.eclipse.jetty.websocket.api.WebSocketPolicy;
import org.eclipse.jetty.websocket.common.SessionTracker;
import org.eclipse.jetty.websocket.common.WebSocketContainer;
import org.eclipse.jetty.websocket.common.WebSocketSessionListener;
@ -44,17 +47,17 @@ import org.eclipse.jetty.websocket.servlet.FrameHandlerFactory;
import org.eclipse.jetty.websocket.servlet.WebSocketCreator;
import org.eclipse.jetty.websocket.servlet.WebSocketMapping;
public class JettyWebSocketServerContainer extends ContainerLifeCycle implements WebSocketContainer, LifeCycle.Listener
public class JettyWebSocketServerContainer extends ContainerLifeCycle implements WebSocketContainer, WebSocketPolicy, LifeCycle.Listener
{
public static JettyWebSocketServerContainer ensureContainer(ServletContext servletContext)
{
ContextHandler contextHandler = ServletContextHandler.getServletContextHandler(servletContext, "Javax Websocket");
ServletContextHandler contextHandler = ServletContextHandler.getServletContextHandler(servletContext, "Javax Websocket");
if (contextHandler.getServer() == null)
throw new IllegalStateException("Server has not been set on the ServletContextHandler");
JettyWebSocketServerContainer container = contextHandler.getBean(JettyWebSocketServerContainer.class);
if (container==null)
if (container == null)
{
// Find Pre-Existing executor
Executor executor = (Executor)servletContext.getAttribute("org.eclipse.jetty.server.Executor");
@ -63,8 +66,10 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements
// Create the Jetty ServerContainer implementation
container = new JettyWebSocketServerContainer(
contextHandler,
WebSocketMapping.ensureMapping(servletContext, WebSocketMapping.DEFAULT_KEY),
WebSocketComponents.ensureWebSocketComponents(servletContext), executor);
servletContext.setAttribute(WebSocketContainer.class.getName(), container);
contextHandler.addManaged(container);
contextHandler.addLifeCycleListener(container);
}
@ -85,32 +90,37 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements
/**
* Main entry point for {@link JettyWebSocketServletContainerInitializer}.
* @param webSocketMapping the {@link WebSocketMapping} that this container belongs to
*
* @param webSocketMapping the {@link WebSocketMapping} that this container belongs to
* @param webSocketComponents the {@link WebSocketComponents} instance to use
* @param executor the {@link Executor} to use
* @param executor the {@link Executor} to use
*/
public JettyWebSocketServerContainer(WebSocketMapping webSocketMapping, WebSocketComponents webSocketComponents, Executor executor)
public JettyWebSocketServerContainer(ServletContextHandler contextHandler, WebSocketMapping webSocketMapping, WebSocketComponents webSocketComponents, Executor executor)
{
this.webSocketMapping = webSocketMapping;
this.webSocketComponents = webSocketComponents;
this.executor = executor;
this.frameHandlerFactory = new JettyServerFrameHandlerFactory(this);
// Ensure there is a FrameHandlerFactory
JettyServerFrameHandlerFactory factory = contextHandler.getBean(JettyServerFrameHandlerFactory.class);
if (factory == null)
{
factory = new JettyServerFrameHandlerFactory(this);
contextHandler.addManaged(factory);
contextHandler.addLifeCycleListener(factory);
}
frameHandlerFactory = factory;
addSessionListener(sessionTracker);
}
public void addMapping(String pathSpec, WebSocketCreator creator)
{
addMapping(WebSocketMapping.parsePathSpec(pathSpec), creator);
}
public void addMapping(PathSpec pathSpec, WebSocketCreator creator) throws WebSocketException
{
if (webSocketMapping.getMapping(pathSpec) != null)
PathSpec ps = WebSocketMapping.parsePathSpec(pathSpec);
if (webSocketMapping.getMapping(ps) != null)
throw new WebSocketException("Duplicate WebSocket Mapping for PathSpec");
webSocketMapping.addMapping(pathSpec, creator, frameHandlerFactory, customizer);
webSocketMapping.addMapping(ps, creator, frameHandlerFactory, customizer);
}
@Override
@ -152,4 +162,70 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements
{
return sessionTracker.getSessions();
}
@Override
public WebSocketBehavior getBehavior()
{
return WebSocketBehavior.SERVER;
}
@Override
public Duration getIdleTimeout()
{
return customizer.getIdleTimeout();
}
@Override
public int getInputBufferSize()
{
return customizer.getInputBufferSize();
}
@Override
public int getOutputBufferSize()
{
return customizer.getOutputBufferSize();
}
@Override
public long getMaxBinaryMessageSize()
{
return customizer.getMaxBinaryMessageSize();
}
@Override
public long getMaxTextMessageSize()
{
return customizer.getMaxTextMessageSize();
}
@Override
public void setIdleTimeout(Duration duration)
{
customizer.setIdleTimeout(duration);
}
@Override
public void setInputBufferSize(int size)
{
customizer.setInputBufferSize(size);
}
@Override
public void setOutputBufferSize(int size)
{
customizer.setOutputBufferSize(size);
}
@Override
public void setMaxBinaryMessageSize(long size)
{
customizer.setMaxBinaryMessageSize(size);
}
@Override
public void setMaxTextMessageSize(long size)
{
customizer.setMaxTextMessageSize(size);
}
}

View File

@ -45,10 +45,9 @@ public class JettyWebSocketServletContainerInitializer implements ServletContain
FilterHolder filterHolder = WebSocketUpgradeFilter.ensureFilter(context.getServletContext());
WebSocketMapping mapping = WebSocketMapping.ensureMapping(context.getServletContext(), WebSocketMapping.DEFAULT_KEY);
JettyWebSocketServerContainer container = JettyWebSocketServerContainer.ensureContainer(context.getServletContext());
JettyServerFrameHandlerFactory factory = JettyServerFrameHandlerFactory.ensureFactory(context.getServletContext());
if (LOG.isDebugEnabled())
LOG.debug("configureContext {} {} {} {} {}", container, mapping, filterHolder, components, factory);
LOG.debug("configureContext {} {} {} {}", container, mapping, filterHolder, components);
return container;
}

View File

@ -74,7 +74,6 @@ public class BadNetworkTest
server.addConnector(connector);
ServletContextHandler context = new ServletContextHandler();
JettyWebSocketServletContainerInitializer.configureContext(context);
context.setContextPath("/");
ServletHolder holder = new ServletHolder(new WebSocketServlet()
{
@ -92,6 +91,7 @@ public class BadNetworkTest
handlers.addHandler(context);
handlers.addHandler(new DefaultHandler());
server.setHandler(handlers);
JettyWebSocketServletContainerInitializer.configureContext(context);
server.start();
}

View File

@ -118,7 +118,6 @@ public class ClientCloseTest
server.addConnector(connector);
ServletContextHandler context = new ServletContextHandler();
JettyWebSocketServletContainerInitializer.configureContext(context);
context.setContextPath("/");
ServletHolder holder = new ServletHolder(new WebSocketServlet()
{
@ -136,6 +135,7 @@ public class ClientCloseTest
handlers.addHandler(context);
handlers.addHandler(new DefaultHandler());
server.setHandler(handlers);
JettyWebSocketServletContainerInitializer.configureContext(context);
server.start();
}

View File

@ -68,7 +68,6 @@ public class ClientSessionsTest
server.addConnector(connector);
ServletContextHandler context = new ServletContextHandler();
JettyWebSocketServletContainerInitializer.configureContext(context);
context.setContextPath("/");
ServletHolder holder = new ServletHolder(new WebSocketServlet()
{
@ -86,6 +85,7 @@ public class ClientSessionsTest
handlers.addHandler(context);
handlers.addHandler(new DefaultHandler());
server.setHandler(handlers);
JettyWebSocketServletContainerInitializer.configureContext(context);
server.start();
}

View File

@ -56,6 +56,7 @@ public abstract class AbstractCloseEndpoint extends WebSocketAdapter
@Override
public void onWebSocketError(Throwable cause)
{
LOG.debug("onWebSocketError({})", cause.getClass().getSimpleName());
errors.offer(cause);
}

View File

@ -84,13 +84,13 @@ public class ServerCloseTest
}
});
context.addServlet(closeEndpoint, "/ws");
JettyWebSocketServletContainerInitializer.configureContext(context);
HandlerList handlers = new HandlerList();
handlers.addHandler(context);
handlers.addHandler(new DefaultHandler());
server.setHandler(handlers);
JettyWebSocketServletContainerInitializer.configureContext(context);
server.start();
}
@ -241,9 +241,9 @@ public class ServerCloseTest
@Test
public void testOpenSessionCleanup() throws Exception
{
fastFail();
fastClose();
dropConnection();
//fastFail();
//fastClose();
//dropConnection();
ClientUpgradeRequest request = new ClientUpgradeRequest();
request.setSubProtocols("container");