Remove logback-access from the core framework

dropwizard-bom/pom.xml

@@ -107,6 +107,11 @@
                 <artifactId>dropwizard-request-logging</artifactId>
                 <version>5.0.0-SNAPSHOT</version>
             </dependency>
+            <dependency>
+                <groupId>io.dropwizard</groupId>
+                <artifactId>dropwizard-request-logging-logback-access</artifactId>
+                <version>5.0.0-SNAPSHOT</version>
+            </dependency>
             <dependency>
                 <groupId>io.dropwizard</groupId>
                 <artifactId>dropwizard-json-logging</artifactId>

dropwizard-core/src/main/java/io/dropwizard/core/server/AbstractServerFactory.java

@@ -20,10 +20,8 @@
 import io.dropwizard.metrics.servlets.AdminServlet;
 import io.dropwizard.metrics.servlets.HealthCheckServlet;
 import io.dropwizard.metrics.servlets.MetricsServlet;
-import io.dropwizard.request.logging.LogbackAccessRequestLog;
-import io.dropwizard.request.logging.LogbackAccessRequestLogAwareHandler;
-import io.dropwizard.request.logging.LogbackAccessRequestLogFactory;
 import io.dropwizard.request.logging.RequestLogFactory;
+import io.dropwizard.request.logging.old.LogbackClassicRequestLogFactory;
 import io.dropwizard.servlets.ThreadNameFilter;
 import io.dropwizard.util.Duration;
 import io.dropwizard.validation.MinDuration;
@@ -56,6 +54,7 @@
 import java.lang.reflect.InvocationTargetException;
 import java.util.EnumSet;
 import java.util.Optional;
+import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.Executors;
@@ -327,7 +326,7 @@ public boolean isThreadPoolSizedCorrectly() {
     public synchronized RequestLogFactory<?> getRequestLogFactory() {
         if (requestLog == null) {
             // Lazy init to avoid a hard dependency to logback
-            requestLog = new LogbackAccessRequestLogFactory();
+            requestLog = new LogbackClassicRequestLogFactory();
         }
         return requestLog;
     }
@@ -734,9 +733,6 @@ protected SetUIDListener buildSetUIDListener() {
     protected void addRequestLog(Server server, String name, MutableServletContextHandler servletContextHandler) {
         if (getRequestLogFactory().isEnabled()) {
             RequestLog log = getRequestLogFactory().build(name);
-            if (log instanceof LogbackAccessRequestLog) {
-                servletContextHandler.insertHandler(new LogbackAccessRequestLogAwareHandler());
-            }
             server.setRequestLog(log);
         }
     }
@@ -768,4 +764,14 @@ protected void printBanner(String name) {
         }
         LOGGER.info(msg);
     }
+
+    protected final Handler customizeHandlerChain(Handler first) {
+
+        ServiceLoader<ServerCustomizer> serverCustomizers = ServiceLoader.load(ServerCustomizer.class);
+        Handler result = first;
+        for (ServerCustomizer serverCustomizer : serverCustomizers) {
+            result = serverCustomizer.customizeHandlerChain(result);
+        }
+        return result;
+    }
 }

dropwizard-core/src/main/java/io/dropwizard/core/server/DefaultServerFactory.java

@@ -192,7 +192,7 @@ public Server build(Environment environment) {
                                                                   applicationHandler,
                                                                   adminHandler);
         final Handler gzipHandler = buildGzipHandler(routingHandler);
-        server.setHandler(addGracefulHandler(gzipHandler));
+        server.setHandler(customizeHandlerChain(addGracefulHandler(gzipHandler)));
         addRequestLog(server, environment.getName(), environment.getApplicationContext());
         return server;
     }

dropwizard-core/src/main/java/io/dropwizard/core/server/ServerCustomizer.java

