This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5540-localized-text-provider-caching in repository https://gitbox.apache.org/repos/asf/struts.git
commit 04cce1c9a68809fcd5b2e6c31215041af41d8afe Author: Lukasz Lenart <[email protected]> AuthorDate: Thu Jul 23 13:25:45 2026 +0200 WW-5540 perf(core): cache package-hierarchy text resolution Cache the *.package traversal in findText the same way as the class hierarchy, with the same keying, fall-through, and invalidation. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../text/AbstractLocalizedTextProvider.java | 55 ++++++++++++++++++++++ .../struts2/text/StrutsLocalizedTextProvider.java | 28 +++-------- .../text/StrutsLocalizedTextProviderTest.java | 30 ++++++++++++ 3 files changed, 91 insertions(+), 22 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java b/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java index dc647d6d2..efe568704 100644 --- a/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java +++ b/core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java @@ -68,6 +68,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { private final Set<String> missingBundles = ConcurrentHashMap.newKeySet(); private final ConcurrentMap<Integer, ClassLoader> delegatedClassLoaderMap = new ConcurrentHashMap<>(); private final ConcurrentMap<TextCacheKey, String> classHierarchyCache = new ConcurrentHashMap<>(); + private final ConcurrentMap<TextCacheKey, String> packageHierarchyCache = new ConcurrentHashMap<>(); @Override public void addDefaultResourceBundle(String bundleName) { @@ -101,6 +102,11 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { return classHierarchyCache.size(); } + /** Test-support accessor: current number of cached package-hierarchy resolutions. */ + protected int packageHierarchyCacheSize() { + return packageHierarchyCache.size(); + } + @Inject(value = StrutsConstants.STRUTS_CUSTOM_I18N_RESOURCES, required = false) public void setCustomI18NResources(String bundles) { if (bundles == null || bundles.isEmpty()) { @@ -199,6 +205,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { final String key = createMissesKey(String.valueOf(getCurrentThreadContextClassLoader().hashCode()), bundleName, locale); final ResourceBundle removedBundle = bundlesMap.remove(key); classHierarchyCache.clear(); + packageHierarchyCache.clear(); LOG.debug("Clearing resource bundle [{}], locale [{}], result: [{}].", bundleName, locale, removedBundle != null); } @@ -217,6 +224,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { protected void clearMissingBundlesCache() { missingBundles.clear(); classHierarchyCache.clear(); + packageHierarchyCache.clear(); LOG.debug("Cleared the missing bundles cache."); } @@ -236,6 +244,7 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { if (!reloaded) { bundlesMap.clear(); classHierarchyCache.clear(); + packageHierarchyCache.clear(); clearResourceBundleClassloaderCaches(); // now, for the true and utter hack, if we're running in tomcat, clear @@ -657,6 +666,52 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider { return cachedRawResult == NOT_FOUND; } + /** + * Raw-pattern walk of the {@code *.package} bundles up the class hierarchy of {@code startClazz}. + * Returns the first raw pattern found (via {@link #getRawMessage}) for the key or its indexed form, + * or {@code null} when none match. + */ + private String findPackageMessageRaw(Class<?> startClazz, String textKey, String indexedTextName, Locale locale) { + for (Class<?> clazz = startClazz; + (clazz != null) && !clazz.equals(Object.class); + clazz = clazz.getSuperclass()) { + + String basePackageName = clazz.getName(); + while (basePackageName.lastIndexOf('.') != -1) { + basePackageName = basePackageName.substring(0, basePackageName.lastIndexOf('.')); + String packageName = basePackageName + ".package"; + String msg = getRawMessage(packageName, locale, textKey); + if (msg != null) { + return msg; + } + if (indexedTextName != null) { + msg = getRawMessage(packageName, locale, indexedTextName); + if (msg != null) { + return msg; + } + } + } + } + return null; + } + + /** + * Cached resolution of the {@code *.package} hierarchy for a key. Returns the raw pattern found, or + * {@link #NOT_FOUND} when absent. Same keying and get + putIfAbsent discipline as + * {@link #resolveClassHierarchyRaw}. + */ + protected String resolvePackageHierarchyRaw(Class<?> startClazz, String textKey, String indexedTextName, Locale locale) { + TextCacheKey cacheKey = new TextCacheKey(currentLoaderHashCode(), startClazz.getName(), textKey, locale); + String cached = packageHierarchyCache.get(cacheKey); + if (cached != null) { + return cached; + } + String raw = findPackageMessageRaw(startClazz, textKey, indexedTextName, locale); + String toStore = (raw != null) ? raw : NOT_FOUND; + packageHierarchyCache.putIfAbsent(cacheKey, toStore); + return toStore; + } + /** * Traverse up class hierarchy looking for message. Looks at class, then implemented interface, * before going up hierarchy. diff --git a/core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java b/core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java index dcee2e579..9c1d36ec5 100644 --- a/core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java +++ b/core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java @@ -119,28 +119,12 @@ public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider { } } - // nothing still? alright, search the package hierarchy now - for (Class<?> clazz = startClazz; - (clazz != null) && !clazz.equals(Object.class); - clazz = clazz.getSuperclass()) { - - String basePackageName = clazz.getName(); - while (basePackageName.lastIndexOf('.') != -1) { - basePackageName = basePackageName.substring(0, basePackageName.lastIndexOf('.')); - String packageName = basePackageName + ".package"; - msg = getMessage(packageName, locale, textKey, valueStack, args); - - if (msg != null) { - return msg; - } - - if (indexedTextName != null) { - msg = getMessage(packageName, locale, indexedTextName, valueStack, args); - - if (msg != null) { - return msg; - } - } + // search the package hierarchy (cached raw resolution; format per call) + String packageRaw = resolvePackageHierarchyRaw(startClazz, textKey, indexedTextName, locale); + if (!isNotFound(packageRaw)) { + msg = formatMessage(packageRaw, locale, valueStack, args); + if (msg != null) { + return msg; } } diff --git a/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java b/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java index b29426e5e..ad35de8e4 100644 --- a/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java +++ b/core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java @@ -637,6 +637,32 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase { assertEquals("clearMissingBundlesCache did not empty class hierarchy cache ?", 0, provider.classHierarchyCacheSize()); } + public void testPackageHierarchyCacheReusesFoundPattern() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + // ModelDrivenAction2 lives in a package that provides "package.properties" = "It works!". + assertEquals("Package cache not empty before lookup ?", 0, provider.packageHierarchyCacheSize()); + String first = provider.findText(org.apache.struts2.test.ModelDrivenAction2.class, "package.properties", Locale.getDefault(), null, null, valueStack); + assertEquals("It works!", first); + assertEquals("Package cache not populated after found lookup ?", 1, provider.packageHierarchyCacheSize()); + + String second = provider.findText(org.apache.struts2.test.ModelDrivenAction2.class, "package.properties", Locale.getDefault(), null, null, valueStack); + assertEquals("Second package lookup differs ?", first, second); + assertEquals("Package cache grew on repeat ?", 1, provider.packageHierarchyCacheSize()); + } + + public void testReloadClearsPackageHierarchyCache() { + TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); + ValueStack valueStack = ActionContext.getContext().getValueStack(); + + provider.findText(org.apache.struts2.test.ModelDrivenAction2.class, "package.properties", Locale.getDefault(), null, null, valueStack); + assertEquals("Package cache not populated ?", 1, provider.packageHierarchyCacheSize()); + + provider.callReloadBundlesForceReload(); + assertEquals("Reload did not clear package hierarchy cache ?", 0, provider.packageHierarchyCacheSize()); + } + public void testDeprecatedFindMessageStillDelegates() { // findMessage leaves findText's hot path in this task; this locks the deprecated delegator. TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider(); @@ -720,6 +746,10 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase { return super.classHierarchyCacheSize(); } + public int packageHierarchyCacheSize() { + return super.packageHierarchyCacheSize(); + } + public String callFindMessage(Class<?> clazz, String key, Locale locale, ValueStack valueStack) { return super.findMessage(clazz, key, null, locale, null, null, valueStack); }
