diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java b/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java index 3df13fafa9..7fc05bf1ec 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java @@ -722,9 +722,7 @@ private void doValidate(Object value, String nodeName) { cascadedValueContext.setTypeParameter( cascadingMetaData.getDeclaredContainerClass(), cascadingMetaData.getDeclaredTypeParameterIndex() ); } - if ( nodeName != null ) { - cascadedTypeArgumentValueContext.appendTypeParameterNode( nodeName ); - } + cascadedTypeArgumentValueContext.appendTypeParameterNode( nodeName ); validateCascadedContainerElementsInContext( value, validationContext, cascadedTypeArgumentValueContext, cascadingMetaData, validationOrder ); } diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/path/PathImpl.java b/engine/src/main/java/org/hibernate/validator/internal/engine/path/PathImpl.java index 55de95d73d..d0a50221ae 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/path/PathImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/path/PathImpl.java @@ -134,6 +134,17 @@ public NodeImpl addContainerElementNode(String nodeName) { return currentLeafNode; } + public boolean needToAddContainerElementNode(String nodeName) { + // if the node name is not null that would mean that we need to add it, + // but otherwise -- we would want to have an empty node + // if a current node is some iterable/multivalued element (E.g. array/list/map etc.). + // If we don't add it -- the path would be broken and would lead to a situation + // where container elements would be pointed to by a container element node itself + // resulting in various node methods like `Node#getIndex()` producing incorrect results. + // As an additional side effect of not adding a node it might lead to the path not being correctly copied. + return nodeName != null || currentLeafNode.isIterable(); + } + public NodeImpl addParameterNode(String nodeName, int index) { requiresWriteableNodeList(); diff --git a/engine/src/main/java/org/hibernate/validator/internal/engine/valuecontext/ValueContext.java b/engine/src/main/java/org/hibernate/validator/internal/engine/valuecontext/ValueContext.java index 47a844fadb..e6bcd6f176 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/engine/valuecontext/ValueContext.java +++ b/engine/src/main/java/org/hibernate/validator/internal/engine/valuecontext/ValueContext.java @@ -102,9 +102,11 @@ public final void appendNode(ConstraintLocation location) { } public final void appendTypeParameterNode(String nodeName) { - PathImpl newPath = PathImpl.createCopy( propertyPath ); - newPath.addContainerElementNode( nodeName ); - propertyPath = newPath; + if ( propertyPath.needToAddContainerElementNode( nodeName ) ) { + PathImpl newPath = PathImpl.createCopy( propertyPath ); + newPath.addContainerElementNode( nodeName ); + propertyPath = newPath; + } } public final void markCurrentPropertyAsIterable() { diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/core/MetaConstraint.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/core/MetaConstraint.java index 948ac2b95e..1598ac7bf2 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/core/MetaConstraint.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/core/MetaConstraint.java @@ -226,10 +226,7 @@ private void doValidate(Object value, String nodeName) { valueContext.setTypeParameter( containerClass, currentValueExtractionPathNode.getTypeParameterIndex() ); } - if ( nodeName != null ) { - valueContext.appendTypeParameterNode( nodeName ); - } - + valueContext.appendTypeParameterNode( nodeName ); valueContext.setCurrentValidatedValue( value ); if ( currentValueExtractionPathNode.hasNext() ) { diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/engine/path/NodeImplTest.java b/engine/src/test/java/org/hibernate/validator/test/internal/engine/path/NodeImplTest.java index 4436873f67..05af389b44 100644 --- a/engine/src/test/java/org/hibernate/validator/test/internal/engine/path/NodeImplTest.java +++ b/engine/src/test/java/org/hibernate/validator/test/internal/engine/path/NodeImplTest.java @@ -39,8 +39,10 @@ import jakarta.validation.Validator; import jakarta.validation.constraints.Max; import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.Pattern; import jakarta.validation.constraints.Size; import jakarta.validation.valueextraction.ExtractedValue; +import jakarta.validation.valueextraction.UnwrapByDefault; import jakarta.validation.valueextraction.ValueExtractor; import org.hibernate.validator.path.ContainerElementNode; @@ -505,6 +507,46 @@ public void testContainerElementNodeGetValueForNestedContainer() { assertFalse( nodeIterator.hasNext() ); } + @Test + @TestForIssue(jiraKey = "HV-1946") + public void testIndexedContainerElementMultipleFailuresCorrectPath() { + class People { + @Pattern( regexp = "[a-z]+") + Person[] people; + + public People(Person... people) { + this.people = people; + } + } + + Validator validator = ValidatorUtil.getConfiguration() + .addValueExtractor( new PersonArrayValueExtractor() ) + .buildValidatorFactory() + .getValidator(); + + Set> constraintViolations = validator.validate( + new People( new Person( "name1" ), new Person( "name2" ), new Person( "name3" ) ) + ); + + assertThat( constraintViolations ).containsOnlyViolations( + violationOf( Pattern.class ) + .withPropertyPath( pathWith() + .property( "people" ) + .containerElement( null, true, null, 0, Person[].class, null ) + ), + violationOf( Pattern.class ) + .withPropertyPath( pathWith() + .property( "people" ) + .containerElement( null, true, null, 1, Person[].class, null ) + ), + violationOf( Pattern.class ) + .withPropertyPath( pathWith() + .property( "people" ) + .containerElement( null, true, null, 2, Person[].class, null ) + ) + ); + } + private void assertConstraintViolationToOneValidation(Set> constraintViolations) { assertThat( constraintViolations ).containsOnlyViolations( violationOf( NotNull.class ) @@ -752,4 +794,15 @@ public void extractValues(CustomContainer originalValue, ValueExtractor.Value } } } + + @UnwrapByDefault + private static final class PersonArrayValueExtractor implements ValueExtractor { + + @Override + public void extractValues(Person[] originalValue, ValueExtractor.ValueReceiver receiver) { + for ( int i = 0; i < originalValue.length; i++ ) { + receiver.indexedValue( null, i, originalValue[i].name ); + } + } + } }