lukaszlenart commented on code in PR #1808:
URL: https://github.com/apache/struts/pull/1808#discussion_r3638055400
##########
core/src/main/java/org/apache/struts2/text/StrutsLocalizedTextProvider.java:
##########
@@ -65,6 +65,11 @@ public String findText(Class<?> startClazz, String textKey,
Locale locale, Strin
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.
+ reloadBundles(valueStack != null ? valueStack.getContext() : null);
Review Comment:
Good catch — this was also flagged by an internal review pass and fixed in
e4166f876 (before this comment was posted): when the value stack is null,
`findText` now falls back to the ActionContext-based `reloadBundles()`
overload, so the `RELOADED` flag is tracked and the caches can warm on that
path.
##########
core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java:
##########
@@ -90,6 +93,20 @@ protected ClassLoader getCurrentThreadContextClassLoader() {
return Thread.currentThread().getContextClassLoader();
}
+ private int currentLoaderHashCode() {
+ return getCurrentThreadContextClassLoader().hashCode();
+ }
Review Comment:
Fixed in b04e0f416 — `currentLoaderHashCode()` now uses
`System.identityHashCode(...)`. The pre-existing maps (`classLoaderMap`,
`createMissesKey`) keep their `hashCode()`-based keying for now; the new caches
don't share keys with them, so the divergence is safe.
##########
core/src/main/java/org/apache/struts2/text/AbstractLocalizedTextProvider.java:
##########
@@ -582,23 +628,113 @@ protected String findMessage(Class<?> clazz, String key,
String indexedKey, Loca
// traverse up hierarchy
if (clazz.isInterface()) {
interfaces = clazz.getInterfaces();
-
for (Class<?> anInterface : interfaces) {
- msg = findMessage(anInterface, key, indexedKey, locale, args,
checked, valueStack);
-
+ 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);
+ return findMessageRaw(clazz.getSuperclass(), key, indexedKey,
locale, checked);
}
}
return null;
}
+ /**
+ * 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);
Review Comment:
Fixed in b04e0f416 using the stronger variant you suggested: the
`indexedKey` parameter was removed and the resolvers derive it internally via
`extractIndexedName(textKey)` (miss-only), so the cache key now covers every
input that influences the resolution result.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