@@ -0,0 +1,19 @@
+package io.dropwizard.core.server;
+
+import org.eclipse.jetty.server.Handler;
+
+/**
+ * A customizer for {@link org.eclipse.jetty.server.Server} objects to be called in a {@link ServerFactory}.
+ */
+public interface ServerCustomizer {
+
+    /**
+     * Allows customizing the server's handler chain. The first handler of the current handler chain built by a
+     * {@link ServerFactory} is provided to allow adding a handler at an arbitrary point of the handler chain.
+     * The execution order of {@link ServerCustomizer ServerCustomizers} is not guaranteed.
+     *
+     * @param first the currently first handler in the {@link org.eclipse.jetty.server.Server Server's} handler chain
+     * @return the new first handler of the handler chain
+     */
+    Handler customizeHandlerChain(Handler first);
+}

dropwizard-core/src/main/java/io/dropwizard/core/server/SimpleServerFactory.java

@@ -132,7 +132,7 @@ public Server build(Environment environment) {
                 adminContextPath, adminHandler);
         final ContextRoutingHandler routingHandler = new ContextRoutingHandler(handlers);
         final Handler gzipHandler = buildGzipHandler(routingHandler);
-        server.setHandler(addGracefulHandler(gzipHandler));
+        server.setHandler(customizeHandlerChain(addGracefulHandler(gzipHandler)));
         addRequestLog(server, environment.getName(), environment.getApplicationContext());
 
         return server;

dropwizard-e2e/pom.xml

@@ -180,6 +180,11 @@
             <artifactId>dropwizard-client</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.dropwizard</groupId>
+            <artifactId>dropwizard-request-logging-logback-access</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <build>

dropwizard-e2e/src/test/java/com/example/request_log/ClassicRequestLogIntegrationTest.java

@@ -20,13 +20,6 @@ class ClassicRequestLogIntegrationTest extends AbstractRequestLogPatternIntegrat
             "127\\.0\\.0\\.1 - - \\[.+\\] \"GET /greet HTTP/1\\.1\" 200 15 \"-\" \"TestApplication \\(test-request-logs\\)\" \\d+"
     );
 
-    @Override
-    protected List<ConfigOverride> configOverrides() {
-        final List<ConfigOverride> configOverrides = new ArrayList<>(super.configOverrides());
-        configOverrides.add(ConfigOverride.config("server.requestLog.type", "classic"));
-        return configOverrides;
-    }
-
     @Test
     void testDefaultPattern() throws Exception {
         String url = String.format("http://localhost:%d/greet?name=Charley", dropwizardAppRule.getLocalPort());

dropwizard-e2e/src/test/java/com/example/request_log/CustomRequestLogPatternIntegrationTest.java

@@ -20,6 +20,7 @@ class CustomRequestLogPatternIntegrationTest extends AbstractRequestLogPatternIn
     protected List<ConfigOverride> configOverrides() {
         final List<ConfigOverride> configOverrides = new ArrayList<>(super.configOverrides());
         configOverrides.add(ConfigOverride.config("server.requestLog.appenders[0].logFormat", LOG_FORMAT));
+        configOverrides.add(ConfigOverride.config("server.requestLog.type", "logback-access"));
         return configOverrides;
     }
 

dropwizard-e2e/src/test/java/com/example/request_log/RequestLogPatternIntegrationTest.java

@@ -1,11 +1,13 @@
 package com.example.request_log;
 
+import io.dropwizard.testing.ConfigOverride;
 import jakarta.ws.rs.core.HttpHeaders;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.condition.DisabledOnOs;
 import org.junit.jupiter.api.condition.OS;
 
 import java.nio.file.Files;
+import java.util.ArrayList;
 import java.util.Base64;
 import java.util.List;
 import java.util.regex.Pattern;
@@ -20,6 +22,13 @@ class RequestLogPatternIntegrationTest extends AbstractRequestLogPatternIntegrat
         "127\\.0\\.0\\.1 - - \\[.+\\] \"GET /greet\\?name=Charley HTTP/1\\.1\" 200 15 \"-\" \"TestApplication \\(test-request-logs\\)\" (-)?\\d+"
     );
 
+    @Override
+    protected List<ConfigOverride> configOverrides() {
+        final List<ConfigOverride> configOverrides = new ArrayList<>(super.configOverrides());
+        configOverrides.add(ConfigOverride.config("server.requestLog.type", "logback-access"));
+        return configOverrides;
+    }
+
     @Test
     void testDefaultPattern() throws Exception {
         String url = String.format("http://localhost:%d/greet?name=Charley", dropwizardAppRule.getLocalPort());

dropwizard-json-logging/pom.xml

@@ -26,10 +26,6 @@
             <artifactId>dropwizard-logging</artifactId>
         </dependency>
 
-        <dependency>
-            <groupId>ch.qos.logback</groupId>
-            <artifactId>logback-access</artifactId>
-        </dependency>
         <dependency>
             <groupId>ch.qos.logback</groupId>
             <artifactId>logback-classic</artifactId>

dropwizard-json-logging/src/main/resources/META-INF/services/io.dropwizard.logging.common.layout.DiscoverableLayoutFactory

@@ -1,2 +1 @@
-io.dropwizard.logging.json.AccessJsonLayoutBaseFactory
 io.dropwizard.logging.json.EventJsonLayoutBaseFactory

dropwizard-json-logging/src/test/java/io/dropwizard/logging/json/LayoutIntegrationTests.java

@@ -1,6 +1,5 @@
 package io.dropwizard.logging.json;
 
-import ch.qos.logback.access.spi.IAccessEvent;
 import ch.qos.logback.classic.Level;
 import ch.qos.logback.classic.spi.ILoggingEvent;
 import ch.qos.logback.core.spi.DeferredProcessingAware;
@@ -16,21 +15,9 @@
 import io.dropwizard.logging.common.BootstrapLogging;
 import io.dropwizard.logging.common.ConsoleAppenderFactory;
 import io.dropwizard.logging.common.DefaultLoggingFactory;
-import io.dropwizard.request.logging.LogbackAccessRequestLogFactory;
 import io.dropwizard.validation.BaseValidator;
-import org.eclipse.jetty.ee10.servlet.ServletChannel;
-import org.eclipse.jetty.ee10.servlet.ServletContextHandler;
-import org.eclipse.jetty.ee10.servlet.ServletContextRequest;
-import org.eclipse.jetty.http.HttpFields;
-import org.eclipse.jetty.http.HttpURI;
-import org.eclipse.jetty.server.ConnectionMetaData;
-import org.eclipse.jetty.server.HttpConfiguration;
-import org.eclipse.jetty.server.Request;
-import org.eclipse.jetty.server.RequestLog;
-import org.eclipse.jetty.server.Response;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import org.mockito.MockedStatic;
 import org.slf4j.LoggerFactory;
 import org.slf4j.Marker;
 import org.slf4j.MarkerFactory;
@@ -39,17 +26,12 @@
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import java.util.Collections;
-import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.assertj.core.api.Assertions.entry;
 import static org.awaitility.Awaitility.await;
-import static org.mockito.Mockito.CALLS_REAL_METHODS;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.mockStatic;
-import static org.mockito.Mockito.when;
 
 class LayoutIntegrationTests {
 
@@ -65,7 +47,7 @@ class LayoutIntegrationTests {
 
     @BeforeEach
     void setUp() {
-        objectMapper.getSubtypeResolver().registerSubtypes(AccessJsonLayoutBaseFactory.class, EventJsonLayoutBaseFactory.class);
+        objectMapper.getSubtypeResolver().registerSubtypes(EventJsonLayoutBaseFactory.class);
     }
 
     @SuppressWarnings("unchecked")
@@ -102,36 +84,6 @@ void testDeserializeJson() throws Exception {
                 .satisfies(exceptionFormat -> assertThat(exceptionFormat.getEvaluators()).contains("io.dropwizard")));
     }
 
-    @Test
-    void testDeserializeAccessJson() throws Exception {
-        assertThat(getAppenderFactory("yaml/json-access-log.yml"))
-            .extracting(ConsoleAppenderFactory::getLayout)
-            .isInstanceOfSatisfying(AccessJsonLayoutBaseFactory.class, accessJsonLayoutBaseFactory -> assertThat(accessJsonLayoutBaseFactory)
-                .satisfies(factory -> assertThat(factory.getTimestampFormat()).isEqualTo("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"))
-                .satisfies(factory -> assertThat(factory.isPrettyPrint()).isFalse())
-                .satisfies(factory -> assertThat(factory.isAppendLineSeparator()).isTrue())
-                .satisfies(factory -> assertThat(factory.getIncludes()).contains(
-                    AccessAttribute.TIMESTAMP,
-                    AccessAttribute.REMOTE_USER,
-                    AccessAttribute.STATUS_CODE,
-                    AccessAttribute.METHOD,
-                    AccessAttribute.REQUEST_URL,
-                    AccessAttribute.REMOTE_HOST,
-                    AccessAttribute.REQUEST_PARAMETERS,
-                    AccessAttribute.REQUEST_CONTENT,
-                    AccessAttribute.TIMESTAMP,
-                    AccessAttribute.USER_AGENT,
-                    AccessAttribute.PATH_QUERY))
-                .satisfies(factory -> assertThat(factory.getResponseHeaders()).containsOnly("X-Request-Id"))
-                .satisfies(factory -> assertThat(factory.getRequestHeaders()).containsOnly("User-Agent", "X-Request-Id"))
-                .satisfies(factory -> assertThat(factory.getCustomFieldNames()).containsOnly(
-                    entry("statusCode", "status_code"),
-                    entry("userAgent", "user_agent")))
-                .satisfies(factory -> assertThat(factory.getAdditionalFields()).containsOnly(
-                    entry("service-name", "shipping-service"),
-                    entry("service-version", "1.2.3"))));
-    }
-
     @Test
     void testLogJsonToConsole() throws Exception {
         ConsoleAppenderFactory<ILoggingEvent> consoleAppenderFactory = getAppenderFactory("yaml/json-log-default.yml");
@@ -176,82 +128,6 @@ void testLogJsonToConsole() throws Exception {
         }
     }
 
-    @Test
-    void testLogAccessJsonToConsole() throws Exception {
-        ConsoleAppenderFactory<IAccessEvent> consoleAppenderFactory = getAppenderFactory("yaml/json-access-log-default.yml");
-        // Use sys.err, because there are some other log configuration messages in std.out
-        consoleAppenderFactory.setTarget(ConsoleAppenderFactory.ConsoleStream.STDERR);
-
-        final LogbackAccessRequestLogFactory requestLogHandler = new LogbackAccessRequestLogFactory();
-        requestLogHandler.setAppenders(Collections.singletonList(consoleAppenderFactory));
-
-        PrintStream old = System.err;
-        ByteArrayOutputStream redirectedStream = new ByteArrayOutputStream();
-        try {
-            System.setErr(new PrintStream(redirectedStream));
-            RequestLog requestLog = requestLogHandler.build("json-access-log-test");
-
-            Request request = mock(Request.class);
-            Response response = mock(Response.class);
-            ConnectionMetaData connectionMetaData = mock(ConnectionMetaData.class);
-            HttpURI httpURI = mock(HttpURI.class);
-
-            when(response.getHeaders()).thenReturn(HttpFields.build());
-            when(request.getConnectionMetaData()).thenReturn(connectionMetaData);
-
-            when(httpURI.getPath()).thenReturn("/test/users");
-            when(httpURI.getQuery()).thenReturn("age=22&city=LA");
-            when(httpURI.getAuthority()).thenReturn("10.0.0.1");
-            when(request.getHttpURI()).thenReturn(httpURI);
-
-            when(connectionMetaData.getHttpConfiguration()).thenReturn(new HttpConfiguration());
-            when(connectionMetaData.getProtocol()).thenReturn("HTTP/1.1");
-
-            HttpFields requestHeaders = HttpFields.build()
-                .add("Connection", "keep-alive")
-                .add("User-Agent", "Mozilla/5.0");
-            when(request.getHeaders()).thenReturn(requestHeaders);
-            when(request.getAttributeNameSet()).thenReturn(Set.of());
-            when(request.getMethod()).thenReturn("GET");
-
-            HttpFields.Mutable responseHeaders = HttpFields.build()
-                .add("Date", "Mon, 16 Nov 2012 05:00:48 GMT")
-                .add("Server", "Apache/2.4.12");
-            when(response.getHeaders()).thenReturn(responseHeaders);
-            when(response.getStatus()).thenReturn(200);
-
-            final ServletContextHandler servletContextHandler = new ServletContextHandler();
-            final ServletChannel servletChannel = new ServletChannel(servletContextHandler, connectionMetaData);
-            ServletContextRequest servletContextRequest = new TestServletContextRequest(servletContextHandler, servletChannel, request, response);
-            servletChannel.associate(servletContextRequest);
-
-            try (MockedStatic<Request> staticRequest = mockStatic(Request.class, CALLS_REAL_METHODS); MockedStatic<Response> staticResponse = mockStatic(Response.class)) {
-                staticRequest.when(() -> Request.getRemoteAddr(servletContextRequest)).thenReturn("10.0.0.1");
-                staticRequest.when(() -> Request.getTimeStamp(servletContextRequest)).thenReturn(TimeUnit.SECONDS.toMillis(1353042047));
-                staticResponse.when(() -> Response.getContentBytesWritten(response)).thenReturn(8290L);
-
-                requestLog.log(servletContextRequest, response);
-                // Need to wait, because the logger is async
-                await().atMost(1, TimeUnit.SECONDS).until(() -> !redirectedStream.toString().isEmpty());
-            }
-
-            JsonNode jsonNode = objectMapper.readTree(redirectedStream.toString());
-            assertThat(jsonNode.fieldNames().next()).isEqualTo("timestamp");
-            assertThat(jsonNode.get("timestamp").isNumber()).isTrue();
-            assertThat(jsonNode.get("requestTime").isNumber()).isTrue();
-            assertThat(jsonNode.get("remoteAddress").asText()).isEqualTo("10.0.0.1");
-            assertThat(jsonNode.get("status").asInt()).isEqualTo(200);
-            assertThat(jsonNode.get("method").asText()).isEqualTo("GET");
-            assertThat(jsonNode.get("uri").asText()).isEqualTo("/test/users");
-            assertThat(jsonNode.get("protocol").asText()).isEqualTo("HTTP/1.1");
-            assertThat(jsonNode.get("userAgent").asText()).isEqualTo("Mozilla/5.0");
-            assertThat(jsonNode.get("contentLength").asInt()).isEqualTo(8290);
-        } finally {
-            System.setErr(old);
-        }
-
-    }
-
     @Test
     void invalidJsonLogLayoutField() {
         assertThatExceptionOfType(ConfigurationValidationException.class)
@@ -266,17 +142,4 @@ public static class CustomJsonLayoutBaseFactory extends EventJsonLayoutBaseFacto
         @Min(1)
         private int messageSize = 8000;
     }
-
-    public static class TestServletContextRequest extends ServletContextRequest {
-
-        public TestServletContextRequest(ServletContextHandler servletContextHandler, ServletChannel servletChannel, Request request, Response response) {
-            super(servletContextHandler.newServletContextApi(), servletChannel, request, response,
-                null, null, null);
-        }
-
-        @Override
-        public Set<String> getAttributeNameSet() {
-            return Set.of();
-        }
-    }
 }