This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new a9ce3e3c9 fix(convention): WW-4421 detect duplicate @Action names when
execute() is annotated (#1579)
a9ce3e3c9 is described below
commit a9ce3e3c99ada4c463cc021710d27c7158f01816
Author: Lukasz Lenart <[email protected]>
AuthorDate: Sat Feb 21 10:12:52 2026 +0100
fix(convention): WW-4421 detect duplicate @Action names when execute() is
annotated (#1579)
The duplicate @Action name detection in PackageBasedActionConfigBuilder
was embedded inside a conditional block that only ran when execute() was
NOT annotated with @Action. This meant two methods could map to the same
action name silently when execute() had an @Action annotation, with one
overwriting the other non-deterministically.
Extract the duplicate check to run unconditionally before the conditional
block, so it applies to all annotated methods regardless of whether
execute() is annotated.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude <[email protected]>
---
.../PackageBasedActionConfigBuilder.java | 78 +++++++-----
.../PackageBasedActionConfigBuilderTest.java | 103 ++++++++++++++-
.../result/ActionLevelResultsNamesAction.java | 10 +-
...cateActionNameWithExecuteAnnotationAction.java} | 27 ++--
...eActionNameWithoutExecuteAnnotationAction.java} | 31 ++---
...21-duplicate-action-annotation-check-skipped.md | 141 +++++++++++++++++++++
6 files changed, 319 insertions(+), 71 deletions(-)
diff --git
a/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java
b/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java
index bbd718497..711b25b72 100644
---
a/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java
+++
b/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java
@@ -738,42 +738,28 @@ public class PackageBasedActionConfigBuilder implements
ActionConfigBuilder {
// Verify that the annotations have no errors and also
determine if the default action
// configuration should still be built or not.
- Map<String, List<Action>> map =
getActionAnnotations(actionClass);
- Set<String> actionNames = new HashSet<>();
+ Map<String, List<Action>> actionAnnotationsByMethod =
getActionAnnotations(actionClass);
boolean hasDefaultMethod =
ReflectionTools.containsMethod(actionClass, DEFAULT_METHOD);
- if (!map.containsKey(DEFAULT_METHOD)
- && hasDefaultMethod
- && actionAnnotation == null && actionsAnnotation ==
null
- && (alwaysMapExecute || map.isEmpty())) {
- boolean found = false;
- for (List<Action> actions : map.values()) {
- for (Action action : actions) {
-
- // Check if there are duplicate action names in
the annotations.
- String actionName =
action.value().equals(Action.DEFAULT_VALUE) ? defaultActionName :
action.value();
- if (actionNames.contains(actionName)) {
- throw new ConfigurationException("The action
class [" + actionClass +
- "] contains two methods with an action
name annotation whose value " +
- "is the same (they both might be empty
as well).");
- } else {
- actionNames.add(actionName);
- }
-
- // Check this annotation is the default action
- if (action.value().equals(Action.DEFAULT_VALUE)) {
- found = true;
- }
+
+ // Check for duplicate action names across all annotated
methods
+ Set<String> actionNames = new HashSet<>();
+ for (List<Action> actions :
actionAnnotationsByMethod.values()) {
+ for (Action action : actions) {
+ String actionName =
action.value().equals(Action.DEFAULT_VALUE) ? defaultActionName :
action.value();
+ if (!actionNames.add(actionName)) {
+ throw new ConfigurationException("The action class
[" + actionClass +
+ "] contains two methods with an action
name annotation whose value " +
+ "is the same (they both might be empty as
well).");
}
}
+ }
- // Build the default
- if (!found) {
- createActionConfig(defaultPackageConfig, actionClass,
defaultActionName, DEFAULT_METHOD, null, allowedMethods);
- }
+ if (shouldMapDefaultExecuteMethod(actionAnnotationsByMethod,
hasDefaultMethod, actionAnnotation, actionsAnnotation)) {
+ createActionConfig(defaultPackageConfig, actionClass,
defaultActionName, DEFAULT_METHOD, null, allowedMethods);
}
// Build the actions for the annotations
- for (Map.Entry<String, List<Action>> entry : map.entrySet()) {
+ for (Map.Entry<String, List<Action>> entry :
actionAnnotationsByMethod.entrySet()) {
String method = entry.getKey();
List<Action> actions = entry.getValue();
for (Action action : actions) {
@@ -789,7 +775,7 @@ public class PackageBasedActionConfigBuilder implements
ActionConfigBuilder {
// some actions will not have any @Action or a default method,
like the rest actions
// where the action mapper is the one that finds the right
method at runtime
- if (map.isEmpty() && mapAllMatches && actionAnnotation == null
&& actionsAnnotation == null) {
+ if (actionAnnotationsByMethod.isEmpty() && mapAllMatches &&
actionAnnotation == null && actionsAnnotation == null) {
createActionConfig(defaultPackageConfig, actionClass,
defaultActionName, null, actionAnnotation, allowedMethods);
}
@@ -840,6 +826,38 @@ public class PackageBasedActionConfigBuilder implements
ActionConfigBuilder {
(actionClass.getModifiers() & Modifier.ABSTRACT) != 0 ||
actionClass.isAnonymousClass();
}
+ /**
+ * Determines whether the default {@code execute()} method should be
automatically mapped as an action.
+ * This is the case when no explicit mapping exists for the default
method, the class has an execute method,
+ * there are no class-level @Action/@Actions annotations, and no other
method already maps to the default action name.
+ *
+ * @param actionAnnotationsByMethod the map of method names to their
@Action annotations
+ * @param hasDefaultMethod whether the action class has an
execute() method
+ * @param classAction the class-level @Action annotation, or
null
+ * @param classActions the class-level @Actions annotation,
or null
+ * @return true if the default execute method should be mapped
+ */
+ protected boolean shouldMapDefaultExecuteMethod(Map<String, List<Action>>
actionAnnotationsByMethod, boolean hasDefaultMethod,
+ Action classAction,
Actions classActions) {
+ if (actionAnnotationsByMethod.containsKey(DEFAULT_METHOD) ||
!hasDefaultMethod) {
+ return false;
+ }
+ if (classAction != null || classActions != null) {
+ return false;
+ }
+ if (!alwaysMapExecute && !actionAnnotationsByMethod.isEmpty()) {
+ return false;
+ }
+ for (List<Action> actions : actionAnnotationsByMethod.values()) {
+ for (Action action : actions) {
+ if (action.value().equals(Action.DEFAULT_VALUE)) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
/**
* Determines the namespace(s) for the action based on the action class.
If there is a {@link Namespace}
* annotation on the class (including parent classes) or on the package
that the class is in, than
diff --git
a/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java
b/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java
index 975aaea35..fce42e617 100644
---
a/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java
+++
b/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java
@@ -30,6 +30,7 @@ import org.apache.struts2.FileManager;
import org.apache.struts2.FileManagerFactory;
import org.apache.struts2.ObjectFactory;
import org.apache.struts2.config.Configuration;
+import org.apache.struts2.config.ConfigurationException;
import org.apache.struts2.config.entities.ActionConfig;
import org.apache.struts2.config.entities.ExceptionMappingConfig;
import org.apache.struts2.config.entities.InterceptorConfig;
@@ -249,6 +250,103 @@ public class PackageBasedActionConfigBuilderTest extends
TestCase {
assertFalse("ClassNotFoundException should be caught and class should
be excluded", result);
}
+ /**
+ * Tests that duplicate @Action name detection works when execute() is
annotated with @Action.
+ * Before the fix for WW-4421, the duplicate check was inside a
conditional block that was
+ * skipped when execute() had an @Action annotation, allowing duplicate
action names silently.
+ *
+ * @see <a href="https://issues.apache.org/jira/browse/WW-4421";>WW-4421</a>
+ */
+ public void testDuplicateActionNameWithAnnotatedExecute() {
+ ResultTypeConfig defaultResult = new
ResultTypeConfig.Builder("dispatcher",
+
ServletDispatcherResult.class.getName()).defaultResultParam("location").build();
+ PackageConfig strutsDefault = makePackageConfig("struts-default",
null, null, "dispatcher",
+ new ResultTypeConfig[]{defaultResult}, null, null, null, true);
+
+ final DummyContainer mockContainer = new DummyContainer();
+ Configuration configuration = new DefaultConfiguration() {
+ @Override
+ public Container getContainer() {
+ return mockContainer;
+ }
+ };
+ configuration.addPackageConfig("struts-default", strutsDefault);
+
+ ActionNameBuilder actionNameBuilder = new SEOActionNameBuilder("true",
"-");
+ ObjectFactory of = new ObjectFactory();
+ of.setContainer(mockContainer);
+
+ mockContainer.setActionNameBuilder(actionNameBuilder);
+ mockContainer.setConventionsService(new ConventionsServiceImpl(""));
+
+ PackageBasedActionConfigBuilder builder = new
PackageBasedActionConfigBuilder(
+ configuration, mockContainer, of, "false", "struts-default",
"false");
+
builder.setActionPackages("org.apache.struts2.convention.duplicate.annotatedexecute");
+ builder.setActionSuffix("Action");
+
+ DefaultFileManagerFactory fileManagerFactory = new
DefaultFileManagerFactory();
+
fileManagerFactory.setContainer(ActionContext.getContext().getContainer());
+ fileManagerFactory.setFileManager(new DefaultFileManager());
+ builder.setFileManagerFactory(fileManagerFactory);
+ builder.setProviderAllowlist(new ProviderAllowlist());
+
+ try {
+ builder.buildActionConfigs();
+ fail("Expected ConfigurationException for duplicate action names");
+ } catch (ConfigurationException e) {
+ assertTrue("Exception message should mention duplicate action
names",
+ e.getMessage().contains("two methods with an action name
annotation whose value is the same"));
+ }
+ }
+
+ /**
+ * Tests that duplicate @Action name detection works when execute() is NOT
annotated with @Action.
+ * This is a regression guard for WW-4421 — this case was already detected
before the fix.
+ *
+ * @see <a href="https://issues.apache.org/jira/browse/WW-4421";>WW-4421</a>
+ */
+ public void testDuplicateActionNameWithoutAnnotatedExecute() throws
Exception {
+ ResultTypeConfig defaultResult = new
ResultTypeConfig.Builder("dispatcher",
+
ServletDispatcherResult.class.getName()).defaultResultParam("location").build();
+ PackageConfig strutsDefault = makePackageConfig("struts-default",
null, null, "dispatcher",
+ new ResultTypeConfig[]{defaultResult}, null, null, null, true);
+
+ final DummyContainer mockContainer = new DummyContainer();
+ Configuration configuration = new DefaultConfiguration() {
+ @Override
+ public Container getContainer() {
+ return mockContainer;
+ }
+ };
+ configuration.addPackageConfig("struts-default", strutsDefault);
+
+ ActionNameBuilder actionNameBuilder = new SEOActionNameBuilder("true",
"-");
+ ObjectFactory of = new ObjectFactory();
+ of.setContainer(mockContainer);
+
+ mockContainer.setActionNameBuilder(actionNameBuilder);
+ mockContainer.setConventionsService(new ConventionsServiceImpl(""));
+
+ PackageBasedActionConfigBuilder builder = new
PackageBasedActionConfigBuilder(
+ configuration, mockContainer, of, "false", "struts-default",
"false");
+
builder.setActionPackages("org.apache.struts2.convention.duplicate.unannotatedexecute");
+ builder.setActionSuffix("Action");
+
+ DefaultFileManagerFactory fileManagerFactory = new
DefaultFileManagerFactory();
+
fileManagerFactory.setContainer(ActionContext.getContext().getContainer());
+ fileManagerFactory.setFileManager(new DefaultFileManager());
+ builder.setFileManagerFactory(fileManagerFactory);
+ builder.setProviderAllowlist(new ProviderAllowlist());
+
+ try {
+ builder.buildActionConfigs();
+ fail("Expected ConfigurationException for duplicate action names");
+ } catch (ConfigurationException e) {
+ assertTrue("Exception message should mention duplicate action
names",
+ e.getMessage().contains("two methods with an action name
annotation whose value is the same"));
+ }
+ }
+
public void testActionPackages() throws MalformedURLException {
run("org.apache.struts2.convention.actions", null, null);
}
@@ -544,7 +642,7 @@ public class PackageBasedActionConfigBuilderTest extends
TestCase {
expect(resultMapBuilder.build(GlobalResultAction.class, null,
"global-result", globalResultPkg)).andReturn(results);
expect(resultMapBuilder.build(GlobalResultOverrideAction.class, null,
"global-result-override", globalResultPkg)).andReturn(results);
expect(resultMapBuilder.build(ActionLevelResultsNamesAction.class,
getAnnotation(ActionLevelResultsNamesAction.class, "execute", Action.class),
"action-level-results-names", resultPkg)).andReturn(results);
- expect(resultMapBuilder.build(ActionLevelResultsNamesAction.class,
getAnnotation(ActionLevelResultsNamesAction.class, "noname", Action.class),
"action-level-results-names", resultPkg)).andReturn(results);
+ expect(resultMapBuilder.build(ActionLevelResultsNamesAction.class,
getAnnotation(ActionLevelResultsNamesAction.class, "noname", Action.class),
"action-level-results-names-noname", resultPkg)).andReturn(results);
/* org.apache.struts2.convention.actions.resultpath */
expect(resultMapBuilder.build(ClassLevelResultPathAction.class, null,
"class-level-result-path", resultPathPkg)).andReturn(results);
@@ -854,11 +952,12 @@ public class PackageBasedActionConfigBuilderTest extends
TestCase {
pkgConfig =
configuration.getPackageConfig("org.apache.struts2.convention.actions.result#struts-default#/result");
assertNotNull(pkgConfig);
checkSmiValue(pkgConfig, strutsDefault, isSmiInheritanceEnabled);
- assertEquals(7, pkgConfig.getActionConfigs().size());
+ assertEquals(8, pkgConfig.getActionConfigs().size());
verifyActionConfig(pkgConfig, "class-level-result",
ClassLevelResultAction.class, "execute", pkgConfig.getName());
verifyActionConfig(pkgConfig, "class-level-results",
ClassLevelResultsAction.class, "execute", pkgConfig.getName());
verifyActionConfig(pkgConfig, "action-level-result",
ActionLevelResultAction.class, "execute", pkgConfig.getName());
verifyActionConfig(pkgConfig, "action-level-results",
ActionLevelResultsAction.class, "execute", pkgConfig.getName());
+ verifyActionConfig(pkgConfig, "action-level-results-names-noname",
ActionLevelResultsNamesAction.class, "noname", pkgConfig.getName());
verifyActionConfig(pkgConfig, "inherited-result-extends",
InheritedResultExtends.class, "execute", pkgConfig.getName());
/* org.apache.struts2.convention.actions.resultpath */
diff --git
a/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/ActionLevelResultsNamesAction.java
b/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/ActionLevelResultsNamesAction.java
index 2136f8d0a..8a8aee64d 100644
---
a/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/ActionLevelResultsNamesAction.java
+++
b/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/ActionLevelResultsNamesAction.java
@@ -28,16 +28,16 @@ import org.apache.struts2.convention.annotation.Result;
*/
public class ActionLevelResultsNamesAction {
@Action(results = {
- @Result(name={"error", "input"}, location="error.jsp"),
- @Result(name="success",
location="/WEB-INF/location/namespace/action-success.jsp"),
- @Result(name="failure",
location="/WEB-INF/location/namespace/action-failure.jsp")
+ @Result(name = {"error", "input"}, location = "error.jsp"),
+ @Result(name = "success", location =
"/WEB-INF/location/namespace/action-success.jsp"),
+ @Result(name = "failure", location =
"/WEB-INF/location/namespace/action-failure.jsp")
})
public String execute() {
return null;
}
- @Action(results = {
- @Result(location="/WEB-INF/location/namespace/action-success.jsp")
+ @Action(value = "action-level-results-names-noname", results = {
+ @Result(location =
"/WEB-INF/location/namespace/action-success.jsp")
})
public String noname() {
return null;
diff --git
a/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/ActionLevelResultsNamesAction.java
b/plugins/convention/src/test/java/org/apache/struts2/convention/duplicate/annotatedexecute/DuplicateActionNameWithExecuteAnnotationAction.java
similarity index 57%
copy from
plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/ActionLevelResultsNamesAction.java
copy to
plugins/convention/src/test/java/org/apache/struts2/convention/duplicate/annotatedexecute/DuplicateActionNameWithExecuteAnnotationAction.java
index 2136f8d0a..89b3256d6 100644
---
a/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/ActionLevelResultsNamesAction.java
+++
b/plugins/convention/src/test/java/org/apache/struts2/convention/duplicate/annotatedexecute/DuplicateActionNameWithExecuteAnnotationAction.java
@@ -16,30 +16,23 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.struts2.convention.actions.result;
+package org.apache.struts2.convention.duplicate.annotatedexecute;
import org.apache.struts2.convention.annotation.Action;
-import org.apache.struts2.convention.annotation.Result;
/**
- * <p>
- * This is a test action with multiple results names.
- * </p>
+ * Test action with duplicate @Action names where execute() is annotated.
+ * This is the case that was previously not detected (WW-4421).
*/
-public class ActionLevelResultsNamesAction {
- @Action(results = {
- @Result(name={"error", "input"}, location="error.jsp"),
- @Result(name="success",
location="/WEB-INF/location/namespace/action-success.jsp"),
- @Result(name="failure",
location="/WEB-INF/location/namespace/action-failure.jsp")
- })
+public class DuplicateActionNameWithExecuteAnnotationAction {
+
+ @Action("duplicate")
public String execute() {
- return null;
+ return "success";
}
- @Action(results = {
- @Result(location="/WEB-INF/location/namespace/action-success.jsp")
- })
- public String noname() {
- return null;
+ @Action("duplicate")
+ public String anotherMethod() {
+ return "success";
}
}
diff --git
a/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/ActionLevelResultsNamesAction.java
b/plugins/convention/src/test/java/org/apache/struts2/convention/duplicate/unannotatedexecute/DuplicateActionNameWithoutExecuteAnnotationAction.java
similarity index 57%
copy from
plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/ActionLevelResultsNamesAction.java
copy to
plugins/convention/src/test/java/org/apache/struts2/convention/duplicate/unannotatedexecute/DuplicateActionNameWithoutExecuteAnnotationAction.java
index 2136f8d0a..8048fa2ff 100644
---
a/plugins/convention/src/test/java/org/apache/struts2/convention/actions/result/ActionLevelResultsNamesAction.java
+++
b/plugins/convention/src/test/java/org/apache/struts2/convention/duplicate/unannotatedexecute/DuplicateActionNameWithoutExecuteAnnotationAction.java
@@ -16,30 +16,27 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.struts2.convention.actions.result;
+package org.apache.struts2.convention.duplicate.unannotatedexecute;
import org.apache.struts2.convention.annotation.Action;
-import org.apache.struts2.convention.annotation.Result;
/**
- * <p>
- * This is a test action with multiple results names.
- * </p>
+ * Test action with duplicate @Action names where execute() is NOT annotated.
+ * This is the case that was already detected before the fix (regression guard
for WW-4421).
*/
-public class ActionLevelResultsNamesAction {
- @Action(results = {
- @Result(name={"error", "input"}, location="error.jsp"),
- @Result(name="success",
location="/WEB-INF/location/namespace/action-success.jsp"),
- @Result(name="failure",
location="/WEB-INF/location/namespace/action-failure.jsp")
- })
+public class DuplicateActionNameWithoutExecuteAnnotationAction {
+
public String execute() {
- return null;
+ return "success";
+ }
+
+ @Action("duplicate")
+ public String method1() {
+ return "success";
}
- @Action(results = {
- @Result(location="/WEB-INF/location/namespace/action-success.jsp")
- })
- public String noname() {
- return null;
+ @Action("duplicate")
+ public String method2() {
+ return "success";
}
}
diff --git
a/thoughts/shared/research/2026-02-15-WW-4421-duplicate-action-annotation-check-skipped.md
b/thoughts/shared/research/2026-02-15-WW-4421-duplicate-action-annotation-check-skipped.md
new file mode 100644
index 000000000..b3f6d51eb
--- /dev/null
+++
b/thoughts/shared/research/2026-02-15-WW-4421-duplicate-action-annotation-check-skipped.md
@@ -0,0 +1,141 @@
+---
+date: 2026-02-15T12:00:00+01:00
+topic: "WW-4421: Duplicate @Action value annotation check skipped"
+tags: [research, codebase, convention-plugin, annotations, duplicate-detection]
+status: complete
+git_commit: fd874258631e999f5bd5ffc3fea8c0e416c61962
+---
+
+# Research: WW-4421 - Duplicate @Action value annotation check skipped
+
+**Date**: 2026-02-15
+**JIRA**: [WW-4421](https://issues.apache.org/jira/browse/WW-4421)
+
+## Research Question
+Investigate the core issue behind duplicate @Action annotation detection being
bypassed in the Convention plugin.
+
+## Summary
+
+The duplicate @Action name detection logic in
`PackageBasedActionConfigBuilder.buildConfiguration` is **guarded by a
conditional that excludes the most common case**: when the `execute()` method
is annotated with `@Action`. The duplicate check only runs when
`!map.containsKey(DEFAULT_METHOD)` — meaning if the class overrides `execute()`
and decorates it with `@Action`, the entire duplicate detection block is
skipped.
+
+Additionally, the code uses `Class.getMethods()` and `HashMap` — both with
**non-deterministic ordering** — making the behavior inconsistent across JVM
versions.
+
+## Detailed Findings
+
+### The Bug: Conditional Guard Excludes Common Case
+
+**File**:
[`PackageBasedActionConfigBuilder.java:744-747`](https://github.com/apache/struts/blob/fd874258631e999f5bd5ffc3fea8c0e416c61962/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java#L744-L747)
+
+```java
+if (!map.containsKey(DEFAULT_METHOD) // <-- THE BUG
+ && hasDefaultMethod
+ && actionAnnotation == null && actionsAnnotation == null
+ && (alwaysMapExecute || map.isEmpty())) {
+ // ... duplicate detection code is INSIDE this block ...
+ Set<String> actionNames = new HashSet<>();
+ for (List<Action> actions : map.values()) {
+ for (Action action : actions) {
+ String actionName = action.value().equals(Action.DEFAULT_VALUE) ?
defaultActionName : action.value();
+ if (actionNames.contains(actionName)) {
+ throw new ConfigurationException("The action class [" +
actionClass +
+ "] contains two methods with an action name annotation
whose value " +
+ "is the same (they both might be empty as well).");
+ } else {
+ actionNames.add(actionName);
+ }
+ }
+ }
+}
+```
+
+The condition `!map.containsKey(DEFAULT_METHOD)` means:
+- **If `execute()` has an `@Action` annotation** → `map` contains key
`"execute"` → condition is `false` → **duplicate check is SKIPPED entirely**
+- **If `execute()` does NOT have an `@Action` annotation** → condition can be
`true` → duplicate check runs
+
+This is backwards from what you'd expect. The most common pattern — overriding
`execute()` with `@Action` — is exactly the case that bypasses validation.
+
+### Scenario: When Duplicate Detection Fails
+
+```java
+// This class will NOT trigger the duplicate detection error:
+public class MethodCheckSkippedOnStartup extends ActionSupport {
+ @Action("myAction") // <-- annotated execute() puts "execute" in
the map
+ public String execute() {
+ return SUCCESS;
+ }
+
+ @Action("myAction") // <-- duplicate name, but no error thrown!
+ public String anotherMethod() {
+ return SUCCESS;
+ }
+}
+```
+
+```java
+// This class WILL trigger the duplicate detection error:
+public class MethodCheckFailsOnStartup extends ActionSupport {
+ // execute() is NOT annotated — not in the map
+ public String execute() {
+ return SUCCESS;
+ }
+
+ @Action("myAction")
+ public String method1() {
+ return SUCCESS;
+ }
+
+ @Action("myAction") // <-- duplicate detected, throws
ConfigurationException
+ public String method2() {
+ return SUCCESS;
+ }
+}
+```
+
+### Secondary Issue: Non-Deterministic Method Ordering
+
+**File**:
[`PackageBasedActionConfigBuilder.java:943-944`](https://github.com/apache/struts/blob/fd874258631e999f5bd5ffc3fea8c0e416c61962/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java#L943-L944)
+
+```java
+Method[] methods = actionClass.getMethods(); // no guaranteed order
+Map<String, List<Action>> map = new HashMap<>(); // no guaranteed iteration
order
+```
+
+- `Class.getMethods()` returns methods in **no particular order** (JVM spec
does not guarantee ordering)
+- Results are stored in a `HashMap`, which also has **no guaranteed iteration
order**
+- This means the second `@Action` annotation that "wins" (overwrites the
mapping) is non-deterministic
+- Java 7 and Java 8 had different internal `HashMap` implementations, causing
different behavior across versions
+
+### The Duplicate Check Should Be Independent
+
+The duplicate detection logic (lines 748-760) is embedded inside a block that
also handles default `execute()` method mapping. These are **two separate
concerns**:
+
+1. **Should we auto-create an action config for `execute()`?** — This depends
on `!map.containsKey(DEFAULT_METHOD)`
+2. **Are there duplicate action names across annotated methods?** — This
should ALWAYS be checked
+
+The fix should extract the duplicate detection loop so it runs unconditionally
(or at least independently of whether `execute()` is in the annotation map).
+
+## Code References
+
+-
[`PackageBasedActionConfigBuilder.java:87`](https://github.com/apache/struts/blob/fd874258631e999f5bd5ffc3fea8c0e416c61962/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java#L87)
— `DEFAULT_METHOD = "execute"`
+-
[`PackageBasedActionConfigBuilder.java:741-773`](https://github.com/apache/struts/blob/fd874258631e999f5bd5ffc3fea8c0e416c61962/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java#L741-L773)
— The buggy block with embedded duplicate check
+-
[`PackageBasedActionConfigBuilder.java:776-788`](https://github.com/apache/struts/blob/fd874258631e999f5bd5ffc3fea8c0e416c61962/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java#L776-L788)
— Action config creation loop (no duplicate check here)
+-
[`PackageBasedActionConfigBuilder.java:942-959`](https://github.com/apache/struts/blob/fd874258631e999f5bd5ffc3fea8c0e416c61962/plugins/convention/src/main/java/org/apache/struts2/convention/PackageBasedActionConfigBuilder.java#L942-L959)
— `getActionAnnotations()` with non-deterministic ordering
+-
[`ReflectionTools.java:38-45`](https://github.com/apache/struts/blob/fd874258631e999f5bd5ffc3fea8c0e416c61962/plugins/convention/src/main/java/org/apache/struts2/convention/ReflectionTools.java#L38-L45)
— `containsMethod()` helper
+
+## Architecture Insights
+
+1. **Separation of concerns violation**: Duplicate detection is tangled with
default-execute-mapping logic inside the same conditional block
+2. **Non-deterministic reflection**: Using `getMethods()` + `HashMap` means
behavior varies across JVMs
+3. **Silent failure mode**: When the check is skipped, one action config
silently overwrites another — no error, no warning, just undefined behavior at
runtime
+
+## Suggested Fix Direction
+
+1. **Extract duplicate detection** out of the
`!map.containsKey(DEFAULT_METHOD)` conditional so it runs for ALL annotated
action classes
+2. **Also check class-level annotations**
(`actionAnnotation`/`actionsAnnotation`) against method-level annotations for
name conflicts
+3. **Consider using `LinkedHashMap`** or sorting methods for deterministic
processing order
+
+## Open Questions
+
+1. Should duplicate detection also consider action names across different
classes in the same namespace?
+2. Should a warning be issued instead of a `ConfigurationException` to allow
intentional overrides?
+3. Are there backward-compatibility concerns if we start detecting duplicates
that were previously silently ignored?
