You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2026/08/24 09:56:14 UTC
(struts) 01/01: WW-5690 perf(dispatcher): load the dev-mode error template on first use
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch fix/WW-5690-lazy-error-template
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 7497fe0f5d72934c0d3af0b069783545112d89c9
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Aug 24 11:56:00 2026 +0200
WW-5690 perf(dispatcher): load the dev-mode error template on first use
DefaultDispatcherErrorHandler.init() built a FreeMarker configuration and
loaded /org/apache/struts2/dispatcher/error.ftl on every startup, including
in production where the problem report is never rendered - handleError()
delegates to the container's error page unless devMode is on.
Defer the load to the first dev-mode error instead. Two threads racing there
may both load the template, which is harmless: FreeMarker caches templates in
its own configuration, and that is cheaper than locking a path taken once per
application.
One behavioural change: a missing or unparsable error.ftl used to fail the
application at boot. It now surfaces on the first dev-mode error, where the
existing catch in handleErrorInDevMode() degrades to sendError() with the
cause. A dev-only template should not stop a production application starting.
This removes the startup cost only; the FreeMarker dependency itself stays.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../dispatcher/DefaultDispatcherErrorHandler.java | 34 ++++++++++---
.../DefaultDispatcherErrorHandlerTest.java | 59 ++++++++++++++++++++++
2 files changed, 85 insertions(+), 8 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java
index d64345854..290db7753 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandler.java
@@ -30,7 +30,6 @@ import org.apache.commons.lang3.BooleanUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
-import org.apache.struts2.StrutsException;
import org.apache.struts2.views.freemarker.FreemarkerManager;
import java.io.IOException;
@@ -48,9 +47,12 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler {
private static final Logger LOG = LogManager.getLogger(DefaultDispatcherErrorHandler.class);
+ private static final String ERROR_TEMPLATE = "/org/apache/struts2/dispatcher/error.ftl";
+
private FreemarkerManager freemarkerManager;
private boolean devMode;
- private Template template;
+ private ServletContext servletContext;
+ private volatile Template template;
@Inject
public void setFreemarkerManager(FreemarkerManager freemarkerManager) {
@@ -63,12 +65,28 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler {
}
public void init(ServletContext ctx) {
- try {
- freemarker.template.Configuration config = freemarkerManager.getConfiguration(ctx);
- template = config.getTemplate("/org/apache/struts2/dispatcher/error.ftl");
- } catch (IOException e) {
- throw new StrutsException(e);
+ this.servletContext = ctx;
+ }
+
+ /**
+ * Loads the problem report template on first use rather than at startup: it is only ever
+ * rendered in devMode, so a production application should never build a FreeMarker
+ * configuration on its behalf.
+ * <p>
+ * Two threads racing on the very first error may both load the template. That is harmless -
+ * FreeMarker caches templates in its own configuration - and cheaper than locking a path taken
+ * once per application.
+ *
+ * @return the problem report template
+ * @throws IOException if the template cannot be loaded
+ */
+ protected Template getTemplate() throws IOException {
+ Template result = template;
+ if (result == null) {
+ result = freemarkerManager.getConfiguration(servletContext).getTemplate(ERROR_TEMPLATE);
+ template = result;
}
+ return result;
}
public void handleError(HttpServletRequest request, HttpServletResponse response, int code, Exception e) {
@@ -115,7 +133,7 @@ public class DefaultDispatcherErrorHandler implements DispatcherErrorHandler {
} while ((cur = cur.getCause()) != null);
Writer writer = new StringWriter();
- template.process(createReportData(e, chain), writer);
+ getTemplate().process(createReportData(e, chain), writer);
response.setContentType("text/html");
response.getWriter().write(writer.toString());
diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java
index 6d44c7dac..f1f521047 100644
--- a/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java
+++ b/core/src/test/java/org/apache/struts2/dispatcher/DefaultDispatcherErrorHandlerTest.java
@@ -18,10 +18,14 @@
*/
package org.apache.struts2.dispatcher;
+import freemarker.template.Configuration;
import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
import java.util.Collections;
import org.apache.struts2.StrutsInternalTestCase;
+import jakarta.servlet.ServletContext;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import org.apache.struts2.views.freemarker.FreemarkerManager;
@@ -130,6 +134,61 @@ public class DefaultDispatcherErrorHandlerTest extends StrutsInternalTestCase {
defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException);
}
+ /**
+ * The dev-mode problem report is the only thing that needs FreeMarker here, so booting the
+ * application must not build a FreeMarker configuration just to have the template ready.
+ */
+ public void testInitDoesNotLoadErrorTemplate() {
+ RecordingFreemarkerManager freemarkerManager = createFreemarkerManager();
+ DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler();
+ defaultDispatcherErrorHandler.setDevMode("true");
+ defaultDispatcherErrorHandler.setFreemarkerManager(freemarkerManager);
+
+ defaultDispatcherErrorHandler.init(dispatcher.servletContext);
+
+ assertFalse("init() must not touch FreeMarker", freemarkerManager.configurationRequested);
+ }
+
+ /**
+ * The deferred load must still happen, otherwise the problem report silently stops rendering.
+ */
+ public void testErrorTemplateLoadedOnFirstDevModeError() throws IOException {
+ RecordingFreemarkerManager freemarkerManager = createFreemarkerManager();
+ DefaultDispatcherErrorHandler defaultDispatcherErrorHandler = new DefaultDispatcherErrorHandler();
+ defaultDispatcherErrorHandler.setDevMode("true");
+ defaultDispatcherErrorHandler.setFreemarkerManager(freemarkerManager);
+ defaultDispatcherErrorHandler.init(dispatcher.servletContext);
+ Exception fakeException = new Exception("Fake Exception, devMode true");
+ responseMock.setContentType("text/html");
+ expectLastCall();
+ expect(responseMock.getWriter()).andStubReturn(new PrintWriter(new StringWriter()));
+ replay(responseMock);
+
+ defaultDispatcherErrorHandler.handleError(requestMock, responseMock, HttpServletResponse.SC_INTERNAL_SERVER_ERROR, fakeException);
+
+ assertTrue("problem report must load the template on first use", freemarkerManager.configurationRequested);
+ }
+
+ private RecordingFreemarkerManager createFreemarkerManager() {
+ RecordingFreemarkerManager freemarkerManager = new RecordingFreemarkerManager();
+ container.inject(freemarkerManager);
+ return freemarkerManager;
+ }
+
+ /**
+ * Records whether the FreeMarker configuration was ever asked for, while still delegating so the
+ * template really renders.
+ */
+ private static class RecordingFreemarkerManager extends FreemarkerManager {
+ private boolean configurationRequested;
+
+ @Override
+ public Configuration getConfiguration(ServletContext servletContext) {
+ configurationRequested = true;
+ return super.getConfiguration(servletContext);
+ }
+ }
+
protected void setUp() {
requestMock = (HttpServletRequest) createMock(HttpServletRequest.class);
responseMock = (HttpServletResponse) createMock(HttpServletResponse.class);