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/23 19:47:10 UTC
(struts) 03/04: WW-2934 feat(core): skip field validators on conversion error behind opt-in flag
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-2934-skip-validators-on-conversion-error
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 4ce1690dd7eb25997f9fba15d661f978a991826b
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Thu Jul 23 21:32:57 2026 +0200
WW-2934 feat(core): skip field validators on conversion error behind opt-in flag
Co-Authored-By: Claude Opus 4.8 <no...@anthropic.com>
---
.../java/org/apache/struts2/StrutsConstants.java | 3 ++
.../validator/DefaultActionValidatorManager.java | 14 ++++++
.../org/apache/struts2/default.properties | 5 ++
.../validator/ConversionErrorSkipAction.java | 48 ++++++++++++++++++
.../DefaultActionValidatorManagerTest.java | 57 ++++++++++++++++++++++
.../ConversionErrorSkipAction-validation.xml | 43 ++++++++++++++++
6 files changed, 170 insertions(+)
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index e76fac4e5..7530bd307 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -419,6 +419,9 @@ public final class StrutsConstants {
*/
public static final String STRUTS_ACTIONVALIDATORMANAGER = "struts.actionValidatorManager";
+ /** @see org.apache.struts2.validator.DefaultActionValidatorManager */
+ public static final String STRUTS_VALIDATORS_SKIP_VALIDATORS_ON_CONVERSION_ERROR = "struts.validators.skipValidatorsOnConversionError";
+
/**
* The {@link org.apache.struts2.util.ValueStackFactory} implementation class
*/
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 2f2b758b7..f5c40a1fb 100644
--- a/core/src/main/java/org/apache/struts2/validator/DefaultActionValidatorManager.java
+++ b/core/src/main/java/org/apache/struts2/validator/DefaultActionValidatorManager.java
@@ -25,6 +25,7 @@ import org.apache.struts2.text.TextProviderFactory;
import org.apache.struts2.inject.Inject;
import org.apache.struts2.util.ClassLoaderUtil;
import org.apache.struts2.util.ValueStack;
+import org.apache.struts2.validator.validators.ConversionErrorFieldValidator;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
@@ -75,6 +76,7 @@ public class DefaultActionValidatorManager implements ActionValidatorManager {
protected ValidatorFileParser validatorFileParser;
protected FileManager fileManager;
protected boolean reloadingConfigs;
+ protected boolean skipValidatorsOnConversionError;
protected TextProviderFactory textProviderFactory;
@Inject
@@ -97,6 +99,11 @@ public class DefaultActionValidatorManager implements ActionValidatorManager {
this.reloadingConfigs = Boolean.parseBoolean(reloadingConfigs);
}
+ @Inject(value = StrutsConstants.STRUTS_VALIDATORS_SKIP_VALIDATORS_ON_CONVERSION_ERROR, required = false)
+ public void setSkipValidatorsOnConversionError(String skipValidatorsOnConversionError) {
+ this.skipValidatorsOnConversionError = Boolean.parseBoolean(skipValidatorsOnConversionError);
+ }
+
@Inject
public void setTextProviderFactory(TextProviderFactory textProviderFactory) {
this.textProviderFactory = textProviderFactory;
@@ -183,6 +190,13 @@ public class DefaultActionValidatorManager implements ActionValidatorManager {
LOG.debug("Short-circuited, skipping");
continue;
}
+
+ if (skipValidatorsOnConversionError
+ && !(validator instanceof ConversionErrorFieldValidator)
+ && ActionContext.getContext().getConversionErrors().containsKey(fullFieldName)) {
+ LOG.debug("Skipping validator {} for field {} due to a conversion error", validator, fullFieldName);
+ continue;
+ }
}
if (validator instanceof ShortCircuitableValidator && ((ShortCircuitableValidator) validator).isShortCircuit()) {
diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties
index 1c501cb33..0dcb5ab80 100644
--- a/core/src/main/resources/org/apache/struts2/default.properties
+++ b/core/src/main/resources/org/apache/struts2/default.properties
@@ -146,6 +146,11 @@ struts.mapper.action.prefix.crossNamespaces = false
### them right away.
struts.devMode = false
+### When set to true, a field's remaining validators are skipped once that field
+### has a type conversion error, avoiding a duplicate error (WW-2934).
+### valid values are: true, false (false is the default)
+struts.validators.skipValidatorsOnConversionError = false
+
### when set to true, resource bundles will be reloaded on _every_ request.
### this is good during development, but should never be used in production
# struts.i18n.reload=false
diff --git a/core/src/test/java/org/apache/struts2/validator/ConversionErrorSkipAction.java b/core/src/test/java/org/apache/struts2/validator/ConversionErrorSkipAction.java
new file mode 100644
index 000000000..e9b22fc3e
--- /dev/null
+++ b/core/src/test/java/org/apache/struts2/validator/ConversionErrorSkipAction.java
@@ -0,0 +1,48 @@
+/*
+ * 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.validator;
+
+import org.apache.struts2.ActionSupport;
+
+/**
+ * Fixture for WW-2934: an Integer field ("age") that carries both a conversion
+ * validator and a required validator, plus an unrelated required String field
+ * ("name") and an action-level validator (see the matching -validation.xml).
+ */
+public class ConversionErrorSkipAction extends ActionSupport {
+
+ private Integer age;
+ private String name;
+
+ public Integer getAge() {
+ return age;
+ }
+
+ public void setAge(Integer age) {
+ this.age = age;
+ }
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
+}
diff --git a/core/src/test/java/org/apache/struts2/validator/DefaultActionValidatorManagerTest.java b/core/src/test/java/org/apache/struts2/validator/DefaultActionValidatorManagerTest.java
index 06914d31f..f2ad1550f 100644
--- a/core/src/test/java/org/apache/struts2/validator/DefaultActionValidatorManagerTest.java
+++ b/core/src/test/java/org/apache/struts2/validator/DefaultActionValidatorManagerTest.java
@@ -18,11 +18,13 @@
*/
package org.apache.struts2.validator;
+import org.apache.struts2.ActionContext;
import org.apache.struts2.FileManagerFactory;
import org.apache.struts2.SimpleAction;
import org.apache.struts2.TestBean;
import org.apache.struts2.ValidationOrderAction;
import org.apache.struts2.XWorkTestCase;
+import org.apache.struts2.conversion.impl.ConversionData;
import org.apache.struts2.interceptor.ValidationAware;
import org.apache.struts2.test.DataAware2;
import org.apache.struts2.test.SimpleAction3;
@@ -375,4 +377,59 @@ public class DefaultActionValidatorManagerTest extends XWorkTestCase {
assertEquals((e.getValue()).get(0), "password hint is required");
}
+ public void testConversionError_bothErrorsWhenFlagDisabledByDefault() throws Exception {
+ ConversionErrorSkipAction action = new ConversionErrorSkipAction();
+ ActionContext.getContext().getConversionErrors()
+ .put("age", new ConversionData(new String[]{"one"}, Integer.class));
+
+ actionValidatorManager.validate(action, null);
+
+ List<String> ageErrors = action.getFieldErrors().get("age");
+ assertNotNull(ageErrors);
+ assertEquals(2, ageErrors.size()); // conversion + required, current behavior
+ assertTrue(ageErrors.contains("Age must be a valid number"));
+ assertTrue(ageErrors.contains("Age is required"));
+ }
+
+ public void testConversionError_fieldValidatorsSkippedWhenEnabled() throws Exception {
+ ConversionErrorSkipAction action = new ConversionErrorSkipAction();
+ ActionContext.getContext().getConversionErrors()
+ .put("age", new ConversionData(new String[]{"one"}, Integer.class));
+ actionValidatorManager.setSkipValidatorsOnConversionError("true");
+
+ actionValidatorManager.validate(action, null);
+
+ List<String> ageErrors = action.getFieldErrors().get("age");
+ assertNotNull(ageErrors);
+ // required is skipped; the conversion validator itself still runs
+ assertEquals(1, ageErrors.size());
+ assertEquals("Age must be a valid number", ageErrors.get(0));
+ }
+
+ public void testConversionError_unrelatedFieldStillValidatedWhenEnabled() throws Exception {
+ ConversionErrorSkipAction action = new ConversionErrorSkipAction();
+ ActionContext.getContext().getConversionErrors()
+ .put("age", new ConversionData(new String[]{"one"}, Integer.class));
+ actionValidatorManager.setSkipValidatorsOnConversionError("true");
+
+ actionValidatorManager.validate(action, null);
+
+ List<String> nameErrors = action.getFieldErrors().get("name");
+ assertNotNull(nameErrors); // "name" has no conversion error, still validated
+ assertEquals(1, nameErrors.size());
+ assertEquals("Name is required", nameErrors.get(0));
+ }
+
+ public void testConversionError_actionLevelValidatorUnaffectedWhenEnabled() throws Exception {
+ ConversionErrorSkipAction action = new ConversionErrorSkipAction();
+ ActionContext.getContext().getConversionErrors()
+ .put("age", new ConversionData(new String[]{"one"}, Integer.class));
+ actionValidatorManager.setSkipValidatorsOnConversionError("true");
+
+ actionValidatorManager.validate(action, null);
+
+ assertTrue(action.hasActionErrors());
+ assertTrue(action.getActionErrors().contains("Action level always fails"));
+ }
+
}
diff --git a/core/src/test/resources/org/apache/struts2/validator/ConversionErrorSkipAction-validation.xml b/core/src/test/resources/org/apache/struts2/validator/ConversionErrorSkipAction-validation.xml
new file mode 100644
index 000000000..e8a6af67e
--- /dev/null
+++ b/core/src/test/resources/org/apache/struts2/validator/ConversionErrorSkipAction-validation.xml
@@ -0,0 +1,43 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+/*
+ * 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.
+ */
+-->
+<!DOCTYPE validators PUBLIC "-//Apache Struts//XWork Validator 1.0.2//EN" "https://struts.apache.org/dtds/xwork-validator-1.0.2.dtd">
+<validators>
+ <field name="age">
+ <field-validator type="conversion">
+ <message>Age must be a valid number</message>
+ </field-validator>
+ <field-validator type="required">
+ <message>Age is required</message>
+ </field-validator>
+ </field>
+
+ <field name="name">
+ <field-validator type="required">
+ <message>Name is required</message>
+ </field-validator>
+ </field>
+
+ <validator type="expression">
+ <param name="expression">false</param>
+ <message>Action level always fails</message>
+ </validator>
+</validators>