[
https://issues.apache.org/jira/browse/WW-5770?focusedWorklogId=1047370&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1047370
]
ASF GitHub Bot logged work on WW-5770:
--------------------------------------
Author: ASF GitHub Bot
Created on: 14/Oct/26 06:15
Start Date: 14/Oct/26 06:15
Worklog Time Spent: 10m
Work Description: lukaszlenart commented on code in PR #2009:
URL: https://github.com/apache/struts/pull/2009#discussion_r4267396133
##########
plugins/convention/src/test/resources/struts-convention-scoped-result-path.xml:
##########
@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<!--
+/*
+ * 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.
+ */
+-->
+
+<!DOCTYPE struts PUBLIC
+ "-//Apache Software Foundation//DTD Struts Configuration 7.5//EN"
+ "https://struts.apache.org/dtds/struts-7.5.dtd";>
+
+<struts>
+ <constant name="struts.convention.result.path" value="/"/>
+ <constant name="struts.convention.default.parent.package"
value="scoped-default"/>
+
+ <package name="convention-default" extends="struts-default"/>
+
Review Comment:
Removed in df4a7b6be — `scoped-default` now extends the plugin-provided
`convention-default`. For the record, the redeclaration did not raise a
duplicate-package error: `XmlDocConfigurationProvider.addPackage` silently
skips a package name that is already loaded, so the line was inert.
##########
core/src/main/java/org/apache/struts2/config/ScopedConstantProvider.java:
##########
@@ -32,4 +34,17 @@ public interface ScopedConstantProvider {
* @throws IllegalArgumentException when {@code name} is not scopable
*/
String getValue(String name);
+
+ /**
+ * @param name a constant registered through a {@link
ScopableConstants} bean
+ * @param packageConfig the package whose scoped constants, inherited ones
included, take precedence; may be
+ * {@code null}
+ * @return the package's scoped value, or else {@link #getValue(String)}
+ * @throws IllegalArgumentException when {@code name} is not scopable
+ * @since 7.5.0
+ */
+ default String getValue(String name, PackageConfig packageConfig) {
+ String value = packageConfig != null ?
packageConfig.getAllScopedConstants().get(name) : null;
+ return value != null ? value : getValue(name);
Review Comment:
Fixed in df4a7b6be by narrowing the `@throws`: the default method checks the
name only on the `getValue(String)` fallback, since package values are
validated against the scopable set at startup
(`DefaultConfiguration.validateScopedConstants`).
`StrutsScopedConstantProvider` overrides it and always checks.
##########
plugins/convention/src/main/java/org/apache/struts2/convention/ConventionsServiceImpl.java:
##########
@@ -51,54 +53,84 @@ public
ConventionsServiceImpl(@Inject(ConventionConstants.CONVENTION_RESULT_PATH
this.resultPath = resultPath;
}
+ @Inject
+ public void setScopedConstantProvider(ScopedConstantProvider
scopedConstantProvider) {
+ this.scopedConstantProvider = scopedConstantProvider;
+ }
+
/**
* {@inheritDoc}
*/
public String determineResultPath(Class<?> actionClass) {
- String localResultPath = resultPath;
- ResultPath resultPathAnnotation =
AnnotationUtils.findAnnotation(actionClass, ResultPath.class);
- if (resultPathAnnotation != null) {
- if (resultPathAnnotation.value().equals("") &&
resultPathAnnotation.property().equals("")) {
- throw new ConfigurationException("The ResultPath annotation
must have either" +
- " a value or property specified.");
- }
-
- String property = resultPathAnnotation.property();
- if (property.equals("")) {
- localResultPath = resultPathAnnotation.value();
- } else {
- try {
- ResourceBundle strutsBundle =
ResourceBundle.getBundle("struts");
- localResultPath = strutsBundle.getString(property);
- } catch (Exception e) {
- throw new ConfigurationException("The action class [" +
actionClass + "] defines" +
- " a @ResultPath annotation and a property definition
however the" +
- " struts.properties could not be found in the
classpath using ResourceBundle" +
- " OR the bundle exists but the property [" + property
+ "] is not defined" +
- " in the file.", e);
- }
- }
- }
+ return determineResultPath(actionClass, null);
+ }
- return localResultPath;
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public String determineResultPath(Class<?> actionClass, PackageConfig
packageConfig) {
+ String annotatedResultPath = findAnnotatedResultPath(actionClass);
+ return annotatedResultPath != null ? annotatedResultPath :
scopedResultPath(packageConfig);
}
/**
* {@inheritDoc}
*/
public String determineResultPath(ActionConfig actionConfig) {
+ return determineResultPath(actionConfig, null);
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ @Override
+ public String determineResultPath(ActionConfig actionConfig, PackageConfig
packageConfig) {
if (actionConfig == null) {
- return resultPath;
+ return scopedResultPath(packageConfig);
}
try {
- return
determineResultPath(ClassLoaderUtil.loadClass(actionConfig.getClassName(),
this.getClass()));
+ return
determineResultPath(ClassLoaderUtil.loadClass(actionConfig.getClassName(),
this.getClass()), packageConfig);
} catch (ClassNotFoundException e) {
throw new RuntimeException("Invalid action class configuration
that references an unknown " +
"class named [" + actionConfig.getClassName() + "]", e);
}
}
+ private String scopedResultPath(PackageConfig packageConfig) {
+ if (packageConfig == null) {
+ return resultPath;
+ }
+ return
scopedConstantProvider.getValue(ConventionConstants.CONVENTION_RESULT_PATH,
packageConfig);
+ }
Review Comment:
Keeping as is. `ScopedConstantProvider` is a required bean: the container
always injects it, the same way `UIBean.setScopedConstantProvider` works since
WW-5768. Code that builds `ConventionsServiceImpl` by hand has to set it, as
the tests here do (`setScopedConstantProvider(name -> ...)`). A silent fallback
to the global path would hide a misconfiguration rather than surface it.
##########
docs/superpowers/specs/2026-10-14-WW-5770-scoped-convention-result-path-design.md:
##########
@@ -0,0 +1,121 @@
+# WW-5770 — Scopable `struts.convention.result.path`
+
+**Ticket:** [WW-5770](https://issues.apache.org/jira/browse/WW-5770) — *Make
struts.convention.result.path scopable per package*
+**Target:** 7.5.0 (ticket fix version)
+**Date:** 2026-10-14
+**Status:** Design approved, pending implementation plan
Review Comment:
Updated in df4a7b6be.
Issue Time Tracking
-------------------
Worklog Id: (was: 1047370)
Time Spent: 50m (was: 40m)
> Make struts.convention.result.path scopable per package
> -------------------------------------------------------
>
> Key: WW-5770
> URL: https://issues.apache.org/jira/browse/WW-5770
> Project: Struts 2
> Issue Type: Improvement
> Components: Plugin - Convention
> Reporter: Lukasz Lenart
> Assignee: Lukasz Lenart
> Priority: Major
> Fix For: 7.5.0
>
> Time Spent: 50m
> Remaining Estimate: 0h
>
> A follow-up to the package-scoped constants from
> [WW-3476|https://issues.apache.org/jira/browse/WW-3476], whose first adopter
> was [WW-5768|https://issues.apache.org/jira/browse/WW-5768]
> ({{struts.ui.theme}}, {{struts.ui.templateDir}}).
> h2. Goal
> Allow a parent package to set the Convention result path for the actions
> placed under it:
> {code:xml}<package name="admin-base" extends="convention-default">
> <scoped-constant name="struts.convention.result.path"
> value="/WEB-INF/admin/"/>
> </package>
> {code}
> Today the same effect needs {{@ResultPath}} on every action class or package.
> h2. Why it needs more than WW-5768
> {{ScopedConstantProvider.getValue(name)}} resolves the package from the
> current {{ActionInvocation}}. The Convention plugin reads the result path in
> two places:
> # *At startup*, in {{DefaultResultMapBuilder.build(...)}} via
> {{ConventionsService.determineResultPath(Class)}}. There is no invocation, so
> {{getValue(name)}} always returns the global value. However, {{build()}}
> receives the action's {{PackageConfig}}. {{PackageBasedActionConfigBuilder}}
> passes {{pkgCfg.build()}}, which was created with {{addParent(parentPkg)}},
> so {{getAllScopedConstants()}} already includes the parent's overrides.
> # *At runtime*, in {{ConventionUnknownHandler.determinePath(...)}} via
> {{determineResultPath(ActionConfig)}}, for action-less results.
> {{actionConfig}} may be {{null}} there.
> Both need a lookup by package rather than by invocation. One option is a
> {{getValue(String name, PackageConfig packageConfig)}} on
> {{ScopedConstantProvider}}, with {{getValue(name)}} delegating to it for the
> current invocation's package.
> h2. Proposed change
> * Register a {{ScopableConstants}} bean in the Convention plugin's
> {{struts-plugin.xml}} returning {{struts.convention.result.path}}.
> * Resolve the result path from the action's package in both places, keeping
> this precedence:
> ## {{@ResultPath}} on the action class or package
> ## the package's scoped constant (including inherited ones)
> ## the global {{struts.convention.result.path}}
> h2. Tests
> * A scoped override on the parent package changes the location of results
> built at startup.
> * {{@ResultPath}} still wins over the override.
> * An action-less result resolved by {{ConventionUnknownHandler}} uses the
> override.
> * With no override, the global path is used.
> h2. Docs
> On the struts-site, list the constant on the scoped-constants section of
> {{core-developers/package-configuration.md}}, and mention it on the
> Convention plugin page.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
