This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5539 in repository https://gitbox.apache.org/repos/asf/struts.git
commit ebf341aeeb991035a04507a7bb2ce131076e67ea Author: Lukasz Lenart <[email protected]> AuthorDate: Tue Jul 21 17:27:37 2026 +0200 WW-5539 Pin down addNoMapping's override semantics Storing the no-mapping sentinel deliberately replaces any mapping cached for the class, matching the pre-7.3.0 effective behaviour where such a class was short-circuited before its cached mapping was ever read. putIfAbsent would instead serve a stale mapping after a failed build. Also asserts the sentinel translation in getMapping directly, and stops the interface javadoc promising a specific empty-map instance that implementations are not required to return. --- .../struts2/conversion/StrutsTypeConverterHolder.java | 10 ++++++++++ .../org/apache/struts2/conversion/TypeConverterHolder.java | 3 +-- .../struts2/conversion/StrutsTypeConverterHolderTest.java | 14 ++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/conversion/StrutsTypeConverterHolder.java b/core/src/main/java/org/apache/struts2/conversion/StrutsTypeConverterHolder.java index 4b53b1dda..4a2bed820 100644 --- a/core/src/main/java/org/apache/struts2/conversion/StrutsTypeConverterHolder.java +++ b/core/src/main/java/org/apache/struts2/conversion/StrutsTypeConverterHolder.java @@ -123,6 +123,16 @@ public class StrutsTypeConverterHolder implements TypeConverterHolder { return mappings.get(clazz) == NO_MAPPING; } + /** + * Stores the {@link #NO_MAPPING} sentinel for the given class, replacing any mapping previously + * cached for it. This matches the pre-7.3.0 effective behaviour, back when a separate no-mapping + * collection was consulted independently of the mappings map: the only in-tree caller, + * {@code XWorkConverter.getConverter}, checked the no-mapping flag first and short-circuited + * before the cached mapping was ever read, so flagging a class as no-mapping made it behave as + * though it had no mapping, real cached mapping or not. {@link Map#putIfAbsent} is deliberately + * not used here: it would leave a stale mapping being served for a class whose conversion build + * subsequently failed, which is a genuine behaviour change rather than a faithful port. + */ @Override public void addNoMapping(Class clazz) { mappings.put(clazz, NO_MAPPING); diff --git a/core/src/main/java/org/apache/struts2/conversion/TypeConverterHolder.java b/core/src/main/java/org/apache/struts2/conversion/TypeConverterHolder.java index eb710e408..1adfe8a3c 100644 --- a/core/src/main/java/org/apache/struts2/conversion/TypeConverterHolder.java +++ b/core/src/main/java/org/apache/struts2/conversion/TypeConverterHolder.java @@ -110,8 +110,7 @@ public interface TypeConverterHolder { /** * Returns the property-converter mapping for the given class, building and caching it on first - * use. Never returns {@code null}: a class known to have no mapping yields - * {@link Collections#emptyMap()}. + * use. Never returns {@code null}: a class known to have no mapping yields an empty map. * * <p>If the builder returns {@code null} or an empty map, the class is recorded in the negative * cache so the builder is not invoked for it again.</p> diff --git a/core/src/test/java/org/apache/struts2/conversion/StrutsTypeConverterHolderTest.java b/core/src/test/java/org/apache/struts2/conversion/StrutsTypeConverterHolderTest.java index 34dc18ad0..686ab5f4d 100644 --- a/core/src/test/java/org/apache/struts2/conversion/StrutsTypeConverterHolderTest.java +++ b/core/src/test/java/org/apache/struts2/conversion/StrutsTypeConverterHolderTest.java @@ -172,6 +172,20 @@ public class StrutsTypeConverterHolderTest extends XWorkTestCase { assertThat(second).isEmpty(); assertThat(builds.get()).as("empty result must be negative cached").isEqualTo(1); assertThat(holder.containsNoMapping(String.class)).isTrue(); + assertThat(holder.getMapping(String.class)).isNull(); + } + + public void testAddNoMappingOverridesPreviouslyCachedMapping() { + StrutsTypeConverterHolder holder = new StrutsTypeConverterHolder(); + Map<String, Object> real = new HashMap<>(); + real.put("someProperty", "someConverter"); + holder.addMapping(String.class, real); + + holder.addNoMapping(String.class); + + assertThat(holder.containsNoMapping(String.class)).isTrue(); + assertThat(holder.getMapping(String.class)).isNull(); + assertThat(holder.computeMappingIfAbsent(String.class, clazz -> real)).isEmpty(); } public void testComputeMappingIfAbsentNegativeCachesNullResult() {
