mirror of
https://github.com/apache/struts.git
synced 2026-08-06 23:27:07 +00:00
WW-5540 Add caching to AbstractLocalizedTextProvider (#1808)
* WW-5540 docs: add caching design spec for AbstractLocalizedTextProvider Design for caching the class/package hierarchy traversal result in findText, keyed on (classloader, class name, textKey, locale). Caches the raw resolved pattern (or a NOT_FOUND marker) only; translation and formatting stay per-call. Wires invalidation into the existing reloadBundles/clearBundle/clearMissingBundlesCache sites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: add implementation plan and refine spec Add the 3-task TDD implementation plan and record the formatWithNullDetection fall-through decision in the spec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: deprecate+delegate findMessage/getMessage in plan Resolve pre-flight duplication/dead-code finding: old traversal helpers delegate to the raw twins and are marked @Deprecated instead of being duplicated. Add a direct characterization test for the findMessage delegator. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 refactor(core): split raw message resolution from formatting Add getRawMessage/formatMessage and a raw twin findMessageRaw. Re-express getMessage via formatMessage and make findMessage delegate to findMessageRaw + formatMessage; deprecate both as legacy extension points superseded by the raw-resolution path. The deprecated findMessage triggers the bundle reload on entry, preserving the reload side effect the old getMessage-per-probe walk provided. Groundwork for the traversal caches. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: refine Task 1 plan (deprecate/delegate + reload-on-entry) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 perf(core): cache class-hierarchy text resolution Cache the class/interface/superclass traversal in findText keyed on (classloader, class name, key, locale), storing the raw pattern or a NOT_FOUND marker. Formatting stays per call and falls through to the next tier when a cached pattern formats to null. Invalidated on reloadBundles/clearBundle/clearMissingBundlesCache; reload is hoisted to the top of findText so caches are cleared before they are read. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: draft follow-up ticket for null-control-flow cleanup Capture the deferred result-wrapper refactor (raised during WW-5540) as a ready-to-file Jira draft; keep WW-5540 focused on caching. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * 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 <noreply@anthropic.com> * WW-5540 test(core): tighten localized-text cache tests Assert single cache entry in the per-call-format tests (proves the raw pattern is cached, not the formatted result), and mirror the package-cache clearBundle/clearMissingBundlesCache invalidation test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: note devMode null-valueStack eager-reload edge Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: link follow-up doc to filed ticket WW-5655 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 chore(core): add ASF license header to CacheFixture.properties Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 chore(core): add since/forRemoval to @Deprecated annotations Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: drop follow-up draft superseded by WW-5655 The ticket is filed; the draft's content now lives in WW-5655 itself. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 fix(core): address fresh-eyes review findings - Document that the deprecated getMessage/findMessage are no longer invoked by findText, and name formatMessage as the override point - Fall back to the ActionContext-based reloadBundles() when findText is called without a value stack, so the RELOADED flag is tracked and the caches can warm on that path in reload/devMode - Narrow resolveClassHierarchyRaw/resolvePackageHierarchyRaw to package-private (the cache key omits indexedKey, which is safe only when derived from textKey as the internal call sites do) - Suppress java:S2129 on the NOT_FOUND identity sentinel Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs: strip stray NUL bytes from design spec Two literal NUL bytes in the sentinel example made git/GitHub treat the whole markdown file as binary and unreviewable in the PR UI; align the example with the shipped sentinel name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 test(core): cover ModelDriven tier, per-locale keys, indexed keys Close the review-noted coverage gaps: the ModelDriven tier resolves via the shared class-hierarchy cache (action miss + model hit), each locale gets its own cache entry backed by a new _de fixture bundle, and indexed keys (name[N] -> name[*]) resolve and cache per full textKey. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 fix(core): address Copilot review comments - Partition the caches by System.identityHashCode of the context classloader so a custom ClassLoader overriding hashCode() cannot collide or collapse the per-loader partitions - Derive the indexed key inside the resolvers (miss-only) instead of accepting it as a parameter, so the cache key trivially covers every input that influences the resolution result Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 fix(core): resolve SonarCloud quality-gate findings - Suppress S4973 on isNotFound: the identity comparison against the non-interned NOT_FOUND sentinel is the design, not a bug - Reduce findMessageRaw cognitive complexity (S3776) by extracting getRawMessageWithAlternate, reused by the package walk - Add missing @Override annotations and suppress the deliberate deprecated-delegator call in the test helper (S1161, S5738) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 fix(core): make findMessageRaw cycle guard effective The `checked` set tested `contains(clazz.getName())` but never added the class, so the diamond-interface cycle guard was a no-op (a latent issue inherited from the original findMessage). Add the class name after the contains-check so repeated interface branches aren't re-traversed. Behavior is unchanged (lookups are idempotent); this only avoids redundant recursion. The deprecated findMessage delegates here, so it's fixed too. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * WW-5540 docs(core): reference WW-5658 in deprecated method javadoc Point the @deprecated javadoc of getMessage and findMessage at WW-5658, the ticket tracking their removal in the next major release. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -56,6 +56,8 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider {
|
||||
private static final String TOMCAT_WEBAPP_CLASSLOADER = "org.apache.catalina.loader.WebappClassLoader";
|
||||
private static final String TOMCAT_WEBAPP_CLASSLOADER_BASE = "org.apache.catalina.loader.WebappClassLoaderBase";
|
||||
private static final String RELOADED = "org.apache.struts2.util.LocalizedTextProvider.reloaded";
|
||||
@SuppressWarnings("java:S2129") // deliberate: a non-interned instance is required for an identity (==) sentinel
|
||||
private static final String NOT_FOUND = new String("__STRUTS_TEXT_NOT_FOUND__"); // unique identity sentinel; compared with ==
|
||||
|
||||
protected final ConcurrentMap<String, ResourceBundle> bundlesMap = new ConcurrentHashMap<>();
|
||||
protected boolean devMode = false;
|
||||
@@ -66,6 +68,8 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider {
|
||||
private final ConcurrentMap<Integer, List<String>> classLoaderMap = new ConcurrentHashMap<>();
|
||||
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) {
|
||||
@@ -90,6 +94,22 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider {
|
||||
return Thread.currentThread().getContextClassLoader();
|
||||
}
|
||||
|
||||
private int currentLoaderHashCode() {
|
||||
// Identity-based on purpose: a custom ClassLoader overriding hashCode() must not be able to
|
||||
// collapse (or collide) the per-classloader cache partitions.
|
||||
return System.identityHashCode(getCurrentThreadContextClassLoader());
|
||||
}
|
||||
|
||||
/** Test-support accessor: current number of cached class-hierarchy resolutions. */
|
||||
protected int classHierarchyCacheSize() {
|
||||
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()) {
|
||||
@@ -187,6 +207,8 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider {
|
||||
protected void clearBundle(final String bundleName, Locale locale) {
|
||||
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);
|
||||
}
|
||||
|
||||
@@ -204,6 +226,8 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider {
|
||||
*/
|
||||
protected void clearMissingBundlesCache() {
|
||||
missingBundles.clear();
|
||||
classHierarchyCache.clear();
|
||||
packageHierarchyCache.clear();
|
||||
LOG.debug("Cleared the missing bundles cache.");
|
||||
}
|
||||
|
||||
@@ -222,6 +246,8 @@ 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
|
||||
@@ -508,8 +534,47 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider {
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the message from the named resource bundle.
|
||||
* Resolves the raw (untranslated, unformatted) message pattern for a key within a single bundle.
|
||||
* Returns {@code null} when the bundle or key is absent. This is the cacheable unit relied upon by
|
||||
* the hierarchy-resolution caches; translation and formatting are applied separately by
|
||||
* {@link #formatMessage(String, Locale, ValueStack, Object[])}.
|
||||
*/
|
||||
private String getRawMessage(String bundleName, Locale locale, String key) {
|
||||
ResourceBundle bundle = findResourceBundle(bundleName, locale);
|
||||
if (bundle == null) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return bundle.getString(key);
|
||||
} catch (MissingResourceException e) {
|
||||
LOG.debug("Missing key [{}] in bundle [{}]!", key, bundleName);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Applies value stack variable translation (when a stack is available) and {@link MessageFormat}
|
||||
* argument substitution to a raw message pattern. Mirrors the rendering previously performed inline
|
||||
* by {@link #getMessage(String, Locale, String, ValueStack, Object[])}.
|
||||
*/
|
||||
protected String formatMessage(String rawPattern, Locale locale, ValueStack valueStack, Object[] args) {
|
||||
String message = (valueStack != null)
|
||||
? TextParseUtil.translateVariables(rawPattern, valueStack)
|
||||
: rawPattern;
|
||||
MessageFormat mf = buildMessageFormat(message, locale);
|
||||
return formatWithNullDetection(mf, args);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return the message from the named resource bundle.
|
||||
* @deprecated since 7.3.0 — superseded by the internal raw-resolution + caching path
|
||||
* ({@link #formatMessage(String, Locale, ValueStack, Object[])} over a raw lookup). Retained for
|
||||
* backward compatibility with descendant classes that call it directly. <strong>No longer invoked
|
||||
* by {@code findText}</strong>: overriding this method does not affect framework message lookup
|
||||
* anymore; override {@link #formatMessage(String, Locale, ValueStack, Object[])} to customize
|
||||
* rendering instead. Scheduled for removal in the next major release (see WW-5658).
|
||||
*/
|
||||
@Deprecated(since = "7.3.0", forRemoval = true)
|
||||
protected String getMessage(String bundleName, Locale locale, String key, ValueStack valueStack, Object[] args) {
|
||||
ResourceBundle bundle = findResourceBundle(bundleName, locale);
|
||||
if (bundle == null) {
|
||||
@@ -519,12 +584,8 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider {
|
||||
reloadBundles(valueStack.getContext());
|
||||
}
|
||||
try {
|
||||
String message = bundle.getString(key);
|
||||
if (valueStack != null) {
|
||||
message = TextParseUtil.translateVariables(bundle.getString(key), valueStack);
|
||||
}
|
||||
MessageFormat mf = buildMessageFormat(message, locale);
|
||||
return formatWithNullDetection(mf, args);
|
||||
String rawPattern = bundle.getString(key);
|
||||
return formatMessage(rawPattern, locale, valueStack, args);
|
||||
} catch (MissingResourceException e) {
|
||||
LOG.debug("Missing key [{}] in bundle [{}]!", key, bundleName);
|
||||
return null;
|
||||
@@ -532,73 +593,158 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider {
|
||||
}
|
||||
|
||||
/**
|
||||
* Traverse up class hierarchy looking for message. Looks at class, then implemented interface,
|
||||
* before going up hierarchy.
|
||||
*
|
||||
* @return the message
|
||||
* Raw-pattern twin of {@link #findMessage}. Walks class, implemented interfaces, then up the
|
||||
* hierarchy, returning the first raw message pattern found (via {@link #getRawMessage}) without
|
||||
* translation or formatting. Used by the cached class-hierarchy resolver.
|
||||
*/
|
||||
protected String findMessage(Class<?> clazz, String key, String indexedKey, Locale locale, Object[] args, Set<String> checked,
|
||||
ValueStack valueStack) {
|
||||
private String findMessageRaw(Class<?> clazz, String key, String indexedKey, Locale locale, Set<String> checked) {
|
||||
if (checked == null) {
|
||||
checked = new TreeSet<>();
|
||||
} else if (checked.contains(clazz.getName())) {
|
||||
return null;
|
||||
}
|
||||
// Record this class so diamond-shaped interface hierarchies aren't re-traversed.
|
||||
checked.add(clazz.getName());
|
||||
|
||||
// look in properties of this class
|
||||
String msg = getMessage(clazz.getName(), locale, key, valueStack, args);
|
||||
|
||||
String msg = getRawMessageWithAlternate(clazz.getName(), locale, key, indexedKey);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
|
||||
if (indexedKey != null) {
|
||||
msg = getMessage(clazz.getName(), locale, indexedKey, valueStack, args);
|
||||
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
|
||||
// look in properties of implemented interfaces
|
||||
Class<?>[] interfaces = clazz.getInterfaces();
|
||||
|
||||
for (Class<?> anInterface : interfaces) {
|
||||
msg = getMessage(anInterface.getName(), locale, key, valueStack, args);
|
||||
|
||||
for (Class<?> anInterface : clazz.getInterfaces()) {
|
||||
msg = getRawMessageWithAlternate(anInterface.getName(), locale, key, indexedKey);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
|
||||
if (indexedKey != null) {
|
||||
msg = getMessage(anInterface.getName(), locale, indexedKey, valueStack, args);
|
||||
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// traverse up hierarchy
|
||||
if (clazz.isInterface()) {
|
||||
interfaces = clazz.getInterfaces();
|
||||
|
||||
for (Class<?> anInterface : interfaces) {
|
||||
msg = findMessage(anInterface, key, indexedKey, locale, args, checked, valueStack);
|
||||
|
||||
for (Class<?> anInterface : clazz.getInterfaces()) {
|
||||
msg = findMessageRaw(anInterface, key, indexedKey, locale, checked);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if (!clazz.equals(Object.class) && !clazz.isPrimitive()) {
|
||||
return findMessage(clazz.getSuperclass(), key, indexedKey, locale, args, checked, valueStack);
|
||||
}
|
||||
} else if (!clazz.equals(Object.class) && !clazz.isPrimitive()) {
|
||||
return findMessageRaw(clazz.getSuperclass(), key, indexedKey, locale, checked);
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the raw message pattern for a key within a single bundle, falling back to the
|
||||
* indexed (general-form) key when the primary key is absent.
|
||||
*/
|
||||
private String getRawMessageWithAlternate(String bundleName, Locale locale, String key, String indexedKey) {
|
||||
String msg = getRawMessage(bundleName, locale, key);
|
||||
if (msg == null && indexedKey != null) {
|
||||
msg = getRawMessage(bundleName, locale, indexedKey);
|
||||
}
|
||||
return msg;
|
||||
}
|
||||
|
||||
/**
|
||||
* Cached resolution of the class/interface/superclass hierarchy for a key. Returns the raw pattern
|
||||
* found, or {@link #NOT_FOUND} when the key is absent from the entire hierarchy. Keyed on the
|
||||
* context classloader hash + class name + key + locale, so no {@link Class} reference is retained.
|
||||
* Uses get + putIfAbsent (never computeIfAbsent) because the child-property path recurses into findText.
|
||||
*/
|
||||
String resolveClassHierarchyRaw(Class<?> clazz, String textKey, Locale locale) {
|
||||
TextCacheKey cacheKey = new TextCacheKey(currentLoaderHashCode(), clazz.getName(), textKey, locale);
|
||||
String cached = classHierarchyCache.get(cacheKey);
|
||||
if (cached != null) {
|
||||
return cached;
|
||||
}
|
||||
// Derived here (miss-only) rather than accepted as a parameter, so the cache key trivially
|
||||
// covers every input that influences the resolution result.
|
||||
String raw = findMessageRaw(clazz, textKey, extractIndexedName(textKey), locale, null);
|
||||
String toStore = (raw != null) ? raw : NOT_FOUND;
|
||||
classHierarchyCache.putIfAbsent(cacheKey, toStore);
|
||||
return toStore;
|
||||
}
|
||||
|
||||
/** @return true when a cached raw-resolution result represents "not found". */
|
||||
@SuppressWarnings("java:S4973") // deliberate identity comparison against the non-interned NOT_FOUND sentinel
|
||||
protected boolean isNotFound(String cachedRawResult) {
|
||||
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 = getRawMessageWithAlternate(packageName, locale, textKey, 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}.
|
||||
*/
|
||||
String resolvePackageHierarchyRaw(Class<?> startClazz, String textKey, Locale locale) {
|
||||
TextCacheKey cacheKey = new TextCacheKey(currentLoaderHashCode(), startClazz.getName(), textKey, locale);
|
||||
String cached = packageHierarchyCache.get(cacheKey);
|
||||
if (cached != null) {
|
||||
return cached;
|
||||
}
|
||||
// Derived here (miss-only) rather than accepted as a parameter, so the cache key trivially
|
||||
// covers every input that influences the resolution result.
|
||||
String raw = findPackageMessageRaw(startClazz, textKey, extractIndexedName(textKey), 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.
|
||||
*
|
||||
* @return the message
|
||||
* @deprecated since 7.3.0 — superseded by the internal raw-resolution + caching path
|
||||
* ({@link #findMessageRaw} + {@link #formatMessage(String, Locale, ValueStack, Object[])}). Retained
|
||||
* for backward compatibility with descendant classes that call it directly. <strong>No longer
|
||||
* invoked by {@code findText}</strong>: overriding this method does not affect framework message
|
||||
* lookup anymore; override {@link #formatMessage(String, Locale, ValueStack, Object[])} to
|
||||
* customize rendering instead. Note: unlike the pre-7.3.0 implementation, a
|
||||
* candidate whose formatted value is the literal {@code "null"} no longer causes the search to
|
||||
* continue deeper in the same hierarchy; this affects only the pathological case of the same key
|
||||
* redefined at multiple hierarchy levels with the shallow value formatting to {@code "null"}.
|
||||
* The bundle-reload check is now triggered once on entry (when reload mode is enabled) rather than
|
||||
* lazily per bundle probe, preserving the reload side effect that the previous getMessage-per-probe
|
||||
* walk provided. Scheduled for removal in the next major release (see WW-5658).
|
||||
*/
|
||||
@Deprecated(since = "7.3.0", forRemoval = true)
|
||||
protected String findMessage(Class<?> clazz, String key, String indexedKey, Locale locale, Object[] args, Set<String> checked,
|
||||
ValueStack valueStack) {
|
||||
if (valueStack != null) {
|
||||
reloadBundles(valueStack.getContext());
|
||||
} else {
|
||||
reloadBundles();
|
||||
}
|
||||
String rawPattern = findMessageRaw(clazz, key, indexedKey, locale, checked);
|
||||
return rawPattern != null ? formatMessage(rawPattern, locale, valueStack, args) : null;
|
||||
}
|
||||
|
||||
protected String extractIndexedName(String textKey) {
|
||||
String indexedTextName = null;
|
||||
// calculate indexedTextName (collection[*]) if applicable
|
||||
@@ -658,6 +804,40 @@ abstract class AbstractLocalizedTextProvider implements LocalizedTextProvider {
|
||||
}
|
||||
}
|
||||
|
||||
static class TextCacheKey {
|
||||
private final int classLoaderHash;
|
||||
private final String className;
|
||||
private final String textKey;
|
||||
private final Locale locale;
|
||||
|
||||
TextCacheKey(int classLoaderHash, String className, String textKey, Locale locale) {
|
||||
this.classLoaderHash = classLoaderHash;
|
||||
this.className = className;
|
||||
this.textKey = textKey;
|
||||
this.locale = locale;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean equals(Object o) {
|
||||
if (this == o) return true;
|
||||
if (o == null || getClass() != o.getClass()) return false;
|
||||
TextCacheKey that = (TextCacheKey) o;
|
||||
return classLoaderHash == that.classLoaderHash
|
||||
&& Objects.equals(className, that.className)
|
||||
&& Objects.equals(textKey, that.textKey)
|
||||
&& Objects.equals(locale, that.locale);
|
||||
}
|
||||
|
||||
@Override
|
||||
public int hashCode() {
|
||||
int result = classLoaderHash;
|
||||
result = 31 * result + (className != null ? className.hashCode() : 0);
|
||||
result = 31 * result + (textKey != null ? textKey.hashCode() : 0);
|
||||
result = 31 * result + (locale != null ? locale.hashCode() : 0);
|
||||
return result;
|
||||
}
|
||||
}
|
||||
|
||||
static class GetDefaultMessageReturnArg {
|
||||
String message;
|
||||
boolean foundInBundle;
|
||||
|
||||
@@ -65,6 +65,17 @@ public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider {
|
||||
LOG.debug("Key is null, short-circuit to default message");
|
||||
return defaultMessage;
|
||||
}
|
||||
|
||||
// Trigger bundle reload (and cache invalidation) once, before any cached hierarchy lookup,
|
||||
// so that in reload/devMode the hierarchy caches are cleared before they are read. With no
|
||||
// value stack, fall back to the ActionContext-based overload so the RELOADED flag is still
|
||||
// tracked and the caches can warm on that path too.
|
||||
if (valueStack != null) {
|
||||
reloadBundles(valueStack.getContext());
|
||||
} else {
|
||||
reloadBundles();
|
||||
}
|
||||
|
||||
String indexedTextName = extractIndexedName(textKey);
|
||||
|
||||
// Allow for and track an early lookup for the message in the default resource bundles first, before searching the class hierarchy.
|
||||
@@ -81,11 +92,14 @@ public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider {
|
||||
}
|
||||
}
|
||||
|
||||
// search up class hierarchy
|
||||
String msg = findMessage(startClazz, textKey, indexedTextName, locale, args, null, valueStack);
|
||||
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
// search up class hierarchy (cached raw resolution; format per call)
|
||||
String classHierarchyRaw = resolveClassHierarchyRaw(startClazz, textKey, locale);
|
||||
String msg = null;
|
||||
if (!isNotFound(classHierarchyRaw)) {
|
||||
msg = formatMessage(classHierarchyRaw, locale, valueStack, args);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
|
||||
if (ModelDriven.class.isAssignableFrom(startClazz)) {
|
||||
@@ -99,37 +113,24 @@ public class StrutsLocalizedTextProvider extends AbstractLocalizedTextProvider {
|
||||
if (action instanceof ModelDriven) {
|
||||
Object model = ((ModelDriven<?>) action).getModel();
|
||||
if (model != null) {
|
||||
msg = findMessage(model.getClass(), textKey, indexedTextName, locale, args, null, valueStack);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
String modelRaw = resolveClassHierarchyRaw(model.getClass(), textKey, locale);
|
||||
if (!isNotFound(modelRaw)) {
|
||||
msg = formatMessage(modelRaw, locale, valueStack, args);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 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, locale);
|
||||
if (!isNotFound(packageRaw)) {
|
||||
msg = formatMessage(packageRaw, locale, valueStack, args);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
/*
|
||||
* 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.struts2.text;
|
||||
|
||||
/**
|
||||
* Simple fixture whose class-associated bundle ({@code CacheFixture.properties}) backs the
|
||||
* localized-text caching tests. The {@code name} property is exposed so OGNL expressions such as
|
||||
* {@code ${name}} can be resolved against a value stack.
|
||||
*/
|
||||
public class CacheFixture {
|
||||
|
||||
private final String name;
|
||||
|
||||
public CacheFixture(String name) {
|
||||
this.name = name;
|
||||
}
|
||||
|
||||
public String getName() {
|
||||
return name;
|
||||
}
|
||||
}
|
||||
@@ -547,6 +547,207 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase {
|
||||
assertEquals("Result of bean2.name lookup not as expected ?", "Okay! You found Me!", messageResult);
|
||||
}
|
||||
|
||||
public void testClassHierarchyCacheReusesFoundPattern() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
assertEquals("Cache not empty before first lookup ?", 0, provider.classHierarchyCacheSize());
|
||||
String first = provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Static cached value", first);
|
||||
assertEquals("Cache not populated after found lookup ?", 1, provider.classHierarchyCacheSize());
|
||||
|
||||
String second = provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Second lookup differs from first ?", first, second);
|
||||
assertEquals("Cache grew on repeated lookup ?", 1, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testClassHierarchyCacheStoresMisses() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
String first = provider.findText(CacheFixture.class, "cache.missing", Locale.ENGLISH, "Fallback", null, valueStack);
|
||||
assertEquals("Fallback", first);
|
||||
assertEquals("Miss not cached ?", 1, provider.classHierarchyCacheSize());
|
||||
|
||||
String second = provider.findText(CacheFixture.class, "cache.missing", Locale.ENGLISH, "Fallback", null, valueStack);
|
||||
assertEquals("Fallback", second);
|
||||
assertEquals("Miss cache grew on repeat ?", 1, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testFormattingIsPerCallNotCached() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
String x = provider.findText(CacheFixture.class, "cache.withparam", Locale.ENGLISH, null, new Object[]{"X"}, valueStack);
|
||||
String y = provider.findText(CacheFixture.class, "cache.withparam", Locale.ENGLISH, null, new Object[]{"Y"}, valueStack);
|
||||
assertEquals("Value with param X", x);
|
||||
assertEquals("Value with param Y", y);
|
||||
assertEquals("Raw pattern should be cached once, not per format ?", 1, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testOgnlTranslationIsPerCall() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
valueStack.push(new CacheFixture("World"));
|
||||
String world = provider.findText(CacheFixture.class, "cache.withognl", Locale.ENGLISH, null, null, valueStack);
|
||||
valueStack.pop();
|
||||
valueStack.push(new CacheFixture("Mars"));
|
||||
String mars = provider.findText(CacheFixture.class, "cache.withognl", Locale.ENGLISH, null, null, valueStack);
|
||||
valueStack.pop();
|
||||
|
||||
assertEquals("Hello World", world);
|
||||
assertEquals("Hello Mars", mars);
|
||||
assertEquals("Raw pattern should be cached once across value stacks ?", 1, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testNullFormattingFallsThroughToDefault() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
// "{0}" with a null arg formats to the literal "null"; findText must fall through to the default.
|
||||
String first = provider.findText(CacheFixture.class, "cache.nullformat", Locale.ENGLISH, "Fallback", new Object[]{null}, valueStack);
|
||||
assertEquals("Fallback", first);
|
||||
// Repeat after the pattern is cached — still falls through.
|
||||
String second = provider.findText(CacheFixture.class, "cache.nullformat", Locale.ENGLISH, "Fallback", new Object[]{null}, valueStack);
|
||||
assertEquals("Fallback", second);
|
||||
}
|
||||
|
||||
public void testReloadClearsClassHierarchyCache() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Cache not populated ?", 1, provider.classHierarchyCacheSize());
|
||||
|
||||
provider.callReloadBundlesForceReload();
|
||||
assertEquals("Reload did not clear class hierarchy cache ?", 0, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testClearBundleAndClearMissingCacheEmptyClassHierarchyCache() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Cache not populated ?", 1, provider.classHierarchyCacheSize());
|
||||
provider.callClearBundleWithLocale("org/apache/struts2/text/CacheFixture", Locale.ENGLISH);
|
||||
assertEquals("clearBundle did not empty class hierarchy cache ?", 0, provider.classHierarchyCacheSize());
|
||||
|
||||
provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Cache not repopulated ?", 1, provider.classHierarchyCacheSize());
|
||||
provider.callClearMissingBundlesCache();
|
||||
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 testClearBundleAndClearMissingCacheEmptyPackageHierarchyCache() {
|
||||
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.callClearBundleWithLocale("org/apache/struts2/test/package", Locale.getDefault());
|
||||
assertEquals("clearBundle did not empty package hierarchy cache ?", 0, provider.packageHierarchyCacheSize());
|
||||
|
||||
provider.findText(org.apache.struts2.test.ModelDrivenAction2.class, "package.properties", Locale.getDefault(), null, null, valueStack);
|
||||
assertEquals("Package cache not repopulated ?", 1, provider.packageHierarchyCacheSize());
|
||||
provider.callClearMissingBundlesCache();
|
||||
assertEquals("clearMissingBundlesCache did not empty 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();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
assertEquals("Static cached value", provider.callFindMessage(CacheFixture.class, "cache.static", Locale.ENGLISH, valueStack));
|
||||
assertNull(provider.callFindMessage(CacheFixture.class, "cache.missing", Locale.ENGLISH, valueStack));
|
||||
}
|
||||
|
||||
public void testModelDrivenTierUsesClassHierarchyCache() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
|
||||
ModelDrivenAction2 action = new ModelDrivenAction2();
|
||||
Mock mockActionInvocation = new Mock(ActionInvocation.class);
|
||||
mockActionInvocation.matchAndReturn("getAction", action);
|
||||
ActionContext.getContext().withActionInvocation((ActionInvocation) mockActionInvocation.proxy());
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
// "invalid.count" resolves only via the model's hierarchy (TestBean2 -> TestBean.properties),
|
||||
// not via the action class hierarchy, so it exercises the ModelDriven tier.
|
||||
String first = provider.findText(ModelDrivenAction2.class, "invalid.count", Locale.ENGLISH, null, null, valueStack);
|
||||
assertNotNull("Model-tier lookup found nothing ?", first);
|
||||
assertTrue("Model-tier lookup did not resolve via the TestBean bundle ?", first.startsWith("TestBean model:"));
|
||||
// Two entries: a miss for the action class hierarchy plus a hit for the model class hierarchy.
|
||||
assertEquals("Class-hierarchy cache should hold action miss + model hit ?", 2, provider.classHierarchyCacheSize());
|
||||
|
||||
String second = provider.findText(ModelDrivenAction2.class, "invalid.count", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Warm model-tier lookup differs from cold ?", first, second);
|
||||
assertEquals("Cache grew on repeated model-tier lookup ?", 2, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testLocaleIsPartOfCacheKey() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
String english = provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
String german = provider.findText(CacheFixture.class, "cache.static", Locale.GERMAN, null, null, valueStack);
|
||||
assertEquals("Static cached value", english);
|
||||
assertEquals("Statischer Wert", german);
|
||||
assertEquals("Each locale should have its own cache entry ?", 2, provider.classHierarchyCacheSize());
|
||||
|
||||
assertEquals("Warm English lookup differs ?", english,
|
||||
provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack));
|
||||
assertEquals("Warm German lookup differs ?", german,
|
||||
provider.findText(CacheFixture.class, "cache.static", Locale.GERMAN, null, null, valueStack));
|
||||
assertEquals("Cache grew on warm per-locale lookups ?", 2, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testIndexedKeyResolvesThroughCache() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
// "cache.indexed[20]" falls back to the general form "cache.indexed[*]" during raw resolution.
|
||||
String first = provider.findText(CacheFixture.class, "cache.indexed[20]", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Indexed cached value", first);
|
||||
assertEquals("Indexed lookup not cached ?", 1, provider.classHierarchyCacheSize());
|
||||
|
||||
String second = provider.findText(CacheFixture.class, "cache.indexed[20]", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Warm indexed lookup differs from cold ?", first, second);
|
||||
assertEquals("Cache grew on warm indexed lookup ?", 1, provider.classHierarchyCacheSize());
|
||||
|
||||
// A different index is a distinct cache key (the cache is keyed on the full textKey),
|
||||
// resolving to the same general form.
|
||||
String other = provider.findText(CacheFixture.class, "cache.indexed[7]", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Indexed cached value", other);
|
||||
assertEquals("A different index should create its own cache entry ?", 2, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void setUp() throws Exception {
|
||||
super.setUp();
|
||||
@@ -616,5 +817,20 @@ public class StrutsLocalizedTextProviderTest extends XWorkTestCase {
|
||||
final Object reloadedObject = ActionContext.getContext().get(RELOADED);
|
||||
return reloadedObject instanceof Boolean && (Boolean) reloadedObject;
|
||||
}
|
||||
|
||||
@Override
|
||||
public int classHierarchyCacheSize() {
|
||||
return super.classHierarchyCacheSize();
|
||||
}
|
||||
|
||||
@Override
|
||||
public int packageHierarchyCacheSize() {
|
||||
return super.packageHierarchyCacheSize();
|
||||
}
|
||||
|
||||
@SuppressWarnings("removal") // deliberately exercises the deprecated delegator
|
||||
public String callFindMessage(Class<?> clazz, String key, Locale locale, ValueStack valueStack) {
|
||||
return super.findMessage(clazz, key, null, locale, null, null, valueStack);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
#
|
||||
# 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
|
||||
cache.withparam=Value with param {0}
|
||||
cache.withognl=Hello ${name}
|
||||
cache.nullformat={0}
|
||||
cache.indexed[*]=Indexed cached value
|
||||
@@ -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=Statischer Wert
|
||||
@@ -0,0 +1,871 @@
|
||||
# WW-5540: LocalizedTextProvider Traversal Caching — Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Cache the class- and package-hierarchy traversal in `StrutsLocalizedTextProvider.findText` so repeated lookups for the same `(classloader, class, key, locale)` collapse to a single map lookup, without changing observable behavior.
|
||||
|
||||
**Architecture:** Split raw-pattern resolution (cacheable) from translation/formatting (per-call). Add two `ConcurrentHashMap` caches in `AbstractLocalizedTextProvider` — one for the class/interface/superclass walk, one for the `*.package` walk — each storing a raw pattern or a shared `NOT_FOUND` sentinel. `findText` reads the caches, then formats per call, and falls through to the next tier when a cached pattern formats to `null` (preserving `formatWithNullDetection` semantics).
|
||||
|
||||
**Tech Stack:** Java, Maven, JUnit 3/4 via `XWorkTestCase` (junit.framework), Mockito/mockobjects (existing test deps).
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Commit messages MUST be prefixed with the Jira ticket: `WW-5540 <type>[scope]: <desc>`.
|
||||
- Cache key stores the class **name (String)**, never a `Class` object — no classloader pinning.
|
||||
- Caches are unbounded `ConcurrentHashMap` (consistent with existing `bundlesMap`/`missingBundles`).
|
||||
- `NOT_FOUND` is a unique `String` instance compared by `==` (identity), never by content.
|
||||
- Use `get` + `putIfAbsent`, never `computeIfAbsent`, on the new caches (the child-property path recurses back into `findText`).
|
||||
- Only the raw pattern is cached; `TextParseUtil.translateVariables` + `MessageFormat` always run per call.
|
||||
- Core tests here extend `XWorkTestCase` — use `testXxx()` methods and `junit.framework` `assert*`; **no `@Test`, no AssertJ** (an `@Test` here silently never runs).
|
||||
- Build/test: `mvn test -DskipAssembly -pl core -Dtest=<Class>#<method>` (single) or `-Dtest=<Class>` (whole class).
|
||||
- Work on branch `WW-5540-localized-text-provider-caching` (already created off `main`).
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Raw / format split (behavior-preserving refactor)
|
||||
|
||||
Extract the "find the raw pattern" and "render a pattern" steps from `getMessage`, and add a raw twin of `findMessage`. No caching yet. Behavior must be byte-identical; the existing test suite is the safety net.
|
||||
|
||||
**Files:**
|
||||
- Modify: `core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java`
|
||||
- Test (regression only): `core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: existing `findResourceBundle`, `buildMessageFormat`, `formatWithNullDetection`, `reloadBundles`.
|
||||
- Produces (used by Task 2 & 3):
|
||||
- `private String getRawMessage(String bundleName, Locale locale, String key)` → raw pattern or `null`.
|
||||
- `protected String formatMessage(String rawPattern, Locale locale, ValueStack valueStack, Object[] args)` → translated+formatted string (or `null` via null-detection).
|
||||
- `private String findMessageRaw(Class<?> clazz, String key, String indexedKey, Locale locale, Set<String> checked)` → first raw pattern found walking class/interface/superclass, or `null`.
|
||||
- Deprecates (retained as legacy `protected` extension points, superseded by the raw path):
|
||||
- `getMessage(...)` — now `@Deprecated`, delegates its formatting to `formatMessage` (behavior identical).
|
||||
- `findMessage(...)` — now `@Deprecated`, delegates to `findMessageRaw` + `formatMessage`.
|
||||
|
||||
- [ ] **Step 1: Add `getRawMessage` and `formatMessage`, and re-express `getMessage` via them (deprecating it)**
|
||||
|
||||
In `AbstractLocalizedTextProvider`, add these two methods (place them just above the existing `getMessage`):
|
||||
|
||||
```java
|
||||
/**
|
||||
* Resolves the raw (untranslated, unformatted) message pattern for a key within a single bundle.
|
||||
* Returns {@code null} when the bundle or key is absent. This is the cacheable unit relied upon by
|
||||
* the hierarchy-resolution caches; translation and formatting are applied separately by
|
||||
* {@link #formatMessage(String, Locale, ValueStack, Object[])}.
|
||||
*/
|
||||
private String getRawMessage(String bundleName, Locale locale, String key) {
|
||||
ResourceBundle bundle = findResourceBundle(bundleName, locale);
|
||||
if (bundle == null) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return bundle.getString(key);
|
||||
} catch (MissingResourceException e) {
|
||||
LOG.debug("Missing key [{}] in bundle [{}]!", key, bundleName);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Applies value stack variable translation (when a stack is available) and {@link MessageFormat}
|
||||
* argument substitution to a raw message pattern. Mirrors the rendering previously performed inline
|
||||
* by {@link #getMessage(String, Locale, String, ValueStack, Object[])}.
|
||||
*/
|
||||
protected String formatMessage(String rawPattern, Locale locale, ValueStack valueStack, Object[] args) {
|
||||
String message = (valueStack != null)
|
||||
? TextParseUtil.translateVariables(rawPattern, valueStack)
|
||||
: rawPattern;
|
||||
MessageFormat mf = buildMessageFormat(message, locale);
|
||||
return formatWithNullDetection(mf, args);
|
||||
}
|
||||
```
|
||||
|
||||
Then replace the body of the existing `getMessage` (currently at `AbstractLocalizedTextProvider.java:513-532`) with the version below, and mark it `@Deprecated` (it is superseded internally by the raw-resolution path and retained only as a legacy extension point):
|
||||
|
||||
```java
|
||||
/**
|
||||
* @return the message from the named resource bundle.
|
||||
* @deprecated since 7.3.0 — superseded by the internal raw-resolution + caching path
|
||||
* ({@link #formatMessage(String, Locale, ValueStack, Object[])} over a raw lookup). Retained for
|
||||
* backward compatibility with descendant classes.
|
||||
*/
|
||||
@Deprecated
|
||||
protected String getMessage(String bundleName, Locale locale, String key, ValueStack valueStack, Object[] args) {
|
||||
ResourceBundle bundle = findResourceBundle(bundleName, locale);
|
||||
if (bundle == null) {
|
||||
return null;
|
||||
}
|
||||
if (valueStack != null) {
|
||||
reloadBundles(valueStack.getContext());
|
||||
}
|
||||
try {
|
||||
String rawPattern = bundle.getString(key);
|
||||
return formatMessage(rawPattern, locale, valueStack, args);
|
||||
} catch (MissingResourceException e) {
|
||||
LOG.debug("Missing key [{}] in bundle [{}]!", key, bundleName);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
(This keeps `getMessage`'s order — `findResourceBundle` → `reloadBundles` → `getString` — identical; only the trailing translate/format is now delegated to `formatMessage`, so behavior is unchanged.)
|
||||
|
||||
- [ ] **Step 2: Add `findMessageRaw`**
|
||||
|
||||
Add next to the existing `findMessage`. It mirrors `findMessage` exactly (including the pre-existing, never-populated `checked` cycle guard) but returns the raw pattern via `getRawMessage` with no translation/formatting/args:
|
||||
|
||||
```java
|
||||
/**
|
||||
* Raw-pattern twin of {@link #findMessage}. Walks class, implemented interfaces, then up the
|
||||
* hierarchy, returning the first raw message pattern found (via {@link #getRawMessage}) without
|
||||
* translation or formatting. Used by the cached class-hierarchy resolver.
|
||||
*/
|
||||
private String findMessageRaw(Class<?> clazz, String key, String indexedKey, Locale locale, Set<String> checked) {
|
||||
if (checked == null) {
|
||||
checked = new TreeSet<>();
|
||||
} else if (checked.contains(clazz.getName())) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// look in properties of this class
|
||||
String msg = getRawMessage(clazz.getName(), locale, key);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
if (indexedKey != null) {
|
||||
msg = getRawMessage(clazz.getName(), locale, indexedKey);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
|
||||
// look in properties of implemented interfaces
|
||||
Class<?>[] interfaces = clazz.getInterfaces();
|
||||
for (Class<?> anInterface : interfaces) {
|
||||
msg = getRawMessage(anInterface.getName(), locale, key);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
if (indexedKey != null) {
|
||||
msg = getRawMessage(anInterface.getName(), locale, indexedKey);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// traverse up hierarchy
|
||||
if (clazz.isInterface()) {
|
||||
interfaces = clazz.getInterfaces();
|
||||
for (Class<?> anInterface : interfaces) {
|
||||
msg = findMessageRaw(anInterface, key, indexedKey, locale, checked);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if (!clazz.equals(Object.class) && !clazz.isPrimitive()) {
|
||||
return findMessageRaw(clazz.getSuperclass(), key, indexedKey, locale, checked);
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Replace `findMessage`'s body with delegation and deprecate it**
|
||||
|
||||
Replace the entire body of the existing `findMessage` (currently at `AbstractLocalizedTextProvider.java:540-600`) with a thin delegation to `findMessageRaw` + `formatMessage`, and mark it `@Deprecated`. This removes the duplicated traversal (the walk now lives only in `findMessageRaw`):
|
||||
|
||||
```java
|
||||
/**
|
||||
* Traverse up class hierarchy looking for message. Looks at class, then implemented interface,
|
||||
* before going up hierarchy.
|
||||
*
|
||||
* @return the message
|
||||
* @deprecated since 7.3.0 — superseded by the internal raw-resolution + caching path
|
||||
* ({@link #findMessageRaw} + {@link #formatMessage(String, Locale, ValueStack, Object[])}). Retained
|
||||
* for backward compatibility with descendant classes. Note: unlike the pre-7.3.0 implementation, a
|
||||
* candidate whose formatted value is the literal {@code "null"} no longer causes the search to
|
||||
* continue deeper in the same hierarchy; this affects only the pathological case of the same key
|
||||
* redefined at multiple hierarchy levels with the shallow value formatting to {@code "null"}.
|
||||
* The bundle-reload check is now triggered once on entry (when reload mode is enabled) rather than
|
||||
* lazily per bundle probe, preserving the reload side effect that the previous {@code getMessage}-per-probe
|
||||
* walk provided.
|
||||
*/
|
||||
@Deprecated
|
||||
protected String findMessage(Class<?> clazz, String key, String indexedKey, Locale locale, Object[] args, Set<String> checked,
|
||||
ValueStack valueStack) {
|
||||
reloadBundles(valueStack != null ? valueStack.getContext() : null);
|
||||
String rawPattern = findMessageRaw(clazz, key, indexedKey, locale, checked);
|
||||
return rawPattern != null ? formatMessage(rawPattern, locale, valueStack, args) : null;
|
||||
}
|
||||
```
|
||||
|
||||
(The reload-on-entry preserves the side effect that the old `getMessage`-per-probe walk carried, so both the deprecated external-caller path and the Task-1 intermediate state — where `findText` still calls `findMessage` — keep triggering reload. The final cached path added in Task 2 relies instead on the reload hoisted to the top of `findText`.)
|
||||
|
||||
- [ ] **Step 4: Compile**
|
||||
|
||||
Run: `mvn -q test-compile -DskipAssembly -pl core`
|
||||
Expected: BUILD SUCCESS (new methods compile; `getRawMessage`/`findMessageRaw` may be flagged unused by the IDE but not by the compiler).
|
||||
|
||||
- [ ] **Step 5: Run the full regression suite for this class**
|
||||
|
||||
Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsLocalizedTextProviderTest`
|
||||
Expected: PASS — all existing tests green. `getMessage` is behavior-identical; `findMessage` is behavior-identical for all single-definition keys (the only tests here), so the suite proves the split. (`findMessage`'s deprecated delegation differs from before only in the pathological multi-level null-format case, which no existing test exercises.)
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java
|
||||
git commit -m "WW-5540 refactor(core): split raw message resolution from formatting
|
||||
|
||||
Add getRawMessage/formatMessage and a raw twin findMessageRaw. Re-express
|
||||
getMessage via formatMessage and make findMessage delegate to
|
||||
findMessageRaw + formatMessage; deprecate both as legacy extension points
|
||||
superseded by the raw-resolution path. Groundwork for the traversal caches.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Class-hierarchy cache (with invalidation, reload hoist, and correctness tests)
|
||||
|
||||
Introduce the first cache end-to-end: key type, sentinel, map, resolver, `findText` wiring for the class + ModelDriven tiers, invalidation at all three clear sites, and the reload hoist. Add a test fixture and correctness tests first.
|
||||
|
||||
**Files:**
|
||||
- Modify: `core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java`
|
||||
- Modify: `core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java`
|
||||
- Create: `core/src/test/java/org/apache/struts2/text/CacheFixture.java`
|
||||
- Create: `core/src/test/resources/org/apache/struts2/text/CacheFixture.properties`
|
||||
- Modify: `core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes (from Task 1): `getRawMessage`, `formatMessage`, `findMessageRaw`.
|
||||
- Produces (used by Task 3 and tests):
|
||||
- `static class TextCacheKey` with fields `int classLoaderHash, String className, String textKey, Locale locale` and `equals`/`hashCode`.
|
||||
- `private static final String NOT_FOUND` — identity sentinel.
|
||||
- `private final ConcurrentMap<TextCacheKey, String> classHierarchyCache`.
|
||||
- `private int currentLoaderHashCode()` → `getCurrentThreadContextClassLoader().hashCode()`.
|
||||
- `protected String resolveClassHierarchyRaw(Class<?> clazz, String textKey, String indexedKey, Locale locale)` → raw pattern or `NOT_FOUND`.
|
||||
- `protected int classHierarchyCacheSize()` → entry count (test support).
|
||||
|
||||
- [ ] **Step 1: Create the test fixture class**
|
||||
|
||||
Create `core/src/test/java/org/apache/struts2/text/CacheFixture.java`:
|
||||
|
||||
```java
|
||||
/*
|
||||
* 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.struts2.text;
|
||||
|
||||
/**
|
||||
* Simple fixture whose class-associated bundle ({@code CacheFixture.properties}) backs the
|
||||
* localized-text caching tests. The {@code name} property is exposed so OGNL expressions such as
|
||||
* {@code ${name}} can be resolved against a value stack.
|
||||
*/
|
||||
public class CacheFixture {
|
||||
|
||||
private final String name;
|
||||
|
||||
public CacheFixture(String name) {
|
||||
this.name = name;
|
||||
}
|
||||
|
||||
public String getName() {
|
||||
return name;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Create the fixture bundle**
|
||||
|
||||
Create `core/src/test/resources/org/apache/struts2/text/CacheFixture.properties`:
|
||||
|
||||
```properties
|
||||
cache.static=Static cached value
|
||||
cache.withparam=Value with param {0}
|
||||
cache.withognl=Hello ${name}
|
||||
cache.nullformat={0}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Add public cache-size accessor to the test helper**
|
||||
|
||||
In `StrutsLocalizedTextProviderTest.java`, inside the nested `TestStrutsLocalizedTextProvider` class (after `getBundlesReloadedIndicatorValue`, before its closing brace), add:
|
||||
|
||||
```java
|
||||
public int classHierarchyCacheSize() {
|
||||
return super.classHierarchyCacheSize();
|
||||
}
|
||||
|
||||
public String callFindMessage(Class<?> clazz, String key, Locale locale, ValueStack valueStack) {
|
||||
return super.findMessage(clazz, key, null, locale, null, null, valueStack);
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Write the failing tests**
|
||||
|
||||
Add these methods to `StrutsLocalizedTextProviderTest` (anywhere among the other `testXxx` methods). They reference `resolveClassHierarchyRaw`/`classHierarchyCacheSize`/`NOT_FOUND` behavior that does not exist yet, so they fail to compile/pass until Steps 5–9:
|
||||
|
||||
```java
|
||||
public void testClassHierarchyCacheReusesFoundPattern() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
assertEquals("Cache not empty before first lookup ?", 0, provider.classHierarchyCacheSize());
|
||||
String first = provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Static cached value", first);
|
||||
assertEquals("Cache not populated after found lookup ?", 1, provider.classHierarchyCacheSize());
|
||||
|
||||
String second = provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Second lookup differs from first ?", first, second);
|
||||
assertEquals("Cache grew on repeated lookup ?", 1, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testClassHierarchyCacheStoresMisses() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
String first = provider.findText(CacheFixture.class, "cache.missing", Locale.ENGLISH, "Fallback", null, valueStack);
|
||||
assertEquals("Fallback", first);
|
||||
assertEquals("Miss not cached ?", 1, provider.classHierarchyCacheSize());
|
||||
|
||||
String second = provider.findText(CacheFixture.class, "cache.missing", Locale.ENGLISH, "Fallback", null, valueStack);
|
||||
assertEquals("Fallback", second);
|
||||
assertEquals("Miss cache grew on repeat ?", 1, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testFormattingIsPerCallNotCached() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
String x = provider.findText(CacheFixture.class, "cache.withparam", Locale.ENGLISH, null, new Object[]{"X"}, valueStack);
|
||||
String y = provider.findText(CacheFixture.class, "cache.withparam", Locale.ENGLISH, null, new Object[]{"Y"}, valueStack);
|
||||
assertEquals("Value with param X", x);
|
||||
assertEquals("Value with param Y", y);
|
||||
}
|
||||
|
||||
public void testOgnlTranslationIsPerCall() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
valueStack.push(new CacheFixture("World"));
|
||||
String world = provider.findText(CacheFixture.class, "cache.withognl", Locale.ENGLISH, null, null, valueStack);
|
||||
valueStack.pop();
|
||||
valueStack.push(new CacheFixture("Mars"));
|
||||
String mars = provider.findText(CacheFixture.class, "cache.withognl", Locale.ENGLISH, null, null, valueStack);
|
||||
valueStack.pop();
|
||||
|
||||
assertEquals("Hello World", world);
|
||||
assertEquals("Hello Mars", mars);
|
||||
}
|
||||
|
||||
public void testNullFormattingFallsThroughToDefault() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
// "{0}" with a null arg formats to the literal "null"; findText must fall through to the default.
|
||||
String first = provider.findText(CacheFixture.class, "cache.nullformat", Locale.ENGLISH, "Fallback", new Object[]{null}, valueStack);
|
||||
assertEquals("Fallback", first);
|
||||
// Repeat after the pattern is cached — still falls through.
|
||||
String second = provider.findText(CacheFixture.class, "cache.nullformat", Locale.ENGLISH, "Fallback", new Object[]{null}, valueStack);
|
||||
assertEquals("Fallback", second);
|
||||
}
|
||||
|
||||
public void testReloadClearsClassHierarchyCache() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Cache not populated ?", 1, provider.classHierarchyCacheSize());
|
||||
|
||||
provider.callReloadBundlesForceReload();
|
||||
assertEquals("Reload did not clear class hierarchy cache ?", 0, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testClearBundleAndClearMissingCacheEmptyClassHierarchyCache() {
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Cache not populated ?", 1, provider.classHierarchyCacheSize());
|
||||
provider.callClearBundleWithLocale("org/apache/struts2/text/CacheFixture", Locale.ENGLISH);
|
||||
assertEquals("clearBundle did not empty class hierarchy cache ?", 0, provider.classHierarchyCacheSize());
|
||||
|
||||
provider.findText(CacheFixture.class, "cache.static", Locale.ENGLISH, null, null, valueStack);
|
||||
assertEquals("Cache not repopulated ?", 1, provider.classHierarchyCacheSize());
|
||||
provider.callClearMissingBundlesCache();
|
||||
assertEquals("clearMissingBundlesCache did not empty class hierarchy cache ?", 0, provider.classHierarchyCacheSize());
|
||||
}
|
||||
|
||||
public void testDeprecatedFindMessageStillDelegates() {
|
||||
// findMessage leaves findText's hot path in this task; this locks the deprecated delegator.
|
||||
TestStrutsLocalizedTextProvider provider = new TestStrutsLocalizedTextProvider();
|
||||
ValueStack valueStack = ActionContext.getContext().getValueStack();
|
||||
|
||||
assertEquals("Static cached value", provider.callFindMessage(CacheFixture.class, "cache.static", Locale.ENGLISH, valueStack));
|
||||
assertNull(provider.callFindMessage(CacheFixture.class, "cache.missing", Locale.ENGLISH, valueStack));
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run the new tests to confirm they fail**
|
||||
|
||||
Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsLocalizedTextProviderTest#testClassHierarchyCacheReusesFoundPattern+testReloadClearsClassHierarchyCache`
|
||||
Expected: compilation failure (`classHierarchyCacheSize()` undefined) — this is the red state.
|
||||
|
||||
- [ ] **Step 6: Add the cache field, sentinel, key type, loader-hash helper, and size accessor**
|
||||
|
||||
In `AbstractLocalizedTextProvider`, add the sentinel near the other constants (after `RELOADED` at line ~58):
|
||||
|
||||
```java
|
||||
private static final String NOT_FOUND = new String("__STRUTS_TEXT_NOT_FOUND__"); // unique identity sentinel; compared with ==
|
||||
```
|
||||
|
||||
Add the field next to the other caches (after `delegatedClassLoaderMap` at line ~68):
|
||||
|
||||
```java
|
||||
private final ConcurrentMap<TextCacheKey, String> classHierarchyCache = new ConcurrentHashMap<>();
|
||||
```
|
||||
|
||||
Add the helper and the size accessor (place near `getCurrentThreadContextClassLoader`):
|
||||
|
||||
```java
|
||||
private int currentLoaderHashCode() {
|
||||
return getCurrentThreadContextClassLoader().hashCode();
|
||||
}
|
||||
|
||||
/** Test-support accessor: current number of cached class-hierarchy resolutions. */
|
||||
protected int classHierarchyCacheSize() {
|
||||
return classHierarchyCache.size();
|
||||
}
|
||||
```
|
||||
|
||||
Add the key class next to the existing `MessageFormatKey` static class:
|
||||
|
||||
```java
|
||||
static class TextCacheKey {
|
||||
private final int classLoaderHash;
|
||||
private final String className;
|
||||
private final String textKey;
|
||||
private final Locale locale;
|
||||
|
||||
TextCacheKey(int classLoaderHash, String className, String textKey, Locale locale) {
|
||||
this.classLoaderHash = classLoaderHash;
|
||||
this.className = className;
|
||||
this.textKey = textKey;
|
||||
this.locale = locale;
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean equals(Object o) {
|
||||
if (this == o) return true;
|
||||
if (o == null || getClass() != o.getClass()) return false;
|
||||
TextCacheKey that = (TextCacheKey) o;
|
||||
return classLoaderHash == that.classLoaderHash
|
||||
&& Objects.equals(className, that.className)
|
||||
&& Objects.equals(textKey, that.textKey)
|
||||
&& Objects.equals(locale, that.locale);
|
||||
}
|
||||
|
||||
@Override
|
||||
public int hashCode() {
|
||||
int result = classLoaderHash;
|
||||
result = 31 * result + (className != null ? className.hashCode() : 0);
|
||||
result = 31 * result + (textKey != null ? textKey.hashCode() : 0);
|
||||
result = 31 * result + (locale != null ? locale.hashCode() : 0);
|
||||
return result;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
(`Objects`, `ConcurrentMap`, `ConcurrentHashMap`, `Locale` are already imported.)
|
||||
|
||||
- [ ] **Step 7: Add the cached class-hierarchy resolver**
|
||||
|
||||
Add to `AbstractLocalizedTextProvider` (near `findMessageRaw`):
|
||||
|
||||
```java
|
||||
/**
|
||||
* Cached resolution of the class/interface/superclass hierarchy for a key. Returns the raw pattern
|
||||
* found, or {@link #NOT_FOUND} when the key is absent from the entire hierarchy. Keyed on the
|
||||
* context classloader hash + class name + key + locale, so no {@link Class} reference is retained.
|
||||
* Uses get + putIfAbsent (never computeIfAbsent) because the child-property path recurses into findText.
|
||||
*/
|
||||
protected String resolveClassHierarchyRaw(Class<?> clazz, String textKey, String indexedKey, Locale locale) {
|
||||
TextCacheKey cacheKey = new TextCacheKey(currentLoaderHashCode(), clazz.getName(), textKey, locale);
|
||||
String cached = classHierarchyCache.get(cacheKey);
|
||||
if (cached != null) {
|
||||
return cached;
|
||||
}
|
||||
String raw = findMessageRaw(clazz, textKey, indexedKey, locale, null);
|
||||
String toStore = (raw != null) ? raw : NOT_FOUND;
|
||||
classHierarchyCache.putIfAbsent(cacheKey, toStore);
|
||||
return toStore;
|
||||
}
|
||||
|
||||
/** @return true when a cached raw-resolution result represents "not found". */
|
||||
protected boolean isNotFound(String cachedRawResult) {
|
||||
return cachedRawResult == NOT_FOUND;
|
||||
}
|
||||
```
|
||||
|
||||
(`NOT_FOUND` is `private` and not visible to the `StrutsLocalizedTextProvider` subclass, so the subclass tests "found?" via `isNotFound(...)` rather than referencing the sentinel directly.)
|
||||
|
||||
- [ ] **Step 8: Wire invalidation into the three clear sites**
|
||||
|
||||
In `reloadBundles(Map<String, Object> context)`, inside the `if (!reloaded)` block, add the clear immediately after `bundlesMap.clear();` (line ~224):
|
||||
|
||||
```java
|
||||
bundlesMap.clear();
|
||||
classHierarchyCache.clear();
|
||||
```
|
||||
|
||||
In `clearBundle(String bundleName, Locale locale)` (line ~187), add after the `bundlesMap.remove(key)` line:
|
||||
|
||||
```java
|
||||
final ResourceBundle removedBundle = bundlesMap.remove(key);
|
||||
classHierarchyCache.clear();
|
||||
```
|
||||
|
||||
In `clearMissingBundlesCache()` (line ~205), add after `missingBundles.clear();`:
|
||||
|
||||
```java
|
||||
missingBundles.clear();
|
||||
classHierarchyCache.clear();
|
||||
```
|
||||
|
||||
- [ ] **Step 9: Rewire `findText` (class + ModelDriven tiers) and hoist the reload**
|
||||
|
||||
In `StrutsLocalizedTextProvider.findText(Class<?>, String, Locale, String, Object[], ValueStack)` (lines 62-195), make these edits.
|
||||
|
||||
Immediately after the `textKey == null` guard (after line 67), add the reload hoist:
|
||||
|
||||
```java
|
||||
// Trigger bundle reload (and cache invalidation) once, before any cached hierarchy lookup,
|
||||
// so that in reload/devMode the hierarchy caches are cleared before they are read.
|
||||
reloadBundles(valueStack != null ? valueStack.getContext() : null);
|
||||
```
|
||||
|
||||
Replace the class-hierarchy block (current lines 84-89):
|
||||
|
||||
```java
|
||||
// search up class hierarchy
|
||||
String msg = findMessage(startClazz, textKey, indexedTextName, locale, args, null, valueStack);
|
||||
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```java
|
||||
// search up class hierarchy (cached raw resolution; format per call)
|
||||
String classHierarchyRaw = resolveClassHierarchyRaw(startClazz, textKey, indexedTextName, locale);
|
||||
String msg = null;
|
||||
if (!isNotFound(classHierarchyRaw)) {
|
||||
msg = formatMessage(classHierarchyRaw, locale, valueStack, args);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
Replace the ModelDriven inner lookup (current lines 102-105):
|
||||
|
||||
```java
|
||||
msg = findMessage(model.getClass(), textKey, indexedTextName, locale, args, null, valueStack);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```java
|
||||
String modelRaw = resolveClassHierarchyRaw(model.getClass(), textKey, indexedTextName, locale);
|
||||
if (!isNotFound(modelRaw)) {
|
||||
msg = formatMessage(modelRaw, locale, valueStack, args);
|
||||
if (msg != null) {
|
||||
return msg;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
(`isNotFound` was added in Step 7; the sentinel itself stays private to `AbstractLocalizedTextProvider`.)
|
||||
|
||||
Leave the package loop (lines 111-134), child-property block, and default-message block unchanged in this task.
|
||||
|
||||
- [ ] **Step 10: Compile and run the new tests**
|
||||
|
||||
Run: `mvn -q test-compile -DskipAssembly -pl core`
|
||||
Expected: BUILD SUCCESS.
|
||||
|
||||
Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsLocalizedTextProviderTest#testClassHierarchyCacheReusesFoundPattern+testClassHierarchyCacheStoresMisses+testFormattingIsPerCallNotCached+testOgnlTranslationIsPerCall+testNullFormattingFallsThroughToDefault+testReloadClearsClassHierarchyCache+testClearBundleAndClearMissingCacheEmptyClassHierarchyCache+testDeprecatedFindMessageStillDelegates`
|
||||
Expected: PASS (8 tests green).
|
||||
|
||||
- [ ] **Step 11: Run the full class to confirm no regression**
|
||||
|
||||
Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsLocalizedTextProviderTest`
|
||||
Expected: PASS — all tests (existing + 6 new) green.
|
||||
|
||||
- [ ] **Step 12: Commit**
|
||||
|
||||
```bash
|
||||
git add core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java \
|
||||
core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java \
|
||||
core/src/test/java/org/apache/struts2/text/CacheFixture.java \
|
||||
core/src/test/resources/org/apache/struts2/text/CacheFixture.properties \
|
||||
core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java
|
||||
git commit -m "WW-5540 perf(core): cache class-hierarchy text resolution
|
||||
|
||||
Cache the class/interface/superclass traversal in findText keyed on
|
||||
(classloader, class name, key, locale), storing the raw pattern or a
|
||||
NOT_FOUND marker. Formatting stays per call and falls through to the
|
||||
next tier when a cached pattern formats to null. Invalidated on
|
||||
reloadBundles/clearBundle/clearMissingBundlesCache; reload is hoisted
|
||||
to the top of findText so caches are cleared before they are read.
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Package-hierarchy cache
|
||||
|
||||
Cache the `*.package` traversal the same way, wire it into `findText`, and invalidate it at the same three sites.
|
||||
|
||||
**Files:**
|
||||
- Modify: `core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java`
|
||||
- Modify: `core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java`
|
||||
- Modify: `core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes (from Task 1/2): `getRawMessage`, `formatMessage`, `isNotFound`, `TextCacheKey`, `NOT_FOUND`, `currentLoaderHashCode`.
|
||||
- Produces:
|
||||
- `private final ConcurrentMap<TextCacheKey, String> packageHierarchyCache`.
|
||||
- `private String findPackageMessageRaw(Class<?> startClazz, String textKey, String indexedTextName, Locale locale)` → first raw `*.package` match or `null`.
|
||||
- `protected String resolvePackageHierarchyRaw(Class<?> startClazz, String textKey, String indexedTextName, Locale locale)` → raw pattern or `NOT_FOUND`.
|
||||
- `protected int packageHierarchyCacheSize()` → entry count (test support).
|
||||
|
||||
- [ ] **Step 1: Add public accessor to the test helper**
|
||||
|
||||
In `StrutsLocalizedTextProviderTest.TestStrutsLocalizedTextProvider`, add:
|
||||
|
||||
```java
|
||||
public int packageHierarchyCacheSize() {
|
||||
return super.packageHierarchyCacheSize();
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Write the failing tests**
|
||||
|
||||
Add to `StrutsLocalizedTextProviderTest`:
|
||||
|
||||
```java
|
||||
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());
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run the new tests to confirm they fail**
|
||||
|
||||
Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsLocalizedTextProviderTest#testPackageHierarchyCacheReusesFoundPattern+testReloadClearsPackageHierarchyCache`
|
||||
Expected: compilation failure (`packageHierarchyCacheSize()` undefined) — red state.
|
||||
|
||||
- [ ] **Step 4: Add the package cache field, raw walk, resolver, and size accessor**
|
||||
|
||||
In `AbstractLocalizedTextProvider`, add the field next to `classHierarchyCache`:
|
||||
|
||||
```java
|
||||
private final ConcurrentMap<TextCacheKey, String> packageHierarchyCache = new ConcurrentHashMap<>();
|
||||
```
|
||||
|
||||
Add the size accessor next to `classHierarchyCacheSize`:
|
||||
|
||||
```java
|
||||
/** Test-support accessor: current number of cached package-hierarchy resolutions. */
|
||||
protected int packageHierarchyCacheSize() {
|
||||
return packageHierarchyCache.size();
|
||||
}
|
||||
```
|
||||
|
||||
Add the raw package walk (mirrors the current package loop in `StrutsLocalizedTextProvider.findText`, using `getRawMessage`) and its cached resolver, next to `resolveClassHierarchyRaw`:
|
||||
|
||||
```java
|
||||
/**
|
||||
* 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;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Invalidate the package cache at the three clear sites**
|
||||
|
||||
Add `packageHierarchyCache.clear();` immediately after each `classHierarchyCache.clear();` added in Task 2 — in `reloadBundles` (the `if (!reloaded)` block), `clearBundle`, and `clearMissingBundlesCache`.
|
||||
|
||||
- [ ] **Step 6: Replace the package loop in `findText` with the cached resolver**
|
||||
|
||||
In `StrutsLocalizedTextProvider.findText`, replace the entire package-hierarchy loop (current lines 111-134):
|
||||
|
||||
```java
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
with:
|
||||
|
||||
```java
|
||||
// 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;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 7: Compile and run the new tests**
|
||||
|
||||
Run: `mvn -q test-compile -DskipAssembly -pl core`
|
||||
Expected: BUILD SUCCESS.
|
||||
|
||||
Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsLocalizedTextProviderTest#testPackageHierarchyCacheReusesFoundPattern+testReloadClearsPackageHierarchyCache`
|
||||
Expected: PASS (2 tests green). In particular `testFindTextInPackage` (existing) must still pass.
|
||||
|
||||
- [ ] **Step 8: Run the full class and the sibling suite**
|
||||
|
||||
Run: `mvn test -DskipAssembly -pl core -Dtest=StrutsLocalizedTextProviderTest`
|
||||
Expected: PASS — all tests green.
|
||||
|
||||
Run: `mvn test -DskipAssembly -pl core -Dtest=GlobalLocalizedTextProviderTest,LocalizedTextUtilTest`
|
||||
Expected: PASS (or "No tests matching" for any class that doesn't exist — confirm the ones that do exist pass). This guards the other `AbstractLocalizedTextProvider` subclass and legacy util.
|
||||
|
||||
- [ ] **Step 9: Commit**
|
||||
|
||||
```bash
|
||||
git add core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java \
|
||||
core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java \
|
||||
core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java
|
||||
git commit -m "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 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Final verification
|
||||
|
||||
- [ ] Run the whole core text package and a broad i18n-touching slice:
|
||||
`mvn test -DskipAssembly -pl core -Dtest='*LocalizedText*,*TextProvider*,TextProviderSupportTest'`
|
||||
Expected: PASS.
|
||||
- [ ] Confirm no `computeIfAbsent` was used on the new caches and no `Class` object is stored in any key.
|
||||
- [ ] Confirm `NOT_FOUND` is only ever compared via `isNotFound(...)` / `==`, never `.equals`.
|
||||
|
||||
## Notes / residual behavior (documented in the spec)
|
||||
|
||||
- A message that formats to the literal `"null"` and is redefined deeper in the *same* class hierarchy may resolve differently than before (shallow `null` short-circuits the cached path). Accepted as pathological — see the spec's "formatWithNullDetection fall-through" section.
|
||||
- Caches are unbounded, consistent with `bundlesMap`/`missingBundles`; see the spec's "Known limitation".
|
||||
@@ -0,0 +1,263 @@
|
||||
# WW-5540: Add caching to AbstractLocalizedTextProvider
|
||||
|
||||
- **Jira:** [WW-5540](https://issues.apache.org/jira/browse/WW-5540) — Improvement (Minor)
|
||||
- **Fix version:** 7.3.0
|
||||
- **Component:** Core
|
||||
- **Affected classes:** `org.apache.struts2.text.AbstractLocalizedTextProvider`, `org.apache.struts2.text.StrutsLocalizedTextProvider`
|
||||
|
||||
## Problem
|
||||
|
||||
`StrutsLocalizedTextProvider.findText(Class, textKey, locale, defaultMessage, args, valueStack)`
|
||||
runs a full **class → interface → superclass → `*.package` hierarchy traversal on every
|
||||
call**, plus an optional child-property recursion. This method is on the hottest path in the
|
||||
framework: it backs every UI tag label and every validation message rendered during a request.
|
||||
|
||||
Three caches already exist in `AbstractLocalizedTextProvider`:
|
||||
|
||||
- `bundlesMap` — resolved `ResourceBundle` by `(classloader, bundle, locale)`
|
||||
- `messageFormats` — `MessageFormat` by `(pattern, locale)`
|
||||
- `missingBundles` — negative cache for missing *bundles*
|
||||
|
||||
None of them caches the **outcome of the hierarchy traversal** keyed on
|
||||
`(classloader, class, textKey, locale)`. Missing keys are especially costly: they walk the
|
||||
entire class hierarchy *and* the entire package hierarchy, each iteration performing a bundle
|
||||
lookup and (for a miss) triggering a swallowed `MissingResourceException`. This work repeats
|
||||
identically on every request.
|
||||
|
||||
## Goal
|
||||
|
||||
Cache the result of the traversal so repeated lookups for the same `(class, key, locale)`
|
||||
collapse to a single map lookup, for both found and not-found keys — **without changing any
|
||||
observable behavior** (lookup order, dynamic-message evaluation, argument formatting,
|
||||
devMode/reload semantics).
|
||||
|
||||
## Non-goals
|
||||
|
||||
- No caching of the final rendered string (would break OGNL/`${...}` and per-call `args`).
|
||||
- No new `StrutsConstant` toggle (consistent with the other, un-toggled caches).
|
||||
- No bounded/LRU eviction — the caches remain unbounded `ConcurrentHashMap`s, matching
|
||||
`bundlesMap` / `missingBundles`. See *Known limitation* below.
|
||||
- No JMH microbenchmark added to the repo.
|
||||
|
||||
## Core invariant
|
||||
|
||||
The return value of `findText` is **not** a pure function of `(class, key, locale)`:
|
||||
|
||||
- The raw message may contain `${...}` OGNL expressions, evaluated per call via
|
||||
`TextParseUtil.translateVariables(pattern, valueStack)`.
|
||||
- `MessageFormat` is applied with per-call `args`.
|
||||
- The `defaultMessage`, the `ModelDriven` model instance, and the child-property recursion
|
||||
all depend on runtime `ActionContext` / `ValueStack` state.
|
||||
|
||||
Therefore **only the raw resolved pattern (or a `NOT_FOUND` marker) is cached.** Translation
|
||||
and formatting always run per call.
|
||||
|
||||
### `formatWithNullDetection` fall-through
|
||||
|
||||
`formatWithNullDetection` returns `null` when a message formats to the literal string `"null"`,
|
||||
and the current `findText` treats that as "not found in this tier" and **keeps searching** the
|
||||
remaining tiers (ModelDriven → package → default). Because formatting depends on per-call `args`
|
||||
/ ValueStack, the raw pattern the hierarchy "resolves to" is not a pure function of
|
||||
`(class, key, locale)` in that case (e.g. a `"{0}"` pattern with a `null` argument formats to
|
||||
`"null"`).
|
||||
|
||||
**Decision:** keep positive + negative caching, and preserve the fall-through: after a positive
|
||||
cache hit, `findText` formats the cached raw pattern and, **only if the formatted result is
|
||||
non-null, returns it** — otherwise it falls through to the next tier exactly as today. This makes
|
||||
the common case (shallow match formats to `null`, fall through to package/default) behave
|
||||
identically.
|
||||
|
||||
**Documented residual difference:** if the *same* key is defined at multiple levels of a single
|
||||
class hierarchy and the shallowest match formats to `"null"` for the given args while a deeper
|
||||
match would format to a real value, the current code returns the deeper match, whereas the cached
|
||||
path returns the shallow `null` and falls through past the deeper match. This requires the same
|
||||
key redefined within one hierarchy with the shallow value formatting to exactly `"null"` — treated
|
||||
as pathological and accepted.
|
||||
|
||||
## Design (Approach B — hierarchy-result cache)
|
||||
|
||||
### 1. Raw / format split
|
||||
|
||||
Add a private helper to `AbstractLocalizedTextProvider` that renders a raw pattern exactly as
|
||||
`getMessage` does inline today:
|
||||
|
||||
```java
|
||||
private String formatMessage(String rawPattern, Locale locale, ValueStack valueStack, Object[] args) {
|
||||
String message = (valueStack != null)
|
||||
? TextParseUtil.translateVariables(rawPattern, valueStack)
|
||||
: rawPattern;
|
||||
MessageFormat mf = buildMessageFormat(message, locale); // already cached via messageFormats
|
||||
return formatWithNullDetection(mf, args);
|
||||
}
|
||||
```
|
||||
|
||||
Add a raw resolver that does only lookup, no rendering:
|
||||
|
||||
```java
|
||||
private String getRawMessage(String bundleName, Locale locale, String key) {
|
||||
ResourceBundle bundle = findResourceBundle(bundleName, locale); // already cached via bundlesMap
|
||||
if (bundle == null) return null;
|
||||
try {
|
||||
return bundle.getString(key);
|
||||
} catch (MissingResourceException e) {
|
||||
LOG.debug("Missing key [{}] in bundle [{}]!", key, bundleName);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`getMessage(bundleName, locale, key, valueStack, args)` keeps its current signature and
|
||||
behavior, re-expressed as `getRawMessage(...)` followed by `formatMessage(...)` so every
|
||||
existing caller is byte-for-byte equivalent.
|
||||
|
||||
### 2. Two caches
|
||||
|
||||
```java
|
||||
private final ConcurrentMap<TextCacheKey, String> classHierarchyCache = new ConcurrentHashMap<>();
|
||||
private final ConcurrentMap<TextCacheKey, String> packageHierarchyCache = new ConcurrentHashMap<>();
|
||||
|
||||
private static final String NOT_FOUND = new String("__STRUTS_TEXT_NOT_FOUND__"); // identity sentinel
|
||||
```
|
||||
|
||||
- Values are either the raw pattern or the shared `NOT_FOUND` sentinel (a distinct `String`
|
||||
instance compared by `==`). A sentinel is required because `ConcurrentHashMap` forbids null
|
||||
values.
|
||||
- `TextCacheKey` is a small static class modelled on the existing `MessageFormatKey`, holding
|
||||
**`int classLoaderHash`, `String className`, `String textKey`, `Locale locale`** with
|
||||
`equals`/`hashCode`.
|
||||
- The key stores the **class name (String), not the `Class` object**, so the cache never pins
|
||||
a `Class`/classloader (consistent with recent classloader-leak remediation). `classLoaderHash`
|
||||
uses `getCurrentThreadContextClassLoader().hashCode()`, matching the keying scheme already
|
||||
used by `bundlesMap` / `missingBundles` / `classLoaderMap`.
|
||||
|
||||
### 3. Cached resolution methods
|
||||
|
||||
```java
|
||||
private String resolveClassHierarchyRaw(Class<?> clazz, String textKey, String indexedKey, Locale locale) {
|
||||
TextCacheKey key = new TextCacheKey(loaderHash(), clazz.getName(), textKey, locale);
|
||||
String cached = classHierarchyCache.get(key);
|
||||
if (cached != null) return cached; // hit: pattern or NOT_FOUND
|
||||
String raw = findMessageRaw(clazz, textKey, indexedKey, locale, null);
|
||||
String toStore = (raw != null) ? raw : NOT_FOUND;
|
||||
classHierarchyCache.putIfAbsent(key, toStore);
|
||||
return toStore;
|
||||
}
|
||||
```
|
||||
|
||||
- `resolvePackageHierarchyRaw(...)` mirrors this around the `*.package` loop.
|
||||
- `findMessageRaw` is the raw twin of `findMessage`: walks class → interfaces → superclass,
|
||||
tries `textKey` then `indexedKey` per class/interface, returns the raw pattern via
|
||||
`getRawMessage` (no translate/format/args). The internal recursion stays uncached; only the
|
||||
top-level entry per `(class, key, locale)` is cached.
|
||||
- Uses **`get` + `putIfAbsent`, not `computeIfAbsent`** — the child-property path recurses back
|
||||
into `findText`, and a nested `computeIfAbsent` on the same map during its own mapping
|
||||
function is unsafe.
|
||||
|
||||
### 4. `findText` flow (StrutsLocalizedTextProvider)
|
||||
|
||||
Order is **identical to the current implementation**; only the traversal steps become cached:
|
||||
|
||||
```
|
||||
if (textKey == null) return defaultMessage;
|
||||
indexedTextName = extractIndexedName(textKey);
|
||||
|
||||
if (searchDefaultBundlesFirst) { ... unchanged early lookup ... }
|
||||
|
||||
// class/interface/superclass hierarchy — CACHED
|
||||
raw = resolveClassHierarchyRaw(startClazz, textKey, indexedTextName, locale);
|
||||
if (raw != NOT_FOUND) { msg = formatMessage(raw, locale, valueStack, args); if (msg != null) return msg; }
|
||||
|
||||
// ModelDriven model hierarchy — CACHED (reuses classHierarchyCache)
|
||||
if (ModelDriven.class.isAssignableFrom(startClazz)) {
|
||||
... resolve model ...
|
||||
raw = resolveClassHierarchyRaw(model.getClass(), textKey, indexedTextName, locale);
|
||||
if (raw != NOT_FOUND) { msg = formatMessage(raw, locale, valueStack, args); if (msg != null) return msg; }
|
||||
}
|
||||
|
||||
// *.package hierarchy — CACHED
|
||||
raw = resolvePackageHierarchyRaw(startClazz, textKey, indexedTextName, locale);
|
||||
if (raw != NOT_FOUND) { msg = formatMessage(raw, locale, valueStack, args); if (msg != null) return msg; }
|
||||
|
||||
// child-property recursion — UNCACHED (valueStack-dependent) — unchanged
|
||||
// default message — unchanged
|
||||
```
|
||||
|
||||
Keeping `classHierarchyCache` and `packageHierarchyCache` separate preserves the exact
|
||||
ordering (class → ModelDriven → package → child-property → default). The `ModelDriven` model
|
||||
lookup reuses the class-hierarchy cache for free.
|
||||
|
||||
## Cache invalidation
|
||||
|
||||
The two new caches are cleared at every site that already clears the bundle caches:
|
||||
|
||||
1. **`reloadBundles(context)`** — inside the guarded block that calls `bundlesMap.clear()`,
|
||||
also `classHierarchyCache.clear()` and `packageHierarchyCache.clear()`. This preserves
|
||||
devMode / `struts.i18n.reload=true` semantics: the caches are wiped once per request
|
||||
(guarded by the `RELOADED` context flag), so edited properties are always seen. In
|
||||
production (`reloadBundles=false`) the caches persist — the intended win. When `findText`
|
||||
is called with a null ValueStack, the hoisted reload now passes a null context; in
|
||||
reload/devMode this triggers an eager cache/bundle clear that the previous per-probe path
|
||||
skipped for null-stack lookups — strictly more eager (never staler), and inert in
|
||||
production where reload is disabled.
|
||||
2. **`clearBundle(bundleName, locale)`** — clear both new caches fully (a cached pattern may
|
||||
originate from the cleared bundle; rare admin operation, full clear acceptable).
|
||||
3. **`clearMissingBundlesCache()`** — clear both new caches fully, honoring its documented
|
||||
intent that previously-missing lookups may now resolve (our `NOT_FOUND` entries are
|
||||
analogous to the missing-bundle cache).
|
||||
|
||||
**Not invalidated by `addDefaultResourceBundle`:** the two caches only ever hold class-named
|
||||
and `*.package` bundle results. Default bundles are consulted solely in the uncached
|
||||
`getDefaultMessage` path, so adding a default bundle cannot stale these caches.
|
||||
|
||||
## Correctness invariants preserved
|
||||
|
||||
- **Dynamic `${...}` messages** — raw pattern cached; `translateVariables` runs per call against
|
||||
the live `ValueStack`, so different stacks yield different outputs.
|
||||
- **Parameterized messages** — `MessageFormat.format(args)` runs per call.
|
||||
- **`defaultMessage`, child-property, `ModelDriven` model resolution** — runtime-dependent,
|
||||
never cached at the result level.
|
||||
- **Lookup order** — unchanged.
|
||||
- **No classloader pinning** — keys hold class names, not `Class` objects.
|
||||
|
||||
## Testing
|
||||
|
||||
Tests live in `core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java`,
|
||||
which extends `XWorkTestCase` — **JUnit 3/4 style: `testXxx()` methods with plain `assert*`
|
||||
(junit.framework), no `@Test`, no AssertJ.** (A `@Test`-annotated method here would silently
|
||||
never run.)
|
||||
|
||||
1. **Found key, repeated lookup** — same message on both calls; second served from cache.
|
||||
2. **Missing key, repeated lookup** — default/null on both; cached as `NOT_FOUND` (verified
|
||||
indirectly: after caching a miss, adding the property + `reloadBundles` makes it resolve).
|
||||
3. **Dynamic `${...}` message after a cache hit** — same `(class, key, locale)` rendered against
|
||||
two different `ValueStack`s yields two different results.
|
||||
4. **Parameterized message** — different `args` across calls format correctly.
|
||||
5. **`reloadBundles` invalidation** — with reload enabled, a changed bundle value is picked up
|
||||
on the next request.
|
||||
6. **`clearBundle` / `clearMissingBundlesCache`** — both drop cached entries.
|
||||
7. **`ModelDriven` path** — model-class messages still resolve, reusing the class-hierarchy cache.
|
||||
8. **Package-hierarchy resolution** — a `*.package` key resolves, caches, and the class-before-package
|
||||
order is unchanged.
|
||||
9. **Regression** — the existing `StrutsLocalizedTextProviderTest` suite and related
|
||||
localized-text tests stay green.
|
||||
|
||||
## Known limitation
|
||||
|
||||
Per decision during design, the new caches are **unbounded** `ConcurrentHashMap`s, matching the
|
||||
existing `bundlesMap` / `missingBundles`. Because `getText`-style tags can accept dynamic /
|
||||
user-influenced keys, an unbounded `(class, key, locale)` key space is a theoretical
|
||||
memory-exhaustion vector. This is consistent with the pre-existing exposure of the other caches
|
||||
and is accepted here for consistency and simplicity; bounding all localized-text caches could be
|
||||
revisited as a separate, cross-cutting change.
|
||||
|
||||
## Files touched
|
||||
|
||||
- `core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java`
|
||||
— `formatMessage`, `getRawMessage`, `findMessageRaw`, `resolveClassHierarchyRaw`,
|
||||
`resolvePackageHierarchyRaw`, `TextCacheKey`, `NOT_FOUND`, the two cache maps, and
|
||||
invalidation hooks in `reloadBundles` / `clearBundle` / `clearMissingBundlesCache`;
|
||||
`getMessage` re-expressed via `getRawMessage` + `formatMessage`.
|
||||
- `core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java`
|
||||
— `findText(Class, ...)` rewired to the cached resolvers.
|
||||
- `core/src/test/java/org/apache/struts2/text/StrutsLocalizedTextProviderTest.java`
|
||||
— new tests.
|
||||
Reference in New Issue
Block a user