Skip to content

Commit

Permalink
Issue #5832 - changes from review
Browse files Browse the repository at this point in the history 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
  • Loading branch information
@lachlan-roberts
lachlan-roberts committed Jan 13, 2021
1 parent 9e19e87 commit 35051df
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 77 deletions.
2 changes: 1 addition & 1 deletion jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletContextHandler.java
View file
Expand Up @@ -703,7 +703,7 @@ else if (handler instanceof ServletHandler)
*/
public void addServletContainerInitializer(ServletContainerInitializer containerInitializer)
{
addManaged(new Initializer(this, containerInitializer));
addServletContainerInitializer(containerInitializer, Collections.emptySet());
}

/**
Expand Down
3 changes: 2 additions & 1 deletion .../src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientShutdown.java
View file
Expand Up @@ -29,7 +29,7 @@ public class JavaxWebSocketClientShutdown extends ContainerLifeCycle implements
@Override
public void onStartup(Set<Class<?>> c, ServletContext ctx) throws ServletException
{
JavaxWebSocketClientContainer.SHUTDOWN_CONTAINER.compareAndSet(null, this);
JavaxWebSocketClientContainer.initialize(this);
ctx.addListener(this);
}

Expand All @@ -44,5 +44,6 @@ public void contextDestroyed(ServletContextEvent sce)
{
LifeCycle.stop(this);
removeBeans();
JavaxWebSocketClientContainer.initialize(null);
}
}
13 changes: 12 additions & 1 deletion ...java/org/eclipse/jetty/websocket/javax/client/internal/JavaxWebSocketClientContainer.java
View file
Expand Up @@ -60,7 +60,12 @@
public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer implements javax.websocket.WebSocketContainer
{
private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketClientContainer.class);
public static final AtomicReference<ContainerLifeCycle> SHUTDOWN_CONTAINER = new AtomicReference<>();
private static final AtomicReference<ContainerLifeCycle> SHUTDOWN_CONTAINER = new AtomicReference<>();

public static void initialize(ContainerLifeCycle container)
{
SHUTDOWN_CONTAINER.set(container);
}

protected WebSocketCoreClient coreClient;
protected Function<WebSocketComponents, WebSocketCoreClient> coreClientFactory;
Expand Down Expand Up @@ -282,6 +287,12 @@ private ClientEndpointConfig getAnnotatedConfig(Object endpoint) throws Deployme
return new AnnotatedClientEndpointConfig(anno);
}

@Override
protected void doStop() throws Exception
{
super.doStop();
}

@Override
protected void doStart() throws Exception
{
Expand Down
63 changes: 24 additions & 39 deletions ...clipse/jetty/websocket/javax/server/config/JavaxWebSocketServletContainerInitializer.java
View file
Expand Up @@ -15,7 +15,6 @@

import java.util.HashSet;
import java.util.Set;
import java.util.function.Consumer;
import javax.servlet.ServletContainerInitializer;
import javax.servlet.ServletContext;
import javax.servlet.ServletException;
Expand Down Expand Up @@ -52,7 +51,17 @@ public class JavaxWebSocketServletContainerInitializer implements ServletContain
public static final String HTTPCLIENT_ATTRIBUTE = "org.eclipse.jetty.websocket.javax.HttpClient";
private static final Logger LOG = LoggerFactory.getLogger(JavaxWebSocketServletContainerInitializer.class);

private Consumer<ServletContext> afterStartupConsumer;
private final Configurator configurator;

public JavaxWebSocketServletContainerInitializer()
{
this(null);
}

public JavaxWebSocketServletContainerInitializer(Configurator configurator)
{
this.configurator = configurator;
}

/**
* Test a ServletContext for {@code init-param} or {@code attribute} at {@code keyName} for
Expand Down Expand Up @@ -98,28 +107,7 @@ public static void configure(ServletContextHandler context, Configurator configu
{
if (!context.isStopped())
throw new IllegalStateException("configure should be called before starting");

// In this embedded-jetty usage, allow ServletContext.addListener() to
// add other ServletContextListeners (such as the ContextDestroyListener) after
// the initialization phase is over. (important for this SCI to function)
context.getServletContext().setExtendedListenerTypes(true);

context.addServletContainerInitializer(new JavaxWebSocketServletContainerInitializer()
.afterStartup((servletContext) ->
{
JavaxWebSocketServerContainer serverContainer = JavaxWebSocketServerContainer.getContainer(servletContext);
if (configurator != null)
{
try
{
configurator.accept(servletContext, serverContainer);
}
catch (DeploymentException e)
{
throw new RuntimeException("Failed to deploy WebSocket Endpoint", e);
}
}
}));
context.addServletContainerInitializer(new JavaxWebSocketServletContainerInitializer(configurator));
}

/**
Expand Down Expand Up @@ -273,8 +261,18 @@ public void onStartup(Set<Class<?>> c, ServletContext context) throws ServletExc
}
}

if (afterStartupConsumer != null)
afterStartupConsumer.accept(context);
// Call the configurator after startup.
if (configurator != null)
{
try
{
configurator.accept(context, container);
}
catch (DeploymentException e)
{
throw new RuntimeException("Failed to deploy WebSocket Endpoint", e);
}
}
}

@SuppressWarnings("unchecked")
Expand All @@ -301,17 +299,4 @@ private void filterClasses(Set<Class<?>> c, Set<Class<? extends Endpoint>> disco
}
}
}

/**
* Add a optional consumer to execute once the {@link ServletContainerInitializer#onStartup(Set, ServletContext)} has
* been called successfully.
*
* @param consumer the consumer to execute after the SCI has executed
* @return this configured {@link JavaxWebSocketServletContainerInitializer} instance.
*/
public JavaxWebSocketServletContainerInitializer afterStartup(Consumer<ServletContext> consumer)
{
this.afterStartupConsumer = consumer;
return this;
}
}
43 changes: 16 additions & 27 deletions .../org/eclipse/jetty/websocket/server/config/JettyWebSocketServletContainerInitializer.java
View file
Expand Up @@ -14,7 +14,6 @@
package org.eclipse.jetty.websocket.server.config;

