This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5539 in repository https://gitbox.apache.org/repos/asf/struts.git
commit 058d59a31d17863c8d8d9f169e4ba41cd7c8e77b Author: Lukasz Lenart <[email protected]> AuthorDate: Tue Jul 21 16:54:30 2026 +0200 WW-5539 docs: make conditionalReload store its rebuilt mapping buildConverterMapping no longer stores its result, so reload mode would have rebuilt from disk on every request without ever caching. --- .../2026-07-21-WW-5539-concurrency-performance.md | 23 +++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-07-21-WW-5539-concurrency-performance.md b/docs/superpowers/plans/2026-07-21-WW-5539-concurrency-performance.md index 1637ddd62..24ef23a4e 100644 --- a/docs/superpowers/plans/2026-07-21-WW-5539-concurrency-performance.md +++ b/docs/superpowers/plans/2026-07-21-WW-5539-concurrency-performance.md @@ -727,6 +727,27 @@ That is, delete this block entirely: } ``` +Now make `conditionalReload` store the rebuilt mapping, which `buildConverterMapping` no longer does. Without this, reload mode returns fresh data but never caches it, rebuilding from disk on every single request. Replace `conditionalReload` (lines 580-591) with: + +```java + @SuppressWarnings("deprecation") + private Map<String, Object> conditionalReload(Class clazz, Map<String, Object> oldValues) throws Exception { + Map<String, Object> mapping = oldValues; + + if (reloadingConfigs) { + URL fileUrl = ClassLoaderUtil.getResource(buildConverterFilename(clazz), clazz); + if (fileManager.fileNeedsReloading(fileUrl)) { + mapping = buildConverterMapping(clazz); + // addMapping is deprecated but remains the correct primitive here: + // computeMappingIfAbsent cannot express an unconditional overwrite. + converterHolder.addMapping(clazz, mapping); + } + } + + return mapping; + } +``` + Update the method's Javadoc to record the behaviour change, since it is `protected` and visible to subclasses: ```java @@ -1165,4 +1186,4 @@ Follow `~/.claude/pr_guideline.md`. **Concurrency tests prove presence, not absence.** A green run on a strongly-ordered x86 machine is weak evidence. The primary correctness argument is the reasoning about the data structures; the tests are a backstop. Do not add sleeps or retry loops to make a flaky assertion pass — if one is flaky, the design reasoning is wrong and needs revisiting. -**`conditionalReload` stays outside the compute** in `getConverter`. It only does work when `struts.configuration.xml.reload` is enabled and it must run on cache *hits* — that is its entire purpose. +**`conditionalReload` stays outside the compute** in `getConverter`. It only does work when `struts.configuration.xml.reload` is enabled and it must run on cache *hits* — that is its entire purpose. It stores its own rebuild via the deprecated `addMapping`, because `computeMappingIfAbsent` cannot express an unconditional overwrite. That deprecated call is deliberate and carries a comment saying so; do not "clean it up".
