WW-3530 Fix visitor-validator cache-key collision under wildcard actions#1811
WW-3530 Fix visitor-validator cache-key collision under wildcard actions#1811lukaszlenart wants to merge 8 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… fix Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…g under wildcard actions Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…wildcard actions Apply the wildcard config-name substitution only when validating the action's own class. Visited objects carry a stable, explicit visitor context that must remain part of the cache key, otherwise two visitor validators on one field with different contexts collide and the second is silently dropped. Fixes https://issues.apache.org/jira/browse/WW-3530 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…sitor limitation Final-review finding: default-context visitor validators under wildcard actions key on the volatile resolved action name for the visited class, reintroducing bounded WW-2996-style cache growth (memory only; correct validators still load). Correct the 'WW-2996 untouched' wording to 'untouched for the action's own class', document the subpath as an accepted limitation folded into the follow-up ticket, and clarify that end-to-end visitor execution is covered by existing visitor suites. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… decision Log the built key together with clazz, context, validatingActionClass, wildcard, and the action config name, so the wildcard-vs-visited-object branch taken in buildValidatorKey can be diagnosed at runtime. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Fixes a long-standing validator cache-key collision affecting visitor field validators under wildcard/named-pattern actions by narrowing when wildcard config-name substitution is applied. This ensures distinct visitor context values don’t collapse into the same cached validator entry for visited model classes, while preserving the WW-2996 wildcard-action caching behavior for the action’s own class.
Changes:
- Adjust
AnnotationActionValidatorManager.buildValidatorKey()to only substituteconfigName|methodfor wildcard actions when validating the action’s own class. - Add unit tests that pin both the WW-3530 visited-class behavior and the WW-2996 action-class behavior under wildcard actions.
- Add design/spec and implementation plan documentation under
docs/superpowers/.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| docs/superpowers/specs/2026-07-25-WW-3530-visitor-validator-cache-key-design.md | Design/spec documenting root cause, fix approach, and known limitations. |
| docs/superpowers/plans/2026-07-25-WW-3530-visitor-validator-cache-key.md | Step-by-step implementation plan and test guidance for WW-3530. |
| core/src/main/java/org/apache/struts2/validator/AnnotationActionValidatorManager.java | Narrow wildcard cache-key substitution to the action’s own class; adds debug logging (flagged in review). |
| core/src/test/java/org/apache/struts2/validator/AnnotationActionValidatorManagerTest.java | Adds targeted unit tests for visited-class vs action-class key behavior under wildcard actions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… comment grammar Address SonarCloud S5785 (assertFalse+equals -> assertNotEquals) and a Copilot grammar nit in the WW-4536 comment. DEBUG logging kept as-is per author decision. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|



Fixes WW-3530
Problem
Two visitor field validators declared on the same field with different
contextparams (e.g.
basicandadditional) collide in the validator cache underwildcard / named-pattern actions: both produce the same cache key, so the
first context's validators are returned for both and the second context's
validators are silently dropped.
Originally reported in 2010 (v2.2.1) when the cache key omitted
contextentirely. WW-2996 / WW-3753 / WW-4536 later reworked the key so
contextisincluded for normal actions — fixing the report for non-wildcard actions. The
defect still reproduces for wildcard actions, where the key intentionally
substitutes the action config name for
context.Root cause
AnnotationActionValidatorManager.buildValidatorKeysubstitutes the action'sconfig name + method for
contextwhenever the current action is awildcard/named-pattern action. That substitution is designed for the action's
own class (where
contextis the volatile URL-derived action name that WW-2996must not key on). But it was applied too broadly — it also fired when validating
a visited object under a visitor validator, whose
contextis a stable,explicit param, and wrongly discarded it.
Fix
Narrow the substitution to the case it was designed for: apply the config-name
key only when the class being validated is the action's own class
(
clazz.equals(invocation.getAction().getClass())). For any other class (avisited object), key on
context— matchingDefaultActionValidatorManager.The request-validation hot path (
ValidationInterceptor→getValidators(action.getClass(), …)) always validates the live actioninstance's own class, so WW-2996's single-cache-entry-per-wildcard-action
behavior is fully preserved. The cache-key change only affects cache reuse; it
never changes which validators load (
buildValidatorConfigsreceivescontextdirectly).
Tests
AnnotationActionValidatorManagerTest:failing-before / passing-after case)
behavior pinned)
AnnotationActionValidatorManagerTest(14/14) andVisitorFieldValidatorTest/VisitorFieldValidatorModelTest/DefaultActionValidatorManagerTest(23/23) pass.coremodule suite has not yet been run in thisbranch — to be completed before marking ready for review.
Known limitations (follow-up ticket to be filed)
Both need an explicit visitor signal threaded through the manager API to fix
cleanly, and are memory-only (correct validators still load):
no explicit
contextdefaults to the resolved action name, which is volatileunder a wildcard action — so the visited-class key moves from stable
(
configName|method) to volatile, causing bounded WW-2996-style cache growth.Disappears when the visitor declares an explicit
context.<s:form>client-side JS-validation render path: resolves the actionclass by name rather than from the live action instance, so for a
cross-action or proxied form the action's own top-level lookup falls to the
contextbranch under a wildcard action (extra cache entries, correctnessunaffected).
Design spec and implementation plan are included under
docs/superpowers/.🤖 Generated with Claude Code