import java.util.Set;
import java.util.function.Consumer;
import javax.servlet.ServletContainerInitializer;
import javax.servlet.ServletContext;

Expand All @@ -32,7 +31,17 @@
public class JettyWebSocketServletContainerInitializer implements ServletContainerInitializer
{
private static final Logger LOG = LoggerFactory.getLogger(JettyWebSocketServletContainerInitializer.class);
private Consumer<ServletContext> afterStartupConsumer;
private final Configurator configurator;

public JettyWebSocketServletContainerInitializer()
{
this(null);
}

public JettyWebSocketServletContainerInitializer(Configurator configurator)
{
this.configurator = configurator;
}

public interface Configurator
{
Expand All @@ -51,16 +60,7 @@ public static void configure(ServletContextHandler context, Configurator configu
{
if (!context.isStopped())
throw new IllegalStateException("configure should be called before starting");

context.addServletContainerInitializer(new JettyWebSocketServletContainerInitializer()
.afterStartup((servletContext) ->
{
if (configurator != null)
{
JettyWebSocketServerContainer container = JettyWebSocketServerContainer.getContainer(servletContext);
configurator.accept(servletContext, container);
}
}));
context.addServletContainerInitializer(new JettyWebSocketServletContainerInitializer());
}

/**
Expand Down Expand Up @@ -101,20 +101,9 @@ public void onStartup(Set<Class<?>> c, ServletContext context)
if (LOG.isDebugEnabled())
LOG.debug("onStartup {}", container);

if (afterStartupConsumer != null)
afterStartupConsumer.accept(context);
}

/**
* Add a optional consumer to execute once the {@link ServletContainerInitializer#onStartup(Set, ServletContext)} has
* been called successfully.
*
* @param consumer the consumer to execute after the SCI has executed
* @return this configured {@link JettyWebSocketServletContainerInitializer} instance.
*/
public JettyWebSocketServletContainerInitializer afterStartup(Consumer<ServletContext> consumer)
{
this.afterStartupConsumer = consumer;
return this;
if (configurator != null)
{
configurator.accept(context, container);
}
}
}
16 changes: 8 additions & 8 deletions tests/test-cdi/src/test/java/org/eclipse/jetty/cdi/tests/EmbeddedWeldTest.java
View file
Expand Up @@ -103,29 +103,29 @@ public static Server createServerWithServletContext(String mode)

case "CdiServletContainerInitializer+Listener":
// Expect:INFO: WELD-ENV-001213: Jetty CDI SPI support detected, CDI injection will be available in Listeners, Servlets and Filters.
context.addBean(new ServletContextHandler.Initializer(context, new CdiServletContainerInitializer()));
context.addServletContainerInitializer(new CdiServletContainerInitializer());
context.addEventListener(new org.jboss.weld.environment.servlet.Listener());
break;

case "CdiServletContainerInitializer(CdiDecoratingListener)+Listener":
// Expect:INFO: WELD-ENV-001212: Jetty CdiDecoratingListener support detected, CDI injection will be available in Listeners, Servlets and Filters
context.setInitParameter(CdiServletContainerInitializer.CDI_INTEGRATION_ATTRIBUTE, CdiDecoratingListener.MODE);
context.addBean(new ServletContextHandler.Initializer(context, new CdiServletContainerInitializer()));
context.addServletContainerInitializer(new CdiServletContainerInitializer());
context.addEventListener(new org.jboss.weld.environment.servlet.Listener());
break;

case "CdiServletContainerInitializer+EnhancedListener":
// Expect:INFO: WELD-ENV-001213: Jetty CDI SPI support detected, CDI injection will be available in Listeners, Servlets and Filters.
context.addBean(new ServletContextHandler.Initializer(context, new CdiServletContainerInitializer()));
context.addBean(new ServletContextHandler.Initializer(context, new org.jboss.weld.environment.servlet.EnhancedListener()));
context.addServletContainerInitializer(new CdiServletContainerInitializer());
context.addServletContainerInitializer(new org.jboss.weld.environment.servlet.EnhancedListener());
break;

// NOTE: This is the preferred mode from the Weld team.
case "CdiServletContainerInitializer(CdiDecoratingListener)+EnhancedListener":
// Expect:INFO: WELD-ENV-001212: Jetty CdiDecoratingListener support detected, CDI injection will be available in Listeners, Servlets and Filters
context.setInitParameter(CdiServletContainerInitializer.CDI_INTEGRATION_ATTRIBUTE, CdiDecoratingListener.MODE);
context.addBean(new ServletContextHandler.Initializer(context, new CdiServletContainerInitializer()));
context.addBean(new ServletContextHandler.Initializer(context, new org.jboss.weld.environment.servlet.EnhancedListener()));
context.addServletContainerInitializer(new CdiServletContainerInitializer());
context.addServletContainerInitializer(new org.jboss.weld.environment.servlet.EnhancedListener());
break;
}

Expand Down Expand Up @@ -190,8 +190,8 @@ public void testWebappContext() throws Exception
server.setHandler(webapp);

webapp.setInitParameter(CdiServletContainerInitializer.CDI_INTEGRATION_ATTRIBUTE, CdiDecoratingListener.MODE);
webapp.addBean(new ServletContextHandler.Initializer(webapp, new CdiServletContainerInitializer()));
webapp.addBean(new ServletContextHandler.Initializer(webapp, new org.jboss.weld.environment.servlet.EnhancedListener()));
webapp.addServletContainerInitializer(new CdiServletContainerInitializer());
webapp.addServletContainerInitializer(new org.jboss.weld.environment.servlet.EnhancedListener());

String pkg = EmbeddedWeldTest.class.getPackage().getName();
webapp.getServerClassMatcher().add("-" + pkg + ".");
Expand Down

0 comments on commit 35051df

Please sign in to comment.