This is an automated email from the ASF dual-hosted git repository.
dsoumis pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new f928ba6794 Improve test coverage for AccessLogValve and
PersistentValve (#979)
f928ba6794 is described below
commit f928ba67940ffd0a543c233b650605e112ef3588
Author: Onkar Raghunath Shelke <[email protected]>
AuthorDate: Thu Apr 23 18:30:02 2026 +0530
Improve test coverage for AccessLogValve and PersistentValve (#979)
* Improve test coverage for AccessLogValve and PersistentValve
---------
Co-authored-by: Onkar <[email protected]>
Co-authored-by: Dimitris Soumis <[email protected]>
---
.../catalina/valves/TestAccessLogValveFile.java | 271 +++++++++++++++++++++
.../catalina/valves/TestPersistentValve.java | 134 +++++++++-
2 files changed, 398 insertions(+), 7 deletions(-)
diff --git a/test/org/apache/catalina/valves/TestAccessLogValveFile.java
b/test/org/apache/catalina/valves/TestAccessLogValveFile.java
new file mode 100644
index 0000000000..719aeb6639
--- /dev/null
+++ b/test/org/apache/catalina/valves/TestAccessLogValveFile.java
@@ -0,0 +1,271 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.valves;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+
+/**
+ * Tests for {@link AccessLogValve} file I/O, rotation, encoding, and cleanup
+ * operations. Pattern-based access log format tests are covered by
+ * {@link TestAccessLogValve}.
+ */
+public class TestAccessLogValveFile extends TomcatBaseTest {
+
+ private File logDir;
+
+
+ @Test
+ public void testLogWritesToFile() throws Exception {
+ createValve("access", Constants.AccessLog.COMBINED_ALIAS);
+ getTomcatInstance().start();
+
+ getUrl("http://localhost:"; + getPort());
+
+ File logFile = new File(logDir, "access.log");
+ awaitFile(logFile);
+
+ String content = Files.readString(logFile.toPath());
+ Assert.assertTrue(content.contains("200"));
+ Assert.assertTrue(content.contains("GET"));
+ }
+
+
+ @Test
+ @SuppressWarnings("unused")
+ public void testRotateWithNewFileName() throws Exception {
+ AccessLogValve valve = createValve("access", "%s");
+ getTomcatInstance().start();
+
+ getUrl("http://localhost:"; + getPort());
+ File logFile = new File(logDir, "access.log");
+ awaitFile(logFile);
+
+ File rotatedFile = new File(logDir, "access_rotated.log");
+ Assert.assertTrue(valve.rotate(rotatedFile.getAbsolutePath()));
+ Assert.assertTrue(rotatedFile.exists());
+
+ getUrl("http://localhost:"; + getPort());
+ awaitFile(logFile);
+
+ File[] logFiles = logDir.listFiles((dir, name) ->
name.startsWith("access") && name.endsWith(".log"));
+ Assert.assertNotNull(logFiles);
+ Assert.assertEquals(2, logFiles.length);
+ }
+
+
+ @Test
+ public void testRotateReturnsFalseWhenNoLogFile() {
+ AccessLogValve valve = new AccessLogValve();
+ // currentLogFile will be null because the valve was never started
+ boolean result = valve.rotate("nonexistent.log");
+ Assert.assertFalse("rotate() should return false when no log file",
result);
+ }
+
+
+ @Test
+ @SuppressWarnings("unused")
+ public void testRenameOnRotate() throws Exception {
+ AccessLogValve valve = createValve("access", "%s");
+ valve.setRotatable(true);
+ valve.setRenameOnRotate(true);
+ getTomcatInstance().start();
+
+ getUrl("http://localhost:"; + getPort());
+
+ // With renameOnRotate, the active log has no date stamp
+ File logFile = new File(logDir, "access.log");
+ awaitFile(logFile);
+ Assert.assertTrue(logFile.exists());
+
+ File[] datedFiles = logDir.listFiles(
+ (dir, name) -> name.startsWith("access.") &&
name.endsWith(".log")
+ && name.length() > "access.log".length());
+ Assert.assertTrue("No dated log file should exist before rotation",
+ datedFiles == null || datedFiles.length == 0);
+ }
+
+
+ @Test
+ public void testMaxDaysCleanup() throws Exception {
+ AccessLogValve valve = createValve("access", "%s");
+ valve.setRotatable(true);
+ valve.setMaxDays(1);
+
+ File oldLog = new File(logDir, "access.old.log");
+ Assert.assertTrue(oldLog.createNewFile());
+ Assert.assertTrue(oldLog.setLastModified(System.currentTimeMillis() -
10L * 24 * 60 * 60 * 1000));
+
+ getTomcatInstance().start();
+
+ valve.backgroundProcess();
+
+ Assert.assertFalse(oldLog.exists());
+ }
+
+
+ @Test
+ public void testBufferedFlush() throws Exception {
+ AccessLogValve valve = createValve("access_buffered", "%s");
+ valve.setBuffered(true);
+ getTomcatInstance().start();
+
+ getUrl("http://localhost:"; + getPort());
+
+ // Flush via backgroundProcess since buffered=true
+ valve.backgroundProcess();
+
+ File logFile = new File(logDir, "access_buffered.log");
+ awaitFile(logFile);
+ String content = Files.readString(logFile.toPath());
+ Assert.assertTrue(content.contains("200"));
+ }
+
+
+ @Test
+ public void testCheckExists() throws Exception {
+ AccessLogValve valve = createValve("access_check", "%s");
+ valve.setCheckExists(true);
+ getTomcatInstance().start();
+
+ getUrl("http://localhost:"; + getPort());
+ File logFile = new File(logDir, "access_check.log");
+ awaitFile(logFile);
+
+ Assert.assertTrue(logFile.delete());
+ Assert.assertFalse(logFile.exists());
+
+ getUrl("http://localhost:"; + getPort());
+ awaitFile(logFile);
+ Assert.assertTrue(logFile.exists());
+ }
+
+
+ @Test
+ public void testCustomEncoding() throws Exception {
+ AccessLogValve valve = createValve("access_iso", "%s");
+ valve.setEncoding("ISO-8859-1");
+ getTomcatInstance().start();
+
+ getUrl("http://localhost:"; + getPort());
+
+ File logFile = new File(logDir, "access_iso.log");
+ awaitFile(logFile);
+
+ String content = Files.readString(logFile.toPath(),
StandardCharsets.ISO_8859_1);
+ Assert.assertTrue(content.contains("200"));
+ }
+
+ @Test
+ public void testGetSetProperties() {
+ AccessLogValve valve = new AccessLogValve();
+
+ Assert.assertEquals("logs", valve.getDirectory());
+ valve.setDirectory("custom-dir");
+ Assert.assertEquals("custom-dir", valve.getDirectory());
+
+ Assert.assertEquals("access_log", valve.getPrefix());
+ valve.setPrefix("myapp");
+ Assert.assertEquals("myapp", valve.getPrefix());
+
+ Assert.assertEquals("", valve.getSuffix());
+ valve.setSuffix(".txt");
+ Assert.assertEquals(".txt", valve.getSuffix());
+
+ Assert.assertTrue(valve.isRotatable());
+ valve.setRotatable(false);
+ Assert.assertFalse(valve.isRotatable());
+
+ Assert.assertFalse(valve.isRenameOnRotate());
+ valve.setRenameOnRotate(true);
+ Assert.assertTrue(valve.isRenameOnRotate());
+
+ Assert.assertTrue(valve.isBuffered());
+ valve.setBuffered(false);
+ Assert.assertFalse(valve.isBuffered());
+
+ Assert.assertFalse(valve.isCheckExists());
+ valve.setCheckExists(true);
+ Assert.assertTrue(valve.isCheckExists());
+
+ Assert.assertEquals(".yyyy-MM-dd", valve.getFileDateFormat());
+ valve.setFileDateFormat(".yyyyMMdd");
+ Assert.assertEquals(".yyyyMMdd", valve.getFileDateFormat());
+
+ Assert.assertEquals(-1, valve.getMaxDays());
+ valve.setMaxDays(30);
+ Assert.assertEquals(30, valve.getMaxDays());
+
+ Assert.assertNull(valve.getEncoding());
+ valve.setEncoding("UTF-16");
+ Assert.assertEquals("UTF-16", valve.getEncoding());
+ }
+
+
+ /**
+ * Creates an {@link AccessLogValve} with common defaults (non-rotatable,
+ * unbuffered) attached to a Tomcat instance ready to start.
+ */
+ private AccessLogValve createValve(String prefix, String pattern)
+ throws Exception {
+ Tomcat tomcat = getTomcatInstance();
+ Context ctx = getProgrammaticRootContext();
+ Tomcat.addServlet(ctx, "hello", new HelloWorldServlet());
+ ctx.addServletMappingDecoded("/", "hello");
+
+ logDir = getLogDir();
+ AccessLogValve valve = new AccessLogValve();
+ valve.setDirectory(logDir.getAbsolutePath());
+ valve.setPrefix(prefix);
+ valve.setSuffix(".log");
+ valve.setRotatable(false);
+ valve.setBuffered(false);
+ valve.setPattern(pattern);
+ tomcat.getHost().getPipeline().addValve(valve);
+ return valve;
+ }
+
+
+ private File getLogDir() throws IOException {
+ File dir = new File(getTemporaryDirectory(), "access-log-test");
+ if (!dir.mkdirs() && !dir.isDirectory()) {
+ throw new IOException("Failed to create log directory: " + dir);
+ }
+ addDeleteOnTearDown(dir);
+ return dir;
+ }
+
+ @SuppressWarnings("BusyWait")
+ private static void awaitFile(File file) throws InterruptedException {
+ long deadline = System.currentTimeMillis() + 2000;
+ while (!file.exists() || file.length() == 0) {
+ if (System.currentTimeMillis() > deadline) {
+ Assert.fail("Timed out waiting for " + file.getName());
+ }
+ Thread.sleep(2);
+ }
+ }
+}
diff --git a/test/org/apache/catalina/valves/TestPersistentValve.java
b/test/org/apache/catalina/valves/TestPersistentValve.java
index b50c17b0cb..49359a1ceb 100644
--- a/test/org/apache/catalina/valves/TestPersistentValve.java
+++ b/test/org/apache/catalina/valves/TestPersistentValve.java
@@ -1,4 +1,5 @@
-/* Licensed to the Apache Software Foundation (ASF) under one or more
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
@@ -25,6 +26,7 @@ import org.junit.Test;
import org.apache.catalina.connector.Request;
import org.apache.catalina.connector.Response;
+import org.apache.catalina.session.StandardSession;
import org.apache.tomcat.unittest.TesterRequest;
import org.apache.tomcat.unittest.TesterResponse;
@@ -61,27 +63,145 @@ public class TestPersistentValve {
});
}
+ for (Thread thread : threads) {
+ thread.start();
+ }
+
+ for (Thread thread : threads) {
+ thread.join();
+ }
+
+ Assert.assertEquals(1, testerValve.getMaximumConcurrency());
+ }
+
+
+ @Test
+ public void testFilterMatchBypassesSession() throws Exception {
+ PersistentValve pv = new PersistentValve();
+ Request request = new TesterRequest();
+ Response response = new TesterResponse();
+ TesterValve testerValve = new TesterValve();
+
+ pv.setFilter(".*\\.html");
+ request.setRequestedSessionId("1234");
+
+ pv.setContainer(request.getContext());
+ pv.setNext(testerValve);
+ pv.init();
+
+ Thread[] threads = new Thread[2];
+
for (int i = 0; i < threads.length; i++) {
- threads[i].start();
+ threads[i] = new Thread(() -> {
+ try {
+ pv.invoke(request, response);
+ } catch (IOException | ServletException e) {
+ throw new RuntimeException(e);
+ }
+ });
+ }
+
+ for (Thread thread : threads) {
+ thread.start();
}
+ for (Thread thread : threads) {
+ thread.join();
+ }
+
+ Assert.assertEquals(2, testerValve.getMaximumConcurrency());
+ }
+
+
+ @Test
+ public void testFilterNoMatchProcessesSession() throws Exception {
+ PersistentValve pv = new PersistentValve();
+ Request request = new TesterRequest();
+ Response response = new TesterResponse();
+ TesterValve testerValve = new TesterValve();
+
+ // The filter defines which requests don't need sessions (e.g. static
resources).
+ pv.setFilter(".*\\css");
+ request.setRequestedSessionId("1234");
+
+ pv.setContainer(request.getContext());
+ pv.setNext(testerValve);
+ pv.init();
+
+ Thread[] threads = new Thread[2];
+
for (int i = 0; i < threads.length; i++) {
- threads[i].join();
+ threads[i] = new Thread(() -> {
+ try {
+ pv.invoke(request, response);
+ } catch (IOException | ServletException e) {
+ throw new RuntimeException(e);
+ }
+ });
+ }
+
+ for (Thread thread : threads) {
+ thread.start();
+ }
+
+ for (Thread thread : threads) {
+ thread.join();
}
Assert.assertEquals(1, testerValve.getMaximumConcurrency());
}
+ @Test
+ public void testIsSessionStale() {
+ PersistentValve pv = new PersistentValve();
+ StandardSession session = new StandardSession(null);
+ session.setId("test-stale", false);
+ session.setValid(true);
+
+ session.setMaxInactiveInterval(1);
+ session.setCreationTime(System.currentTimeMillis() - 60 * 1000);
+ Assert.assertTrue(pv.isSessionStale(session,
System.currentTimeMillis()));
+
+ session.setMaxInactiveInterval(3600);
+ session.setCreationTime(System.currentTimeMillis());
+ Assert.assertFalse(pv.isSessionStale(session,
System.currentTimeMillis()));
+
+ session.setMaxInactiveInterval(-1);
+ Assert.assertFalse(pv.isSessionStale(session,
System.currentTimeMillis()));
+
+ Assert.assertFalse(pv.isSessionStale(null,
System.currentTimeMillis()));
+ }
+
+ @Test
+ public void testRequestWithoutSessionNoFilter() {
+ PersistentValve pv = new PersistentValve();
+ Assert.assertFalse(pv.isRequestWithoutSession("/index.html"));
+ }
+
+
+ @Test
+ public void testRequestWithoutSessionWithFilter() {
+ PersistentValve pv = new PersistentValve();
+ pv.setContainer(new TesterRequest().getContext());
+
+ pv.setFilter(".*\\.(css|js|png)");
+
+ Assert.assertTrue(pv.isRequestWithoutSession("/style.css"));
+ Assert.assertTrue(pv.isRequestWithoutSession("/app.js"));
+ Assert.assertTrue(pv.isRequestWithoutSession("/logo.png"));
+ Assert.assertFalse(pv.isRequestWithoutSession("/index.html"));
+ Assert.assertFalse(pv.isRequestWithoutSession("/api/data"));
+ }
private static class TesterValve extends ValveBase {
- private static AtomicInteger maximumConcurrency = new AtomicInteger();
- private static AtomicInteger concurrency = new AtomicInteger();
+ private static final AtomicInteger maximumConcurrency = new
AtomicInteger();
+ private static final AtomicInteger concurrency = new AtomicInteger();
@Override
- public void invoke(Request request, Response response) throws
IOException, ServletException {
+ public void invoke(Request request, Response response) {
int c = concurrency.incrementAndGet();
- maximumConcurrency.getAndUpdate((v) -> c > v ? c : v);
+ maximumConcurrency.getAndUpdate(v -> Math.max(c, v));
try {
Thread.sleep(1000);
} catch (InterruptedException e) {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
