You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2026/07/21 18:56:01 UTC
(struts) 14/14: WW-5539 Address final review findings
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 4de29a145f703fdde2bb81e867ce29af41142e5d
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Tue Jul 21 20:45:38 2026 +0200
WW-5539 Address final review findings
Restores the protected unknownMappings field verbatim as a deprecated,
unused vestige: retyping it changed the field descriptor, so a subclass
compiled against 7.2.0 would have hit NoSuchFieldError on upgrade without
recompiling. Real storage moves to a private concurrent set.
Also stops conditionalReload running for negative-cached classes, which
had been costing a failed classloader resource scan per property per
request in devMode, and restores the unknown-mapping clearing that the
null-converter guard was skipping.
---
.../struts2/conversion/StrutsTypeConverterHolder.java | 19 ++++++++++++++-----
.../struts2/conversion/TypeConverterHolder.java | 4 ++++
.../struts2/conversion/impl/XWorkConverter.java | 4 +++-
.../validator/DefaultActionValidatorManager.java | 2 +-
4 files changed, 22 insertions(+), 7 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 4a2bed820..e6ec37b47 100644
--- a/core/src/main/java/org/apache/struts2/conversion/StrutsTypeConverterHolder.java
+++ b/core/src/main/java/org/apache/struts2/conversion/StrutsTypeConverterHolder.java
@@ -23,6 +23,7 @@ import org.apache.logging.log4j.Logger;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
@@ -79,19 +80,27 @@ public class StrutsTypeConverterHolder implements TypeConverterHolder {
* - String -> classname as String
* </pre>
*
- * @deprecated since 7.3.0, this field is an implementation detail and will be made private.
+ * @deprecated since 7.3.0, unused - superseded by internal concurrent storage. Retained only
+ * for binary compatibility with subclasses compiled against earlier versions, and will be
+ * removed in a future release.
*/
@Deprecated
- protected final Set<String> unknownMappings = ConcurrentHashMap.newKeySet(); // non-action (eg. returned value)
+ protected HashSet<String> unknownMappings = new HashSet<>();
+
+ /**
+ * Actual storage for classes with no registered converter. Concurrent, so that lock-free
+ * readers in {@code XWorkConverter.lookup} cannot race writers.
+ */
+ private final Set<String> unknownMappingsInternal = ConcurrentHashMap.newKeySet();
@Override
public void addDefaultMapping(String className, TypeConverter typeConverter) {
+ unknownMappingsInternal.remove(className);
if (typeConverter == null) {
LOG.warn("Ignoring null TypeConverter registered for class [{}]", className);
return;
}
defaultMappings.put(className, typeConverter);
- unknownMappings.remove(className);
}
@Override
@@ -148,12 +157,12 @@ public class StrutsTypeConverterHolder implements TypeConverterHolder {
@Override
public boolean containsUnknownMapping(String className) {
- return unknownMappings.contains(className);
+ return unknownMappingsInternal.contains(className);
}
@Override
public void addUnknownMapping(String className) {
- unknownMappings.add(className);
+ unknownMappingsInternal.add(className);
}
}
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 ccfa7d1e1..7518fc30d 100644
--- a/core/src/main/java/org/apache/struts2/conversion/TypeConverterHolder.java
+++ b/core/src/main/java/org/apache/struts2/conversion/TypeConverterHolder.java
@@ -54,6 +54,10 @@ public interface TypeConverterHolder {
/**
* Target class conversion Mappings.
*
+ * <p>Returns {@code null} if the class has been flagged as having no mapping via
+ * {@link #addNoMapping(Class)}, even if a real mapping was previously stored for it with
+ * {@link #addMapping(Class, Map)}.</p>
+ *
* @param clazz class to convert to/from
* @return {@link TypeConverter} for given class
* @deprecated since 7.3.0, use {@link #computeMappingIfAbsent(Class, Function)} which resolves
diff --git a/core/src/main/java/org/apache/struts2/conversion/impl/XWorkConverter.java b/core/src/main/java/org/apache/struts2/conversion/impl/XWorkConverter.java
index 58a075efd..419e8b005 100644
--- a/core/src/main/java/org/apache/struts2/conversion/impl/XWorkConverter.java
+++ b/core/src/main/java/org/apache/struts2/conversion/impl/XWorkConverter.java
@@ -419,7 +419,9 @@ public class XWorkConverter extends DefaultTypeConverter {
}
try {
Map<String, Object> mapping = converterHolder.computeMappingIfAbsent(clazz, this::buildConverterMappingUnchecked);
- mapping = conditionalReload(clazz, mapping);
+ if (!mapping.isEmpty()) {
+ mapping = conditionalReload(clazz, mapping);
+ }
Object converter = mapping.get(property);
if (converter == null && LOG.isDebugEnabled()) {
diff --git a/core/src/main/java/org/apache/struts2/validator/DefaultActionValidatorManager.java b/core/src/main/java/org/apache/struts2/validator/DefaultActionValidatorManager.java
index 530b89599..7da4bccf8 100644
--- a/core/src/main/java/org/apache/struts2/validator/DefaultActionValidatorManager.java
+++ b/core/src/main/java/org/apache/struts2/validator/DefaultActionValidatorManager.java
@@ -283,7 +283,7 @@ public class DefaultActionValidatorManager implements ActionValidatorManager {
if (checked == null) {
checked = new TreeSet<>();
} else if (checked.contains(clazz.getName())) {
- return validatorConfigs;
+ return Collections.emptyList();
}
if (clazz.isInterface()) {