This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-3784-annotated-wildcard-specificity-ordering in repository https://gitbox.apache.org/repos/asf/struts.git
commit 5de416ae21a69ae80fba09e85deb0578cc7e5c19 Author: Lukasz Lenart <[email protected]> AuthorDate: Sun Jul 26 05:29:33 2026 +0200 WW-3784 docs: design for specificity-ordered wildcard matching in annotated actions Co-Authored-By: Claude Opus 4.8 <[email protected]> --- ...notated-wildcard-specificity-ordering-design.md | 171 +++++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/docs/superpowers/specs/2026-07-26-WW-3784-annotated-wildcard-specificity-ordering-design.md b/docs/superpowers/specs/2026-07-26-WW-3784-annotated-wildcard-specificity-ordering-design.md new file mode 100644 index 000000000..4f385ae4f --- /dev/null +++ b/docs/superpowers/specs/2026-07-26-WW-3784-annotated-wildcard-specificity-ordering-design.md @@ -0,0 +1,171 @@ +# WW-3784 — Specificity-ordered wildcard matching for annotated actions + +- **Jira:** [WW-3784](https://issues.apache.org/jira/browse/WW-3784) — *Greedy and non-greedy matching behaviour should work in action methods using annotated wildcards* +- **Type:** Bug (Core Actions) +- **Fix version:** 7.3.0 +- **Date:** 2026-07-26 + +## Problem + +Struts matches wildcard action patterns on a **first-match-wins, insertion-order** basis. +`AbstractMatcher.match()` iterates its `compiledPatterns` list and `break`s on the first hit: + +```java +for (Mapping<E> m : compiledPatterns) { + if (wildcard.match(vars, potentialMatch, m.pattern())) { + config = convert(potentialMatch, m.target(), vars); + break; + } +} +``` + +In **XML** configuration, precedence is controlled by *physically ordering* mappings — specific +patterns are placed before general ones, so first-match-wins does the right thing. + +In **annotation-based** configuration (Convention plugin `@Action` wildcards), there is **no ordering +guarantee**. The registration order is derived from `Set<Class<?>>` class-scan order in +`PackageBasedActionConfigBuilder.buildConfiguration(...)`, which is effectively arbitrary and +non-deterministic across JVMs/classloaders. Consequently a general pattern such as `some/*` can be +evaluated before, and shadow, a more specific pattern such as `some/usefull/*`, leaving the specific +action unreachable. + +### Concrete example (from the ticket) + +| Pattern (annotated) | Intent | +|---|---| +| `some/usefull/*` | specific — should handle `/some/usefull/sleeping` | +| `some/*` | general — should handle `/some/eating` | + +With the default `WildcardHelper` matcher, `*` is greedy and crosses `/`, so `some/*` also matches +`/some/usefull/sleeping`. If `some/*` happens to be registered first, it wins and `some/usefull/*` +never matches. + +### Matchers in play + +- `WildcardHelper` (default bean `struts`): `*` and `**`, greedy, `*` crosses `/`. +- `NamedVariablePatternMatcher` (bean `namedVariable`): `{var}` → `([^/]+)`, does **not** cross `/`. + +The defect is fundamentally about **match precedence / ordering**, not the regex semantics +themselves. + +## Scope decision + +**Automatic specificity-ordering, annotation-sourced configs only.** XML keeps its explicit +file-order semantics untouched. No configuration flag — always on for Convention. This is safe +because the prior Convention order was non-deterministic, so no application could reliably depend on +it. + +Rejected alternatives: + +- *Specificity-ordering for all configs incl. XML* — would change long-standing XML first-match-wins + behavior and risk breaking configs that rely on order. +- *Opt-in flag* — pushes the burden onto users who would need to discover it; the bug should just be + fixed for annotations. + +## Architecture & placement + +The entire change lives in the **Convention plugin**. Core matchers +(`AbstractMatcher` / `ActionConfigMatcher`) and the XML configuration providers are **not modified**, +so XML behavior is fully preserved. + +We change *the order in which Convention registers wildcard action patterns into each +`PackageConfig`*. That `LinkedHashMap` insertion order is exactly what flows through +`DefaultConfiguration` into the per-namespace `ActionConfigMatcher` and drives the runtime +first-match-wins loop. + +``` +@Action wildcards + │ (Set<Class> scan order — arbitrary) + ▼ +PackageBasedActionConfigBuilder.buildConfiguration() + │ ── NEW: reorder each PackageConfig.Builder's actions by specificity + ▼ +PackageConfig.actionConfigs (LinkedHashMap, now specific-first) + │ + ▼ +DefaultConfiguration → ActionConfigMatcher (first-match-wins over specific-first list) +``` + +Two pieces: + +1. **`ActionNameSpecificityComparator`** — new pure `Comparator<String>` over action-name patterns, + in `org.apache.struts2.convention`. +2. **A sort pass** in `PackageBasedActionConfigBuilder`, applied to each `PackageConfig.Builder` + after all actions are collected and before packages are handed to the configuration. + +One small, generic helper is added to core so the plugin can reorder a builder's map: +`PackageConfig.Builder.reorderActionConfigs(Comparator<String> byActionName)`, which clears and +re-inserts `actionConfigs` in sorted key order. It is a neutral utility that XML code never calls. + +## The specificity comparator + +`ActionNameSpecificityComparator implements Comparator<String>` orders action-name patterns +**most-specific first** using these keys, in order: + +1. **Fewer wildcard tokens** first. A token is a `*` / `**` run (WildcardHelper) or a `{var}` group + (NamedVariable). +2. **More literal characters** first — total pattern length minus the characters consumed by wildcard + tokens. +3. **`*` before `**`** — fewer path-spanning (`**`) tokens is more specific. +4. **Alphabetical** on the raw pattern string — deterministic tiebreak. + +Notes: + +- The comparator is **matcher-agnostic**: it counts both `*`/`**` runs and `{var}` groups as + wildcards, so it behaves correctly whether the application uses `WildcardHelper` or + `NamedVariablePatternMatcher`. +- Literal (wildcard-free) names naturally sort first (0 wildcards, all-literal). This is harmless: + literal action names are resolved by exact-map lookup (`actions.get(name)`) **before** the wildcard + loop runs, so their relative order never affects matching. +- **Secondary benefit:** ordering becomes **deterministic** across JVMs/classloaders, which it is not + today. + +### Worked ranking — ticket case + +| Pattern | wildcards | literal chars | `**` count | order | +|---|---|---|---|---| +| `some/usefull/*` | 1 | 13 | 0 | **1st (specific)** | +| `some/*` | 1 | 5 | 0 | 2nd (general) | + +Tie on key 1 (both 1 wildcard) → key 2 decides: `some/usefull/*` has more literal characters, so it +is tried first. Result: `/some/usefull/sleeping` → specific action; `/some/eating` → general action — +regardless of scan order. + +## Integration point + +In `PackageBasedActionConfigBuilder.buildConfiguration(Set<Class<?>> classes)`, after the `classes` +loop finishes populating the `packageConfigs` map, iterate each `PackageConfig.Builder` and reorder +its action configs via `reorderActionConfigs(new ActionNameSpecificityComparator())`. Existing +downstream steps (index actions, adding packages to the configuration) run unchanged on the reordered +builders. + +## Edge cases & limitations + +- **Per-package scope.** Sorting is applied within each `PackageConfig`. Convention places all actions + of a given namespace into the same package builder, so the common "several action classes, one + namespace" case is fully covered. Competing wildcards spread across *different* convention packages + that share a namespace remain in package-registration order. This is documented as a known limit and + is out of scope; addressing it would require moving ordering into core `DefaultConfiguration`, which + would risk affecting XML. +- **Genuine ties.** Patterns of identical specificity that truly overlap resolve alphabetically — + deterministic, if arbitrary. Documented behavior. +- **No config flag.** Always on for Convention (per scope decision). The prior order was + non-deterministic, so nothing could reliably depend on it. + +## Testing + +- **Unit — `ActionNameSpecificityComparator`:** + - Ticket case: `some/usefull/*` ranks before `some/*`. + - `*` vs `**`: single-star ranks before double-star at equal literal length. + - `{var}` patterns ranked consistently with `*` patterns. + - Literals rank before any wildcard pattern. + - **Shuffle-invariance:** a randomized input list produces an identical sorted output. +- **Integration — Convention plugin:** register competing annotated wildcards on action classes and + assert that `/some/usefull/sleeping` resolves to the specific action and `/some/eating` to the + general action, independent of class-registration order. + +## Out of scope + +- Changes to XML wildcard precedence or core matcher semantics. +- Namespace-wide (cross-package) ordering. +- Any change to greedy vs. non-greedy regex behavior of `WildcardHelper` / `NamedVariablePatternMatcher`.
