This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 963a81f43 WW-5647 Use ConcurrentHashMap for XSLT template cache (#1781)
963a81f43 is described below
commit 963a81f43df9b17a80f0892ff441d328f7979681
Author: Arun <[email protected]>
AuthorDate: Mon Jul 20 00:10:25 2026 +0530
WW-5647 Use ConcurrentHashMap for XSLT template cache (#1781)
* Use ConcurrentHashMap for XSLT template cache and add double-check locking
* Prevent noCache from polluting shared template cache; add dedup and
noCache regression tests
---
.../org/apache/struts2/result/xslt/XSLTResult.java | 38 +++---
.../apache/struts2/result/xslt/XSLTResultTest.java | 148 +++++++++++++++++++++
.../src/test/resources/XSLTResultTestDedup.xsl | 29 ++++
.../resources/XSLTResultTestNoCachePollution.xsl | 29 ++++
4 files changed, 226 insertions(+), 18 deletions(-)
diff --git
a/plugins/xslt/src/main/java/org/apache/struts2/result/xslt/XSLTResult.java
b/plugins/xslt/src/main/java/org/apache/struts2/result/xslt/XSLTResult.java
index b6d8ec62a..12f57ff34 100644
--- a/plugins/xslt/src/main/java/org/apache/struts2/result/xslt/XSLTResult.java
+++ b/plugins/xslt/src/main/java/org/apache/struts2/result/xslt/XSLTResult.java
@@ -45,7 +45,7 @@ import javax.xml.transform.stream.StreamSource;
import java.io.IOException;
import java.io.PrintWriter;
import java.net.URL;
-import java.util.HashMap;
+import java.util.concurrent.ConcurrentHashMap;
import java.util.Map;
/**
@@ -68,11 +68,7 @@ public class XSLTResult implements Result {
/**
* Cache of all templates.
*/
- private static final Map<String, Templates> templatesCache;
-
- static {
- templatesCache = new HashMap<>();
- }
+ private static final Map<String, Templates> templatesCache = new
ConcurrentHashMap<>();
// Configurable Parameters
@@ -283,19 +279,25 @@ public class XSLTResult implements Result {
if (noCache || (templates == null)) {
synchronized (templatesCache) {
- URL resource =
ServletActionContext.getServletContext().getResource(path);
-
- if (resource == null) {
- throw new TransformerException("Stylesheet " + path + "
not found in resources.");
+ // Re-check after acquiring lock to avoid redundant compilation
+ templates = templatesCache.get(path);
+ if (noCache || (templates == null)) {
+ URL resource =
ServletActionContext.getServletContext().getResource(path);
+
+ if (resource == null) {
+ throw new TransformerException("Stylesheet " + path +
" not found in resources.");
+ }
+
+ LOG.debug("Preparing XSLT stylesheet templates: {}", path);
+
+ TransformerFactory factory = createTransformerFactory();
+ factory.setURIResolver(getURIResolver());
+ factory.setErrorListener(buildErrorListener());
+ templates = factory.newTemplates(new
StreamSource(resource.openStream()));
+ if (!noCache) {
+ templatesCache.put(path, templates);
+ }
}
-
- LOG.debug("Preparing XSLT stylesheet templates: {}", path);
-
- TransformerFactory factory = createTransformerFactory();
- factory.setURIResolver(getURIResolver());
- factory.setErrorListener(buildErrorListener());
- templates = factory.newTemplates(new
StreamSource(resource.openStream()));
- templatesCache.put(path, templates);
}
}
diff --git
a/plugins/xslt/src/test/java/org/apache/struts2/result/xslt/XSLTResultTest.java
b/plugins/xslt/src/test/java/org/apache/struts2/result/xslt/XSLTResultTest.java
index 51efb818a..127dc33c7 100644
---
a/plugins/xslt/src/test/java/org/apache/struts2/result/xslt/XSLTResultTest.java
+++
b/plugins/xslt/src/test/java/org/apache/struts2/result/xslt/XSLTResultTest.java
@@ -31,13 +31,21 @@ import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.mock.web.MockHttpServletResponse;
import org.springframework.mock.web.MockServletContext;
+import javax.xml.transform.ErrorListener;
import javax.xml.transform.Source;
+import javax.xml.transform.Templates;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerConfigurationException;
import javax.xml.transform.TransformerException;
+import javax.xml.transform.TransformerFactory;
import javax.xml.transform.URIResolver;
import javax.xml.transform.stream.StreamSource;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
/**
* Unit test for {@link XSLTResult}.
@@ -167,6 +175,146 @@ public class XSLTResultTest extends StrutsTestCase {
TestCase.assertTrue(out.contains("<validators>"));
}
+ public void testConcurrentGetTemplatesCompilesOnce() throws Exception {
+ final AtomicInteger compileCount = new AtomicInteger(0);
+ final CountDownLatch compileStarted = new CountDownLatch(1);
+ final CountDownLatch releaseCompile = new CountDownLatch(1);
+
+ result = new XSLTResult() {
+ protected TransformerFactory createTransformerFactory() {
+ final TransformerFactory delegate =
super.createTransformerFactory();
+ return new TransformerFactory() {
+ public Transformer newTransformer(Source source) throws
TransformerConfigurationException {
+ return delegate.newTransformer(source);
+ }
+
+ public Transformer newTransformer() throws
TransformerConfigurationException {
+ return delegate.newTransformer();
+ }
+
+ public Templates newTemplates(Source source) throws
TransformerConfigurationException {
+ compileCount.incrementAndGet();
+ compileStarted.countDown();
+ try {
+ releaseCompile.await();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ return delegate.newTemplates(source);
+ }
+
+ public Source getAssociatedStylesheet(Source source,
String media, String title, String charset) throws
TransformerConfigurationException {
+ return delegate.getAssociatedStylesheet(source, media,
title, charset);
+ }
+
+ public void setURIResolver(URIResolver resolver) {
+ delegate.setURIResolver(resolver);
+ }
+
+ public URIResolver getURIResolver() {
+ return delegate.getURIResolver();
+ }
+
+ public void setFeature(String name, boolean value) throws
TransformerConfigurationException {
+ delegate.setFeature(name, value);
+ }
+
+ public boolean getFeature(String name) {
+ return delegate.getFeature(name);
+ }
+
+ public void setAttribute(String name, Object value) {
+ delegate.setAttribute(name, value);
+ }
+
+ public Object getAttribute(String name) {
+ return delegate.getAttribute(name);
+ }
+
+ public void setErrorListener(ErrorListener listener) {
+ delegate.setErrorListener(listener);
+ }
+
+ public ErrorListener getErrorListener() {
+ return delegate.getErrorListener();
+ }
+ };
+ }
+ };
+
+ final String path = "XSLTResultTestDedup.xsl";
+ final ActionContext context = ActionContext.getContext();
+ final Templates[] compiled = new Templates[2];
+ final Exception[] errors = new Exception[2];
+
+ Thread first = new Thread(() -> {
+ ActionContext.bind(context);
+ try {
+ compiled[0] = result.getTemplates(path);
+ } catch (Exception e) {
+ errors[0] = e;
+ } finally {
+ ActionContext.clear();
+ }
+ });
+ first.start();
+
+ TestCase.assertTrue("First thread should have started compiling",
+ compileStarted.await(5, TimeUnit.SECONDS));
+
+ Thread second = new Thread(() -> {
+ ActionContext.bind(context);
+ try {
+ compiled[1] = result.getTemplates(path);
+ } catch (Exception e) {
+ errors[1] = e;
+ } finally {
+ ActionContext.clear();
+ }
+ });
+ second.start();
+
+ // Give the second thread time to reach the synchronized block and
park on
+ // the lock held by the first thread, so it hits the concurrent-miss
race
+ // rather than a plain cache hit.
+ Thread.sleep(200);
+
+ releaseCompile.countDown();
+
+ first.join(5000);
+ second.join(5000);
+
+ TestCase.assertNull(errors[0]);
+ TestCase.assertNull(errors[1]);
+ TestCase.assertEquals("Templates should be compiled exactly once
despite the concurrent miss",
+ 1, compileCount.get());
+ TestCase.assertSame("Both callers should observe the same cached
Templates instance",
+ compiled[0], compiled[1]);
+ }
+
+ public void testNoCacheDoesNotPollutePersistentCache() throws Exception {
+ final String path = "XSLTResultTestNoCachePollution.xsl";
+
+ // First, a normal (cached) compile populates the shared static cache.
+ result.setNoCache("false");
+ Templates cached = result.getTemplates(path);
+ TestCase.assertNotNull(cached);
+
+ // A noCache=true caller must still get a fresh compile...
+ result.setNoCache("true");
+ Templates fresh = result.getTemplates(path);
+ TestCase.assertNotNull(fresh);
+ TestCase.assertNotSame("noCache=true should always recompile rather
than reuse the cache",
+ cached, fresh);
+
+ // ...but must NOT overwrite the shared cache entry other, cached
callers rely on.
+ XSLTResult cachedCaller = new XSLTResult();
+ cachedCaller.setNoCache("false");
+ Templates stillCached = cachedCaller.getTemplates(path);
+ TestCase.assertSame("A noCache=true call must not pollute the shared
cache for cached callers",
+ cached, stillCached);
+ }
+
public void testTransform4WithBadDocumentInclude() {
result = new XSLTResult(){
protected URIResolver getURIResolver() {
diff --git a/plugins/xslt/src/test/resources/XSLTResultTestDedup.xsl
b/plugins/xslt/src/test/resources/XSLTResultTestDedup.xsl
new file mode 100644
index 000000000..f9582dfd1
--- /dev/null
+++ b/plugins/xslt/src/test/resources/XSLTResultTestDedup.xsl
@@ -0,0 +1,29 @@
+<!--
+/*
+ * 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.
+ */
+-->
+<xsl:stylesheet version="1.0"
+ xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
+ xmlns="http://www.w3.org/TR/xhtml1/strict";>
+
+ <xsl:template match="/">
+ <result/>
+ </xsl:template>
+
+</xsl:stylesheet>
diff --git a/plugins/xslt/src/test/resources/XSLTResultTestNoCachePollution.xsl
b/plugins/xslt/src/test/resources/XSLTResultTestNoCachePollution.xsl
new file mode 100644
index 000000000..f9582dfd1
--- /dev/null
+++ b/plugins/xslt/src/test/resources/XSLTResultTestNoCachePollution.xsl
@@ -0,0 +1,29 @@
+<!--
+/*
+ * 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.
+ */
+-->
+<xsl:stylesheet version="1.0"
+ xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
+ xmlns="http://www.w3.org/TR/xhtml1/strict";>
+
+ <xsl:template match="/">
+ <result/>
+ </xsl:template>
+
+</xsl:stylesheet>
