diff --git a/dropwizard-core/src/main/java/io/dropwizard/server/AbstractServerFactory.java b/dropwizard-core/src/main/java/io/dropwizard/server/AbstractServerFactory.java index 582c44b528e..0d7df303c00 100644 --- a/dropwizard-core/src/main/java/io/dropwizard/server/AbstractServerFactory.java +++ b/dropwizard-core/src/main/java/io/dropwizard/server/AbstractServerFactory.java @@ -16,7 +16,6 @@ import io.dropwizard.jersey.validation.HibernateValidationBinder; import io.dropwizard.jetty.GzipHandlerFactory; import io.dropwizard.jetty.MutableServletContextHandler; -import io.dropwizard.jetty.NonblockingServletHolder; import io.dropwizard.jetty.ServerPushFilterFactory; import io.dropwizard.lifecycle.setup.LifecycleEnvironment; import io.dropwizard.request.logging.LogbackAccessRequestLogFactory; @@ -31,6 +30,7 @@ import org.eclipse.jetty.server.handler.ErrorHandler; import org.eclipse.jetty.server.handler.RequestLogHandler; import org.eclipse.jetty.server.handler.StatisticsHandler; +import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.setuid.RLimit; import org.eclipse.jetty.setuid.SetUIDListener; import org.eclipse.jetty.util.BlockingArrayQueue; @@ -558,9 +558,8 @@ protected Handler createAdminServlet(Server server, handler.setServer(server); handler.getServletContext().setAttribute(MetricsServlet.METRICS_REGISTRY, metrics); handler.getServletContext().setAttribute(HealthCheckServlet.HEALTH_CHECK_REGISTRY, healthChecks); - handler.addServlet(new NonblockingServletHolder(new AdminServlet()), "/*"); - final String allowedMethodsParam = allowedMethods.stream() - .collect(Collectors.joining(",")); + handler.addServlet(AdminServlet.class, "/*"); + final String allowedMethodsParam = String.join(",", allowedMethods); handler.addFilter(AllowedMethodsFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST)) .setInitParameter(AllowedMethodsFilter.ALLOWED_METHODS_PARAM, allowedMethodsParam); return handler; @@ -584,8 +583,7 @@ protected Handler createAppServlet(Server server, @Nullable Servlet jerseyContainer, MetricRegistry metricRegistry) { configureSessionsAndSecurity(handler, server); - final String allowedMethodsParam = allowedMethods.stream() - .collect(Collectors.joining(",")); + final String allowedMethodsParam = String.join(",", allowedMethods); handler.addFilter(AllowedMethodsFilter.class, "/*", EnumSet.of(DispatcherType.REQUEST)) .setInitParameter(AllowedMethodsFilter.ALLOWED_METHODS_PARAM, allowedMethodsParam); if (enableThreadNameFilter) { @@ -599,7 +597,7 @@ protected Handler createAppServlet(Server server, if (registerDefaultExceptionMappers == null || registerDefaultExceptionMappers) { jersey.register(new ExceptionMapperBinder(detailedJsonProcessingExceptionMapper)); } - handler.addServlet(new NonblockingServletHolder(jerseyContainer), jersey.getUrlPattern()); + handler.addServlet(new ServletHolder("jersey", jerseyContainer), jersey.getUrlPattern()); } final InstrumentedHandler instrumented = new InstrumentedHandler(metricRegistry); instrumented.setServer(server); diff --git a/dropwizard-jetty/src/main/java/io/dropwizard/jetty/NonblockingServletHolder.java b/dropwizard-jetty/src/main/java/io/dropwizard/jetty/NonblockingServletHolder.java index fdee90b3d84..4f8f14de00b 100644 --- a/dropwizard-jetty/src/main/java/io/dropwizard/jetty/NonblockingServletHolder.java +++ b/dropwizard-jetty/src/main/java/io/dropwizard/jetty/NonblockingServletHolder.java @@ -13,7 +13,12 @@ /** * A {@link ServletHolder} subclass which removes the synchronization around servlet initialization * by requiring a pre-initialized servlet holder. + * + * @deprecated If necessary, use {@link ServletHolder} or {@link org.eclipse.jetty.servlet.FilterHolder} directly. + * This class will be removed in Dropwizard 2.1.0. */ +@SuppressWarnings("unused") +@Deprecated public class NonblockingServletHolder extends ServletHolder { private final Servlet servlet; diff --git a/dropwizard-jetty/src/main/java/io/dropwizard/jetty/setup/ServletEnvironment.java b/dropwizard-jetty/src/main/java/io/dropwizard/jetty/setup/ServletEnvironment.java index 0a2cd9052c4..a95a15efe41 100644 --- a/dropwizard-jetty/src/main/java/io/dropwizard/jetty/setup/ServletEnvironment.java +++ b/dropwizard-jetty/src/main/java/io/dropwizard/jetty/setup/ServletEnvironment.java @@ -1,10 +1,10 @@ package io.dropwizard.jetty.setup; import io.dropwizard.jetty.MutableServletContextHandler; -import io.dropwizard.jetty.NonblockingServletHolder; import org.eclipse.jetty.security.SecurityHandler; import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.servlet.FilterHolder; +import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceCollection; @@ -43,9 +43,9 @@ public ServletEnvironment(MutableServletContextHandler handler) { * configuration */ public ServletRegistration.Dynamic addServlet(String name, Servlet servlet) { - final ServletHolder holder = new NonblockingServletHolder(requireNonNull(servlet)); - holder.setName(name); - handler.getServletHandler().addServlet(holder); + final ServletHolder holder = new ServletHolder(name, servlet); + final ServletHandler servletHandler = handler.getServletHandler(); + servletHandler.addServlet(holder); final ServletRegistration.Dynamic registration = holder.getRegistration(); checkDuplicateRegistration(name, servlets, "servlet"); @@ -61,9 +61,9 @@ public ServletRegistration.Dynamic addServlet(String name, Servlet servlet) { * @return a {@link javax.servlet.ServletRegistration.Dynamic} instance allowing for further configuration */ public ServletRegistration.Dynamic addServlet(String name, Class klass) { - final ServletHolder holder = new ServletHolder(requireNonNull(klass)); - holder.setName(name); - handler.getServletHandler().addServlet(holder); + final ServletHolder holder = new ServletHolder(name, klass); + final ServletHandler servletHandler = handler.getServletHandler(); + servletHandler.addServlet(holder); final ServletRegistration.Dynamic registration = holder.getRegistration(); checkDuplicateRegistration(name, servlets, "servlet"); diff --git a/dropwizard-jetty/src/test/java/io/dropwizard/jetty/NonblockingServletHolderTest.java b/dropwizard-jetty/src/test/java/io/dropwizard/jetty/NonblockingServletHolderTest.java index c3be2b876c6..0022cce1aa6 100644 --- a/dropwizard-jetty/src/test/java/io/dropwizard/jetty/NonblockingServletHolderTest.java +++ b/dropwizard-jetty/src/test/java/io/dropwizard/jetty/NonblockingServletHolderTest.java @@ -21,6 +21,7 @@ public class NonblockingServletHolderTest { private final Servlet servlet = mock(Servlet.class); + @SuppressWarnings("deprecation") private final NonblockingServletHolder holder = new NonblockingServletHolder(servlet); private final Request baseRequest = mock(Request.class); private final ServletRequest request = mock(ServletRequest.class); diff --git a/dropwizard-jetty/src/test/java/io/dropwizard/jetty/setup/ServletEnvironmentTest.java b/dropwizard-jetty/src/test/java/io/dropwizard/jetty/setup/ServletEnvironmentTest.java index a49ff1d6c31..fbc1f6a99e5 100644 --- a/dropwizard-jetty/src/test/java/io/dropwizard/jetty/setup/ServletEnvironmentTest.java +++ b/dropwizard-jetty/src/test/java/io/dropwizard/jetty/setup/ServletEnvironmentTest.java @@ -1,23 +1,23 @@ package io.dropwizard.jetty.setup; import io.dropwizard.jetty.MutableServletContextHandler; -import org.eclipse.jetty.http.MimeTypes; +import org.eclipse.jetty.security.ConstraintSecurityHandler; import org.eclipse.jetty.security.SecurityHandler; +import org.eclipse.jetty.server.DebugListener; import org.eclipse.jetty.server.session.SessionHandler; import org.eclipse.jetty.servlet.FilterHolder; import org.eclipse.jetty.servlet.ServletHandler; import org.eclipse.jetty.servlet.ServletHolder; +import org.eclipse.jetty.servlets.ConcatServlet; import org.eclipse.jetty.servlets.WelcomeFilter; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceCollection; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import org.mockito.ArgumentCaptor; import javax.servlet.Filter; import javax.servlet.FilterRegistration; -import javax.servlet.GenericServlet; import javax.servlet.Servlet; import javax.servlet.ServletContextListener; import javax.servlet.ServletRegistration; @@ -25,193 +25,170 @@ import java.nio.file.Path; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; -public class ServletEnvironmentTest { - private final ServletHandler servletHandler = mock(ServletHandler.class); - private final MutableServletContextHandler handler = mock(MutableServletContextHandler.class); - private final ServletEnvironment environment = new ServletEnvironment(handler); +class ServletEnvironmentTest { + + private MutableServletContextHandler handler; + private ServletHandler servletHandler; + private ServletEnvironment environment; @BeforeEach - public void setUp() throws Exception { - when(handler.getServletHandler()).thenReturn(servletHandler); + void setUp() { + handler = new MutableServletContextHandler(); + servletHandler = handler.getServletHandler(); + environment = new ServletEnvironment(handler); } @Test - public void addsServletInstances() throws Exception { - final Servlet servlet = mock(Servlet.class); - + void addsServletInstances() throws Exception { + final Servlet servlet = new ConcatServlet(); final ServletRegistration.Dynamic builder = environment.addServlet("servlet", servlet); - assertThat(builder) - .isNotNull(); - - final ArgumentCaptor holder = ArgumentCaptor.forClass(ServletHolder.class); - verify(servletHandler).addServlet(holder.capture()); + assertThat(builder).isNotNull(); - assertThat(holder.getValue().getName()) - .isEqualTo("servlet"); + try { + servletHandler.start(); - assertThat(holder.getValue().getServlet()) - .isEqualTo(servlet); + final ServletHolder servletHolder = servletHandler.getServlet("servlet"); + assertThat(servletHolder.getServlet()).isEqualTo(servlet); + } finally { + servletHandler.stop(); + } } @Test - public void addsServletClasses() throws Exception { - final ServletRegistration.Dynamic builder = environment.addServlet("servlet", GenericServlet.class); - assertThat(builder) - .isNotNull(); - - final ArgumentCaptor holder = ArgumentCaptor.forClass(ServletHolder.class); - verify(servletHandler).addServlet(holder.capture()); - - assertThat(holder.getValue().getName()) - .isEqualTo("servlet"); - - // this is ugly, but comparing classes sucks with these type bounds - assertThat(holder.getValue().getHeldClass().equals(GenericServlet.class)) - .isTrue(); + void addsServletClasses() throws Exception { + final Class servletClass = ConcatServlet.class; + final ServletRegistration.Dynamic builder = environment.addServlet("servlet", servletClass); + assertThat(builder).isNotNull(); + + try { + servletHandler.start(); + + final ServletHolder servletHolder = servletHandler.getServlet("servlet"); + assertThat(servletHolder.getServlet()).isExactlyInstanceOf(servletClass); + } finally { + servletHandler.stop(); + } } @Test - public void addsFilterInstances() throws Exception { + void addsFilterInstances() throws Exception { final Filter filter = new WelcomeFilter(); final FilterRegistration.Dynamic builder = environment.addFilter("filter", filter); - assertThat(builder) - .isNotNull(); - - final ArgumentCaptor holder = ArgumentCaptor.forClass(FilterHolder.class); - verify(servletHandler).addFilter(holder.capture()); + assertThat(builder).isNotNull(); - assertThat(holder.getValue().getName()) - .isEqualTo("filter"); + try { + servletHandler.start(); - assertThat(holder.getValue()).hasFieldOrPropertyWithValue("_instance", filter); + final FilterHolder filterHolder = servletHandler.getFilter("filter"); + assertThat(filterHolder.getFilter()).isEqualTo(filter); + } finally { + servletHandler.stop(); + } } @Test - public void addsFilterClasses() throws Exception { - final FilterRegistration.Dynamic builder = environment.addFilter("filter", WelcomeFilter.class); - assertThat(builder) - .isNotNull(); - - final ArgumentCaptor holder = ArgumentCaptor.forClass(FilterHolder.class); - verify(servletHandler).addFilter(holder.capture()); - - assertThat(holder.getValue().getName()) - .isEqualTo("filter"); - - // this is ugly, but comparing classes sucks with these type bounds - assertThat(holder.getValue().getHeldClass().equals(WelcomeFilter.class)) - .isTrue(); + void addsFilterClasses() throws Exception { + final Class filterClass = WelcomeFilter.class; + final FilterRegistration.Dynamic builder = environment.addFilter("filter", filterClass); + assertThat(builder).isNotNull(); + + try { + servletHandler.start(); + + final FilterHolder filterHolder = servletHandler.getFilter("filter"); + assertThat(filterHolder.getFilter()).isExactlyInstanceOf(filterClass); + } finally { + servletHandler.stop(); + } } @Test - public void addsServletListeners() throws Exception { - final ServletContextListener listener = mock(ServletContextListener.class); + void addsServletListeners() { + final ServletContextListener listener = new DebugListener(); environment.addServletListeners(listener); - verify(handler).addEventListener(listener); + assertThat(handler.getEventListeners()).contains(listener); } @Test - public void addsProtectedTargets() throws Exception { + void addsProtectedTargets() { environment.setProtectedTargets("/woo"); - verify(handler).setProtectedTargets(new String[]{"/woo"}); + assertThat(handler.getProtectedTargets()).contains("/woo"); } @Test - public void setsBaseResource(@TempDir Path tempDir) throws Exception { + void setsBaseResource(@TempDir Path tempDir) throws Exception { final Resource testResource = Resource.newResource(tempDir.resolve("dir").toUri()); environment.setBaseResource(testResource); - verify(handler).setBaseResource(testResource); + assertThat(handler.getBaseResource()).isEqualTo(testResource); } @Test - public void setsBaseResourceList(@TempDir Path tempDir) throws Exception { + void setsBaseResourceList(@TempDir Path tempDir) throws Exception { Resource wooResource = Resource.newResource(Files.createDirectory(tempDir.resolve("dir-1"))); Resource fooResource = Resource.newResource(Files.createDirectory(tempDir.resolve("dir-2"))); final Resource[] testResources = new Resource[]{wooResource, fooResource}; environment.setBaseResource(testResources); - ArgumentCaptor captor = ArgumentCaptor.forClass(Resource.class); - verify(handler).setBaseResource(captor.capture()); - - Resource actualResource = captor.getValue(); - assertThat(actualResource).isInstanceOf(ResourceCollection.class); - - ResourceCollection actualResourceCollection = (ResourceCollection) actualResource; - assertThat(actualResourceCollection.getResources()).contains(wooResource, fooResource); - + assertThat(handler.getBaseResource()).isExactlyInstanceOf(ResourceCollection.class); + assertThat(((ResourceCollection) handler.getBaseResource()).getResources()).contains(wooResource, fooResource); } @Test - public void setsResourceBase() throws Exception { + void setsResourceBase() throws Exception { environment.setResourceBase("/woo"); - verify(handler).setResourceBase("/woo"); + assertThat(handler.getResourceBase()).isEqualTo(handler.newResource("/woo").toString()); } @Test - public void setsBaseResourceStringList(@TempDir Path tempDir) throws Exception { + void setsBaseResourceStringList(@TempDir Path tempDir) throws Exception { String wooResource = Files.createDirectory(tempDir.resolve("dir-1")).toString(); String fooResource = Files.createDirectory(tempDir.resolve("dir-2")).toString(); final String[] testResources = new String[]{wooResource, fooResource}; environment.setBaseResource(testResources); - ArgumentCaptor captor = ArgumentCaptor.forClass(Resource.class); - verify(handler).setBaseResource(captor.capture()); - - Resource actualResource = captor.getValue(); - assertThat(actualResource).isInstanceOf(ResourceCollection.class); - - ResourceCollection actualResourceCollection = (ResourceCollection) actualResource; - assertThat(actualResourceCollection.getResources()).contains(Resource.newResource(wooResource), - Resource.newResource(fooResource)); - + assertThat(handler.getBaseResource()).isExactlyInstanceOf(ResourceCollection.class); + assertThat(((ResourceCollection) handler.getBaseResource()).getResources()) + .contains(Resource.newResource(wooResource), Resource.newResource(fooResource)); } @Test - public void setsInitParams() throws Exception { + void setsInitParams() { environment.setInitParameter("a", "b"); - verify(handler).setInitParameter("a", "b"); + assertThat(handler.getInitParameter("a")).isEqualTo("b"); } @Test - public void setsSessionHandlers() throws Exception { - final SessionHandler sessionHandler = mock(SessionHandler.class); - + void setsSessionHandlers() { + final SessionHandler sessionHandler = new SessionHandler(); environment.setSessionHandler(sessionHandler); - verify(handler).setSessionHandler(sessionHandler); - verify(handler).setSessionsEnabled(true); + assertThat(handler.getSessionHandler()).isEqualTo(sessionHandler); + assertThat(handler.isSessionsEnabled()).isTrue(); } @Test - public void setsSecurityHandlers() throws Exception { - final SecurityHandler securityHandler = mock(SecurityHandler.class); - + void setsSecurityHandlers() { + final SecurityHandler securityHandler = new ConstraintSecurityHandler(); environment.setSecurityHandler(securityHandler); - verify(handler).setSecurityHandler(securityHandler); - verify(handler).setSecurityEnabled(true); + assertThat(handler.getSecurityHandler()).isEqualTo(securityHandler); + assertThat(handler.isSecurityEnabled()).isTrue(); } @Test - public void addsMimeMapping() { - final MimeTypes mimeTypes = mock(MimeTypes.class); - when(handler.getMimeTypes()).thenReturn(mimeTypes); - - environment.addMimeMapping("example/foo", "foo"); + void addsMimeMapping() { + environment.addMimeMapping("foo", "example/foo"); - verify(mimeTypes).addMimeMapping("example/foo", "foo"); + assertThat(handler.getMimeTypes().getMimeMap()).containsEntry("foo", "example/foo"); } }