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 d3a0310c57ea87f069c4c0db7bc5be46d18c716b Author: Lukasz Lenart <[email protected]> AuthorDate: Mon Nov 2 11:04:11 2026 +0100 WW-4164 feat(core): let action mappers declare support for declared extensions Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../org/apache/struts2/dispatcher/Dispatcher.java | 8 +-- .../struts2/dispatcher/mapper/ActionMapper.java | 10 ++++ .../dispatcher/mapper/CompositeActionMapper.java | 5 ++ .../dispatcher/mapper/DefaultActionMapper.java | 5 ++ .../dispatcher/mapper/PrefixBasedActionMapper.java | 5 ++ .../apache/struts2/dispatcher/DispatcherTest.java | 20 ++++++- .../mapper/CompositeActionMapperTest.java | 22 ++++++++ .../dispatcher/mapper/DefaultActionMapperTest.java | 4 ++ .../mapper/PrefixBasedActionMapperTest.java | 61 ++++++++++++++++++++++ .../mapper/Restful2ActionMapperTest.java | 4 ++ .../dispatcher/mapper/RestfulActionMapperTest.java | 4 ++ ...6-11-01-WW-4164-per-action-extensions-design.md | 13 +++-- .../org/apache/struts2/rest/RestActionMapper.java | 5 ++ .../apache/struts2/rest/RestActionMapperTest.java | 4 ++ 14 files changed, 161 insertions(+), 9 deletions(-) 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 cee6fa5c5..4d497b0f5 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -59,7 +59,6 @@ import org.apache.struts2.config.entities.PackageConfig; import org.apache.struts2.config.providers.XmlConfigurationProvider; import org.apache.struts2.dispatcher.mapper.ActionMapper; import org.apache.struts2.dispatcher.mapper.ActionMapping; -import org.apache.struts2.dispatcher.mapper.DefaultActionMapper; import org.apache.struts2.dispatcher.multipart.MultiPartRequest; import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; import org.apache.struts2.inject.Container; @@ -753,15 +752,16 @@ public class Dispatcher { } private void warnIfExtensionsIgnoredByMapper(Container container) { - if (container.getInstance(ActionMapper.class).getClass() == DefaultActionMapper.class) { + ActionMapper mapper = container.getInstance(ActionMapper.class); + if (mapper.supportsDeclaredExtensions()) { return; } boolean declared = container.getInstance(Configuration.class).getRuntimeConfiguration().getActionConfigs().values().stream() .flatMap(actions -> actions.values().stream()) .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()); + LOG.warn("Actions declare the extension attribute but the configured {} [{}] does not support declared extensions, they are not applied", + ActionMapper.class.getSimpleName(), mapper.getClass().getName()); } } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionMapper.java b/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionMapper.java index cdd1760cf..c2163f15a 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionMapper.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/ActionMapper.java @@ -74,4 +74,14 @@ public interface ActionMapper { * @return The URI string that represents this mapping */ String getUriFromActionMapping(ActionMapping mapping); + + /** + * Whether this mapper honours the extension lists declared on packages and actions + * + * @return true if declared extensions are applied when mapping a request + * @since 7.5.0 + */ + default boolean supportsDeclaredExtensions() { + return false; + } } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/CompositeActionMapper.java b/core/src/main/java/org/apache/struts2/dispatcher/mapper/CompositeActionMapper.java index f1e59eab9..690f61eb7 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/CompositeActionMapper.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/CompositeActionMapper.java @@ -108,4 +108,9 @@ public class CompositeActionMapper implements ActionMapper { LOG.debug("exhausted from ActionMapper that could return an ActionMapping"); return null; } + + @Override + public boolean supportsDeclaredExtensions() { + return !actionMappers.isEmpty() && actionMappers.stream().allMatch(ActionMapper::supportsDeclaredExtensions); + } } 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 c39bd8be4..2d46913eb 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 @@ -264,6 +264,11 @@ public class DefaultActionMapper implements ActionMapper { return parseActionName(mapping); } + @Override + public boolean supportsDeclaredExtensions() { + return true; + } + public boolean isSlashesInActionNames() { return allowSlashesInActionNames; } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/mapper/PrefixBasedActionMapper.java b/core/src/main/java/org/apache/struts2/dispatcher/mapper/PrefixBasedActionMapper.java index a7126e80a..319af48d9 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/mapper/PrefixBasedActionMapper.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/mapper/PrefixBasedActionMapper.java @@ -144,4 +144,9 @@ public class PrefixBasedActionMapper extends DefaultActionMapper implements Acti return null; } + @Override + public boolean supportsDeclaredExtensions() { + return !actionMappers.isEmpty() && actionMappers.values().stream().allMatch(ActionMapper::supportsDeclaredExtensions); + } + } diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java index 35debebc6..37ca8f574 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java @@ -45,6 +45,8 @@ import org.apache.struts2.StrutsConstants; import org.apache.struts2.test.LogCapture; import org.apache.struts2.StrutsJUnit4InternalTestCase; import org.apache.struts2.dispatcher.mapper.ActionMapping; +import org.apache.struts2.dispatcher.mapper.DefaultActionMapper; +import org.apache.struts2.dispatcher.mapper.RestfulActionMapper; import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; import org.apache.struts2.util.ObjectFactoryDestroyable; import org.junit.Test; @@ -643,7 +645,7 @@ public class DispatcherTest extends StrutsJUnit4InternalTestCase { initDispatcher(params); assertThat(capture.messages(Level.WARN)).singleElement().asString() - .contains("declared extensions may not be applied"); + .contains("[" + RestfulActionMapper.class.getName() + "] does not support declared extensions"); } } @@ -659,6 +661,19 @@ public class DispatcherTest extends StrutsJUnit4InternalTestCase { } } + @Test + public void testNoMapperWarningWithDefaultMapperSubclass() { + Map<String, String> params = new HashMap<>(); + params.put("config", "struts-default.xml,struts-plugin.xml,org/apache/struts2/config/providers/struts-extension.xml"); + params.put(StrutsConstants.STRUTS_MAPPER_CLASS, ExtendedDefaultActionMapper.class.getName()); + + try (LogCapture capture = new LogCapture(Dispatcher.class)) { + initDispatcher(params); + + assertThat(capture.messages(Level.WARN)).noneMatch(message -> message.contains("declared extensions")); + } + } + @Test public void testGetLocale_With_BadDefaultLocale_WarnsWithResolvedLocaleNotRequestLocale() { Map<String, String> params = new HashMap<>(); @@ -778,6 +793,9 @@ public class DispatcherTest extends StrutsJUnit4InternalTestCase { public boolean isDestroyed = false; } + public static class ExtendedDefaultActionMapper extends DefaultActionMapper { + } + public static class InnerDestroyableObjectFactory extends ObjectFactory implements ObjectFactoryDestroyable { public boolean destroyed = false; diff --git a/core/src/test/java/org/apache/struts2/dispatcher/mapper/CompositeActionMapperTest.java b/core/src/test/java/org/apache/struts2/dispatcher/mapper/CompositeActionMapperTest.java index 462bb87cf..52be10c99 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/mapper/CompositeActionMapperTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/mapper/CompositeActionMapperTest.java @@ -74,6 +74,28 @@ public class CompositeActionMapperTest extends TestCase { assertNull(uri); } + public void testSupportsDeclaredExtensionsWhenAllMappersSupportThem() { + mockContainer.expectAndReturn("getInstance", C.args(C.eq(ActionMapper.class), C.eq("mapper1")), new DefaultActionMapper()); + mockContainer.expectAndReturn("getInstance", C.args(C.eq(ActionMapper.class), C.eq("mapper2")), new Restful2ActionMapper()); + CompositeActionMapper compositeActionMapper = new CompositeActionMapper((Container) mockContainer.proxy(), "mapper1,mapper2"); + + assertTrue(compositeActionMapper.supportsDeclaredExtensions()); + } + + public void testDoesNotSupportDeclaredExtensionsWhenOneMapperDoesNot() { + mockContainer.expectAndReturn("getInstance", C.args(C.eq(ActionMapper.class), C.eq("mapper1")), new DefaultActionMapper()); + mockContainer.expectAndReturn("getInstance", C.args(C.eq(ActionMapper.class), C.eq("mapper2")), new RestfulActionMapper()); + CompositeActionMapper compositeActionMapper = new CompositeActionMapper((Container) mockContainer.proxy(), "mapper1,mapper2"); + + assertFalse(compositeActionMapper.supportsDeclaredExtensions()); + } + + public void testDoesNotSupportDeclaredExtensionsWithoutMappers() { + CompositeActionMapper compositeActionMapper = new CompositeActionMapper((Container) mockContainer.proxy(), ""); + + assertFalse(compositeActionMapper.supportsDeclaredExtensions()); + } + public static class InnerActionMapper1 implements ActionMapper { public static ActionMapping actionMapping = new ActionMapping(); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java b/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java index cee45edc0..10aa6ad2f 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/mapper/DefaultActionMapperTest.java @@ -75,6 +75,10 @@ public class DefaultActionMapperTest extends StrutsInternalTestCase { }; } + public void testSupportsDeclaredExtensions() { + assertTrue(new DefaultActionMapper().supportsDeclaredExtensions()); + } + public void testGetMapping() { req.setRequestURI("/my/namespace/actionName.action"); req.setServletPath("/my/namespace/actionName.action"); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/mapper/PrefixBasedActionMapperTest.java b/core/src/test/java/org/apache/struts2/dispatcher/mapper/PrefixBasedActionMapperTest.java new file mode 100644 index 000000000..2b3ad47a8 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/dispatcher/mapper/PrefixBasedActionMapperTest.java @@ -0,0 +1,61 @@ +/* + * 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.dispatcher.mapper; + +import junit.framework.TestCase; +import org.apache.struts2.inject.Container; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class PrefixBasedActionMapperTest extends TestCase { + + private Container container; + private PrefixBasedActionMapper mapper; + + @Override + protected void setUp() throws Exception { + super.setUp(); + container = mock(Container.class); + mapper = new PrefixBasedActionMapper(); + mapper.setContainer(container); + } + + public void testSupportsDeclaredExtensionsWhenAllMappersSupportThem() { + when(container.getInstance(ActionMapper.class, "struts")).thenReturn(new DefaultActionMapper()); + when(container.getInstance(ActionMapper.class, "restful2")).thenReturn(new Restful2ActionMapper()); + mapper.setPrefixBasedActionMappers("/app:struts,/api:restful2"); + + assertTrue(mapper.supportsDeclaredExtensions()); + } + + public void testDoesNotSupportDeclaredExtensionsWhenOneMapperDoesNot() { + when(container.getInstance(ActionMapper.class, "struts")).thenReturn(new DefaultActionMapper()); + when(container.getInstance(ActionMapper.class, "restful")).thenReturn(new RestfulActionMapper()); + mapper.setPrefixBasedActionMappers("/app:struts,/api:restful"); + + assertFalse(mapper.supportsDeclaredExtensions()); + } + + public void testDoesNotSupportDeclaredExtensionsWithoutMappers() { + mapper.setPrefixBasedActionMappers(""); + + assertFalse(mapper.supportsDeclaredExtensions()); + } +} diff --git a/core/src/test/java/org/apache/struts2/dispatcher/mapper/Restful2ActionMapperTest.java b/core/src/test/java/org/apache/struts2/dispatcher/mapper/Restful2ActionMapperTest.java index b037922e0..5ab473f49 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/mapper/Restful2ActionMapperTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/mapper/Restful2ActionMapperTest.java @@ -61,6 +61,10 @@ public class Restful2ActionMapperTest extends StrutsInternalTestCase { }; } + public void testSupportsDeclaredExtensions() { + assertTrue(mapper.supportsDeclaredExtensions()); + } + public void testGetIndex() throws Exception { req.setRequestURI("/my/namespace/foo/"); req.setServletPath("/my/namespace/foo/"); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapperTest.java b/core/src/test/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapperTest.java index aa1652f24..0678f38b8 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapperTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/mapper/RestfulActionMapperTest.java @@ -35,6 +35,10 @@ public class RestfulActionMapperTest extends StrutsInternalTestCase { private RestfulActionMapper mapper; + public void testDoesNotSupportDeclaredExtensions() { + assertFalse(mapper.supportsDeclaredExtensions()); + } + public void testGetUri() { ActionMapping am = new ActionMapping(); am.setName("view"); diff --git a/docs/superpowers/specs/2026-11-01-WW-4164-per-action-extensions-design.md b/docs/superpowers/specs/2026-11-01-WW-4164-per-action-extensions-design.md index 0724d50c2..fb9bc5c66 100644 --- a/docs/superpowers/specs/2026-11-01-WW-4164-per-action-extensions-design.md +++ b/docs/superpowers/specs/2026-11-01-WW-4164-per-action-extensions-design.md @@ -37,7 +37,8 @@ govern both: - Convention plugin / annotation support (an `@Action`-level or class-level extension). Follow-up ticket, to be filed once this lands. - `RestActionMapper`, `Restful*ActionMapper`, `PrefixBasedActionMapper`, `CompositeActionMapper`: - no changes. They keep using the global list. + no mapping changes. They keep using the global list, except where they reach + `DefaultActionMapper.getMapping` (`Restful2ActionMapper` and other subclasses calling `super`). - Redirecting a wrong-extension request to the canonical URL. - Resolving ambiguous names by trying several strip candidates (e.g. an action literally named `report.json` in a package whose list is empty-only). The mapper strips in one fixed order; the @@ -118,8 +119,12 @@ govern both: - **Other mappers.** `RestActionMapper` extends `DefaultActionMapper` but overrides the mapping path, and the Restful/Prefix/Composite mappers have their own logic. Declaring `extension` under them is the application's responsibility: a doc note, and a startup WARN when an action - declares an extension list while the configured `ActionMapper` is not exactly - `DefaultActionMapper`. No marker on `ActionMapping`. + declares an extension list while the configured `ActionMapper` returns false from + `ActionMapper.supportsDeclaredExtensions()`. The interface default is false; + `DefaultActionMapper` (and so its subclasses, including `Restful2ActionMapper`) returns true, + `RestActionMapper` overrides back to false, and `CompositeActionMapper` / + `PrefixBasedActionMapper` return true only when they have delegates and every delegate does. + No marker on `ActionMapping`. - **Rejection**: return a configurable result name (placeholder `bad-extension`, property like `HttpMethodInterceptor.badRequestResultName`) when the action can resolve it (action result, package global result, `*`, or an unknown handler — same lookup as `HttpMethodInterceptor`). @@ -172,7 +177,7 @@ govern both: today (keeps static `.html`/JSP serving). - Enforcement only on the URL's own action. Chain and `<s:action>` are not enforced; `action:` buttons are, against the form URL's extension. -- Non-default mappers: WARN + doc note, no API marker. +- Non-supporting mappers: WARN + doc note, driven by `ActionMapper.supportsDeclaredExtensions()`; no marker on `ActionMapping`. ## Regression tests owed by the anti-goal diff --git a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java index a5a8e832e..b8ecdaf85 100644 --- a/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java +++ b/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java @@ -185,6 +185,11 @@ public class RestActionMapper extends DefaultActionMapper { this.allowDynamicMethodCalls = "true".equalsIgnoreCase(allowDynamicMethodCalls); } + @Override + public boolean supportsDeclaredExtensions() { + return false; + } + public ActionMapping getMapping(HttpServletRequest request, ConfigurationManager configManager) { ActionMapping mapping = new ActionMapping(); diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java index 88c0de5d3..9c8c2c6cb 100644 --- a/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java +++ b/plugins/rest/src/test/java/org/apache/struts2/rest/RestActionMapperTest.java @@ -55,6 +55,10 @@ public class RestActionMapperTest extends TestCase { }; } + public void testDoesNotSupportDeclaredExtensions() { + assertFalse(mapper.supportsDeclaredExtensions()); + } + public void testRootMapping() throws Exception { req.setRequestURI("/myapp/"); req.setServletPath("/");
