This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-3530-visitor-validator-cache-key in repository https://gitbox.apache.org/repos/asf/struts.git
commit f4c72f9b33994b40c3f597a1ecd7182df9602066 Author: Lukasz Lenart <[email protected]> AuthorDate: Sat Jul 25 09:34:18 2026 +0200 WW-3530 docs: correct WW-2996 scope claim and note default-context visitor 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 <[email protected]> --- ...026-07-25-WW-3530-visitor-validator-cache-key.md | 1 + ...25-WW-3530-visitor-validator-cache-key-design.md | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/plans/2026-07-25-WW-3530-visitor-validator-cache-key.md b/docs/superpowers/plans/2026-07-25-WW-3530-visitor-validator-cache-key.md index fc688cf18..be6e22a13 100644 --- a/docs/superpowers/plans/2026-07-25-WW-3530-visitor-validator-cache-key.md +++ b/docs/superpowers/plans/2026-07-25-WW-3530-visitor-validator-cache-key.md @@ -218,6 +218,7 @@ Verification-only task; nothing to commit if the suite passes. ## Self-Review Notes +- **Test scope note:** Tasks 1-2 verify the fix at the `buildValidatorKey` unit level (distinct cache keys per context), a deterministic proxy for spec test #1. End-to-end "both visitor validators actually execute" is covered by the existing `VisitorFieldValidatorTest` / `VisitorFieldValidatorModelTest` suites (run as regression in Task 2 Step 4), not by a new integration test here. - **Spec coverage:** Root-cause narrowing → Task 2. Reproduction test (distinct cache entries per context for visited objects, spec test #1) → Task 1 Step 2. WW-2996 regression guard (spec test #2) → Task 1 Step 3. Non-wildcard visitor + `DefaultActionValidatorManager` unchanged (spec test #3) → Task 2 Step 4. Null-action guard (spec edge case) → `action != null` in Task 2 Step 1. Self-visiting wildcard limitation is intentionally not exercised (accepted known limitation per spec). - **Placeholder scan:** none. - **Type consistency:** `installInvocation(String, Object)`, `buildValidatorKey(Class, String)`, and `configName`/`wildcard`/`validatingActionClass` locals are consistent across tasks. diff --git a/docs/superpowers/specs/2026-07-25-WW-3530-visitor-validator-cache-key-design.md b/docs/superpowers/specs/2026-07-25-WW-3530-visitor-validator-cache-key-design.md index 4a2889aa5..c12730723 100644 --- a/docs/superpowers/specs/2026-07-25-WW-3530-visitor-validator-cache-key-design.md +++ b/docs/superpowers/specs/2026-07-25-WW-3530-visitor-validator-cache-key-design.md @@ -93,8 +93,12 @@ protected String buildValidatorKey(Class clazz, String context) { The only difference: when `clazz` is **not** the action class (a visited object under a visitor validator), the key now includes `context` even if the current action is a wildcard. `basic` and `additional` therefore get distinct cache -entries and both run. The wildcard-action caching path (WW-2996) is untouched, -because it now fires only when `clazz == action.getClass()`. +entries and both run. The wildcard-action caching path (WW-2996) is untouched +**for the action's own class**, because the `configName|method` branch now fires +only when `clazz == action.getClass()` — the request-validation hot path +(`ValidationInterceptor` → `getValidators(action.getClass(), …)`) is fully +preserved. See the default-context visitor note below for the one subpath where +keying moves from stable to volatile. ## Scope, edge cases, non-goals @@ -107,6 +111,19 @@ because it now fires only when `clazz == action.getClass()`. because `clazz == action.getClass()` takes the wildcard branch. Accepted known limitation — extremely contrived, and resolving it would require reintroducing the volatile-vs-stable-context ambiguity this fix avoids. Documented, not fixed. +- **Default-context visitor under a wildcard action** (visitor validator with no + explicit `context` param): `VisitorFieldValidator` (`VisitorFieldValidator.java:141`) + defaults `visitorContext` to the *resolved* action name when `context == null`. + Under a wildcard action that name is volatile (per URL). Before this change the + visited object keyed on the stable `configName|method`; after it, keying on + `context` (= the volatile action name) creates one cache entry per resolved name + for that visited class — WW-2996-style cache growth. This is a **caching-efficiency** + regression only (correct validators still load; the growth is bounded by the + number of resolved wildcard names, narrower than the original WW-2996 leak), and + it disappears the moment the visitor validator declares an explicit `context`. + Accepted for this change; folded into the same follow-up ticket as the render-path + item (a clean fix needs the rejected "Approach C" — an explicit visitor signal + through the manager API). - **`<s:form>` client-side JS-validation render path** (`Form.getValidators`, `Form.java:295`): this path resolves `actionClass` by *name* (via `ServletUrlRenderer`), not from the live action instance. When the form targets
