Skip to content

WW-5767 Apply value substitution to scoped-constant values - #2007

Merged
lukaszlenart merged 2 commits into
mainfrom
WW-5767-scoped-constant-value-substitution
Oct 12, 2026
Merged

lukaszlenart merged 2 commits into
mainfrom
WW-5767-scoped-constant-value-substitution

Conversation

@lukaszlenart

@lukaszlenart lukaszlenart commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Fixes WW-5767

Summary

<scoped-constant> values (added in WW-3476) are now run through the injected ValueSubstitutor when the configuration loads, the same way <constant> values have been since 2.5.6. ${env.NAME} and ${system.property} now resolve instead of being kept as literal text.

<package name="admin" extends="struts-default">
    <scoped-constant name="some.scopable.constant" value="${env.SOME_VALUE}"/>
</package>

registerConstant and loadScopedConstants now call one private helper, substituteValue, so the two elements can't drift apart. Substitution still runs once, at load time, and produces a static string. This is not the per-request evaluation that WW-3476 ruled out. PackageConfig, the startup validation and ScopedConstantProvider are unchanged.

Tests

ScopedConstantsSubstitutionTest (new, JUnit 4) with a new fixture, struts-scoped-constants-substitution.xml:

  • A scoped constant set to ${ww5767.value} resolves to the system property the test sets, then clears.
  • A literal value is kept as written.

The end-to-end test loads struts-default.xml in Dispatcher order, so loadPackages() gets its ValueSubstitutor from the main container, as in production, where it comes from struts-beans.xml.

XmlConfigurationProviderPackagesTest.testScopedConstantIsKeptAsWrittenWithoutValueSubstitutor covers the other branch. The substitutor is injected with required = false, so a provider used without injection keeps values as written. Every harness in the suite injects one through DefaultConfiguration.bootstrapFactories, so no existing test reached that branch.

Before the fix, the end-to-end test failed with but was: "${ww5767.value}". The existing XmlConfigurationProviderEnvsSubstitutionTest still passes after the refactor. The full core suite passes: 3448 tests, 0 failures.

Follow-up

The struts-site page core-developers/package-configuration.md says value substitution is not supported for scoped constants (apache/struts-site#351). That sentence needs updating once this is merged.

🤖 Generated with Claude Code

<scoped-constant> values now go through the injected ValueSubstitutor at
load time, the same as <constant>, so ${env.NAME} and ${system.property} resolve
instead of being stored literally. Both paths share one helper.

The test loads struts-default.xml so the substitutor comes from the main
container that loadPackages() is injected with; the ConfigurationTestBase
container registers none and could not observe the change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@lukaszlenart
lukaszlenart marked this pull request as ready for review October 12, 2026 13:02
@lukaszlenart
lukaszlenart requested a balanced review from Copilot October 12, 2026 14:06
…bstitutor

The substitutor is injected with required = false, so a provider used
without injection keeps values as written. Every harness in the suite
injects one through DefaultConfiguration.bootstrapFactories, so nothing
exercised that branch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

0 open findings

What changed in this PR

Applies ValueSubstitutor to <scoped-constant> values at configuration load time (matching existing <constant> behavior) to resolve ${env.*} / ${system.*}-style placeholders, and adds regression tests for WW-5767.

Changes:

  • Refactors value substitution into a shared helper and applies it to both global constants and scoped constants.
  • Adds a new XML fixture and a new JUnit test validating scoped-constant substitution at load time.
  • Adds an additional test case to verify behavior when no ValueSubstitutor is configured.
File Description
core/​src/​main/​java/​org/​apache/​struts2/​config/​providers/​XmlDocConfigurationProvider.java Centralizes substitution logic and applies it to scoped constants during config load.
core/​src/​test/​java/​org/​apache/​struts2/​config/​ScopedConstantsSubstitutionTest.java New regression test verifying scoped-constant values are substituted from system properties.
core/​src/​test/​resources/​org/​apache/​struts2/​config/​struts-scoped-constants-substitution.xml New fixture defining scoped constants used by the regression test.
core/​src/​test/​java/​org/​apache/​struts2/​config/​providers/​XmlConfigurationProviderPackagesTest.java Adds a test intended to assert literals remain unchanged when no substitutor is present.

🧠 Review effort: Lite


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@sonarqubecloud

Copy link
Copy Markdown

@lukaszlenart
lukaszlenart merged commit da2bcf6 into main Oct 12, 2026
16 checks passed
@lukaszlenart
lukaszlenart deleted the WW-5767-scoped-constant-value-substitution branch October 12, 2026 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants