Skip to content

WW-3784 Order annotated wildcard actions most-specific-first - #1813

Open
lukaszlenart wants to merge 9 commits into
mainfrom
WW-3784-annotated-wildcard-specificity-ordering
Open

WW-3784 Order annotated wildcard actions most-specific-first#1813
lukaszlenart wants to merge 9 commits into
mainfrom
WW-3784-annotated-wildcard-specificity-ordering

Conversation

@lukaszlenart

@lukaszlenart lukaszlenart commented Jul 26, 2026

Copy link
Copy Markdown
Member

Fixes WW-3784

Problem

Wildcard action patterns are matched first-match-wins, in insertion order (AbstractMatcher.match() iterates compiledPatterns and breaks on the first hit). In XML you control precedence by physically ordering mappings (specific before general). Annotation-based configs (Convention plugin @Action wildcards) have no ordering guarantee — registration order comes from Set<Class<?>> class-scan order, which is arbitrary and non-deterministic across JVMs/classloaders. So when two annotated wildcard patterns genuinely overlap, the general one can be evaluated first and shadow the specific one, leaving the specific action unreachable.

Solution

Order each Convention-built package's wildcard action patterns most-specific-first, so first-match-wins does the right thing — without touching XML or core matchers.

  • ActionNameSpecificityComparator (convention) — a Comparator<String> ranking action-name patterns by: (1) fewer wildcard tokens, (2) more literal characters, (3) fewer path-spanning ** tokens, (4) natural order (deterministic tiebreak). Matcher-agnostic: recognises both */** (WildcardHelper) and {var} (NamedVariablePatternMatcher).
  • PackageConfig.Builder.reorderActionConfigs(Comparator<String>) (core) — a neutral, generic helper that re-inserts the action-config LinkedHashMap in comparator order. Core does not depend on the plugin; XML providers never call it.
  • PackageBasedActionConfigBuilder — applies the comparator to every package in buildConfiguration(...), after buildIndexActions(...) and before registration.

Scope: annotation-sourced configs only. XML keeps its explicit file-order semantics untouched. No config flag — always on for Convention (safe, since the prior order was non-deterministic). A welcome side effect is that Convention action ordering is now deterministic within a package (cross-package order still follows package-creation order — see Known limitations).

Known limitations (documented in the design spec)

  • Ordering is per-package. Convention places all actions of a namespace into one package, so the common case is covered; competing wildcards spread across different convention packages sharing a namespace remain in package-registration order.
  • The comparator's primary key (fewer wildcard tokens) can rank a broad ** (one token) ahead of a narrower */* (two tokens). Accepted as a known limitation and noted for a possible future refinement.

Testing

  • ActionNameSpecificityComparatorTest — ranking rules incl. the ticket case, * vs **, {var}, literals, shuffle-invariance, and the alphabetical tiebreak.
  • PackageConfigBuilderReorderTest (core) — the reorder helper genuinely re-sorts by the supplied comparator.
  • PackageBasedActionConfigBuilderReorderTest — the wiring orders general-before-specific input into specific-first output.
  • WildcardSpecificityRoutingTest — end-to-end proof through the real ActionConfigMatcher/WildcardHelper: a genuinely-overlapping pair (some/** vs some/usefull/*, which collide on some/usefull/sleeping because ** crosses /) is shadowed when the general pattern is registered first, and the specific action becomes reachable after specificity ordering.
  • Full suites green: core 3053/0, plugins/convention 52/0; apache-rat clean.

Design and plan: docs/superpowers/specs/2026-07-26-WW-3784-...-design.md, docs/superpowers/plans/2026-07-26-WW-3784-...md.

🤖 Generated with Claude Code

lukaszlenart and others added 9 commits July 26, 2026 05:29
…fic-first

Sorts each convention-built package's action configs by pattern specificity so a
specific pattern (some/usefull/*) is matched before a general one (some/*),
regardless of class-scan order. Also makes convention action ordering deterministic.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…or limitations

The spec incorrectly stated that WildcardHelper's single `*` is greedy
and crosses `/`, and that `some/*` shadows `some/usefull/*`. Verified
against WildcardHelper.java and NamedVariablePatternMatcher.java: only
`**` crosses `/`, so those two patterns are actually disjoint (different
segment counts) and never compete for the same request. Correct the
Problem narrative, ticket example, and matcher bullets to state this
accurately, and document two known limitations of the specificity
comparator (raw wildcard-token-count key can misrank `**` ahead of
narrower multi-token patterns; parent-package actions bypass sorting).
Also add a test asserting the natural-order alphabetical tiebreak key.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
…hadowing end-to-end

Adds an end-to-end routing test driving the production reorder
(PackageConfig.Builder.reorderActionConfigs + ActionNameSpecificityComparator)
through the real ActionConfigMatcher/WildcardHelper. some/** and some/usefull/*
genuinely overlap for some/usefull/sleeping (** crosses '/'), so the test asserts
the general pattern shadows the specific one when registered first, and that
specificity ordering makes the specific action reachable again.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ensures Convention (annotation-sourced) wildcard action mappings are evaluated most-specific-first to avoid non-deterministic “general pattern shadows specific pattern” routing caused by classpath scan order, while preserving XML’s explicit ordering semantics.

Changes:

  • Add ActionNameSpecificityComparator to rank action-name patterns by specificity (wildcard count, literal chars, ** count, deterministic tiebreak).
  • Add a core helper PackageConfig.Builder.reorderActionConfigs(Comparator<String>) to reorder a package’s action config map in comparator order.
  • Wire Convention’s package build process to reorder actions before package registration; add unit/integration/end-to-end tests plus design/plan docs.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
plugins/convention/src/main/java/org/apache/struts2/convention/ActionNameSpecificityComparator.java Implements a specificity comparator for action-name patterns used by Convention ordering.
plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java Applies specificity reordering to each Convention-built package before registration.
core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java Adds a generic core builder helper to reorder action configs via a supplied comparator.
plugins/convention/src/test/java/org/apache/struts2/convention/ActionNameSpecificityComparatorTest.java Unit tests for comparator ranking and determinism.
plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderReorderTest.java Verifies Convention wiring reorders action configs within packages.
plugins/convention/src/test/java/org/apache/struts2/convention/WildcardSpecificityRoutingTest.java End-to-end routing proof using the real matcher (ActionConfigMatcher + WildcardHelper).
core/src/test/java/org/apache/struts2/config/entities/PackageConfigBuilderReorderTest.java Core test proving the reorder helper actually re-sorts by comparator.
docs/superpowers/specs/2026-07-26-WW-3784-annotated-wildcard-specificity-ordering-design.md Design spec capturing scope, rationale, and known limitations.
docs/superpowers/plans/2026-07-26-WW-3784-annotated-wildcard-specificity-ordering.md Implementation plan documenting steps, constraints, and test strategy.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@lukaszlenart
lukaszlenart marked this pull request as ready for review July 26, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants