This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch fix/WW-5579-add-double-short-range-validators-to-container in repository https://gitbox.apache.org/repos/asf/struts.git
commit c01354afa679538064440f8632bc2f93fa779481 Author: Lukasz Lenart <[email protected]> AuthorDate: Fri Oct 17 10:16:29 2025 +0200 WW-5579 Add missing DoubleRangeFieldValidator and ShortRangeFieldValidator to @Validations container This commit addresses a long-standing gap in the Struts validation framework where @DoubleRangeFieldValidator and @ShortRangeFieldValidator annotations could not be used within the @Validations container annotation, despite being fully functional as standalone annotations. Changes made: - Added doubleRangeFields() and shortRangeFields() methods to @Validations annotation interface - Updated JavaDoc to document the new fields and corrected the parameters table - Added processing loops for both validators in AnnotationValidationConfigurationBuilder - Created comprehensive test coverage in AnnotationValidationConfigurationBuilderTest - Added AnnotationValidationsContainerAction test class demonstrating proper usage The implementation follows the exact same pattern used by IntRangeFieldValidator and LongRangeFieldValidator, ensuring consistency across all range validators in the framework. All existing tests continue to pass, and new tests verify that multiple validators of each type can be properly configured within the @Validations container. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --- .../AnnotationValidationConfigurationBuilder.java | 20 +- .../struts2/validator/annotations/Validations.java | 27 +- ...notationValidationConfigurationBuilderTest.java | 45 +++ .../AnnotationValidationsContainerAction.java | 50 ++++ ...W-5579-double-short-range-validators-missing.md | 301 +++++++++++++++++++++ 5 files changed, 439 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilder.java b/core/src/main/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilder.java index fa0a23a07..7c9650a05 100644 --- a/core/src/main/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilder.java +++ b/core/src/main/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilder.java @@ -312,6 +312,24 @@ public class AnnotationValidationConfigurationBuilder { } } } + DoubleRangeFieldValidator[] doublerfv = validations.doubleRangeFields(); + if (doublerfv != null) { + for (DoubleRangeFieldValidator v : doublerfv) { + ValidatorConfig temp = processDoubleRangeFieldValidatorAnnotation(v, fieldName, methodName); + if (temp != null) { + result.add(temp); + } + } + } + ShortRangeFieldValidator[] shortfv = validations.shortRangeFields(); + if (shortfv != null) { + for (ShortRangeFieldValidator v : shortfv) { + ValidatorConfig temp = processShortRangeFieldValidatorAnnotation(v, fieldName, methodName); + if (temp != null) { + result.add(temp); + } + } + } RegexFieldValidator[] rfv = validations.regexFields(); if (rfv != null) { for (RegexFieldValidator v : rfv) { @@ -567,7 +585,7 @@ public class AnnotationValidationConfigurationBuilder { if (StringUtils.isNotEmpty(v.minLengthExpression())) { params.put("minLengthExpression", v.minLengthExpression()); } - if (StringUtils.isNotEmpty(v.trimExpression())){ + if (StringUtils.isNotEmpty(v.trimExpression())) { params.put("trimExpression", v.trimExpression()); } else { params.put("trim", String.valueOf(v.trim())); diff --git a/core/src/main/java/org/apache/struts2/validator/annotations/Validations.java b/core/src/main/java/org/apache/struts2/validator/annotations/Validations.java index 87c89d2fb..aee0bfe85 100644 --- a/core/src/main/java/org/apache/struts2/validator/annotations/Validations.java +++ b/core/src/main/java/org/apache/struts2/validator/annotations/Validations.java @@ -29,13 +29,13 @@ import java.lang.annotation.Target; * <!-- END SNIPPET: description --> * * <p><u>Annotation usage:</u></p> - * + * <p> * <!-- START SNIPPET: usage --> * <p>Used at METHOD level.</p> * <!-- END SNIPPET: usage --> * * <p><u>Annotation parameters:</u></p> - * + * <p> * <!-- START SNIPPET: parameters --> * <table class='confluenceTable' summary=''> * @@ -82,6 +82,21 @@ import java.lang.annotation.Target; * <td class='confluenceTd'> Add list of IntRangeFieldValidators </td> * </tr> * <tr> + * <td class='confluenceTd'> longRangeFields </td> + * <td class='confluenceTd'> no </td> + * <td class='confluenceTd'> Add list of LongRangeFieldValidators </td> + * </tr> + * <tr> + * <td class='confluenceTd'> doubleRangeFields </td> + * <td class='confluenceTd'> no </td> + * <td class='confluenceTd'> Add list of DoubleRangeFieldValidators </td> + * </tr> + * <tr> + * <td class='confluenceTd'> shortRangeFields </td> + * <td class='confluenceTd'> no </td> + * <td class='confluenceTd'> Add list of ShortRangeFieldValidators </td> + * </tr> + * <tr> * <td class='confluenceTd'> requiredStrings </td> * <td class='confluenceTd'> no </td> * <td class='confluenceTd'> Add list of RequiredStringValidators </td> @@ -133,6 +148,8 @@ import java.lang.annotation.Target; * { @IntRangeFieldValidator(type = ValidatorType.SIMPLE, fieldName = "intfield", min = "6", max = "10", message = "bar must be between ${min} and ${max}, current value is ${bar}.")}, * longRangeFields = * { @LongRangeFieldValidator(type = ValidatorType.SIMPLE, fieldName = "intfield", min = "6", max = "10", message = "bar must be between ${min} and ${max}, current value is ${bar}.")}, + * doubleRangeFields = + * { @DoubleRangeFieldValidator(type = ValidatorType.SIMPLE, fieldName = "doublefield", minInclusive = "0.01", maxInclusive = "999.99", message = "bar must be between ${minInclusive} and ${maxInclusive}, current value is ${bar}.")}, * shortRangeFields = * { @ShortRangeFieldValidator(type = ValidatorType.SIMPLE, fieldName = "shortfield", min = "1", max = "128", message = "bar must be between ${min} and ${max}, current value is ${bar}.")}, * dateRangeFields = @@ -154,7 +171,7 @@ import java.lang.annotation.Target; * @author jepjep * @author Rainer Hermanns */ -@Target( { ElementType.METHOD, ElementType.TYPE }) +@Target({ElementType.METHOD, ElementType.TYPE}) @Retention(RetentionPolicy.RUNTIME) public @interface Validations { @@ -179,6 +196,10 @@ public @interface Validations { LongRangeFieldValidator[] longRangeFields() default {}; + DoubleRangeFieldValidator[] doubleRangeFields() default {}; + + ShortRangeFieldValidator[] shortRangeFields() default {}; + RequiredFieldValidator[] requiredFields() default {}; RequiredStringValidator[] requiredStrings() default {}; diff --git a/core/src/test/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilderTest.java b/core/src/test/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilderTest.java index 6f4430ba0..0e55dbb79 100644 --- a/core/src/test/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilderTest.java +++ b/core/src/test/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilderTest.java @@ -310,6 +310,51 @@ public class AnnotationValidationConfigurationBuilderTest extends XWorkTestCase assertTrue(Arrays.equals(new String[]{"one", "two", "three"}, validator.getMessageParameters())); } + public void testValidationsContainerWithRangeValidators() throws Exception { + // given + AnnotationActionValidatorManager manager = createValidationManager( AnnotationValidationsContainerAction.class, Locale.US); + + // when + List<Validator> validators = manager.getValidators(AnnotationValidationsContainerAction.class, null); + + // then + assertEquals(4, validators.size()); + + int doubleValidators = 0; + int shortValidators = 0; + + for (Validator validator : validators) { + if (validator.getValidatorType().equals("double")) { + DoubleRangeFieldValidator drfv = (DoubleRangeFieldValidator) validator; + doubleValidators++; + if ("price".equals(drfv.getFieldName())) { + assertEquals(0.01, drfv.getMinInclusive()); + assertEquals(999999.99, drfv.getMaxInclusive()); + assertEquals("Price must be between 0.01 and 999999.99", drfv.getDefaultMessage()); + } else if ("discount".equals(drfv.getFieldName())) { + assertEquals(0.0, drfv.getMinInclusive()); + assertEquals(100.0, drfv.getMaxInclusive()); + assertEquals("Discount must be between 0.0 and 100.0", drfv.getDefaultMessage()); + } + } else if (validator.getValidatorType().equals("short")) { + ShortRangeFieldValidator srfv = (ShortRangeFieldValidator) validator; + shortValidators++; + if ("quantity".equals(srfv.getFieldName())) { + assertEquals(Short.valueOf("1"), srfv.getMin()); + assertEquals(Short.valueOf("1000"), srfv.getMax()); + assertEquals("Quantity must be between 1 and 1000", srfv.getDefaultMessage()); + } else if ("priority".equals(srfv.getFieldName())) { + assertEquals(Short.valueOf("1"), srfv.getMin()); + assertEquals(Short.valueOf("10"), srfv.getMax()); + assertEquals("Priority must be between 1 and 10", srfv.getDefaultMessage()); + } + } + } + + assertEquals(2, doubleValidators); + assertEquals(2, shortValidators); + } + private AnnotationActionValidatorManager createValidationManager(final Class<? extends ActionSupport> actionClass, Locale locale) throws Exception { loadConfigurationProviders(new ConfigurationProvider() { public void destroy() { diff --git a/core/src/test/java/org/apache/struts2/validator/AnnotationValidationsContainerAction.java b/core/src/test/java/org/apache/struts2/validator/AnnotationValidationsContainerAction.java new file mode 100644 index 000000000..be18a85b7 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/validator/AnnotationValidationsContainerAction.java @@ -0,0 +1,50 @@ +/* + * 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; +import org.apache.struts2.validator.annotations.DoubleRangeFieldValidator; +import org.apache.struts2.validator.annotations.ShortRangeFieldValidator; +import org.apache.struts2.validator.annotations.Validations; + +/** + * Test action to verify @Validations container annotation with doubleRangeFields and shortRangeFields + */ +public class AnnotationValidationsContainerAction extends ActionSupport { + + @Validations( + doubleRangeFields = { + @DoubleRangeFieldValidator(fieldName = "price", minInclusive = "0.01", maxInclusive = "999999.99", + message = "Price must be between 0.01 and 999999.99"), + @DoubleRangeFieldValidator(fieldName = "discount", minInclusive = "0.0", maxInclusive = "100.0", + message = "Discount must be between 0.0 and 100.0") + }, + shortRangeFields = { + @ShortRangeFieldValidator(fieldName = "quantity", min = "1", max = "1000", + message = "Quantity must be between 1 and 1000"), + @ShortRangeFieldValidator(fieldName = "priority", min = "1", max = "10", + message = "Priority must be between 1 and 10") + } + ) + @Override + public String execute() { + return SUCCESS; + } + +} diff --git a/thoughts/shared/research/2025-10-17-WW-5579-double-short-range-validators-missing.md b/thoughts/shared/research/2025-10-17-WW-5579-double-short-range-validators-missing.md new file mode 100644 index 000000000..317f7a136 --- /dev/null +++ b/thoughts/shared/research/2025-10-17-WW-5579-double-short-range-validators-missing.md @@ -0,0 +1,301 @@ +--- +date: 2025-10-17T07:12:23+0000 +topic: "@DoubleRangeFieldValidator and @ShortRangeFieldValidator missing from @Validations container" +tags: [research, validation, annotations, bug, WW-5579] +status: complete +jira: WW-5579 +git_commit: 06f9f9303387edf0557d128bbd7123bded4f24f5 +--- + +# Research: Missing Range Validators in @Validations Container + +**Date**: 2025-10-17T07:12:23+0000 +**JIRA**: [WW-5579](https://issues.apache.org/jira/browse/WW-5579) + +## Research Question + +Why can't `@DoubleRangeFieldValidator` and `@ShortRangeFieldValidator` be used within the `@Validations` container annotation, while other range validators like `@IntRangeFieldValidator` and `@LongRangeFieldValidator` work fine? + +## Summary + +This is a **genuine bug/oversight** in Apache Struts. Both `@DoubleRangeFieldValidator` and `@ShortRangeFieldValidator` annotations exist and work as standalone annotations, but they are **completely missing** from the `@Validations` container annotation interface definition. This forces developers to either: +1. Use these validators as standalone annotations (limiting to one per method) +2. Fall back to XML-based validation configuration +3. Scatter validations across multiple locations + +The infrastructure to support these validators already exists - the processing methods are implemented, the validators work standalone - only the container fields are missing. + +## Detailed Findings + +### 1. The @Validations Container Annotation + +**File**: `core/src/main/java/org/apache/struts2/validator/annotations/Validations.java` + +The `@Validations` interface defines container fields for various validators (lines 159-197): + +```java +@Target({ElementType.METHOD, ElementType.TYPE}) +@Retention(RetentionPolicy.RUNTIME) +public @interface Validations { + CustomValidator[] customValidators() default {}; + ConversionErrorFieldValidator[] conversionErrorFields() default {}; + DateRangeFieldValidator[] dateRangeFields() default {}; // ✅ Present + EmailValidator[] emails() default {}; + CreditCardValidator[] creditCards() default {}; + FieldExpressionValidator[] fieldExpressions() default {}; + IntRangeFieldValidator[] intRangeFields() default {}; // ✅ Present + LongRangeFieldValidator[] longRangeFields() default {}; // ✅ Present + RequiredFieldValidator[] requiredFields() default {}; + RequiredStringValidator[] requiredStrings() default {}; + StringLengthFieldValidator[] stringLengthFields() default {}; + UrlValidator[] urls() default {}; + ConditionalVisitorFieldValidator[] conditionalVisitorFields() default {}; + VisitorFieldValidator[] visitorFields() default {}; + RegexFieldValidator[] regexFields() default {}; + ExpressionValidator[] expressions() default {}; + + // ❌ MISSING: DoubleRangeFieldValidator[] doubleRangeFields() + // ❌ MISSING: ShortRangeFieldValidator[] shortRangeFields() +} +``` + +**Inconsistency Found**: The JavaDoc example at line 136 **incorrectly shows** `shortRangeFields` being used: + +```java +* shortRangeFields = +* { @ShortRangeFieldValidator(...) }, +``` + +This is misleading documentation - the field doesn't actually exist! + +### 2. The Validators Exist and Work Standalone + +**File**: `core/src/main/java/org/apache/struts2/validator/annotations/DoubleRangeFieldValidator.java` + +The `@DoubleRangeFieldValidator` annotation is fully implemented with support for: +- Inclusive ranges: `minInclusive`, `maxInclusive` +- Exclusive ranges: `minExclusive`, `maxExclusive` +- Expression-based values: `minInclusiveExpression`, `maxInclusiveExpression`, etc. + +**File**: `core/src/main/java/org/apache/struts2/validator/annotations/ShortRangeFieldValidator.java` + +The `@ShortRangeFieldValidator` annotation exists with similar range support. + +**Both validators work perfectly when used directly on methods**, just not within `@Validations`. + +### 3. Processing Infrastructure Exists + +**File**: `core/src/main/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilder.java` + +The processing methods are **fully implemented**: + +- **Line 171-176**: `processAnnotations()` handles `@DoubleRangeFieldValidator` when used standalone +- **Line 163-169**: `processAnnotations()` handles `@ShortRangeFieldValidator` when used standalone +- **Line 758-806**: `processDoubleRangeFieldValidatorAnnotation()` - complete processing logic +- **Line 723-756**: `processShortRangeFieldValidatorAnnotation()` - complete processing logic + +However, the `processValidationAnnotation()` method (lines 232-378) that handles validators within `@Validations` container **does not include loops for doubleRangeFields or shortRangeFields**. + +**Comparison with working validators:** + +```java +// Lines 297-305: IntRangeFieldValidator - PRESENT in container processing +IntRangeFieldValidator[] irfv = validations.intRangeFields(); +if (irfv != null) { + for (IntRangeFieldValidator v : irfv) { + ValidatorConfig temp = processIntRangeFieldValidatorAnnotation(v, fieldName, methodName); + if (temp != null) { + result.add(temp); + } + } +} + +// Lines 306-314: LongRangeFieldValidator - PRESENT in container processing +LongRangeFieldValidator[] lrfv = validations.longRangeFields(); +if (lrfv != null) { + for (LongRangeFieldValidator v : lrfv) { + ValidatorConfig temp = processLongRangeFieldValidatorAnnotation(v, fieldName, methodName); + if (temp != null) { + result.add(temp); + } + } +} + +// ❌ NO SIMILAR LOOP FOR doubleRangeFields +// ❌ NO SIMILAR LOOP FOR shortRangeFields +``` + +## Code References + +- `core/src/main/java/org/apache/struts2/validator/annotations/Validations.java:159-197` - Container annotation definition +- `core/src/main/java/org/apache/struts2/validator/annotations/Validations.java:136` - Misleading JavaDoc example +- `core/src/main/java/org/apache/struts2/validator/annotations/DoubleRangeFieldValidator.java` - Full validator annotation +- `core/src/main/java/org/apache/struts2/validator/annotations/ShortRangeFieldValidator.java` - Full validator annotation +- `core/src/main/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilder.java:758-806` - Double range processing method +- `core/src/main/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilder.java:723-756` - Short range processing method +- `core/src/main/java/org/apache/struts2/validator/AnnotationValidationConfigurationBuilder.java:232-378` - Container processing method (missing loops) + +## Architecture Insights + +### How Validation Annotations Work + +The Struts validation annotation system has three components: + +1. **Annotation Definitions** (`@DoubleRangeFieldValidator`, etc.) - Define the metadata +2. **Container Annotation** (`@Validations`) - Allows multiple validators of the same type +3. **Configuration Builder** (`AnnotationValidationConfigurationBuilder`) - Processes annotations into `ValidatorConfig` objects + +The flow is: +``` +Action Method with @Validations + ↓ +AnnotationActionValidatorManager.buildAnnotationValidatorConfigs() + ↓ +AnnotationValidationConfigurationBuilder.processAnnotations() + ↓ +processValidationAnnotation() [for @Validations container] + ↓ +Individual processXxxValidatorAnnotation() methods + ↓ +ValidatorConfig objects created +``` + +### Why This Bug Exists + +Looking at the git history: + +- **ae90857f1** (2013): WW-4004 - "Improves DoubleRangeFieldValidator annotation to match DoubleRangeFieldValidator class" +- **4460ddae6** (2013): WW-4011 - "Improves ShortRangeFieldValidator annotation to match ShortRangeFieldValidator class" + +Both issues focused on **improving the individual annotations** (adding inclusive/exclusive range support), but **neither added container support** to `@Validations`. This was likely an oversight during the enhancement process. + +The fact that the JavaDoc example shows `shortRangeFields` suggests that someone **intended** to add this support but never completed the implementation. + +## Historical Context + +### Related JIRA Issues + +- **WW-4004** (Fixed in 2.3.14): Improved `@DoubleRangeFieldValidator` to support inclusive/exclusive ranges +- **WW-4011** (Fixed in 2.3.14): Improved `@ShortRangeFieldValidator` similarly +- **WW-5579** (Current): Add missing container support for both validators + +### Why It Went Unnoticed + +1. **Standalone usage works**: Developers can use these validators directly on methods +2. **XML alternative exists**: Can fall back to XML-based validation +3. **Less common validators**: Double and short ranges are less frequently used than int/long +4. **Documentation inconsistency**: The misleading JavaDoc may have caused confusion + +## Impact Analysis + +### Current Workarounds + +**Option 1**: Use standalone annotations (one per method) +```java +@DoubleRangeFieldValidator( + fieldName = "price", + minInclusive = "0.01", + maxInclusive = "999999.99", + message = "Price must be between ${minInclusive} and ${maxInclusive}" +) +public String execute() { + return SUCCESS; +} +``` + +**Option 2**: Use XML validation +```xml +<validators> + <field name="price"> + <field-validator type="double"> + <param name="minInclusive">0.01</param> + <param name="maxInclusive">999999.99</param> + <message>Price must be between ${minInclusive} and ${maxInclusive}</message> + </field-validator> + </field> +</validators> +``` + +**Option 3**: Multiple standalone annotations (cannot group) +```java +@DoubleRangeFieldValidator(fieldName = "price", ...) +@DoubleRangeFieldValidator(fieldName = "discount", ...) // ❌ Cannot have multiple +@ShortRangeFieldValidator(fieldName = "quantity", ...) +``` + +### Desired Usage (After Fix) + +```java +@Validations( + requiredStrings = { + @RequiredStringValidator(fieldName = "productName", message = "Product name is required") + }, + doubleRangeFields = { + @DoubleRangeFieldValidator(fieldName = "price", minInclusive = "0.01", maxInclusive = "999999.99"), + @DoubleRangeFieldValidator(fieldName = "discount", minInclusive = "0.0", maxInclusive = "100.0") + }, + shortRangeFields = { + @ShortRangeFieldValidator(fieldName = "quantity", min = "1", max = "1000") + } +) +public String execute() { + return SUCCESS; +} +``` + +## Implementation Plan + +### Changes Required + +**1. Update `Validations.java`** +- Add `DoubleRangeFieldValidator[] doubleRangeFields() default {};` +- Add `ShortRangeFieldValidator[] shortRangeFields() default {};` +- Fix JavaDoc to accurately reflect available fields + +**2. Update `AnnotationValidationConfigurationBuilder.java`** +- Add processing loop for `doubleRangeFields` in `processValidationAnnotation()` method (after line 314) +- Add processing loop for `shortRangeFields` in `processValidationAnnotation()` method +- Follow the existing pattern used for `intRangeFields` and `longRangeFields` + +**3. Add Tests** +- Add test cases in `AnnotationValidationConfigurationBuilderTest.java` +- Test multiple double range validators in container +- Test multiple short range validators in container +- Test mixed usage with other validators + +**4. Update Documentation** +- Update Struts documentation site to include examples +- Ensure JavaDoc examples are accurate + +### Estimated Complexity + +**Low**: The infrastructure exists, we're just adding: +- 2 method declarations to an interface +- 2 processing loops (~20 lines each) +- Test cases (~50 lines) +- Documentation updates + +Total: ~150 lines of code changes. + +## Related Research + +None found in thoughts/ directory - this is the first documentation of this issue. + +## Open Questions + +1. **Should we backport to older versions?** This is a minor feature addition that could be considered for 6.x and possibly 2.x branches. + +2. **Are there other validators missing from container?** We should audit all validators to ensure none are missing. + +3. **Should we add validation in `@Validations` to catch configuration errors?** For example, checking that field names are valid at compile time (probably not possible without annotation processing). + +## Conclusion + +This is a clear gap in the Struts validation framework that has existed since at least 2013. The fix is straightforward since all the underlying infrastructure exists. Once implemented, it will provide consistency across all range validators and improve the developer experience by allowing all validations to be grouped logically within the `@Validations` container. + +The bug exemplifies how incomplete feature implementations can persist for years when: +- Workarounds exist (standalone annotations, XML) +- The feature is less commonly used (double/short ranges vs int/long) +- Documentation is misleading (JavaDoc showing non-existent fields) + +**Next Steps**: Implement the fix as outlined above after JIRA issue WW-5579 is created. \ No newline at end of file
