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
commit e5e430a0557b0efefa656620d4ac0ad5859d6e19 Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Nov 2 09:31:33 2026 +0100 WW-4164 refactor(core): use an empty list for undeclared extensions Also drops the redundant @since comment on Component.actionExtensionResolver. Co-Authored-By: Claude Sonnet 5.5 <[email protected]> --- .../org/apache/struts2/components/Component.java | 3 --- .../apache/struts2/config/entities/ActionConfig.java | 14 +++++++------- .../struts2/config/entities/PackageConfig.java | 20 ++++++++++---------- .../struts2/config/impl/DefaultConfiguration.java | 4 ++-- .../providers/XmlDocConfigurationProvider.java | 4 ++-- .../org/apache/struts2/dispatcher/Dispatcher.java | 2 +- .../dispatcher/mapper/ActionExtensionResolver.java | 4 ++-- .../dispatcher/mapper/DefaultActionMapper.java | 4 ++-- .../interceptor/ActionExtensionInterceptor.java | 2 +- .../XmlConfigurationProviderExtensionTest.java | 4 ++-- .../interceptor/ActionExtensionInterceptorTest.java | 2 +- 11 files changed, 30 insertions(+), 33 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/components/Component.java b/core/src/main/java/org/apache/struts2/components/Component.java index e4d9bc5fa..d1e9f7d2c 100644 --- a/core/src/main/java/org/apache/struts2/components/Component.java +++ b/core/src/main/java/org/apache/struts2/components/Component.java @@ -82,9 +82,6 @@ public class Component { protected ValueStack stack; protected Map<String, Object> attributes; protected ActionMapper actionMapper; - /** - * @since 7.5.0 - */ protected ActionExtensionResolver actionExtensionResolver; protected boolean throwExceptionOnELFailure; protected boolean performClearTagStateForTagPoolingServers = false; diff --git a/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java b/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java index cd0df08a5..8e27e5375 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/ActionConfig.java @@ -69,7 +69,7 @@ public class ActionConfig extends Located implements Serializable { protected boolean strictMethodInvocation = true; protected AllowedMethods allowedMethods; private Set<HttpMethod> httpMethods; - private List<String> extensions; + private List<String> extensions = List.of(); protected ActionConfig(String packageName, String name, String className) { this.packageName = packageName; @@ -99,7 +99,7 @@ public class ActionConfig extends Located implements Serializable { this.allowedMethods = orig.allowedMethods; this.httpMethods = EnumSet.noneOf(HttpMethod.class); this.httpMethods.addAll(orig.httpMethods); - this.extensions = orig.extensions != null ? List.copyOf(orig.extensions) : null; + this.extensions = orig.extensions; this.location = orig.location; this.strictMethodInvocation = orig.strictMethodInvocation; } @@ -133,7 +133,7 @@ public class ActionConfig extends Located implements Serializable { } /** - * @return action extensions in declared order, null when neither the action nor its package declares any + * @return action extensions in declared order, empty when neither the action nor its package declares any * @since 7.5.0 */ public List<String> getExtensions() { @@ -214,7 +214,7 @@ public class ActionConfig extends Located implements Serializable { return false; } - return Objects.equals(extensions, actionConfig.extensions); + return extensions.equals(actionConfig.extensions); } @Override @@ -230,7 +230,7 @@ public class ActionConfig extends Located implements Serializable { result = 31 * result + (name != null ? name.hashCode() : 0); result = 31 * result + (allowedMethods != null ? allowedMethods.hashCode() : 0); result = 31 * result + (httpMethods != null ? httpMethods.hashCode() : 0); - result = 31 * result + (extensions != null ? extensions.hashCode() : 0); + result = 31 * result + extensions.hashCode(); return result; } @@ -249,7 +249,7 @@ public class ActionConfig extends Located implements Serializable { if (!httpMethods.isEmpty()) { sb.append(" - ").append(httpMethods); } - if (extensions != null) { + if (!extensions.isEmpty()) { sb.append(" - extensions ").append(extensions); } sb.append("}"); @@ -390,7 +390,7 @@ public class ActionConfig extends Located implements Serializable { * @since 7.5.0 */ public Builder extensions(List<String> extensions) { - target.extensions = extensions != null ? List.copyOf(extensions) : null; + target.extensions = extensions != null ? List.copyOf(extensions) : List.of(); return this; } 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 60a58d481..1d76cec34 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,7 +59,7 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, protected boolean isFinal = false; // a final package is unextendable protected boolean needsRefresh; protected boolean strictMethodInvocation = true; - private List<String> extensions; + private List<String> extensions = List.of(); private Map<String, String> scopedConstants; private Map<String, String> allScopedConstants; @@ -95,7 +95,7 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, this.parents = new ArrayList<>(orig.parents); this.location = orig.location; this.strictMethodInvocation = orig.strictMethodInvocation; - this.extensions = orig.extensions != null ? List.copyOf(orig.extensions) : null; + this.extensions = orig.extensions; this.scopedConstants = new LinkedHashMap<>(orig.scopedConstants); this.allScopedConstants = orig.allScopedConstants; } @@ -324,7 +324,7 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, } /** - * @return extensions declared on this package, or null when it declares none + * @return extensions declared on this package, or empty when it declares none * @since 7.5.0 */ public List<String> getExtensions() { @@ -332,20 +332,20 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, } /** - * @return extensions declared on this package, else on the first parent that declares them, else null + * @return extensions declared on this package, else on the first parent that declares them, else empty * @since 7.5.0 */ public List<String> getFullExtensions() { - if (extensions != null) { + if (!extensions.isEmpty()) { return extensions; } for (PackageConfig parent : parents) { List<String> parentExtensions = parent.getFullExtensions(); - if (parentExtensions != null) { + if (!parentExtensions.isEmpty()) { return parentExtensions; } } - return null; + return List.of(); } /** @@ -449,7 +449,7 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, return false; if (!Objects.equals(defaultClassRef, that.defaultClassRef)) return false; - if (!Objects.equals(extensions, that.extensions)) return false; + if (!extensions.equals(that.extensions)) return false; if (!name.equals(that.name)) return false; return Objects.equals(namespace, that.namespace); } @@ -474,7 +474,7 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, result = 31 * result + (isFinal ? 1 : 0); result = 31 * result + (needsRefresh ? 1 : 0); result = 31 * result + (strictMethodInvocation ? 1 : 0); - result = 31 * result + (extensions != null ? extensions.hashCode() : 0); + result = 31 * result + extensions.hashCode(); return result; } @@ -709,7 +709,7 @@ public class PackageConfig extends Located implements Comparable<PackageConfig>, * @since 7.5.0 */ public Builder extensions(List<String> extensions) { - target.extensions = extensions != null ? List.copyOf(extensions) : null; + target.extensions = extensions != null ? List.copyOf(extensions) : List.of(); return this; } 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 6862a0ac4..a53170105 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 @@ -563,7 +563,7 @@ public class DefaultConfiguration implements Configuration { warnIfHttpMethodNotEnforced(baseConfig, interceptors); - List<String> extensions = baseConfig.getExtensions() != null + List<String> extensions = !baseConfig.getExtensions().isEmpty() ? baseConfig.getExtensions() : packageContext.getFullExtensions(); warnIfExtensionNotEnforced(baseConfig, extensions, interceptors); @@ -596,7 +596,7 @@ public class DefaultConfiguration implements Configuration { } private void warnIfExtensionNotEnforced(ActionConfig baseConfig, List<String> extensions, List<InterceptorMapping> interceptors) { - if (extensions != null + if (!extensions.isEmpty() && interceptors.stream().noneMatch(mapping -> mapping.getInterceptor() instanceof ActionExtensionInterceptor)) { LOG.warn("Action [{}] in package [{}] declares extension {} but its interceptors do not include {}, the restriction will not be enforced", baseConfig.getName(), baseConfig.getPackageName(), extensions, ActionExtensionInterceptor.class.getSimpleName()); 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 90f034f8e..c07dedc8e 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 @@ -537,12 +537,12 @@ public abstract class XmlDocConfigurationProvider implements ConfigurationProvid } /** - * @return parsed extension list, or null when the element has no extension attribute + * @return parsed extension list, or empty when the element has no extension attribute * @since 7.5.0 */ protected List<String> buildExtensions(Element element, Location location) { if (!element.hasAttribute("extension")) { - return null; + return List.of(); } List<String> extensions = ActionExtensions.parse(element.getAttribute("extension")); for (String extension : extensions) { diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index 9ad214785..331648c1c 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -758,7 +758,7 @@ public class Dispatcher { } boolean declared = configurationManager.getConfiguration().getRuntimeConfiguration().getActionConfigs().values().stream() .flatMap(actions -> actions.values().stream()) - .anyMatch(config -> config.getExtensions() != null); + .anyMatch(config -> !config.getExtensions().isEmpty()); if (declared) { LOG.warn("Actions declare the extension attribute but the configured {} is not {}, the declared extensions may not be applied", ActionMapper.class.getSimpleName(), DefaultActionMapper.class.getSimpleName()); diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolver.java b/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolver.java index a223d1d65..6849bdb56 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolver.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionExtensionResolver.java @@ -82,7 +82,7 @@ public class ActionExtensionResolver { if (actionConfig == null) { return; } - List<String> effective = actionConfig.getExtensions() != null ? actionConfig.getExtensions() : extensions; + List<String> effective = !actionConfig.getExtensions().isEmpty() ? actionConfig.getExtensions() : extensions; if (effective == null) { return; } @@ -97,7 +97,7 @@ public class ActionExtensionResolver { } boolean anyDeclared = runtimeConfiguration.getActionConfigs().values().stream() .flatMap(actionConfigs -> actionConfigs.values().stream()) - .anyMatch(actionConfig -> actionConfig.getExtensions() != null); + .anyMatch(actionConfig -> !actionConfig.getExtensions().isEmpty()); declarationState = new DeclarationState(runtimeConfiguration, anyDeclared); return anyDeclared; } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java index 9fc17241f..bb624688b 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapper.java @@ -515,7 +515,7 @@ public class DefaultActionMapper implements ActionMapper { int bang = name.indexOf('!'); String actionName = bang == -1 ? name : name.substring(0, bang); ActionConfig config = runtimeConfiguration.getMatchedActionConfig(probe.getNamespace(), actionName); - return config != null && config.getExtensions() != null; + return config != null && !config.getExtensions().isEmpty(); } private DeclaredExtensions declaredExtensions(RuntimeConfiguration runtimeConfiguration) { @@ -526,7 +526,7 @@ public class DefaultActionMapper implements ActionMapper { Set<String> declared = new HashSet<>(); for (Map<String, ActionConfig> actionConfigs : runtimeConfiguration.getActionConfigs().values()) { for (ActionConfig actionConfig : actionConfigs.values()) { - if (actionConfig.getExtensions() != null) { + if (!actionConfig.getExtensions().isEmpty()) { declared.addAll(actionConfig.getExtensions()); } } diff --git a/core/src/main/java/org/apache/struts2/interceptor/ActionExtensionInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ActionExtensionInterceptor.java index 464ecdc5e..c05e48141 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ActionExtensionInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ActionExtensionInterceptor.java @@ -52,7 +52,7 @@ public class ActionExtensionInterceptor extends AbstractInterceptor { public String intercept(ActionInvocation invocation) throws Exception { ActionProxy proxy = invocation.getProxy(); List<String> allowed = proxy.getConfig().getExtensions(); - if (allowed == null) { + if (allowed.isEmpty()) { return invocation.invoke(); } diff --git a/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderExtensionTest.java b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderExtensionTest.java index b07016fbd..1bfa84dfc 100644 --- a/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderExtensionTest.java +++ b/core/src/test/java/org/apache/struts2/config/providers/XmlConfigurationProviderExtensionTest.java @@ -49,10 +49,10 @@ public class XmlConfigurationProviderExtensionTest extends StrutsInternalTestCas assertThat(actionConfig("/child", "own").getExtensions()).isEqualTo(List.of("json", "")); } - public void testUndeclaredIsNull() { + public void testUndeclaredIsEmpty() { loadConfigurationProviders(new StrutsXmlConfigurationProvider(CONFIG)); - assertThat(actionConfig("/plain", "undeclared").getExtensions()).isNull(); + assertThat(actionConfig("/plain", "undeclared").getExtensions()).isEmpty(); } public void testEmptyAttributeMeansNoExtensionOnly() { diff --git a/core/src/test/java/org/apache/struts2/interceptor/ActionExtensionInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ActionExtensionInterceptorTest.java index 0510e23e8..81e8eb2a5 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ActionExtensionInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ActionExtensionInterceptorTest.java @@ -48,7 +48,7 @@ public class ActionExtensionInterceptorTest extends StrutsInternalTestCase { } public void testUndeclaredPasses() throws Exception { - prepare(config(null), "second", "second", "html"); + prepare(config(List.of()), "second", "second", "html"); assertThat(interceptor.intercept(invocation)).isEqualTo("success"); }
