This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5668-i18n-cache-bounds-6x in repository https://gitbox.apache.org/repos/asf/struts.git
commit e18b02ca92130ab3dc677bbacd2172725e09f3b7 Author: Lukasz Lenart <[email protected]> AuthorDate: Sat Aug 1 09:21:31 2026 +0200 WW-5668 Bound the localized-text provider caches with configurable size Converts bundlesMap, messageFormats and missingBundles to the existing OgnlCache abstraction, configurable via struts.i18n.cacheType and struts.i18n.cacheMaxSize (wtlfu / 10000 by default). The caches are kept transient and rebuilt in readObject so the providers stay serializable, and bundlesMap-related synchronization moves to a dedicated monitor since the field is now reassignable. Co-Authored-By: Claude Opus 5 <[email protected]> --- .../xwork2/util/AbstractLocalizedTextProvider.java | 102 ++++++++++++++++++--- .../xwork2/util/GlobalLocalizedTextProvider.java | 2 + .../xwork2/util/StrutsLocalizedTextProvider.java | 2 + .../java/org/apache/struts2/StrutsConstants.java | 16 ++++ .../org/apache/struts2/default.properties | 7 ++ .../com/opensymphony/xwork2/util/CacheFixture.java | 28 ++++++ .../util/StrutsLocalizedTextProviderTest.java | 75 +++++++++++++++ .../xwork2/util/CacheFixture.properties | 19 ++++ 8 files changed, 238 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java b/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java index 2d6a7c678..e0f45e64a 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/AbstractLocalizedTextProvider.java @@ -21,6 +21,10 @@ package com.opensymphony.xwork2.util; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.LocalizedTextProvider; import com.opensymphony.xwork2.inject.Inject; +import com.opensymphony.xwork2.ognl.DefaultOgnlCacheFactory; +import com.opensymphony.xwork2.ognl.OgnlCache; +import com.opensymphony.xwork2.ognl.OgnlCacheFactory.CacheType; +import org.apache.commons.lang3.EnumUtils; import org.apache.commons.lang3.ObjectUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -45,6 +49,8 @@ import java.util.concurrent.CopyOnWriteArrayList; abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { + private static final long serialVersionUID = 1L; + private static final Logger LOG = LogManager.getLogger(AbstractLocalizedTextProvider.class); public static final String XWORK_MESSAGES_BUNDLE = "com/opensymphony/xwork2/xwork-messages"; @@ -56,16 +62,35 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { private static final String TOMCAT_WEBAPP_CLASSLOADER_BASE = "org.apache.catalina.loader.WebappClassLoaderBase"; private static final String RELOADED = "com.opensymphony.xwork2.util.LocalizedTextProvider.reloaded"; - protected final ConcurrentMap<String, ResourceBundle> bundlesMap = new ConcurrentHashMap<>(); protected boolean devMode = false; protected boolean reloadBundles = false; protected boolean searchDefaultBundlesFirst = false; // Search default resource bundles first. Note: This flag may not be meaningful to all implementations. - private final ConcurrentMap<MessageFormatKey, MessageFormat> messageFormats = new ConcurrentHashMap<>(); private final ConcurrentMap<Integer, List<String>> classLoaderMap = new ConcurrentHashMap<>(); - private final Set<String> missingBundles = ConcurrentHashMap.newKeySet(); private final ConcurrentMap<Integer, ClassLoader> delegatedClassLoaderMap = new ConcurrentHashMap<>(); + // Dedicated monitor for bundlesMap-related synchronization: bundlesMap is reassigned by + // rebuildI18nCaches(), so locking on it directly would lock on a monitor that can change identity. + // transient + reinitialised in readObject: a bare Object is not Serializable. + private transient Object bundlesMapLock = new Object(); + + private volatile CacheType i18nCacheType = CacheType.WTLFU; + private volatile int i18nCacheMaxSize = 10000; + + private <K, V> OgnlCache<K, V> buildI18nCache() { + return new DefaultOgnlCacheFactory<K, V>(i18nCacheMaxSize, i18nCacheType).buildOgnlCache(); + } + + // The OgnlCache implementations are themselves thread-safe; volatile only safely publishes the + // reference when rebuildI18nCaches() replaces a cache (during injection / readObject), so S3077 + // ("volatile is not enough") does not apply here. + @SuppressWarnings("java:S3077") + protected transient volatile OgnlCache<String, ResourceBundle> bundlesMap = buildI18nCache(); + @SuppressWarnings("java:S3077") + private transient volatile OgnlCache<MessageFormatKey, MessageFormat> messageFormats = buildI18nCache(); + @SuppressWarnings("java:S3077") + private transient volatile OgnlCache<String, Boolean> missingBundles = buildI18nCache(); + /** * Adds the bundle to the internal list of default bundles. * If the bundle already exists in the list it will be re-added. @@ -99,6 +124,21 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { return Thread.currentThread().getContextClassLoader(); } + /** Test-support accessor: current number of cached resource bundles. */ + protected int bundlesMapSize() { + return bundlesMap.size(); + } + + /** Test-support accessor: current number of cached missing-bundle markers. */ + protected int missingBundlesSize() { + return missingBundles.size(); + } + + /** Test-support accessor: current number of cached message formats. */ + protected int messageFormatsSize() { + return messageFormats.size(); + } + @Inject(value = StrutsConstants.STRUTS_CUSTOM_I18N_RESOURCES, required = false) public void setCustomI18NResources(String bundles) { if (bundles != null && bundles.length() > 0) { @@ -221,7 +261,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { * @param classLoader a {@link ClassLoader} to look up the bundle from if none can be found on the current thread's classloader */ public void setDelegatedClassLoader(final ClassLoader classLoader) { - synchronized (bundlesMap) { + synchronized (bundlesMapLock) { delegatedClassLoaderMap.put(getCurrentThreadContextClassLoader().hashCode(), classLoader); } } @@ -443,6 +483,44 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { this.searchDefaultBundlesFirst = Boolean.parseBoolean(searchDefaultBundlesFirst); } + /** + * @param cacheType the type of cache to use for the localized-text caches + * + * @since 6.11.0 + */ + @Inject(value = StrutsConstants.STRUTS_I18N_CACHE_TYPE, required = false) + public void setI18nCacheType(String cacheType) { + this.i18nCacheType = EnumUtils.getEnumIgnoreCase(CacheType.class, cacheType, CacheType.WTLFU); + rebuildI18nCaches(); + } + + /** + * @param cacheMaxSize the maximum size of each localized-text cache + * + * @since 6.11.0 + */ + @Inject(value = StrutsConstants.STRUTS_I18N_CACHE_MAXSIZE, required = false) + public void setI18nCacheMaxSize(String cacheMaxSize) { + this.i18nCacheMaxSize = Integer.parseInt(cacheMaxSize); + rebuildI18nCaches(); + } + + /** + * Rebuilds the localized-text caches from the current type/size. Called during dependency injection + * (single-threaded startup, before the provider serves lookups); discards any warm-up entries. + */ + private void rebuildI18nCaches() { + bundlesMap = buildI18nCache(); + messageFormats = buildI18nCache(); + missingBundles = buildI18nCache(); + } + + private void readObject(java.io.ObjectInputStream in) throws java.io.IOException, ClassNotFoundException { + in.defaultReadObject(); + bundlesMapLock = new Object(); + rebuildI18nCaches(); + } + /** * Finds the given resource bundle by it's name. * <p> @@ -458,34 +536,32 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { ClassLoader classLoader = getCurrentThreadContextClassLoader(); String key = createMissesKey(String.valueOf(classLoader.hashCode()), aBundleName, locale); - if (missingBundles.contains(key)) { + if (missingBundles.get(key) != null) { return null; } ResourceBundle bundle = null; try { - if (bundlesMap.containsKey(key)) { - bundle = bundlesMap.get(key); - } else { + bundle = bundlesMap.get(key); + if (bundle == null) { bundle = ResourceBundle.getBundle(aBundleName, locale, classLoader); bundlesMap.putIfAbsent(key, bundle); } } catch (MissingResourceException ex) { if (delegatedClassLoaderMap.containsKey(classLoader.hashCode())) { try { - if (bundlesMap.containsKey(key)) { - bundle = bundlesMap.get(key); - } else { + bundle = bundlesMap.get(key); + if (bundle == null) { bundle = ResourceBundle.getBundle(aBundleName, locale, delegatedClassLoaderMap.get(classLoader.hashCode())); bundlesMap.putIfAbsent(key, bundle); } } catch (MissingResourceException e) { LOG.debug("Missing resource bundle [{}]!", aBundleName, e); - missingBundles.add(key); + missingBundles.put(key, Boolean.TRUE); } } else { LOG.debug("Missing resource bundle [{}]!", aBundleName); - missingBundles.add(key); + missingBundles.put(key, Boolean.TRUE); } } return bundle; diff --git a/core/src/main/java/com/opensymphony/xwork2/util/GlobalLocalizedTextProvider.java b/core/src/main/java/com/opensymphony/xwork2/util/GlobalLocalizedTextProvider.java index d86682fd1..b2359eb9e 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/GlobalLocalizedTextProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/GlobalLocalizedTextProvider.java @@ -33,6 +33,8 @@ import java.util.ResourceBundle; */ public class GlobalLocalizedTextProvider extends AbstractLocalizedTextProvider { + private static final long serialVersionUID = 1L; + private static final Logger LOG = LogManager.getLogger(GlobalLocalizedTextProvider.class); public GlobalLocalizedTextProvider() { diff --git a/core/src/main/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProvider.java b/core/src/main/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProvider.java index 60ef3477a..b95a0fbbc 100644 --- a/core/src/main/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProvider.java +++ b/core/src/main/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProvider.java @@ -36,6 +36,8 @@ import java.util.ResourceBundle; */ public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider { + private static final long serialVersionUID = 1L; + private static final Logger LOG = LogManager.getLogger(StrutsLocalizedTextProvider.class); /** diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 0ac751640..22c753bab 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -288,6 +288,22 @@ public final class StrutsConstants { */ public static final String STRUTS_OGNL_BEANINFO_CACHE_FACTORY = "struts.ognl.beanInfoCacheFactory"; + /** + * Specifies the type of cache to use for the localized-text provider caches. Valid values defined in + * {@link com.opensymphony.xwork2.ognl.OgnlCacheFactory.CacheType}. + * + * @since 6.11.0 + */ + public static final String STRUTS_I18N_CACHE_TYPE = "struts.i18n.cacheType"; + + /** + * Specifies the maximum size of each localized-text provider cache. Configure based on the cache type + * chosen and application-specific needs. + * + * @since 6.11.0 + */ + public static final String STRUTS_I18N_CACHE_MAXSIZE = "struts.i18n.cacheMaxSize"; + /** * Specifies the type of cache to use for BeanInfo objects. * @since 6.4.0 diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index 3eedc2437..e48bbd015 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -240,6 +240,13 @@ struts.ognl.expressionCacheType=wtlfu ### chosen and application-specific needs. struts.ognl.expressionCacheMaxSize=10000 +### Specifies the type of cache to use for the localized-text provider caches. See StrutsConstants for details. +struts.i18n.cacheType=wtlfu + +### Specifies the maximum size of each localized-text provider cache. This should be configured based on the +### cache type chosen and application-specific needs. +struts.i18n.cacheMaxSize=10000 + ### Specifies the type of cache to use for BeanInfo objects. See StrutsConstants class for further information. struts.ognl.beanInfoCacheType=wtlfu diff --git a/core/src/test/java/com/opensymphony/xwork2/util/CacheFixture.java b/core/src/test/java/com/opensymphony/xwork2/util/CacheFixture.java new file mode 100644 index 000000000..027aec163 --- /dev/null +++ b/core/src/test/java/com/opensymphony/xwork2/util/CacheFixture.java @@ -0,0 +1,28 @@ +/* + * 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 com.opensymphony.xwork2.util; + +/** + * Simple fixture whose class-associated bundle ({@code CacheFixture.properties}) backs the + * localized-text caching tests. + * + * @since 6.11.0 + */ +public class CacheFixture { +} diff --git a/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java b/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java index 7bb8af612..bea67becb 100644 --- a/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java +++ b/core/src/test/java/com/opensymphony/xwork2/util/StrutsLocalizedTextProviderTest.java @@ -34,6 +34,10 @@ import com.opensymphony.xwork2.test.TestBean2; import org.apache.struts2.config.StrutsXmlConfigurationProvider; import org.apache.struts2.interceptor.parameter.StrutsParameter; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; import java.text.DateFormat; import java.text.ParseException; import java.util.Date; @@ -563,6 +567,77 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase { assertEquals("Result of bean2.name lookup not as expected ?", "Okay! You found Me!", messageResult); } + public void testCachesAreBoundedByConfiguredMaxSize() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + provider.setI18nCacheMaxSize("100"); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + for (int i = 0; i < 20000; i++) { + Locale locale = Locale.forLanguageTag("en-US-x" + String.format("%05d", i)); + provider.findText(CacheFixture.class, "cache.missing", locale, "Fallback", null, valueStack); + } + + assertTrue("bundlesMap not bounded ?", provider.bundlesMapSize() <= 2000); + assertTrue("missingBundles not bounded ?", provider.missingBundlesSize() <= 2000); + assertTrue("messageFormats not bounded ?", provider.messageFormatsSize() <= 2000); + } + + public void testCorrectTextStillReturnedUnderEviction() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + provider.setI18nCacheMaxSize("50"); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + // Force heavy eviction with many distinct locales. + for (int i = 0; i < 5000; i++) { + Locale locale = Locale.forLanguageTag("en-US-x" + String.format("%05d", i)); + provider.findText(CacheFixture.class, "cache.missing", locale, "Fallback", null, valueStack); + } + + // A real key in a real locale still resolves correctly after eviction pressure. + String result = provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack); + assertEquals("Static cached value", result); + } + + public void testReloadClearsBoundedCaches() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + provider.findText(CacheFixture.class, "cache.missing", Locale.ENGLISH, "Fallback", null, valueStack); + assertTrue("missingBundles not populated ?", provider.missingBundlesSize() > 0); + + provider.callReloadBundlesForceReload(); + assertEquals("reload did not clear bundlesMap ?", 0, provider.bundlesMapSize()); + } + + public void testProviderIsUsableAfterDeserialization() throws Exception { + StrutsLocalizedTextProvider provider = new StrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + try (ObjectOutputStream oos = new ObjectOutputStream(baos)) { + oos.writeObject(provider); + } + Object restored; + try (ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(baos.toByteArray()))) { + restored = ois.readObject(); + } + StrutsLocalizedTextProvider deserialized = (StrutsLocalizedTextProvider) restored; + // Caches were transient (null right after defaultReadObject) but readObject rebuilds them: + assertEquals("Deserialized caches not rebuilt empty", 0, deserialized.bundlesMapSize()); + String result = deserialized.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack); + assertEquals("Static cached value", result); + } + + public void testCacheTypeSelectionKeepsProviderWorking() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + provider.setI18nCacheType("basic"); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + String result = provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack); + assertEquals("Static cached value", result); + assertTrue("bundlesMap should populate", provider.bundlesMapSize() >= 1); + } + @Override protected void setUp() throws Exception { super.setUp(); diff --git a/core/src/test/resources/com/opensymphony/xwork2/util/CacheFixture.properties b/core/src/test/resources/com/opensymphony/xwork2/util/CacheFixture.properties new file mode 100644 index 000000000..0e0511b92 --- /dev/null +++ b/core/src/test/resources/com/opensymphony/xwork2/util/CacheFixture.properties @@ -0,0 +1,19 @@ +# +# 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. +# +cache.static=Static cached value
