From 88fd4c7d6da9c12e227853a8f03ece52468d431b Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 16:30:44 +0200 Subject: [PATCH 01/16] WW-3476 docs: add design spec for package-scoped constants Co-Authored-By: Claude Opus 5.5 --- ...6-10-10-WW-3476-scoped-constants-design.md | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 docs/superpowers/specs/2026-10-10-WW-3476-scoped-constants-design.md diff --git a/docs/superpowers/specs/2026-10-10-WW-3476-scoped-constants-design.md b/docs/superpowers/specs/2026-10-10-WW-3476-scoped-constants-design.md new file mode 100644 index 0000000000..f6a6be2e84 --- /dev/null +++ b/docs/superpowers/specs/2026-10-10-WW-3476-scoped-constants-design.md @@ -0,0 +1,179 @@ +# WW-3476 — Package-scoped constants + +**Ticket:** [WW-3476](https://issues.apache.org/jira/browse/WW-3476) — *Allow overwriting of constants for each package* +**Target:** 7.5.0 (ticket fix version) +**Date:** 2026-10-10 +**Status:** Design approved, pending implementation plan + +## Problem + +The ticket (2010, 2.2.1) asks to override constants per package — its examples are the ActionMapper, the locale +and the JSP location. Struts constants cannot do this today: + +- Constants are global. They are injected once, at bootstrap, into container singletons + (`@Inject(StrutsConstants.X)` in ~30 core classes) or read via `container.getInstance(String.class, name)`. +- `` has no constant-like child (`struts-6.5.dtd`), and `PackageConfig` has nowhere to store one. + +A scoped value therefore cannot transparently replace what a singleton already holds. Making every constant +package-aware would need a container per package — an 8.0-sized change. This design instead adds an opt-in +mechanism that fits 7.x. + +## Goals + +- A package can declare an override for a constant; child packages inherit it through `extends`. +- A component that wants a scoped value reads it at request time against the current invocation's package, + falling back to the global constant. +- The shared singleton is never mutated — the same rule `WithLazyParams` follows since 7.3.0. +- A misconfigured override fails at startup, never silently. +- Purely additive: no published DTD, constructor or interface changes. + +## Non-goals + +- **No adopters in this change.** No existing constant becomes scopable. The mechanism ships alone; each adopter + (UI theme, i18n, Convention result path, …) is a follow-up ticket. +- No `${...}` expressions — values are static strings. +- No action-level scope (`` inside ``). +- No Convention-plugin annotation. Convention-built packages extend an XML parent package and inherit its + overrides. +- No typed conversion — values are `String`, as with `container.getInstance(String.class, name)`. +- No user-overridable provider (no `StrutsBeanSelectionProvider` alias / new constant) until there is a need. + +## Inherently non-scopable constants + +Anything consumed before the action is resolved can never be scoped, because the package is not yet known: +the ActionMapper (it is what determines the package), multipart parsing limits (`struts.multipart.*`, applied +in `Dispatcher` before mapping), and similar bootstrap/dispatch settings. The design does not special-case +these — they are simply never registered as scopable. The per-URL-space ActionMapper need from the ticket is +already served by `PrefixBasedActionMapper` (`struts.mapper.prefixMapping`, WW-3260). + +Security switches consumed at request time (`struts.parameters.requireAnnotations`, CSP nonce source, +`struts.enable.DynamicMethodInvocation`, `devMode`) are technically scopable but should not be registered +without a deliberate decision: a package-level override could quietly relax an application-wide hardening flag. + +## Design + +### 1. Configuration syntax — `struts-7.5.dtd` + +A new element, distinct from the global `` so it is never mistaken for one: + +```xml + + + + + + + +``` + +- New `core/src/main/resources/struts-7.5.dtd`: a copy of `struts-6.5.dtd` with + - `scoped-constant*` added first in the `package` content model; + - `` with `name CDATA #REQUIRED` and `value CDATA #REQUIRED`; + - the header comment's public id / URL updated to 7.5. +- Registered in `StrutsXmlConfigurationProvider` next to the 6.5 mapping (`StrutsXmlConfigurationProvider.java:54`). +- Published DTDs are unchanged. Precedent: WW-5409 added `struts-6.5.dtd` for the single `final` attribute. +- The DTD name follows the ticket's fix version; if the release ships under another number, rename it with it. + +### 2. Storage — `PackageConfig` + +- New field: the package's own scoped constants, `Map`, insertion-ordered. +- `Builder.addScopedConstant(String name, String value)`. +- `getScopedConstants()` — own declarations only (unmodifiable). +- `getAllScopedConstants()` — merged map: parents first, then own entries, so the nearest declaration wins. + Multiple parents merge in the same `putAll` order as `getAllResultTypeConfigs()`. +- The merged map is computed eagerly in `Builder.build()` and stored unmodifiable. Parents are always built + before their children, so the request-time lookup does not walk the hierarchy. +- The copy constructor `PackageConfig(PackageConfig orig)` copies both maps. + +### 3. Parsing — `XmlDocConfigurationProvider` + +- `addPackage(Element)` reads `scoped-constant` children and calls `addScopedConstant`. +- A duplicate `name` within one package is a `ConfigurationException` carrying the element's location. +- Under an older DOCTYPE, the validating parser already rejects the element — no extra code. + +### 4. Registry — `ScopableConstants` + +```java +package org.apache.struts2.config; + +public interface ScopableConstants { + Set getNames(); +} +``` + +- Adopters register a named bean, in `struts-beans.xml` (core) or a plugin's `struts-plugin.xml`: + `` +- The registry is the union of all registered beans' names, collected via + `container.getInstanceNames(ScopableConstants.class)` — the existing pattern for `UnknownHandler`, + `FileManager` and `PackageProvider`. +- This change registers no beans; only tests do. + +### 5. Resolution — `ScopedConstantProvider` + +```java +package org.apache.struts2.config; + +public interface ScopedConstantProvider { + String getValue(String name); +} +``` + +Default implementation `StrutsScopedConstantProvider`, bound in `struts-beans.xml`. It injects `Container` and +`Configuration` (the latter is a container factory, `DefaultConfiguration.java:291`). `getValue(name)`: + +1. `name` not in the registry → `IllegalArgumentException`. Only an adopter's coding error reaches this; user + configuration cannot. +2. No `ActionContext`, or no `ActionInvocation` on it (bootstrap, a JSP rendered without an action) → the global + value, `container.getInstance(String.class, name)`. +3. Otherwise: the invocation's `ActionConfig.getPackageName()` → `configuration.getPackageConfig(...)` → + `getAllScopedConstants().get(name)`; when absent, the global value. + +Config reload needs no extra code: the container and `Configuration` are rebuilt, and lookups read the new +`PackageConfig`s. + +### 6. Validation — `DefaultConfiguration.rebuildRuntimeConfiguration()` + +After all package providers have loaded, for every package, each name in `getScopedConstants()` (own +declarations — inherited ones were validated on the parent) must be in the registry. Otherwise a +`ConfigurationException`: + +> Package [admin] declares scoped constant [some.constant], which is not scopable. Scopable constants: [a, b]. + +or, when the registry is empty, "No constants are scopable in this configuration." The exception carries the +package's location (`PackageConfig` is `Located`); per-element locations are not stored. + +## Testing + +JUnit 3/4 only — match each target file's existing style; no Jupiter. + +- **Parsing** — `XmlConfigurationProviderPackagesTest` (extends `ConfigurationTestBase`), with new fixtures next to + `xwork-test-package-final.xml`: + - a 7.5 fixture with `` loads, and the values land on `PackageConfig`; + - a duplicate name in one package fails with `ConfigurationException`; + - `` under the 6.5 DOCTYPE fails validation; + - existing 6.5/6.0/2.x fixtures still load (covered by the current suite). +- **Merging** — `PackageConfigTest`: child overrides parent; grandchild inherits through two levels; a package + with no declarations inherits everything; the multiple-parents winner is pinned. +- **Validation** — a test-only `ScopableConstants` bean: a registered name loads; an unregistered name throws with + package and constant in the message; with no beans registered, any `` throws. +- **Provider** — `StrutsScopedConstantProviderTest`: the package value wins inside an invocation; the global value + is returned with no context, no invocation, or no override; an inherited value resolves for a child package; + an unregistered name throws; the global `container.getInstance(String.class, name)` is unchanged after a scoped + read. +- **Watch it fail** — mutate each guarded behaviour (merge order, the validation call, the fallback) with Edit, + confirm via `git diff --stat`, see the test go red, restore. + +## Compatibility + +Additive only: a new DTD, a new element, two new interfaces, one new default bean, new `PackageConfig` methods. +Existing constructors, builders, interfaces and published DTDs are untouched — suitable for the 7.x line. + +## Delivery + +1. Core PR: `WW-3476 feat(config): add package-scoped constants`. +2. struts-site PR: publish `dtds/struts-7.5.dtd`, and document `` and the adopter API + (`WW-3476 docs: ...`). The public DTD URL must resolve before the release ships. +3. Version Notes entry at release time. +4. Follow-up tickets, one per adopter, filed when wanted. From 43115aa513820de8361c96290071004f64426425 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 17:03:46 +0200 Subject: [PATCH 02/16] WW-3476 docs: add implementation plan for package-scoped constants Wire the provider like other pluggable core services (struts-beans.xml, StrutsBeanSelectionProvider alias, struts.scopedConstantProvider). Co-Authored-By: Claude Opus 5.5 --- .../2026-10-10-WW-3476-scoped-constants.md | 1074 +++++++++++++++++ ...6-10-10-WW-3476-scoped-constants-design.md | 11 +- 2 files changed, 1081 insertions(+), 4 deletions(-) create mode 100644 docs/superpowers/plans/2026-10-10-WW-3476-scoped-constants.md diff --git a/docs/superpowers/plans/2026-10-10-WW-3476-scoped-constants.md b/docs/superpowers/plans/2026-10-10-WW-3476-scoped-constants.md new file mode 100644 index 0000000000..67d798f113 --- /dev/null +++ b/docs/superpowers/plans/2026-10-10-WW-3476-scoped-constants.md @@ -0,0 +1,1074 @@ +# WW-3476 Package-Scoped Constants Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let a `` override selected Struts constants with ``, inherited through `extends`, read at request time through a `ScopedConstantProvider` that falls back to the global constant. + +**Architecture:** `PackageConfig` stores each package's own overrides and, at `build()`, an eagerly merged view (parents first, own entries win). Components opt a constant in by registering a `ScopableConstants` bean; `DefaultConfiguration` rejects any `` whose name no bean registered. `StrutsScopedConstantProvider.getValue(name)` reads the current `ActionInvocation`'s package and falls back to `container.getInstance(String.class, name)` — the singleton's injected value is never touched. + +**Tech Stack:** Java 17, Struts core config/DI (`org.apache.struts2.inject`), XML DTD, JUnit 3 (`XWorkTestCase`) and JUnit 4, AssertJ 3.27. + +**Spec:** `docs/superpowers/specs/2026-10-10-WW-3476-scoped-constants-design.md` + +## Global Constraints + +- Branch: `WW-3476-scoped-constants` (already exists, holds the spec commit). Never push to `main`. +- Commit messages start with `WW-3476 ` and end with `Co-Authored-By: Claude Opus 5.5 `. +- Stage explicit paths only — never `git add -A` / `git add .`; check `git diff --cached --name-only` before every commit. +- Never `git stash` in this repo. +- Tests are JUnit 3 or 4 — never Jupiter. In a class extending `XWorkTestCase`, methods are `public void testXxx()` with no `@Test`. +- Maven: the Bash tool has no `mvn`; use `./mvnw` from the repo root, log to `core/target/ww3476.log`, and read Maven's exit code — never trust a grep over the output alone. +- Every new file starts with the ASF license header, copied verbatim from `core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java` (Java) or `core/src/test/resources/org/apache/struts2/config/providers/xwork-test-package-final.xml` (XML). +- New public API carries `@since 7.5.0`. Comments: default to none; short Javadoc on new public types/methods only. +- Purely additive: do not edit `struts-6.5.dtd` or any older DTD; no existing constructor/interface signature changes. +- No constant is registered as scopable in `src/main` — `ScopableConstants` beans exist only under `src/test`. +- Default implementation name prefix is `Struts*`. +- Watch-it-fail mutations are applied with the Edit tool and confirmed with `git diff --stat` (BSD `sed -i` silently no-ops here), then reverted with Edit. + +## Review Focus + +1. **An abstract package declares an unregistered scoped constant** — must still fail at startup, even though `buildRuntimeConfiguration` skips abstract packages when collecting actions. Test: Task 3 `abstractPackageIsValidatedToo`. +2. **``** — an empty override is a real value; the provider must return `""`, not fall back to the global constant. Test: Task 4 `testEmptyOverrideIsReturnedAsIs`. +3. **Invocation without a resolvable package** (proxy null, `ActionConfig` null, package name not in `Configuration`) — must return the global value, never throw. Tests: Task 4 `testGlobalValueWhenInvocationHasNoProxy`, `testGlobalValueWhenProxyHasNoConfig`, `testGlobalValueWhenPackageIsUnknown`. +4. **Scopable constant with no global value** — outside an override the provider returns `null` (as `container.getInstance` does), it must not throw. Test: Task 4 `testUnsetConstantWithoutOverrideIsNull`. +5. **Several `ScopableConstants` beans** — the scopable set is the union; a name from the second bean must validate. Test: Task 3 `namesFromAllBeansAreScopable`. + +--- + +## File Structure + +| File | Responsibility | +|---|---| +| `core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java` (modify) | Store own + merged scoped constants | +| `core/src/main/resources/struts-7.5.dtd` (create) | DTD with `` | +| `core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java` (modify) | Map the 7.5 public id to the DTD | +| `core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java` (modify) | Parse `` | +| `core/src/main/java/org/apache/struts2/config/ScopableConstants.java` (create) | Registry contract + union helper | +| `core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java` (modify) | Startup validation | +| `core/src/main/java/org/apache/struts2/config/ScopedConstantProvider.java` (create) | Lookup contract | +| `core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java` (create) | Default lookup | +| `core/src/main/java/org/apache/struts2/StrutsConstants.java` (modify) | Bean-selection constant | +| `core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java` (modify) | Alias the default bean | +| `core/src/main/resources/struts-beans.xml` (modify) | Runtime binding | +| `core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java` (modify) | Test-harness binding | +| `core/src/test/java/org/apache/struts2/config/SampleScopableConstants.java`, `SecondScopableConstants.java` (create) | Test-only registry beans | +| Tests + fixtures listed per task | | + +--- + +### Task 1: `PackageConfig` stores and merges scoped constants + +**Files:** +- Modify: `core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java` +- Test: `core/src/test/java/org/apache/struts2/config/entities/PackageConfigTest.java` (JUnit 3, extends `XWorkTestCase`) + +**Interfaces:** +- Produces: `PackageConfig.Builder addScopedConstant(String name, String value)`; `Map PackageConfig.getScopedConstants()` (own only, unmodifiable after `build()`); `Map PackageConfig.getAllScopedConstants()` (merged, unmodifiable, empty before `build()`). + +- [ ] **Step 1: Write the failing tests** — append to `PackageConfigTest` (add `import java.util.Map;`): + +```java + public void testScopedConstantsChildOverridesParent() { + PackageConfig parent = new PackageConfig.Builder("parent") + .addScopedConstant("first", "parent-first") + .addScopedConstant("second", "parent-second") + .build(); + + PackageConfig child = new PackageConfig.Builder("child") + .addParent(parent) + .addScopedConstant("second", "child-second") + .build(); + + assertEquals(Map.of("second", "child-second"), child.getScopedConstants()); + assertEquals(Map.of("first", "parent-first", "second", "child-second"), child.getAllScopedConstants()); + } + + public void testScopedConstantsInheritedThroughTwoLevels() { + PackageConfig grandparent = new PackageConfig.Builder("grandparent") + .addScopedConstant("first", "grandparent-first") + .build(); + PackageConfig parent = new PackageConfig.Builder("parent") + .addParent(grandparent) + .build(); + + PackageConfig child = new PackageConfig.Builder("child") + .addParent(parent) + .build(); + + assertEquals(Map.of(), child.getScopedConstants()); + assertEquals(Map.of("first", "grandparent-first"), child.getAllScopedConstants()); + } + + public void testScopedConstantsFirstAddedParentWins() { + PackageConfig first = new PackageConfig.Builder("first") + .addScopedConstant("shared", "from-first") + .build(); + PackageConfig second = new PackageConfig.Builder("second") + .addScopedConstant("shared", "from-second") + .build(); + + PackageConfig child = new PackageConfig.Builder("child") + .addParent(first) + .addParent(second) + .build(); + + assertEquals("from-first", child.getAllScopedConstants().get("shared")); + } + + public void testScopedConstantsSurviveBuilderCopy() { + PackageConfig parent = new PackageConfig.Builder("parent") + .addScopedConstant("first", "parent-first") + .build(); + PackageConfig original = new PackageConfig.Builder("child") + .addParent(parent) + .addScopedConstant("second", "child-second") + .build(); + + PackageConfig copy = new PackageConfig.Builder(original).build(); + + assertEquals(original.getScopedConstants(), copy.getScopedConstants()); + assertEquals(original.getAllScopedConstants(), copy.getAllScopedConstants()); + } + + public void testScopedConstantsAreUnmodifiableAfterBuild() { + PackageConfig config = new PackageConfig.Builder("pkg") + .addScopedConstant("first", "value") + .build(); + + assertThrows(UnsupportedOperationException.class, () -> config.getScopedConstants().put("x", "y")); + assertThrows(UnsupportedOperationException.class, () -> config.getAllScopedConstants().put("x", "y")); + } + + public void testScopedConstantsTakePartInEquality() { + PackageConfig plain = new PackageConfig.Builder("pkg").build(); + PackageConfig scoped = new PackageConfig.Builder("pkg").addScopedConstant("first", "value").build(); + + assertFalse(plain.equals(scoped)); + } +``` + +Also add `import static org.junit.Assert.assertThrows;` — JUnit 4.13 provides it, and it works inside this JUnit 3 class. + +- [ ] **Step 2: Run to verify they fail** + +Run: `./mvnw test -DskipAssembly -pl core -Dtest=PackageConfigTest -Dsurefire.failIfNoSpecifiedTests=false > core/target/ww3476.log 2>&1; echo "exit=$?"; grep -E "ERROR|Tests run|cannot find symbol" core/target/ww3476.log | head -20` +Expected: `exit=1`, compilation error `cannot find symbol ... addScopedConstant`. + +- [ ] **Step 3: Implement** in `PackageConfig.java`. + +Fields, after `protected boolean strictMethodInvocation = true;`: + +```java + protected Map scopedConstants; + protected Map allScopedConstants; +``` + +In `PackageConfig(String name)`, after `parents = new ArrayList<>();`: + +```java + scopedConstants = new LinkedHashMap<>(); + allScopedConstants = Collections.emptyMap(); +``` + +In `PackageConfig(PackageConfig orig)`, after `this.strictMethodInvocation = orig.strictMethodInvocation;`: + +```java + this.scopedConstants = new LinkedHashMap<>(orig.scopedConstants); + this.allScopedConstants = orig.allScopedConstants; +``` + +Getters, directly after `getAllResultTypeConfigs()`: + +```java + /** + * @return scoped constants declared by this package itself, without inherited ones + * @since 7.5.0 + */ + public Map getScopedConstants() { + return scopedConstants; + } + + /** + * @return scoped constants in effect for this package: inherited ones, overridden by its own + * @since 7.5.0 + */ + public Map getAllScopedConstants() { + return allScopedConstants; + } +``` + +In `equals`, after the `parents` comparison line: + +```java + if (!Objects.equals(scopedConstants, that.scopedConstants)) return false; +``` + +In `hashCode`, after the `parents` line: + +```java + result = 31 * result + (scopedConstants != null ? scopedConstants.hashCode() : 0); +``` + +In `Builder`, after `addResultTypeConfig(...)`: + +```java + /** + * @since 7.5.0 + */ + public Builder addScopedConstant(String name, String value) { + target.scopedConstants.put(name, value); + return this; + } +``` + +In `Builder.build()`, before `PackageConfig result = target;`: + +```java + target.scopedConstants = Collections.unmodifiableMap(target.scopedConstants); + target.allScopedConstants = mergeScopedConstants(target); +``` + +And in `Builder`, after `build()`: + +```java + private static Map mergeScopedConstants(PackageConfig config) { + Map merged = new LinkedHashMap<>(); + for (PackageConfig parent : config.parents) { + merged.putAll(parent.getAllScopedConstants()); + } + merged.putAll(config.scopedConstants); + return Collections.unmodifiableMap(merged); + } +``` + +(`addParent` inserts at index 0, so iterating `parents` and `putAll`-ing lets the first-added parent win — the same order as `getAllResultTypeConfigs()`; `testScopedConstantsFirstAddedParentWins` pins it.) + +- [ ] **Step 4: Run to verify they pass** — same command as Step 2. Expected: `exit=0`, `Tests run: N, Failures: 0, Errors: 0`. + +- [ ] **Step 5: Watch it fail** — with Edit, swap the merge order in `mergeScopedConstants` (move `merged.putAll(config.scopedConstants);` above the `for` loop). `git diff --stat` must show `PackageConfig.java` changed. Re-run Step 2's command: expect `testScopedConstantsChildOverridesParent` to FAIL. Revert with Edit; re-run; expect `exit=0`. + +- [ ] **Step 6: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java core/src/test/java/org/apache/struts2/config/entities/PackageConfigTest.java +git diff --cached --name-only +git commit -m "WW-3476 feat(config): store scoped constants on PackageConfig + +Co-Authored-By: Claude Opus 5.5 " +``` + +--- + +### Task 2: `struts-7.5.dtd` and parsing `` + +**Files:** +- Create: `core/src/main/resources/struts-7.5.dtd` +- Modify: `core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java:47-54` +- Modify: `core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java` (`addPackage`, ~line 411) +- Create fixtures in `core/src/test/resources/org/apache/struts2/config/providers/`: `xwork-test-scoped-constants.xml`, `xwork-test-scoped-constants-duplicate.xml`, `xwork-test-scoped-constants-old-dtd.xml` +- Test: `core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java` (JUnit 3, extends `ConfigurationTestBase`) + +**Interfaces:** +- Consumes: `PackageConfig.Builder.addScopedConstant`, `getScopedConstants`, `getAllScopedConstants` (Task 1). +- Produces: public id `-//Apache Software Foundation//DTD Struts Configuration 7.5//EN` → `struts-7.5.dtd`; `protected void XmlDocConfigurationProvider.loadScopedConstants(PackageConfig.Builder, Element)`. + +- [ ] **Step 1: Create the fixtures** (license header first in each, per Global Constraints). + +`xwork-test-scoped-constants.xml`: + +```xml + + + + + + + + + + + +``` + +`xwork-test-scoped-constants-duplicate.xml`: + +```xml + + + + + + + +``` + +`xwork-test-scoped-constants-old-dtd.xml`: + +```xml + + + + + + +``` + +- [ ] **Step 2: Write the failing tests** — append to `XmlConfigurationProviderPackagesTest` (add `import java.util.Map;` and `import static org.assertj.core.api.Assertions.assertThat;`): + +```java + public void testScopedConstantsLoadAndInherit() throws ConfigurationException { + buildConfigurationProvider(getXmlConfigFilePath("xwork-test-scoped-constants.xml")); + + PackageConfig parent = configuration.getPackageConfig("parent"); + PackageConfig child = configuration.getPackageConfig("child"); + + assertEquals(Map.of("first", "parent-first", "second", "parent-second"), parent.getScopedConstants()); + assertEquals(Map.of("second", "child-second"), child.getScopedConstants()); + assertEquals(Map.of("first", "parent-first", "second", "child-second"), child.getAllScopedConstants()); + } + + public void testDuplicateScopedConstantInOnePackageFails() { + try { + buildConfigurationProvider(getXmlConfigFilePath("xwork-test-scoped-constants-duplicate.xml")); + fail("Should have thrown a ConfigurationException"); + } catch (ConfigurationException e) { + assertThat(e).hasStackTraceContaining("Package [dup] declares scoped constant [first] more than once"); + } + } + + public void testScopedConstantIsRejectedUnderOlderDtd() { + try { + buildConfigurationProvider(getXmlConfigFilePath("xwork-test-scoped-constants-old-dtd.xml")); + fail("Should have thrown a ConfigurationException"); + } catch (ConfigurationException e) { + assertThat(e).hasStackTraceContaining("\"scoped-constant\" must be declared"); + } + } +``` + +(The old-DTD assertion matches the parser's `Element type "scoped-constant" must be declared.` — not the fixture file name, which also contains `scoped-constant`.) + +- [ ] **Step 3: Run to verify they fail** + +Run: `./mvnw test -DskipAssembly -pl core -Dtest=XmlConfigurationProviderPackagesTest -Dsurefire.failIfNoSpecifiedTests=false > core/target/ww3476.log 2>&1; echo "exit=$?"; grep -E "Tests run|FAIL|testScoped|testDuplicate" core/target/ww3476.log | head -20` +Expected: `exit=1`; `testScopedConstantsLoadAndInherit` and `testDuplicateScopedConstantInOnePackageFails` fail/error (7.5 public id is unmapped). `testScopedConstantIsRejectedUnderOlderDtd` already passes — that is fine, it guards that the 6.5 DTD stays unchanged. + +- [ ] **Step 4: Create the DTD** + +```bash +cp core/src/main/resources/struts-6.5.dtd core/src/main/resources/struts-7.5.dtd +``` + +Then, with Edit, in `struts-7.5.dtd`: +- In the header comment, replace `"-//Apache Software Foundation//DTD Struts Configuration 6.5//EN"` with `"-//Apache Software Foundation//DTD Struts Configuration 7.5//EN"` and `"https://struts.apache.org/dtds/struts-6.5.dtd"` with `"https://struts.apache.org/dtds/struts-7.5.dtd"`. +- Replace + +``` + +``` + +with + +``` + +``` + +- Directly after the `` block, add: + +``` + + +``` + +Check: `diff core/src/main/resources/struts-6.5.dtd core/src/main/resources/struts-7.5.dtd` shows only those changes. + +- [ ] **Step 5: Register the DTD** — in `StrutsXmlConfigurationProvider.STRUTS_DTD_MAPPINGS`, change the last entry to: + +```java + "-//Apache Software Foundation//DTD Struts Configuration 6.5//EN", "struts-6.5.dtd", + "-//Apache Software Foundation//DTD Struts Configuration 7.5//EN", "struts-7.5.dtd"); +``` + +- [ ] **Step 6: Parse the element** — in `XmlDocConfigurationProvider.addPackage`, directly after `LOG.debug("Loaded {}", newPackage);`: + +```java + loadScopedConstants(newPackage, packageElement); +``` + +And add, directly before `loadGlobalResults(...)`'s Javadoc: + +```java + /** + * @since 7.5.0 + */ + protected void loadScopedConstants(PackageConfig.Builder packageContext, Element packageElement) { + Set declared = new HashSet<>(); + iterateChildrenByTagName(packageElement, "scoped-constant", constantElement -> { + String name = constantElement.getAttribute("name"); + if (!declared.add(name)) { + throw new ConfigurationException(format("Package [%s] declares scoped constant [%s] more than once", + packageContext.getName(), name), constantElement); + } + packageContext.addScopedConstant(name, constantElement.getAttribute("value")); + }); + } +``` + +(The static `format` import, `Set` and `HashSet` are already present in this file.) + +- [ ] **Step 7: Run to verify they pass** — Step 3's command. Expected: `exit=0`, all tests in the class pass (including the pre-existing 6.5/2.x fixtures). + +- [ ] **Step 8: Watch it fail** — with Edit, change `if (!declared.add(name)) {` to `if (false && !declared.add(name)) {`; `git diff --stat` shows the file; re-run: `testDuplicateScopedConstantInOnePackageFails` FAILS. Revert. Then, with Edit, add `scoped-constant*, ` to the `package` content model in **`struts-6.5.dtd`** plus the same `` block; re-run: `testScopedConstantIsRejectedUnderOlderDtd` FAILS. Revert with Edit; `git diff --stat core/src/main/resources/struts-6.5.dtd` must be empty. Re-run: `exit=0`. + +- [ ] **Step 9: Commit** + +```bash +git add core/src/main/resources/struts-7.5.dtd \ + core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java \ + core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java \ + core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java \ + core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants.xml \ + core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-duplicate.xml \ + core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-old-dtd.xml +git diff --cached --name-only +git commit -m "WW-3476 feat(config): add struts-7.5.dtd with + +Co-Authored-By: Claude Opus 5.5 " +``` + +--- + +### Task 3: `ScopableConstants` registry and startup validation + +**Files:** +- Create: `core/src/main/java/org/apache/struts2/config/ScopableConstants.java` +- Modify: `core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java` (`buildRuntimeConfiguration`, ~line 451) +- Create (test): `core/src/test/java/org/apache/struts2/config/SampleScopableConstants.java`, `core/src/test/java/org/apache/struts2/config/SecondScopableConstants.java` +- Create fixtures in `core/src/test/resources/org/apache/struts2/config/`: `scoped-constants-registered.xml`, `scoped-constants-unregistered.xml`, `scoped-constants-abstract-unregistered.xml`, `scoped-constants-no-registry.xml`, `scoped-constants-two-beans.xml` +- Test: `core/src/test/java/org/apache/struts2/config/ScopedConstantsValidationTest.java` (new, JUnit 4) + +**Interfaces:** +- Consumes: `PackageConfig.getScopedConstants()` (Task 1); `struts-7.5.dtd` mapping (Task 2). +- Produces: `interface ScopableConstants { Set getNames(); static Set collectNames(Container container); }`; test beans `SampleScopableConstants` → `{"sample.scopable", "sample.unset"}`, `SecondScopableConstants` → `{"second.scopable"}`. + +- [ ] **Step 1: Create the registry contract** — `ScopableConstants.java`: + +```java +package org.apache.struts2.config; + +import org.apache.struts2.inject.Container; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +/** + * Declares constants a package may override with {@code }. Register implementations as named beans; + * the scopable set is the union over all of them. + * + * @since 7.5.0 + */ +public interface ScopableConstants { + + Set getNames(); + + static Set collectNames(Container container) { + Set names = new HashSet<>(); + for (String beanName : container.getInstanceNames(ScopableConstants.class)) { + names.addAll(container.getInstance(ScopableConstants.class, beanName).getNames()); + } + return Collections.unmodifiableSet(names); + } +} +``` + +- [ ] **Step 2: Create the test beans** + +`SampleScopableConstants.java`: + +```java +package org.apache.struts2.config; + +import java.util.Set; + +public class SampleScopableConstants implements ScopableConstants { + + @Override + public Set getNames() { + return Set.of("sample.scopable", "sample.unset"); + } +} +``` + +`SecondScopableConstants.java`: + +```java +package org.apache.struts2.config; + +import java.util.Set; + +public class SecondScopableConstants implements ScopableConstants { + + @Override + public Set getNames() { + return Set.of("second.scopable"); + } +} +``` + +- [ ] **Step 3: Create the fixtures.** Each file is: the XML license header, then this DOCTYPE, then the `` body shown below. + +```xml + +``` + +`scoped-constants-registered.xml`: + +```xml + + + + + + + +``` + +`scoped-constants-unregistered.xml`: + +```xml + + + + + + + +``` + +`scoped-constants-abstract-unregistered.xml`: + +```xml + + + + + + + +``` + +`scoped-constants-no-registry.xml`: + +```xml + + + + + +``` + +`scoped-constants-two-beans.xml`: + +```xml + + + + + + + + + +``` + +- [ ] **Step 4: Write the failing tests** — `ScopedConstantsValidationTest.java`: + +```java +package org.apache.struts2.config; + +import org.apache.struts2.ActionContext; +import org.apache.struts2.config.providers.StrutsDefaultConfigurationProvider; +import org.apache.struts2.inject.Container; +import org.junit.After; +import org.junit.Test; + +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class ScopedConstantsValidationTest { + + private ConfigurationManager configurationManager; + + @After + public void tearDown() { + if (configurationManager != null) { + configurationManager.destroyConfiguration(); + } + ActionContext.clear(); + } + + @Test + public void registeredNameLoads() { + Configuration configuration = load("scoped-constants-registered.xml"); + + assertThat(configuration.getPackageConfig("registered").getScopedConstants()) + .isEqualTo(Map.of("sample.scopable", "x")); + } + + @Test + public void unregisteredNameFails() { + assertThatThrownBy(() -> load("scoped-constants-unregistered.xml")) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("Package [unregistered] declares scoped constant [sample.unknown], which is not scopable. " + + "Scopable constants: [sample.scopable, sample.unset]."); + } + + @Test + public void abstractPackageIsValidatedToo() { + assertThatThrownBy(() -> load("scoped-constants-abstract-unregistered.xml")) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("Package [abstractBase] declares scoped constant [sample.unknown], which is not scopable."); + } + + @Test + public void anyScopedConstantFailsWhenNothingIsScopable() { + assertThatThrownBy(() -> load("scoped-constants-no-registry.xml")) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("Package [unregistered] declares scoped constant [sample.scopable], which is not scopable. " + + "No constants are scopable in this configuration."); + } + + @Test + public void namesFromAllBeansAreScopable() { + Configuration configuration = load("scoped-constants-two-beans.xml"); + + assertThat(configuration.getPackageConfig("both").getScopedConstants()) + .isEqualTo(Map.of("sample.scopable", "x", "second.scopable", "y")); + } + + private Configuration load(String fixture) { + configurationManager = new ConfigurationManager(Container.DEFAULT_NAME); + configurationManager.addContainerProvider(new StrutsDefaultConfigurationProvider()); + configurationManager.addContainerProvider(new StrutsXmlConfigurationProvider("org/apache/struts2/config/" + fixture)); + return configurationManager.getConfiguration(); + } +} +``` + +- [ ] **Step 5: Run to verify they fail** + +Run: `./mvnw test -DskipAssembly -pl core -Dtest=ScopedConstantsValidationTest -Dsurefire.failIfNoSpecifiedTests=false > core/target/ww3476.log 2>&1; echo "exit=$?"; grep -E "Tests run|FAIL|Expecting" core/target/ww3476.log | head -20` +Expected: `exit=1`; the three `...Fails`/`...Too` tests fail (no exception thrown); `registeredNameLoads` and `namesFromAllBeansAreScopable` pass. + +- [ ] **Step 6: Implement validation** — in `DefaultConfiguration.java`, add the import `org.apache.struts2.config.ScopableConstants` (`java.util.Set` and `java.util.TreeSet` are already imported). As the first statement of `buildRuntimeConfiguration()`: + +```java + validateScopedConstants(); +``` + +And add, directly after `buildRuntimeConfiguration()`: + +```java + /** + * @since 7.5.0 + */ + protected void validateScopedConstants() throws ConfigurationException { + Set scopableNames = ScopableConstants.collectNames(container); + for (PackageConfig packageConfig : packageContexts.values()) { + for (String name : packageConfig.getScopedConstants().keySet()) { + if (!scopableNames.contains(name)) { + throw new ConfigurationException(String.format("Package [%s] declares scoped constant [%s], which is not scopable. %s", + packageConfig.getName(), name, describeScopable(scopableNames)), packageConfig); + } + } + } + } + + private static String describeScopable(Set scopableNames) { + if (scopableNames.isEmpty()) { + return "No constants are scopable in this configuration."; + } + return "Scopable constants: " + new TreeSet<>(scopableNames) + "."; + } +``` + +- [ ] **Step 7: Run to verify they pass** — Step 5's command. Expected: `exit=0`, 5 tests, 0 failures. + +- [ ] **Step 8: Watch it fail** — with Edit, comment out the `validateScopedConstants();` call; `git diff --stat` shows `DefaultConfiguration.java`; re-run: the three failure-path tests FAIL. Revert; re-run: `exit=0`. + +- [ ] **Step 9: Regression check on config loading** — `./mvnw test -DskipAssembly -pl core -Dtest='*Configuration*Test,XmlConfigurationProvider*Test' -Dsurefire.failIfNoSpecifiedTests=false > core/target/ww3476.log 2>&1; echo "exit=$?"; grep -E "Tests run:.*Fail" core/target/ww3476.log | tail -3`. Expected `exit=0`. + +- [ ] **Step 10: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/config/ScopableConstants.java \ + core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java \ + core/src/test/java/org/apache/struts2/config/SampleScopableConstants.java \ + core/src/test/java/org/apache/struts2/config/SecondScopableConstants.java \ + core/src/test/java/org/apache/struts2/config/ScopedConstantsValidationTest.java \ + core/src/test/resources/org/apache/struts2/config/scoped-constants-registered.xml \ + core/src/test/resources/org/apache/struts2/config/scoped-constants-unregistered.xml \ + core/src/test/resources/org/apache/struts2/config/scoped-constants-abstract-unregistered.xml \ + core/src/test/resources/org/apache/struts2/config/scoped-constants-no-registry.xml \ + core/src/test/resources/org/apache/struts2/config/scoped-constants-two-beans.xml +git diff --cached --name-only +git commit -m "WW-3476 feat(config): reject unregistered scoped constants at startup + +Co-Authored-By: Claude Opus 5.5 " +``` + +--- + +### Task 4: `ScopedConstantProvider` and its wiring + +**Files:** +- Create: `core/src/main/java/org/apache/struts2/config/ScopedConstantProvider.java` +- Create: `core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java` +- Modify: `core/src/main/java/org/apache/struts2/StrutsConstants.java` (after `STRUTS_PARAMETER_ALLOWLISTER`, ~line 629) +- Modify: `core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java` (~line 456) +- Modify: `core/src/main/resources/struts-beans.xml` (after the `ParameterAllowlister` bean, ~line 258) +- Modify: `core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java` (`register` chain) +- Create fixture: `core/src/test/resources/org/apache/struts2/config/scoped-constants-provider.xml` +- Test: `core/src/test/java/org/apache/struts2/config/StrutsScopedConstantProviderTest.java` (new, JUnit 3, extends `XWorkTestCase`) + +**Interfaces:** +- Consumes: `ScopableConstants.collectNames(Container)`, `SampleScopableConstants` (Task 3); `PackageConfig.getAllScopedConstants()` (Task 1); 7.5 DTD (Task 2). +- Produces: `interface ScopedConstantProvider { String getValue(String name); }`; `StrutsScopedConstantProvider`; `StrutsConstants.STRUTS_SCOPED_CONSTANT_PROVIDER = "struts.scopedConstantProvider"`. + +- [ ] **Step 1: Create the contract** — `ScopedConstantProvider.java`: + +```java +package org.apache.struts2.config; + +/** + * Resolves a scopable constant for the current action invocation: the package's {@code }, inherited + * through {@code extends}, or else the global constant. + * + * @since 7.5.0 + */ +public interface ScopedConstantProvider { + + /** + * @param name a constant registered through a {@link ScopableConstants} bean + * @return the scoped value, or the global value ({@code null} when there is none) + * @throws IllegalArgumentException when {@code name} is not scopable + */ + String getValue(String name); +} +``` + +- [ ] **Step 2: Create the fixture** — `scoped-constants-provider.xml`: the XML license header, then this DOCTYPE, then the body below. + +```xml + +``` + +```xml + + + + + + + + + + + + + + + + + + + + + +``` + +- [ ] **Step 3: Write the failing tests** — `StrutsScopedConstantProviderTest.java`: + +```java +package org.apache.struts2.config; + +import org.apache.struts2.ActionContext; +import org.apache.struts2.ActionSupport; +import org.apache.struts2.XWorkTestCase; +import org.apache.struts2.config.entities.ActionConfig; +import org.apache.struts2.mock.MockActionInvocation; +import org.apache.struts2.mock.MockActionProxy; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class StrutsScopedConstantProviderTest extends XWorkTestCase { + + private ScopedConstantProvider provider; + + @Override + protected void setUp() throws Exception { + super.setUp(); + loadConfigurationProviders(new StrutsXmlConfigurationProvider("org/apache/struts2/config/scoped-constants-provider.xml")); + provider = container.getInstance(ScopedConstantProvider.class); + } + + public void testDefaultProviderIsStrutsImplementation() { + assertTrue(provider instanceof StrutsScopedConstantProvider); + } + + public void testPackageValueWinsInsideInvocation() { + enterPackage("override"); + + assertEquals("override", provider.getValue("sample.scopable")); + } + + public void testInheritedValueResolvesForChildPackage() { + enterPackage("child"); + + assertEquals("base", provider.getValue("sample.scopable")); + } + + public void testEmptyOverrideIsReturnedAsIs() { + enterPackage("empty"); + + assertEquals("", provider.getValue("sample.scopable")); + } + + public void testGlobalValueWithoutOverride() { + enterPackage("plain"); + + assertEquals("global", provider.getValue("sample.scopable")); + } + + public void testGlobalValueWithoutInvocation() { + assertEquals("global", provider.getValue("sample.scopable")); + } + + public void testGlobalValueWithoutActionContext() { + ActionContext context = ActionContext.getContext(); + ActionContext.clear(); + try { + assertEquals("global", provider.getValue("sample.scopable")); + } finally { + context.bind(); + } + } + + public void testGlobalValueWhenInvocationHasNoProxy() { + ActionContext.getContext().withActionInvocation(new MockActionInvocation()); + + assertEquals("global", provider.getValue("sample.scopable")); + } + + public void testGlobalValueWhenProxyHasNoConfig() { + MockActionInvocation invocation = new MockActionInvocation(); + invocation.setProxy(new MockActionProxy()); + ActionContext.getContext().withActionInvocation(invocation); + + assertEquals("global", provider.getValue("sample.scopable")); + } + + public void testGlobalValueWhenPackageIsUnknown() { + enterPackage("missing"); + + assertEquals("global", provider.getValue("sample.scopable")); + } + + public void testUnsetConstantWithoutOverrideIsNull() { + enterPackage("plain"); + + assertNull(provider.getValue("sample.unset")); + } + + public void testUnregisteredNameThrows() { + assertThatThrownBy(() -> provider.getValue("sample.unknown")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("[sample.unknown]"); + } + + public void testGlobalConstantUntouchedByScopedRead() { + enterPackage("override"); + provider.getValue("sample.scopable"); + + assertEquals("global", container.getInstance(String.class, "sample.scopable")); + } + + private void enterPackage(String packageName) { + MockActionProxy proxy = new MockActionProxy(); + proxy.setConfig(new ActionConfig.Builder(packageName, "test", ActionSupport.class.getName()).build()); + MockActionInvocation invocation = new MockActionInvocation(); + invocation.setProxy(proxy); + ActionContext.getContext().withActionInvocation(invocation); + } +} +``` + +- [ ] **Step 4: Run to verify they fail** + +Run: `./mvnw test -DskipAssembly -pl core -Dtest=StrutsScopedConstantProviderTest -Dsurefire.failIfNoSpecifiedTests=false > core/target/ww3476.log 2>&1; echo "exit=$?"; grep -E "ERROR|Tests run|cannot find symbol" core/target/ww3476.log | head -20` +Expected: `exit=1`, `cannot find symbol ... StrutsScopedConstantProvider`. + +- [ ] **Step 5: Implement the default provider** — `StrutsScopedConstantProvider.java`: + +```java +package org.apache.struts2.config; + +import org.apache.struts2.ActionContext; +import org.apache.struts2.ActionInvocation; +import org.apache.struts2.ActionProxy; +import org.apache.struts2.config.entities.PackageConfig; +import org.apache.struts2.inject.Container; +import org.apache.struts2.inject.Inject; + +import java.util.Optional; +import java.util.Set; + +/** + * @since 7.5.0 + */ +public class StrutsScopedConstantProvider implements ScopedConstantProvider { + + private Container container; + private Configuration configuration; + private volatile Set scopableNames; + + @Inject + public void setContainer(Container container) { + this.container = container; + } + + @Inject + public void setConfiguration(Configuration configuration) { + this.configuration = configuration; + } + + @Override + public String getValue(String name) { + if (!getScopableNames().contains(name)) { + throw new IllegalArgumentException(String.format( + "Constant [%s] is not scopable; register it through a %s bean", name, ScopableConstants.class.getName())); + } + return currentPackage() + .map(packageConfig -> packageConfig.getAllScopedConstants().get(name)) + .orElseGet(() -> container.getInstance(String.class, name)); + } + + private Optional currentPackage() { + ActionContext context = ActionContext.getContext(); + ActionInvocation invocation = context != null ? context.getActionInvocation() : null; + ActionProxy proxy = invocation != null ? invocation.getProxy() : null; + if (proxy == null || proxy.getConfig() == null) { + return Optional.empty(); + } + return Optional.ofNullable(configuration.getPackageConfig(proxy.getConfig().getPackageName())); + } + + private Set getScopableNames() { + Set names = scopableNames; + if (names == null) { + names = ScopableConstants.collectNames(container); + scopableNames = names; + } + return names; + } +} +``` + +Note: `Optional.map` treats a `null` mapping result as empty, so a package without the override falls through to `orElseGet`; an empty-string override is non-null and is returned as is. + +- [ ] **Step 6: Wire it** + +`StrutsConstants.java`, directly after `STRUTS_PARAMETER_ALLOWLISTER`: + +```java + + /** + * The {@link org.apache.struts2.config.ScopedConstantProvider} implementation class. + * + * @since 7.5.0 + */ + public static final String STRUTS_SCOPED_CONSTANT_PROVIDER = "struts.scopedConstantProvider"; +``` + +`StrutsBeanSelectionProvider.java`, directly after the `ParameterAllowlister` alias (same package — no import): + +```java + alias(ScopedConstantProvider.class, StrutsConstants.STRUTS_SCOPED_CONSTANT_PROVIDER, builder, props, Scope.SINGLETON); +``` + +`struts-beans.xml`, directly after the `ParameterAllowlister` bean: + +```xml + + +``` + +`StrutsDefaultConfigurationProvider.register`, directly after `.factory(UnknownHandlerManager.class, DefaultUnknownHandlerManager.class, Scope.SINGLETON)`: + +```java + + .factory(ScopedConstantProvider.class, StrutsScopedConstantProvider.class, Scope.SINGLETON) +``` + +with imports `org.apache.struts2.config.ScopedConstantProvider` and `org.apache.struts2.config.StrutsScopedConstantProvider`. + +- [ ] **Step 7: Run to verify they pass** — Step 4's command. Expected: `exit=0`, 13 tests, 0 failures. + +- [ ] **Step 8: Watch it fail** — with Edit, replace `.map(packageConfig -> packageConfig.getAllScopedConstants().get(name))` with `.map(packageConfig -> (String) null)`; `git diff --stat` shows the file; re-run: `testPackageValueWinsInsideInvocation`, `testInheritedValueResolvesForChildPackage`, `testEmptyOverrideIsReturnedAsIs` FAIL. Revert. Then replace `.orElseGet(() -> container.getInstance(String.class, name))` with `.orElse(null)`; re-run: the `testGlobalValue*` tests FAIL. Revert; re-run: `exit=0`. + +- [ ] **Step 9: Commit** + +```bash +git add core/src/main/java/org/apache/struts2/config/ScopedConstantProvider.java \ + core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java \ + core/src/main/java/org/apache/struts2/StrutsConstants.java \ + core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java \ + core/src/main/resources/struts-beans.xml \ + core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java \ + core/src/test/java/org/apache/struts2/config/StrutsScopedConstantProviderTest.java \ + core/src/test/resources/org/apache/struts2/config/scoped-constants-provider.xml +git diff --cached --name-only +git commit -m "WW-3476 feat(config): add ScopedConstantProvider resolving package overrides + +Co-Authored-By: Claude Opus 5.5 " +``` + +--- + +### Task 5: Full verification + +**Files:** none changed unless a regression appears. + +- [ ] **Step 1: Check for stray files** — `git status --short` must show no `??` entries from this work (review agents may leave probe tests; delete those, never commit them). + +- [ ] **Step 2: Run core and the Convention plugin** (Convention compares `PackageConfig`s with `equals`, which Task 1 changed): + +Run: `./mvnw test -DskipAssembly -pl core,plugins/convention > core/target/ww3476-full.log 2>&1; echo "exit=$?"; grep -E "Tests run:.*Fail|BUILD|ERROR\]" core/target/ww3476-full.log | tail -10` +Expected: `exit=0`, `BUILD SUCCESS`. If a failure is in `DateTest.testJavaSqlDate` or another test unrelated to config, re-run that single test on `main` to confirm it is pre-existing before reporting it. + +- [ ] **Step 3: Confirm no adopter slipped in** — `grep -rn "ScopableConstants" core/src/main plugins/*/src/main` must list only `ScopableConstants.java`, `StrutsScopedConstantProvider.java`, `ScopedConstantProvider.java`, `DefaultConfiguration.java` — no ``. diff --git a/docs/superpowers/specs/2026-10-10-WW-3476-scoped-constants-design.md b/docs/superpowers/specs/2026-10-10-WW-3476-scoped-constants-design.md index f6a6be2e84..8a5c08c553 100644 --- a/docs/superpowers/specs/2026-10-10-WW-3476-scoped-constants-design.md +++ b/docs/superpowers/specs/2026-10-10-WW-3476-scoped-constants-design.md @@ -36,7 +36,6 @@ mechanism that fits 7.x. - No Convention-plugin annotation. Convention-built packages extend an XML parent package and inherit its overrides. - No typed conversion — values are `String`, as with `container.getInstance(String.class, name)`. -- No user-overridable provider (no `StrutsBeanSelectionProvider` alias / new constant) until there is a need. ## Inherently non-scopable constants @@ -120,8 +119,11 @@ public interface ScopedConstantProvider { } ``` -Default implementation `StrutsScopedConstantProvider`, bound in `struts-beans.xml`. It injects `Container` and -`Configuration` (the latter is a container factory, `DefaultConfiguration.java:291`). `getValue(name)`: +Default implementation `StrutsScopedConstantProvider`, wired like every other pluggable core service +(`ParameterAuthorizer`, `OgnlGuard`): `name="struts"` in `struts-beans.xml`, aliased in `StrutsBeanSelectionProvider` +through a new `StrutsConstants.STRUTS_SCOPED_CONSTANT_PROVIDER = "struts.scopedConstantProvider"`, and registered +in `StrutsDefaultConfigurationProvider` for the test harness. It injects `Container` and `Configuration` (the +latter is a container factory, `DefaultConfiguration.java:291`). `getValue(name)`: 1. `name` not in the registry → `IllegalArgumentException`. Only an adopter's coding error reaches this; user configuration cannot. @@ -167,7 +169,8 @@ JUnit 3/4 only — match each target file's existing style; no Jupiter. ## Compatibility -Additive only: a new DTD, a new element, two new interfaces, one new default bean, new `PackageConfig` methods. +Additive only: a new DTD, a new element, two new interfaces, one new default bean with its selection constant, +new `PackageConfig` methods. Existing constructors, builders, interfaces and published DTDs are untouched — suitable for the 7.x line. ## Delivery From b2a5b0d058eaae229bb950eb79a1d4b7b06569b7 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 17:08:54 +0200 Subject: [PATCH 03/16] WW-3476 feat(config): store scoped constants on PackageConfig Co-Authored-By: Claude Opus 5.5 --- .../config/entities/PackageConfig.java | 43 ++++++++++ .../config/entities/PackageConfigTest.java | 82 +++++++++++++++++++ 2 files changed, 125 insertions(+) diff --git a/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java b/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java index 391ead116d..85f580a828 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java @@ -59,6 +59,8 @@ public class PackageConfig extends Located implements Comparable, protected boolean isFinal = false; // a final package is unextendable protected boolean needsRefresh; protected boolean strictMethodInvocation = true; + protected Map scopedConstants; + protected Map allScopedConstants; protected PackageConfig(String name) { this.name = name; @@ -69,6 +71,8 @@ protected PackageConfig(String name) { resultTypeConfigs = new LinkedHashMap<>(); globalExceptionMappingConfigs = new ArrayList<>(); parents = new ArrayList<>(); + scopedConstants = new LinkedHashMap<>(); + allScopedConstants = Collections.emptyMap(); } protected PackageConfig(PackageConfig orig) { @@ -90,6 +94,8 @@ protected PackageConfig(PackageConfig orig) { this.parents = new ArrayList<>(orig.parents); this.location = orig.location; this.strictMethodInvocation = orig.strictMethodInvocation; + this.scopedConstants = new LinkedHashMap<>(orig.scopedConstants); + this.allScopedConstants = orig.allScopedConstants; } public boolean isAbstract() { @@ -189,6 +195,22 @@ public Map getAllResultTypeConfigs() { return retMap; } + /** + * @return scoped constants declared by this package itself, without inherited ones + * @since 7.5.0 + */ + public Map getScopedConstants() { + return scopedConstants; + } + + /** + * @return scoped constants in effect for this package: inherited ones, overridden by its own + * @since 7.5.0 + */ + public Map getAllScopedConstants() { + return allScopedConstants; + } + /** * returns the List of all the ExceptionMappingConfigs available in the current package. * ExceptionMappingConfigs defined in ancestor packages will be included in this list. @@ -391,6 +413,7 @@ public boolean equals(Object o) { if (!Objects.equals(globalExceptionMappingConfigs, that.globalExceptionMappingConfigs)) return false; if (!Objects.equals(parents, that.parents)) return false; + if (!Objects.equals(scopedConstants, that.scopedConstants)) return false; if (!Objects.equals(defaultInterceptorRef, that.defaultInterceptorRef)) return false; if (!Objects.equals(defaultActionRef, that.defaultActionRef)) @@ -412,6 +435,7 @@ public int hashCode() { result = 31 * result + (resultTypeConfigs != null ? resultTypeConfigs.hashCode() : 0); result = 31 * result + (globalExceptionMappingConfigs != null ? globalExceptionMappingConfigs.hashCode() : 0); result = 31 * result + (parents != null ? parents.hashCode() : 0); + result = 31 * result + (scopedConstants != null ? scopedConstants.hashCode() : 0); result = 31 * result + (defaultInterceptorRef != null ? defaultInterceptorRef.hashCode() : 0); result = 31 * result + (defaultActionRef != null ? defaultActionRef.hashCode() : 0); result = 31 * result + (defaultResultType != null ? defaultResultType.hashCode() : 0); @@ -605,6 +629,14 @@ public Builder addResultTypeConfig(ResultTypeConfig config) { return this; } + /** + * @since 7.5.0 + */ + public Builder addScopedConstant(String name, String value) { + target.scopedConstants.put(name, value); + return this; + } + public Builder location(Location loc) { target.location = loc; return this; @@ -655,11 +687,22 @@ public PackageConfig build() { target.resultTypeConfigs = Collections.unmodifiableMap(target.resultTypeConfigs); target.globalExceptionMappingConfigs = Collections.unmodifiableList(target.globalExceptionMappingConfigs); target.parents = Collections.unmodifiableList(target.parents); + target.scopedConstants = Collections.unmodifiableMap(target.scopedConstants); + target.allScopedConstants = mergeScopedConstants(target); PackageConfig result = target; target = new PackageConfig(result); return result; } + private static Map mergeScopedConstants(PackageConfig config) { + Map merged = new LinkedHashMap<>(); + for (PackageConfig parent : config.parents) { + merged.putAll(parent.getAllScopedConstants()); + } + merged.putAll(config.scopedConstants); + return Collections.unmodifiableMap(merged); + } + @Override public String toString() { return "[BUILDER] " + target.toString(); diff --git a/core/src/test/java/org/apache/struts2/config/entities/PackageConfigTest.java b/core/src/test/java/org/apache/struts2/config/entities/PackageConfigTest.java index b0fd702d75..8619a8b5c9 100644 --- a/core/src/test/java/org/apache/struts2/config/entities/PackageConfigTest.java +++ b/core/src/test/java/org/apache/struts2/config/entities/PackageConfigTest.java @@ -20,6 +20,10 @@ import org.apache.struts2.XWorkTestCase; +import java.util.Map; + +import static org.junit.Assert.assertThrows; + public class PackageConfigTest extends XWorkTestCase { public void testFullDefaultInterceptorRef() { @@ -92,4 +96,82 @@ public void testStrictDMIInheritanceDisabledInChildPackage() { assertFalse(child.isStrictMethodInvocation()); } + public void testScopedConstantsChildOverridesParent() { + PackageConfig parent = new PackageConfig.Builder("parent") + .addScopedConstant("first", "parent-first") + .addScopedConstant("second", "parent-second") + .build(); + + PackageConfig child = new PackageConfig.Builder("child") + .addParent(parent) + .addScopedConstant("second", "child-second") + .build(); + + assertEquals(Map.of("second", "child-second"), child.getScopedConstants()); + assertEquals(Map.of("first", "parent-first", "second", "child-second"), child.getAllScopedConstants()); + } + + public void testScopedConstantsInheritedThroughTwoLevels() { + PackageConfig grandparent = new PackageConfig.Builder("grandparent") + .addScopedConstant("first", "grandparent-first") + .build(); + PackageConfig parent = new PackageConfig.Builder("parent") + .addParent(grandparent) + .build(); + + PackageConfig child = new PackageConfig.Builder("child") + .addParent(parent) + .build(); + + assertEquals(Map.of(), child.getScopedConstants()); + assertEquals(Map.of("first", "grandparent-first"), child.getAllScopedConstants()); + } + + public void testScopedConstantsFirstAddedParentWins() { + PackageConfig first = new PackageConfig.Builder("first") + .addScopedConstant("shared", "from-first") + .build(); + PackageConfig second = new PackageConfig.Builder("second") + .addScopedConstant("shared", "from-second") + .build(); + + PackageConfig child = new PackageConfig.Builder("child") + .addParent(first) + .addParent(second) + .build(); + + assertEquals("from-first", child.getAllScopedConstants().get("shared")); + } + + public void testScopedConstantsSurviveBuilderCopy() { + PackageConfig parent = new PackageConfig.Builder("parent") + .addScopedConstant("first", "parent-first") + .build(); + PackageConfig original = new PackageConfig.Builder("child") + .addParent(parent) + .addScopedConstant("second", "child-second") + .build(); + + PackageConfig copy = new PackageConfig.Builder(original).build(); + + assertEquals(original.getScopedConstants(), copy.getScopedConstants()); + assertEquals(original.getAllScopedConstants(), copy.getAllScopedConstants()); + } + + public void testScopedConstantsAreUnmodifiableAfterBuild() { + PackageConfig config = new PackageConfig.Builder("pkg") + .addScopedConstant("first", "value") + .build(); + + assertThrows(UnsupportedOperationException.class, () -> config.getScopedConstants().put("x", "y")); + assertThrows(UnsupportedOperationException.class, () -> config.getAllScopedConstants().put("x", "y")); + } + + public void testScopedConstantsTakePartInEquality() { + PackageConfig plain = new PackageConfig.Builder("pkg").build(); + PackageConfig scoped = new PackageConfig.Builder("pkg").addScopedConstant("first", "value").build(); + + assertFalse(plain.equals(scoped)); + } + } From 06d6e3c67a4b901f9b427c2d420418358983fd1b Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 17:11:12 +0200 Subject: [PATCH 04/16] WW-3476 feat(config): add struts-7.5.dtd with Co-Authored-By: Claude Opus 5.5 --- .../StrutsXmlConfigurationProvider.java | 3 +- .../XmlDocConfigurationProvider.java | 17 ++ core/src/main/resources/struts-7.5.dtd | 164 ++++++++++++++++++ .../XmlConfigurationProviderPackagesTest.java | 31 ++++ .../xwork-test-scoped-constants-duplicate.xml | 30 ++++ .../xwork-test-scoped-constants-old-dtd.xml | 29 ++++ .../providers/xwork-test-scoped-constants.xml | 34 ++++ 7 files changed, 307 insertions(+), 1 deletion(-) create mode 100644 core/src/main/resources/struts-7.5.dtd create mode 100644 core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-duplicate.xml create mode 100644 core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-old-dtd.xml create mode 100644 core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants.xml diff --git a/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java index 12a57a3a89..6b0a9dce99 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsXmlConfigurationProvider.java @@ -51,7 +51,8 @@ public class StrutsXmlConfigurationProvider extends XmlConfigurationProvider { "-//Apache Software Foundation//DTD Struts Configuration 2.3//EN", "struts-2.3.dtd", "-//Apache Software Foundation//DTD Struts Configuration 2.5//EN", "struts-2.5.dtd", "-//Apache Software Foundation//DTD Struts Configuration 6.0//EN", "struts-6.0.dtd", - "-//Apache Software Foundation//DTD Struts Configuration 6.5//EN", "struts-6.5.dtd"); + "-//Apache Software Foundation//DTD Struts Configuration 6.5//EN", "struts-6.5.dtd", + "-//Apache Software Foundation//DTD Struts Configuration 7.5//EN", "struts-7.5.dtd"); private File baseDir = null; private final String filename; diff --git a/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java b/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java index da44b71f64..b6c606e66d 100644 --- a/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java +++ b/core/src/main/java/org/apache/struts2/config/providers/XmlDocConfigurationProvider.java @@ -424,6 +424,8 @@ protected PackageConfig addPackage(Element packageElement) throws ConfigurationE LOG.debug("Loaded {}", newPackage); + loadScopedConstants(newPackage, packageElement); + // add result types (and default result) to this package addResultTypes(newPackage, packageElement); @@ -839,6 +841,21 @@ protected void loadDefaultActionRef(PackageConfig.Builder packageContext, Elemen } } + /** + * @since 7.5.0 + */ + protected void loadScopedConstants(PackageConfig.Builder packageContext, Element packageElement) { + Set declared = new HashSet<>(); + iterateChildrenByTagName(packageElement, "scoped-constant", constantElement -> { + String name = constantElement.getAttribute("name"); + if (!declared.add(name)) { + throw new ConfigurationException(format("Package [%s] declares scoped constant [%s] more than once", + packageContext.getName(), name), constantElement); + } + packageContext.addScopedConstant(name, constantElement.getAttribute("value")); + }); + } + /** * Load all the global results for this package from the XML element. * diff --git a/core/src/main/resources/struts-7.5.dtd b/core/src/main/resources/struts-7.5.dtd new file mode 100644 index 0000000000..307f5f0e0a --- /dev/null +++ b/core/src/main/resources/struts-7.5.dtd @@ -0,0 +1,164 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java index 0c4bd9714f..25350d5c18 100644 --- a/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java +++ b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java @@ -24,7 +24,9 @@ import org.apache.struts2.config.entities.PackageConfig; import java.util.List; +import java.util.Map; +import static org.assertj.core.api.Assertions.assertThat; /** * Created by IntelliJ IDEA. @@ -198,6 +200,35 @@ public void testDefaultClassRef() throws ConfigurationException { assertEquals(expectedNoDefaultClassRefPackage, configuration.getPackageConfig(noDefaultClassRefPkgName)); } + public void testScopedConstantsLoadAndInherit() throws ConfigurationException { + buildConfigurationProvider(getXmlConfigFilePath("xwork-test-scoped-constants.xml")); + + PackageConfig parent = configuration.getPackageConfig("parent"); + PackageConfig child = configuration.getPackageConfig("child"); + + assertEquals(Map.of("first", "parent-first", "second", "parent-second"), parent.getScopedConstants()); + assertEquals(Map.of("second", "child-second"), child.getScopedConstants()); + assertEquals(Map.of("first", "parent-first", "second", "child-second"), child.getAllScopedConstants()); + } + + public void testDuplicateScopedConstantInOnePackageFails() { + try { + buildConfigurationProvider(getXmlConfigFilePath("xwork-test-scoped-constants-duplicate.xml")); + fail("Should have thrown a ConfigurationException"); + } catch (ConfigurationException e) { + assertThat(e).hasStackTraceContaining("Package [dup] declares scoped constant [first] more than once"); + } + } + + public void testScopedConstantIsRejectedUnderOlderDtd() { + try { + buildConfigurationProvider(getXmlConfigFilePath("xwork-test-scoped-constants-old-dtd.xml")); + fail("Should have thrown a ConfigurationException"); + } catch (ConfigurationException e) { + assertThat(e).hasStackTraceContaining("\"scoped-constant\" must be declared"); + } + } + private String getXmlConfigFilePath(String fileName) { return "org/apache/struts2/config/providers/" + fileName; } diff --git a/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-duplicate.xml b/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-duplicate.xml new file mode 100644 index 0000000000..92b9ec1163 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-duplicate.xml @@ -0,0 +1,30 @@ + + + + + + + + + diff --git a/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-old-dtd.xml b/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-old-dtd.xml new file mode 100644 index 0000000000..35817e2600 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-old-dtd.xml @@ -0,0 +1,29 @@ + + + + + + + + diff --git a/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants.xml b/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants.xml new file mode 100644 index 0000000000..ef0797de83 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants.xml @@ -0,0 +1,34 @@ + + + + + + + + + + + + + From 2050da78c94b0cc7d96cb4de5909cf1af3b3cb0e Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 17:13:04 +0200 Subject: [PATCH 05/16] WW-3476 feat(config): reject unregistered scoped constants at startup Co-Authored-By: Claude Opus 5.5 --- .../struts2/config/ScopableConstants.java | 44 +++++++++ .../config/impl/DefaultConfiguration.java | 25 ++++++ .../config/SampleScopableConstants.java | 29 ++++++ .../config/ScopedConstantsValidationTest.java | 89 +++++++++++++++++++ .../config/SecondScopableConstants.java | 29 ++++++ ...scoped-constants-abstract-unregistered.xml | 32 +++++++ .../config/scoped-constants-no-registry.xml | 29 ++++++ .../config/scoped-constants-registered.xml | 32 +++++++ .../config/scoped-constants-two-beans.xml | 35 ++++++++ .../config/scoped-constants-unregistered.xml | 32 +++++++ 10 files changed, 376 insertions(+) create mode 100644 core/src/main/java/org/apache/struts2/config/ScopableConstants.java create mode 100644 core/src/test/java/org/apache/struts2/config/SampleScopableConstants.java create mode 100644 core/src/test/java/org/apache/struts2/config/ScopedConstantsValidationTest.java create mode 100644 core/src/test/java/org/apache/struts2/config/SecondScopableConstants.java create mode 100644 core/src/test/resources/org/apache/struts2/config/scoped-constants-abstract-unregistered.xml create mode 100644 core/src/test/resources/org/apache/struts2/config/scoped-constants-no-registry.xml create mode 100644 core/src/test/resources/org/apache/struts2/config/scoped-constants-registered.xml create mode 100644 core/src/test/resources/org/apache/struts2/config/scoped-constants-two-beans.xml create mode 100644 core/src/test/resources/org/apache/struts2/config/scoped-constants-unregistered.xml diff --git a/core/src/main/java/org/apache/struts2/config/ScopableConstants.java b/core/src/main/java/org/apache/struts2/config/ScopableConstants.java new file mode 100644 index 0000000000..fd21ac513f --- /dev/null +++ b/core/src/main/java/org/apache/struts2/config/ScopableConstants.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.config; + +import org.apache.struts2.inject.Container; + +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +/** + * Declares constants a package may override with {@code }. Register implementations as named beans; + * the scopable set is the union over all of them. + * + * @since 7.5.0 + */ +public interface ScopableConstants { + + Set getNames(); + + static Set collectNames(Container container) { + Set names = new HashSet<>(); + for (String beanName : container.getInstanceNames(ScopableConstants.class)) { + names.addAll(container.getInstance(ScopableConstants.class, beanName).getNames()); + } + return Collections.unmodifiableSet(names); + } +} diff --git a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java index 53bf20dc85..4f908bfb63 100644 --- a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java @@ -36,6 +36,7 @@ import org.apache.struts2.config.FileManagerProvider; import org.apache.struts2.config.PackageProvider; import org.apache.struts2.config.RuntimeConfiguration; +import org.apache.struts2.config.ScopableConstants; import org.apache.struts2.config.entities.ActionConfig; import org.apache.struts2.config.entities.InterceptorMapping; import org.apache.struts2.config.entities.PackageConfig; @@ -449,6 +450,8 @@ public static ContainerBuilder bootstrapTypeConverters(ContainerBuilder builder) * @throws ConfigurationException in case of any configuration errors */ protected synchronized RuntimeConfiguration buildRuntimeConfiguration() throws ConfigurationException { + validateScopedConstants(); + Map> namespaceActionConfigs = new LinkedHashMap<>(); Map namespaceConfigs = new LinkedHashMap<>(); @@ -488,6 +491,28 @@ protected synchronized RuntimeConfiguration buildRuntimeConfiguration() throws C Collections.unmodifiableMap(namespaceConfigs), matcher, appendNamedParameters, fallbackToEmptyNamespace); } + /** + * @since 7.5.0 + */ + protected void validateScopedConstants() throws ConfigurationException { + Set scopableNames = ScopableConstants.collectNames(container); + for (PackageConfig packageConfig : packageContexts.values()) { + for (String name : packageConfig.getScopedConstants().keySet()) { + if (!scopableNames.contains(name)) { + throw new ConfigurationException(String.format("Package [%s] declares scoped constant [%s], which is not scopable. %s", + packageConfig.getName(), name, describeScopable(scopableNames)), packageConfig); + } + } + } + } + + private static String describeScopable(Set scopableNames) { + if (scopableNames.isEmpty()) { + return "No constants are scopable in this configuration."; + } + return "Scopable constants: " + new TreeSet<>(scopableNames) + "."; + } + private void setDefaultResults(Map results, PackageConfig packageContext) { String defaultResult = packageContext.getFullDefaultResultType(); diff --git a/core/src/test/java/org/apache/struts2/config/SampleScopableConstants.java b/core/src/test/java/org/apache/struts2/config/SampleScopableConstants.java new file mode 100644 index 0000000000..c09d58755a --- /dev/null +++ b/core/src/test/java/org/apache/struts2/config/SampleScopableConstants.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.config; + +import java.util.Set; + +public class SampleScopableConstants implements ScopableConstants { + + @Override + public Set getNames() { + return Set.of("sample.scopable", "sample.unset"); + } +} diff --git a/core/src/test/java/org/apache/struts2/config/ScopedConstantsValidationTest.java b/core/src/test/java/org/apache/struts2/config/ScopedConstantsValidationTest.java new file mode 100644 index 0000000000..5cbb1a3418 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/config/ScopedConstantsValidationTest.java @@ -0,0 +1,89 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.config; + +import org.apache.struts2.ActionContext; +import org.apache.struts2.config.providers.StrutsDefaultConfigurationProvider; +import org.apache.struts2.inject.Container; +import org.junit.After; +import org.junit.Test; + +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class ScopedConstantsValidationTest { + + private ConfigurationManager configurationManager; + + @After + public void tearDown() { + if (configurationManager != null) { + configurationManager.destroyConfiguration(); + } + ActionContext.clear(); + } + + @Test + public void registeredNameLoads() { + Configuration configuration = load("scoped-constants-registered.xml"); + + assertThat(configuration.getPackageConfig("registered").getScopedConstants()) + .isEqualTo(Map.of("sample.scopable", "x")); + } + + @Test + public void unregisteredNameFails() { + assertThatThrownBy(() -> load("scoped-constants-unregistered.xml")) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("Package [unregistered] declares scoped constant [sample.unknown], which is not scopable. " + + "Scopable constants: [sample.scopable, sample.unset]."); + } + + @Test + public void abstractPackageIsValidatedToo() { + assertThatThrownBy(() -> load("scoped-constants-abstract-unregistered.xml")) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("Package [abstractBase] declares scoped constant [sample.unknown], which is not scopable."); + } + + @Test + public void anyScopedConstantFailsWhenNothingIsScopable() { + assertThatThrownBy(() -> load("scoped-constants-no-registry.xml")) + .isInstanceOf(ConfigurationException.class) + .hasStackTraceContaining("Package [unregistered] declares scoped constant [sample.scopable], which is not scopable. " + + "No constants are scopable in this configuration."); + } + + @Test + public void namesFromAllBeansAreScopable() { + Configuration configuration = load("scoped-constants-two-beans.xml"); + + assertThat(configuration.getPackageConfig("both").getScopedConstants()) + .isEqualTo(Map.of("sample.scopable", "x", "second.scopable", "y")); + } + + private Configuration load(String fixture) { + configurationManager = new ConfigurationManager(Container.DEFAULT_NAME); + configurationManager.addContainerProvider(new StrutsDefaultConfigurationProvider()); + configurationManager.addContainerProvider(new StrutsXmlConfigurationProvider("org/apache/struts2/config/" + fixture)); + return configurationManager.getConfiguration(); + } +} diff --git a/core/src/test/java/org/apache/struts2/config/SecondScopableConstants.java b/core/src/test/java/org/apache/struts2/config/SecondScopableConstants.java new file mode 100644 index 0000000000..9534dab7f4 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/config/SecondScopableConstants.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.config; + +import java.util.Set; + +public class SecondScopableConstants implements ScopableConstants { + + @Override + public Set getNames() { + return Set.of("second.scopable"); + } +} diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-abstract-unregistered.xml b/core/src/test/resources/org/apache/struts2/config/scoped-constants-abstract-unregistered.xml new file mode 100644 index 0000000000..6eb2c623cd --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/scoped-constants-abstract-unregistered.xml @@ -0,0 +1,32 @@ + + + + + + + + + + diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-no-registry.xml b/core/src/test/resources/org/apache/struts2/config/scoped-constants-no-registry.xml new file mode 100644 index 0000000000..6d622d8ac9 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/scoped-constants-no-registry.xml @@ -0,0 +1,29 @@ + + + + + + + + diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-registered.xml b/core/src/test/resources/org/apache/struts2/config/scoped-constants-registered.xml new file mode 100644 index 0000000000..f3e5a5401f --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/scoped-constants-registered.xml @@ -0,0 +1,32 @@ + + + + + + + + + + diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-two-beans.xml b/core/src/test/resources/org/apache/struts2/config/scoped-constants-two-beans.xml new file mode 100644 index 0000000000..d79022cf61 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/scoped-constants-two-beans.xml @@ -0,0 +1,35 @@ + + + + + + + + + + + + diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-unregistered.xml b/core/src/test/resources/org/apache/struts2/config/scoped-constants-unregistered.xml new file mode 100644 index 0000000000..0fd512212c --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/scoped-constants-unregistered.xml @@ -0,0 +1,32 @@ + + + + + + + + + + From 617faf1b1607c0eb8cbf06ea4f23a16839310981 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 17:14:56 +0200 Subject: [PATCH 06/16] WW-3476 feat(config): add ScopedConstantProvider resolving package overrides Co-Authored-By: Claude Opus 5.5 --- .../org/apache/struts2/StrutsConstants.java | 7 + .../config/ScopedConstantProvider.java | 35 +++++ .../config/StrutsBeanSelectionProvider.java | 1 + .../config/StrutsScopedConstantProvider.java | 79 +++++++++++ .../StrutsDefaultConfigurationProvider.java | 4 + core/src/main/resources/struts-beans.xml | 3 + .../StrutsScopedConstantProviderTest.java | 129 ++++++++++++++++++ .../config/scoped-constants-provider.xml | 46 +++++++ 8 files changed, 304 insertions(+) create mode 100644 core/src/main/java/org/apache/struts2/config/ScopedConstantProvider.java create mode 100644 core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java create mode 100644 core/src/test/java/org/apache/struts2/config/StrutsScopedConstantProviderTest.java create mode 100644 core/src/test/resources/org/apache/struts2/config/scoped-constants-provider.xml diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 3d3fef3282..cfa3e30a7b 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -628,6 +628,13 @@ public final class StrutsConstants { */ public static final String STRUTS_PARAMETER_ALLOWLISTER = "struts.parameterAllowlister"; + /** + * The {@link org.apache.struts2.config.ScopedConstantProvider} implementation class. + * + * @since 7.5.0 + */ + public static final String STRUTS_SCOPED_CONSTANT_PROVIDER = "struts.scopedConstantProvider"; + /** * Enables evaluation of OGNL expressions * diff --git a/core/src/main/java/org/apache/struts2/config/ScopedConstantProvider.java b/core/src/main/java/org/apache/struts2/config/ScopedConstantProvider.java new file mode 100644 index 0000000000..53e655483f --- /dev/null +++ b/core/src/main/java/org/apache/struts2/config/ScopedConstantProvider.java @@ -0,0 +1,35 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.config; + +/** + * Resolves a scopable constant for the current action invocation: the package's {@code }, inherited + * through {@code extends}, or else the global constant. + * + * @since 7.5.0 + */ +public interface ScopedConstantProvider { + + /** + * @param name a constant registered through a {@link ScopableConstants} bean + * @return the scoped value, or the global value ({@code null} when there is none) + * @throws IllegalArgumentException when {@code name} is not scopable + */ + String getValue(String name); +} diff --git a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java index b0dda639b2..8c141c6c73 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java @@ -454,6 +454,7 @@ public void register(ContainerBuilder builder, LocatableProperties props) { alias(ProxyService.class, StrutsConstants.STRUTS_PROXYSERVICE, builder, props, Scope.SINGLETON); alias(ParameterAuthorizer.class, StrutsConstants.STRUTS_PARAMETER_AUTHORIZER, builder, props, Scope.SINGLETON); alias(ParameterAllowlister.class, StrutsConstants.STRUTS_PARAMETER_ALLOWLISTER, builder, props, Scope.SINGLETON); + alias(ScopedConstantProvider.class, StrutsConstants.STRUTS_SCOPED_CONSTANT_PROVIDER, builder, props, Scope.SINGLETON); alias(SecurityMemberAccess.class, StrutsConstants.STRUTS_MEMBER_ACCESS, builder, props, Scope.PROTOTYPE); alias(OgnlGuard.class, StrutsConstants.STRUTS_OGNL_GUARD, builder, props, Scope.SINGLETON); diff --git a/core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java new file mode 100644 index 0000000000..2bbb40e311 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.config; + +import org.apache.struts2.ActionContext; +import org.apache.struts2.ActionInvocation; +import org.apache.struts2.ActionProxy; +import org.apache.struts2.config.entities.PackageConfig; +import org.apache.struts2.inject.Container; +import org.apache.struts2.inject.Inject; + +import java.util.Optional; +import java.util.Set; + +/** + * @since 7.5.0 + */ +public class StrutsScopedConstantProvider implements ScopedConstantProvider { + + private Container container; + private Configuration configuration; + private volatile Set scopableNames; + + @Inject + public void setContainer(Container container) { + this.container = container; + } + + @Inject + public void setConfiguration(Configuration configuration) { + this.configuration = configuration; + } + + @Override + public String getValue(String name) { + if (!getScopableNames().contains(name)) { + throw new IllegalArgumentException(String.format( + "Constant [%s] is not scopable; register it through a %s bean", name, ScopableConstants.class.getName())); + } + return currentPackage() + .map(packageConfig -> packageConfig.getAllScopedConstants().get(name)) + .orElseGet(() -> container.getInstance(String.class, name)); + } + + private Optional currentPackage() { + ActionContext context = ActionContext.getContext(); + ActionInvocation invocation = context != null ? context.getActionInvocation() : null; + ActionProxy proxy = invocation != null ? invocation.getProxy() : null; + if (proxy == null || proxy.getConfig() == null) { + return Optional.empty(); + } + return Optional.ofNullable(configuration.getPackageConfig(proxy.getConfig().getPackageName())); + } + + private Set getScopableNames() { + Set names = scopableNames; + if (names == null) { + names = ScopableConstants.collectNames(container); + scopableNames = names; + } + return names; + } +} diff --git a/core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java b/core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java index 40d757586d..c66911bb90 100644 --- a/core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java +++ b/core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java @@ -26,6 +26,8 @@ import org.apache.struts2.config.Configuration; import org.apache.struts2.config.ConfigurationException; import org.apache.struts2.config.ConfigurationProvider; +import org.apache.struts2.config.ScopedConstantProvider; +import org.apache.struts2.config.StrutsScopedConstantProvider; import org.apache.struts2.config.impl.DefaultConfiguration; import org.apache.struts2.conversion.NullHandler; import org.apache.struts2.conversion.impl.ArrayConverter; @@ -130,6 +132,8 @@ public void register(ContainerBuilder builder, LocatableProperties props) throws .factory(UnknownHandlerManager.class, DefaultUnknownHandlerManager.class, Scope.SINGLETON) + .factory(ScopedConstantProvider.class, StrutsScopedConstantProvider.class, Scope.SINGLETON) + // silly workarounds for ognl since there is no way to flush its caches .factory(PropertyAccessor.class, List.class.getName(), XWorkListPropertyAccessor.class, Scope.SINGLETON) .factory(PropertyAccessor.class, ArrayList.class.getName(), XWorkListPropertyAccessor.class, Scope.SINGLETON) diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml index d12535b9c3..81109bf069 100644 --- a/core/src/main/resources/struts-beans.xml +++ b/core/src/main/resources/struts-beans.xml @@ -258,6 +258,9 @@ + + provider.getValue("sample.unknown")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("[sample.unknown]"); + } + + public void testGlobalConstantUntouchedByScopedRead() { + enterPackage("override"); + provider.getValue("sample.scopable"); + + assertEquals("global", container.getInstance(String.class, "sample.scopable")); + } + + private void enterPackage(String packageName) { + MockActionProxy proxy = new MockActionProxy(); + proxy.setConfig(new ActionConfig.Builder(packageName, "test", ActionSupport.class.getName()).build()); + MockActionInvocation invocation = new MockActionInvocation(); + invocation.setProxy(proxy); + ActionContext.getContext().withActionInvocation(invocation); + } +} diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-provider.xml b/core/src/test/resources/org/apache/struts2/config/scoped-constants-provider.xml new file mode 100644 index 0000000000..84a163ad60 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/scoped-constants-provider.xml @@ -0,0 +1,46 @@ + + + + + + + + + + + + + + + + + + + + + + + + From 545c5703f95013a990691db98d1d2eb228bb9262 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 17:49:15 +0200 Subject: [PATCH 07/16] WW-3476 test(config): pin ScopedConstantProvider wiring on the Dispatcher path Co-Authored-By: Claude Opus 5.5 --- .../ScopedConstantProviderWiringTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java diff --git a/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java b/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java new file mode 100644 index 0000000000..f241530760 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.config; + +import org.apache.struts2.StrutsInternalTestCase; + +public class ScopedConstantProviderWiringTest extends StrutsInternalTestCase { + + public void testDispatcherResolvesStrutsScopedConstantProvider() { + ScopedConstantProvider provider = dispatcher.getContainer().getInstance(ScopedConstantProvider.class); + + assertTrue(provider instanceof StrutsScopedConstantProvider); + } +} From 4931d342134e23d5e4e66b2dd0fb45bdf20bbeb2 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 18:08:29 +0200 Subject: [PATCH 08/16] WW-3476 test(config): drop the xwork prefix from scoped-constant fixtures Co-Authored-By: Claude Opus 5.5 --- .../providers/XmlConfigurationProviderPackagesTest.java | 6 +++--- ...nstants-duplicate.xml => scoped-constants-duplicate.xml} | 0 ...coped-constants.xml => scoped-constants-inheritance.xml} | 0 ...d-constants-old-dtd.xml => scoped-constants-old-dtd.xml} | 0 4 files changed, 3 insertions(+), 3 deletions(-) rename core/src/test/resources/org/apache/struts2/config/providers/{xwork-test-scoped-constants-duplicate.xml => scoped-constants-duplicate.xml} (100%) rename core/src/test/resources/org/apache/struts2/config/providers/{xwork-test-scoped-constants.xml => scoped-constants-inheritance.xml} (100%) rename core/src/test/resources/org/apache/struts2/config/providers/{xwork-test-scoped-constants-old-dtd.xml => scoped-constants-old-dtd.xml} (100%) diff --git a/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java index 25350d5c18..7a6a7f8d23 100644 --- a/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java +++ b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java @@ -201,7 +201,7 @@ public void testDefaultClassRef() throws ConfigurationException { } public void testScopedConstantsLoadAndInherit() throws ConfigurationException { - buildConfigurationProvider(getXmlConfigFilePath("xwork-test-scoped-constants.xml")); + buildConfigurationProvider(getXmlConfigFilePath("scoped-constants-inheritance.xml")); PackageConfig parent = configuration.getPackageConfig("parent"); PackageConfig child = configuration.getPackageConfig("child"); @@ -213,7 +213,7 @@ public void testScopedConstantsLoadAndInherit() throws ConfigurationException { public void testDuplicateScopedConstantInOnePackageFails() { try { - buildConfigurationProvider(getXmlConfigFilePath("xwork-test-scoped-constants-duplicate.xml")); + buildConfigurationProvider(getXmlConfigFilePath("scoped-constants-duplicate.xml")); fail("Should have thrown a ConfigurationException"); } catch (ConfigurationException e) { assertThat(e).hasStackTraceContaining("Package [dup] declares scoped constant [first] more than once"); @@ -222,7 +222,7 @@ public void testDuplicateScopedConstantInOnePackageFails() { public void testScopedConstantIsRejectedUnderOlderDtd() { try { - buildConfigurationProvider(getXmlConfigFilePath("xwork-test-scoped-constants-old-dtd.xml")); + buildConfigurationProvider(getXmlConfigFilePath("scoped-constants-old-dtd.xml")); fail("Should have thrown a ConfigurationException"); } catch (ConfigurationException e) { assertThat(e).hasStackTraceContaining("\"scoped-constant\" must be declared"); diff --git a/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-duplicate.xml b/core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-duplicate.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-duplicate.xml rename to core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-duplicate.xml diff --git a/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants.xml b/core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-inheritance.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants.xml rename to core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-inheritance.xml diff --git a/core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-old-dtd.xml b/core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-old-dtd.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/providers/xwork-test-scoped-constants-old-dtd.xml rename to core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-old-dtd.xml From 6562ac22fa995a418fb0ff5518052c33215715dd Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 18:09:04 +0200 Subject: [PATCH 09/16] WW-3476 fix(config): skip ScopableConstants bean names without an instance Co-Authored-By: Claude Opus 5.5 --- .../struts2/config/ScopableConstants.java | 5 ++- .../struts2/config/ScopableConstantsTest.java | 41 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 core/src/test/java/org/apache/struts2/config/ScopableConstantsTest.java diff --git a/core/src/main/java/org/apache/struts2/config/ScopableConstants.java b/core/src/main/java/org/apache/struts2/config/ScopableConstants.java index fd21ac513f..de25a41a81 100644 --- a/core/src/main/java/org/apache/struts2/config/ScopableConstants.java +++ b/core/src/main/java/org/apache/struts2/config/ScopableConstants.java @@ -37,7 +37,10 @@ public interface ScopableConstants { static Set collectNames(Container container) { Set names = new HashSet<>(); for (String beanName : container.getInstanceNames(ScopableConstants.class)) { - names.addAll(container.getInstance(ScopableConstants.class, beanName).getNames()); + ScopableConstants scopableConstants = container.getInstance(ScopableConstants.class, beanName); + if (scopableConstants != null) { + names.addAll(scopableConstants.getNames()); + } } return Collections.unmodifiableSet(names); } diff --git a/core/src/test/java/org/apache/struts2/config/ScopableConstantsTest.java b/core/src/test/java/org/apache/struts2/config/ScopableConstantsTest.java new file mode 100644 index 0000000000..bdcf3121fe --- /dev/null +++ b/core/src/test/java/org/apache/struts2/config/ScopableConstantsTest.java @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.config; + +import org.apache.struts2.mock.MockContainer; +import org.junit.Test; + +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; + +public class ScopableConstantsTest { + + @Test + public void beanNameWithoutInstanceContributesNoNames() { + MockContainer container = new MockContainer() { + @Override + public Set getInstanceNames(Class type) { + return Set.of("unresolvable"); + } + }; + + assertThat(ScopableConstants.collectNames(container)).isEmpty(); + } +} From dc4305acdc07503c79a6e4299c98e115034deb34 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 18:09:29 +0200 Subject: [PATCH 10/16] WW-3476 refactor(config): resolve Configuration and scopable names from the container once Co-Authored-By: Claude Opus 5.5 --- .../config/StrutsScopedConstantProvider.java | 20 ++++--------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java index 2bbb40e311..806d8fc6e8 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java @@ -35,21 +35,18 @@ public class StrutsScopedConstantProvider implements ScopedConstantProvider { private Container container; private Configuration configuration; - private volatile Set scopableNames; + private Set scopableNames; @Inject public void setContainer(Container container) { this.container = container; - } - - @Inject - public void setConfiguration(Configuration configuration) { - this.configuration = configuration; + this.configuration = container.getInstance(Configuration.class); + this.scopableNames = ScopableConstants.collectNames(container); } @Override public String getValue(String name) { - if (!getScopableNames().contains(name)) { + if (!scopableNames.contains(name)) { throw new IllegalArgumentException(String.format( "Constant [%s] is not scopable; register it through a %s bean", name, ScopableConstants.class.getName())); } @@ -67,13 +64,4 @@ private Optional currentPackage() { } return Optional.ofNullable(configuration.getPackageConfig(proxy.getConfig().getPackageName())); } - - private Set getScopableNames() { - Set names = scopableNames; - if (names == null) { - names = ScopableConstants.collectNames(container); - scopableNames = names; - } - return names; - } } From 7645bc63c7f3332c7ccbf25a50d9516251ccfa65 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 18:10:01 +0200 Subject: [PATCH 11/16] WW-3476 refactor(config): address Sonar findings on scoped constants Co-Authored-By: Claude Opus 5.5 --- .../apache/struts2/config/entities/PackageConfig.java | 4 ++-- .../struts2/config/entities/PackageConfigTest.java | 10 +++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java b/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java index 85f580a828..88e1961920 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/PackageConfig.java @@ -59,8 +59,8 @@ public class PackageConfig extends Located implements Comparable, protected boolean isFinal = false; // a final package is unextendable protected boolean needsRefresh; protected boolean strictMethodInvocation = true; - protected Map scopedConstants; - protected Map allScopedConstants; + private Map scopedConstants; + private Map allScopedConstants; protected PackageConfig(String name) { this.name = name; diff --git a/core/src/test/java/org/apache/struts2/config/entities/PackageConfigTest.java b/core/src/test/java/org/apache/struts2/config/entities/PackageConfigTest.java index 8619a8b5c9..af4e4506df 100644 --- a/core/src/test/java/org/apache/struts2/config/entities/PackageConfigTest.java +++ b/core/src/test/java/org/apache/struts2/config/entities/PackageConfigTest.java @@ -22,6 +22,7 @@ import java.util.Map; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertThrows; public class PackageConfigTest extends XWorkTestCase { @@ -163,15 +164,18 @@ public void testScopedConstantsAreUnmodifiableAfterBuild() { .addScopedConstant("first", "value") .build(); - assertThrows(UnsupportedOperationException.class, () -> config.getScopedConstants().put("x", "y")); - assertThrows(UnsupportedOperationException.class, () -> config.getAllScopedConstants().put("x", "y")); + Map own = config.getScopedConstants(); + Map all = config.getAllScopedConstants(); + + assertThrows(UnsupportedOperationException.class, () -> own.put("x", "y")); + assertThrows(UnsupportedOperationException.class, () -> all.put("x", "y")); } public void testScopedConstantsTakePartInEquality() { PackageConfig plain = new PackageConfig.Builder("pkg").build(); PackageConfig scoped = new PackageConfig.Builder("pkg").addScopedConstant("first", "value").build(); - assertFalse(plain.equals(scoped)); + assertNotEquals(plain, scoped); } } From 2b251847019e401a088a646ec3801deda10c4762 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 18:11:40 +0200 Subject: [PATCH 12/16] WW-3476 test(config): name scoped-constant fixtures struts-*.xml Co-Authored-By: Claude Opus 5.5 --- .../struts2/config/ScopedConstantsValidationTest.java | 10 +++++----- .../config/StrutsScopedConstantProviderTest.java | 2 +- .../XmlConfigurationProviderPackagesTest.java | 6 +++--- ...icate.xml => struts-scoped-constants-duplicate.xml} | 0 ...nce.xml => struts-scoped-constants-inheritance.xml} | 0 ...old-dtd.xml => struts-scoped-constants-old-dtd.xml} | 0 ... struts-scoped-constants-abstract-unregistered.xml} | 0 ...try.xml => struts-scoped-constants-no-registry.xml} | 0 ...ovider.xml => struts-scoped-constants-provider.xml} | 0 ...ered.xml => struts-scoped-constants-registered.xml} | 0 ...beans.xml => struts-scoped-constants-two-beans.xml} | 0 ...ed.xml => struts-scoped-constants-unregistered.xml} | 0 12 files changed, 9 insertions(+), 9 deletions(-) rename core/src/test/resources/org/apache/struts2/config/providers/{scoped-constants-duplicate.xml => struts-scoped-constants-duplicate.xml} (100%) rename core/src/test/resources/org/apache/struts2/config/providers/{scoped-constants-inheritance.xml => struts-scoped-constants-inheritance.xml} (100%) rename core/src/test/resources/org/apache/struts2/config/providers/{scoped-constants-old-dtd.xml => struts-scoped-constants-old-dtd.xml} (100%) rename core/src/test/resources/org/apache/struts2/config/{scoped-constants-abstract-unregistered.xml => struts-scoped-constants-abstract-unregistered.xml} (100%) rename core/src/test/resources/org/apache/struts2/config/{scoped-constants-no-registry.xml => struts-scoped-constants-no-registry.xml} (100%) rename core/src/test/resources/org/apache/struts2/config/{scoped-constants-provider.xml => struts-scoped-constants-provider.xml} (100%) rename core/src/test/resources/org/apache/struts2/config/{scoped-constants-registered.xml => struts-scoped-constants-registered.xml} (100%) rename core/src/test/resources/org/apache/struts2/config/{scoped-constants-two-beans.xml => struts-scoped-constants-two-beans.xml} (100%) rename core/src/test/resources/org/apache/struts2/config/{scoped-constants-unregistered.xml => struts-scoped-constants-unregistered.xml} (100%) diff --git a/core/src/test/java/org/apache/struts2/config/ScopedConstantsValidationTest.java b/core/src/test/java/org/apache/struts2/config/ScopedConstantsValidationTest.java index 5cbb1a3418..59dfedc023 100644 --- a/core/src/test/java/org/apache/struts2/config/ScopedConstantsValidationTest.java +++ b/core/src/test/java/org/apache/struts2/config/ScopedConstantsValidationTest.java @@ -43,7 +43,7 @@ public void tearDown() { @Test public void registeredNameLoads() { - Configuration configuration = load("scoped-constants-registered.xml"); + Configuration configuration = load("struts-scoped-constants-registered.xml"); assertThat(configuration.getPackageConfig("registered").getScopedConstants()) .isEqualTo(Map.of("sample.scopable", "x")); @@ -51,7 +51,7 @@ public void registeredNameLoads() { @Test public void unregisteredNameFails() { - assertThatThrownBy(() -> load("scoped-constants-unregistered.xml")) + assertThatThrownBy(() -> load("struts-scoped-constants-unregistered.xml")) .isInstanceOf(ConfigurationException.class) .hasStackTraceContaining("Package [unregistered] declares scoped constant [sample.unknown], which is not scopable. " + "Scopable constants: [sample.scopable, sample.unset]."); @@ -59,14 +59,14 @@ public void unregisteredNameFails() { @Test public void abstractPackageIsValidatedToo() { - assertThatThrownBy(() -> load("scoped-constants-abstract-unregistered.xml")) + assertThatThrownBy(() -> load("struts-scoped-constants-abstract-unregistered.xml")) .isInstanceOf(ConfigurationException.class) .hasStackTraceContaining("Package [abstractBase] declares scoped constant [sample.unknown], which is not scopable."); } @Test public void anyScopedConstantFailsWhenNothingIsScopable() { - assertThatThrownBy(() -> load("scoped-constants-no-registry.xml")) + assertThatThrownBy(() -> load("struts-scoped-constants-no-registry.xml")) .isInstanceOf(ConfigurationException.class) .hasStackTraceContaining("Package [unregistered] declares scoped constant [sample.scopable], which is not scopable. " + "No constants are scopable in this configuration."); @@ -74,7 +74,7 @@ public void anyScopedConstantFailsWhenNothingIsScopable() { @Test public void namesFromAllBeansAreScopable() { - Configuration configuration = load("scoped-constants-two-beans.xml"); + Configuration configuration = load("struts-scoped-constants-two-beans.xml"); assertThat(configuration.getPackageConfig("both").getScopedConstants()) .isEqualTo(Map.of("sample.scopable", "x", "second.scopable", "y")); diff --git a/core/src/test/java/org/apache/struts2/config/StrutsScopedConstantProviderTest.java b/core/src/test/java/org/apache/struts2/config/StrutsScopedConstantProviderTest.java index be3ccc53e6..8a2ebaaaff 100644 --- a/core/src/test/java/org/apache/struts2/config/StrutsScopedConstantProviderTest.java +++ b/core/src/test/java/org/apache/struts2/config/StrutsScopedConstantProviderTest.java @@ -34,7 +34,7 @@ public class StrutsScopedConstantProviderTest extends XWorkTestCase { @Override protected void setUp() throws Exception { super.setUp(); - loadConfigurationProviders(new StrutsXmlConfigurationProvider("org/apache/struts2/config/scoped-constants-provider.xml")); + loadConfigurationProviders(new StrutsXmlConfigurationProvider("org/apache/struts2/config/struts-scoped-constants-provider.xml")); provider = container.getInstance(ScopedConstantProvider.class); } diff --git a/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java index 7a6a7f8d23..35e2cdb2f4 100644 --- a/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java +++ b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderPackagesTest.java @@ -201,7 +201,7 @@ public void testDefaultClassRef() throws ConfigurationException { } public void testScopedConstantsLoadAndInherit() throws ConfigurationException { - buildConfigurationProvider(getXmlConfigFilePath("scoped-constants-inheritance.xml")); + buildConfigurationProvider(getXmlConfigFilePath("struts-scoped-constants-inheritance.xml")); PackageConfig parent = configuration.getPackageConfig("parent"); PackageConfig child = configuration.getPackageConfig("child"); @@ -213,7 +213,7 @@ public void testScopedConstantsLoadAndInherit() throws ConfigurationException { public void testDuplicateScopedConstantInOnePackageFails() { try { - buildConfigurationProvider(getXmlConfigFilePath("scoped-constants-duplicate.xml")); + buildConfigurationProvider(getXmlConfigFilePath("struts-scoped-constants-duplicate.xml")); fail("Should have thrown a ConfigurationException"); } catch (ConfigurationException e) { assertThat(e).hasStackTraceContaining("Package [dup] declares scoped constant [first] more than once"); @@ -222,7 +222,7 @@ public void testDuplicateScopedConstantInOnePackageFails() { public void testScopedConstantIsRejectedUnderOlderDtd() { try { - buildConfigurationProvider(getXmlConfigFilePath("scoped-constants-old-dtd.xml")); + buildConfigurationProvider(getXmlConfigFilePath("struts-scoped-constants-old-dtd.xml")); fail("Should have thrown a ConfigurationException"); } catch (ConfigurationException e) { assertThat(e).hasStackTraceContaining("\"scoped-constant\" must be declared"); diff --git a/core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-duplicate.xml b/core/src/test/resources/org/apache/struts2/config/providers/struts-scoped-constants-duplicate.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-duplicate.xml rename to core/src/test/resources/org/apache/struts2/config/providers/struts-scoped-constants-duplicate.xml diff --git a/core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-inheritance.xml b/core/src/test/resources/org/apache/struts2/config/providers/struts-scoped-constants-inheritance.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-inheritance.xml rename to core/src/test/resources/org/apache/struts2/config/providers/struts-scoped-constants-inheritance.xml diff --git a/core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-old-dtd.xml b/core/src/test/resources/org/apache/struts2/config/providers/struts-scoped-constants-old-dtd.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/providers/scoped-constants-old-dtd.xml rename to core/src/test/resources/org/apache/struts2/config/providers/struts-scoped-constants-old-dtd.xml diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-abstract-unregistered.xml b/core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-abstract-unregistered.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/scoped-constants-abstract-unregistered.xml rename to core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-abstract-unregistered.xml diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-no-registry.xml b/core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-no-registry.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/scoped-constants-no-registry.xml rename to core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-no-registry.xml diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-provider.xml b/core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-provider.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/scoped-constants-provider.xml rename to core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-provider.xml diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-registered.xml b/core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-registered.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/scoped-constants-registered.xml rename to core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-registered.xml diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-two-beans.xml b/core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-two-beans.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/scoped-constants-two-beans.xml rename to core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-two-beans.xml diff --git a/core/src/test/resources/org/apache/struts2/config/scoped-constants-unregistered.xml b/core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-unregistered.xml similarity index 100% rename from core/src/test/resources/org/apache/struts2/config/scoped-constants-unregistered.xml rename to core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-unregistered.xml From 197f2cc30cb69184dd030dca1ddeeb473b45b731 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 18:24:06 +0200 Subject: [PATCH 13/16] WW-3476 fix(mock): return an empty set from MockContainer.getInstanceNames Matches the Container contract ContainerImpl follows; callers iterate the result without a null check. Co-Authored-By: Claude Opus 5.5 --- .../src/main/java/org/apache/struts2/mock/MockContainer.java | 3 ++- .../org/apache/struts2/config/ScopableConstantsTest.java | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/mock/MockContainer.java b/core/src/main/java/org/apache/struts2/mock/MockContainer.java index a03ec2d678..209b8a123f 100644 --- a/core/src/main/java/org/apache/struts2/mock/MockContainer.java +++ b/core/src/main/java/org/apache/struts2/mock/MockContainer.java @@ -21,6 +21,7 @@ import org.apache.struts2.inject.Container; import org.apache.struts2.inject.Scope; +import java.util.Collections; import java.util.Set; /** @@ -45,7 +46,7 @@ public T getInstance(Class type) { } public Set getInstanceNames(Class type) { - return null; + return Collections.emptySet(); } public void setScopeStrategy(Scope.Strategy scopeStrategy) { diff --git a/core/src/test/java/org/apache/struts2/config/ScopableConstantsTest.java b/core/src/test/java/org/apache/struts2/config/ScopableConstantsTest.java index bdcf3121fe..ee1257422f 100644 --- a/core/src/test/java/org/apache/struts2/config/ScopableConstantsTest.java +++ b/core/src/test/java/org/apache/struts2/config/ScopableConstantsTest.java @@ -38,4 +38,9 @@ public Set getInstanceNames(Class type) { assertThat(ScopableConstants.collectNames(container)).isEmpty(); } + + @Test + public void containerWithoutScopableBeansContributesNoNames() { + assertThat(ScopableConstants.collectNames(new MockContainer())).isEmpty(); + } } From 8a81250463253e1da350d43c02816f98f95301b0 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sat, 10 Oct 2026 18:27:25 +0200 Subject: [PATCH 14/16] WW-3476 docs(config): document ScopedConstantProvider as a bean-selection extension point Lists it in the StrutsBeanSelectionProvider extension-point table, adds struts.scopedConstantProvider to default.properties, and pins that the constant selects a custom bean. Co-Authored-By: Claude Opus 5.5 --- .../org/apache/struts2/StrutsConstants.java | 3 +- .../config/StrutsBeanSelectionProvider.java | 6 ++++ .../org/apache/struts2/default.properties | 3 ++ .../config/CustomScopedConstantProvider.java | 27 +++++++++++++++++ .../ScopedConstantProviderWiringTest.java | 10 ++++++- ...truts-scoped-constants-custom-provider.xml | 30 +++++++++++++++++++ 6 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 core/src/test/java/org/apache/struts2/config/CustomScopedConstantProvider.java create mode 100644 core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-custom-provider.xml diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index cfa3e30a7b..4c9ca1719b 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -629,7 +629,8 @@ public final class StrutsConstants { public static final String STRUTS_PARAMETER_ALLOWLISTER = "struts.parameterAllowlister"; /** - * The {@link org.apache.struts2.config.ScopedConstantProvider} implementation class. + * Name of the Struts bean implementing {@link org.apache.struts2.config.ScopedConstantProvider}, selected by + * {@link org.apache.struts2.config.StrutsBeanSelectionProvider}. * * @since 7.5.0 */ diff --git a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java index 8c141c6c73..866bad44f7 100644 --- a/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java +++ b/core/src/main/java/org/apache/struts2/config/StrutsBeanSelectionProvider.java @@ -359,6 +359,12 @@ * singleton * Provides access to resource bundles used to localise messages (since 2.5.11) * + * + * org.apache.struts2.config.ScopedConstantProvider + * struts.scopedConstantProvider + * singleton + * Resolves package-scoped constants declared with <scoped-constant> (since 7.5.0) + * * *

* diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index cacf316da3..05e0613b9e 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -334,6 +334,9 @@ struts.proxy.cacheFactory=struts ### Specifies the ProxyService implementation class. struts.proxyService=struts +### Name of the ScopedConstantProvider bean resolving package-scoped constants (since 7.5.0) +struts.scopedConstantProvider=struts + ### Indicates if Dispatcher should handle unexpected exceptions by calling sendError() ### or simply rethrow it as a ServletException to allow future processing by other frameworks like Spring Security struts.handle.exception=true diff --git a/core/src/test/java/org/apache/struts2/config/CustomScopedConstantProvider.java b/core/src/test/java/org/apache/struts2/config/CustomScopedConstantProvider.java new file mode 100644 index 0000000000..f5c1a5f82c --- /dev/null +++ b/core/src/test/java/org/apache/struts2/config/CustomScopedConstantProvider.java @@ -0,0 +1,27 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.config; + +public class CustomScopedConstantProvider implements ScopedConstantProvider { + + @Override + public String getValue(String name) { + return "custom"; + } +} diff --git a/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java b/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java index f241530760..2d5514d9e9 100644 --- a/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java +++ b/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java @@ -23,8 +23,16 @@ public class ScopedConstantProviderWiringTest extends StrutsInternalTestCase { public void testDispatcherResolvesStrutsScopedConstantProvider() { - ScopedConstantProvider provider = dispatcher.getContainer().getInstance(ScopedConstantProvider.class); + ScopedConstantProvider provider = container.getInstance(ScopedConstantProvider.class); assertTrue(provider instanceof StrutsScopedConstantProvider); } + + public void testScopedConstantProviderConstantSelectsCustomBean() { + initDispatcherWithConfigs("struts-default.xml,org/apache/struts2/config/struts-scoped-constants-custom-provider.xml"); + + ScopedConstantProvider provider = container.getInstance(ScopedConstantProvider.class); + + assertTrue(provider instanceof CustomScopedConstantProvider); + } } diff --git a/core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-custom-provider.xml b/core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-custom-provider.xml new file mode 100644 index 0000000000..df4dfcbea7 --- /dev/null +++ b/core/src/test/resources/org/apache/struts2/config/struts-scoped-constants-custom-provider.xml @@ -0,0 +1,30 @@ + + + + + + + + From c93980593e978feddf71d67ec35826ef33e50117 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 11 Oct 2026 09:51:40 +0200 Subject: [PATCH 15/16] WW-3476 refactor(config): move StrutsScopedConstantProvider to config.providers Co-Authored-By: Claude Opus 5.5 --- .../config/{ => providers}/StrutsScopedConstantProvider.java | 0 .../config/{ => providers}/StrutsScopedConstantProviderTest.java | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename core/src/main/java/org/apache/struts2/config/{ => providers}/StrutsScopedConstantProvider.java (100%) rename core/src/test/java/org/apache/struts2/config/{ => providers}/StrutsScopedConstantProviderTest.java (100%) diff --git a/core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java b/core/src/main/java/org/apache/struts2/config/providers/StrutsScopedConstantProvider.java similarity index 100% rename from core/src/main/java/org/apache/struts2/config/StrutsScopedConstantProvider.java rename to core/src/main/java/org/apache/struts2/config/providers/StrutsScopedConstantProvider.java diff --git a/core/src/test/java/org/apache/struts2/config/StrutsScopedConstantProviderTest.java b/core/src/test/java/org/apache/struts2/config/providers/StrutsScopedConstantProviderTest.java similarity index 100% rename from core/src/test/java/org/apache/struts2/config/StrutsScopedConstantProviderTest.java rename to core/src/test/java/org/apache/struts2/config/providers/StrutsScopedConstantProviderTest.java From c8fa01ca81f9125ef910df9399b0997516477995 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Sun, 11 Oct 2026 09:51:59 +0200 Subject: [PATCH 16/16] WW-3476 refactor(config): complete the StrutsScopedConstantProvider move c93980593 committed only the renames; this adds the package declarations, imports and the struts-beans.xml class name. Co-Authored-By: Claude Opus 5.5 --- .../config/providers/StrutsDefaultConfigurationProvider.java | 1 - .../config/providers/StrutsScopedConstantProvider.java | 5 ++++- core/src/main/resources/struts-beans.xml | 2 +- .../struts2/config/ScopedConstantProviderWiringTest.java | 1 + .../config/providers/StrutsScopedConstantProviderTest.java | 4 +++- 5 files changed, 9 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java b/core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java index c66911bb90..736af6c911 100644 --- a/core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java +++ b/core/src/main/java/org/apache/struts2/config/providers/StrutsDefaultConfigurationProvider.java @@ -27,7 +27,6 @@ import org.apache.struts2.config.ConfigurationException; import org.apache.struts2.config.ConfigurationProvider; import org.apache.struts2.config.ScopedConstantProvider; -import org.apache.struts2.config.StrutsScopedConstantProvider; import org.apache.struts2.config.impl.DefaultConfiguration; import org.apache.struts2.conversion.NullHandler; import org.apache.struts2.conversion.impl.ArrayConverter; diff --git a/core/src/main/java/org/apache/struts2/config/providers/StrutsScopedConstantProvider.java b/core/src/main/java/org/apache/struts2/config/providers/StrutsScopedConstantProvider.java index 806d8fc6e8..ebfc968f58 100644 --- a/core/src/main/java/org/apache/struts2/config/providers/StrutsScopedConstantProvider.java +++ b/core/src/main/java/org/apache/struts2/config/providers/StrutsScopedConstantProvider.java @@ -16,11 +16,14 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.struts2.config; +package org.apache.struts2.config.providers; import org.apache.struts2.ActionContext; import org.apache.struts2.ActionInvocation; import org.apache.struts2.ActionProxy; +import org.apache.struts2.config.Configuration; +import org.apache.struts2.config.ScopableConstants; +import org.apache.struts2.config.ScopedConstantProvider; import org.apache.struts2.config.entities.PackageConfig; import org.apache.struts2.inject.Container; import org.apache.struts2.inject.Inject; diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml index 81109bf069..abfca2af69 100644 --- a/core/src/main/resources/struts-beans.xml +++ b/core/src/main/resources/struts-beans.xml @@ -259,7 +259,7 @@ class="org.apache.struts2.interceptor.parameter.OgnlParameterAllowlister" scope="singleton"/> + class="org.apache.struts2.config.providers.StrutsScopedConstantProvider" scope="singleton"/> diff --git a/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java b/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java index 2d5514d9e9..cedf54592f 100644 --- a/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java +++ b/core/src/test/java/org/apache/struts2/config/ScopedConstantProviderWiringTest.java @@ -19,6 +19,7 @@ package org.apache.struts2.config; import org.apache.struts2.StrutsInternalTestCase; +import org.apache.struts2.config.providers.StrutsScopedConstantProvider; public class ScopedConstantProviderWiringTest extends StrutsInternalTestCase { diff --git a/core/src/test/java/org/apache/struts2/config/providers/StrutsScopedConstantProviderTest.java b/core/src/test/java/org/apache/struts2/config/providers/StrutsScopedConstantProviderTest.java index 8a2ebaaaff..c00ded1c5e 100644 --- a/core/src/test/java/org/apache/struts2/config/providers/StrutsScopedConstantProviderTest.java +++ b/core/src/test/java/org/apache/struts2/config/providers/StrutsScopedConstantProviderTest.java @@ -16,11 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.struts2.config; +package org.apache.struts2.config.providers; import org.apache.struts2.ActionContext; import org.apache.struts2.ActionSupport; import org.apache.struts2.XWorkTestCase; +import org.apache.struts2.config.ScopedConstantProvider; +import org.apache.struts2.config.StrutsXmlConfigurationProvider; import org.apache.struts2.config.entities.ActionConfig; import org.apache.struts2.mock.MockActionInvocation; import org.apache.struts2.mock.MockActionProxy;