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 <[email protected]> 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 <[email protected]> --- .../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>
