(struts) 06/06: WW-3530 docs: correct WW-2996 scope claim and note default-context visitor limitation

[lu...@apache.org]

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 <lu...@apache.org>
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 <no...@anthropic.com>
---
 ...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