Skip to content

Commit 27d66ba

Browse files
gsmetgunnarmorling
gsmet
authored and
gunnarmorling
committed
HV-1481 Fix legacy cascading support for collection/array types
It was not working anymore when the fact that it is a collection or an array can only be detected at runtime.
1 parent f41146c commit 27d66ba

File tree

5 files changed

+239
-0
lines changed

5 files changed

+239
-0
lines changed

‎engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorImpl.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaData;
5555
import org.hibernate.validator.internal.metadata.aggregated.ContainerCascadingMetaData;
5656
import org.hibernate.validator.internal.metadata.aggregated.ExecutableMetaData;
57+
import org.hibernate.validator.internal.metadata.aggregated.NonContainerCascadingMetaData;
5758
import org.hibernate.validator.internal.metadata.aggregated.ParameterMetaData;
5859
import org.hibernate.validator.internal.metadata.aggregated.PropertyMetaData;
5960
import org.hibernate.validator.internal.metadata.aggregated.ReturnValueMetaData;
@@ -583,6 +584,21 @@ private void validateCascadedConstraints(ValidationContext<?> validationContext,
583584
// validate cascading on the annotated object
584585
if ( cascadingMetaData.isCascading() ) {
585586
validateCascadedAnnotatedObjectForCurrentGroup( value, validationContext, valueContext, cascadingMetaData );
587+
588+
if ( !cascadingMetaData.isContainer() ) {
589+
// We might be in the case where the legacy @Valid support for collections and arrays has been used on a type for which
590+
// it cannot be detected at bootstrap time: thus we need to check with the runtime type.
591+
// Note that if it has been detected at bootstrap time, the cascading metadata has been modified to include it
592+
// and we are then in the container case.
593+
NonContainerCascadingMetaData nonContainerCascadingMetaData = cascadingMetaData.as( NonContainerCascadingMetaData.class );
594+
ContainerCascadingMetaData legacyContainerCascadingMetaData = nonContainerCascadingMetaData
595+
.getLegacyContainerCascadingMetaData( value.getClass() );
596+
597+
if ( legacyContainerCascadingMetaData != null ) {
598+
validateCascadedContainerElementsForCurrentGroup( value, validationContext, valueContext,
599+
Collections.singletonList( legacyContainerCascadingMetaData ) );
600+
}
601+
}
586602
}
587603

588604
if ( cascadingMetaData.isContainer() ) {

‎engine/src/main/java/org/hibernate/validator/internal/engine/valueextraction/LegacyCollectionSupportValueExtractors.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/*
2+
* Hibernate Validator, declare and validate application constraints
3+
*
4+
* License: Apache License, Version 2.0
5+
* See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>.
6+
*/
7+
package org.hibernate.validator.internal.engine.valueextraction;
8+
9+
import java.util.Collections;
10+
import java.util.Set;
11+
12+
import org.hibernate.validator.internal.util.CollectionHelper;
13+
14+
public class LegacyCollectionSupportValueExtractors {
15+
16+
public static final Set<ValueExtractorDescriptor> LIST = Collections.singleton( ListValueExtractor.DESCRIPTOR );
17+
public static final Set<ValueExtractorDescriptor> MAP = Collections.singleton( MapValueExtractor.DESCRIPTOR );
18+
public static final Set<ValueExtractorDescriptor> ITERABLE = Collections.singleton( IterableValueExtractor.DESCRIPTOR );
19+
public static final Set<ValueExtractorDescriptor> OPTIONAL = Collections.singleton( OptionalValueExtractor.DESCRIPTOR );
20+
public static final Set<ValueExtractorDescriptor> ARRAY = CollectionHelper.asSet(
21+
BooleanArrayValueExtractor.DESCRIPTOR,
22+
ByteArrayValueExtractor.DESCRIPTOR,
23+
CharArrayValueExtractor.DESCRIPTOR,
24+
DoubleArrayValueExtractor.DESCRIPTOR,
25+
FloatArrayValueExtractor.DESCRIPTOR,
26+
IntArrayValueExtractor.DESCRIPTOR,
27+
LongArrayValueExtractor.DESCRIPTOR,
28+
ObjectArrayValueExtractor.DESCRIPTOR,
29+
ShortArrayValueExtractor.DESCRIPTOR
30+
);
31+
32+
private LegacyCollectionSupportValueExtractors() {
33+
}
34+
}

‎engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/ContainerCascadingMetaData.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,19 @@ private ContainerCascadingMetaData(ValueExtractorManager valueExtractorManager,
131131
}
132132
}
133133

134+
ContainerCascadingMetaData(Type enclosingType, TypeVariable<?> typeParameter, Class<?> declaredContainerClass, TypeVariable<?> declaredTypeParameter,
135+
GroupConversionHelper groupConversionHelper, Set<ValueExtractorDescriptor> valueExtractorCandidates) {
136+
this.enclosingType = enclosingType;
137+
this.typeParameter = typeParameter;
138+
this.declaredContainerClass = declaredContainerClass;
139+
this.declaredTypeParameter = declaredTypeParameter;
140+
this.containerElementTypesCascadingMetaData = Collections.emptyList();
141+
this.cascading = true;
142+
this.groupConversionHelper = groupConversionHelper;
143+
this.hasContainerElementsMarkedForCascading = false;
144+
this.valueExtractorCandidates = valueExtractorCandidates;
145+
}
146+
134147
@Override
135148
public boolean isContainer() {
136149
return true;

‎engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/NonContainerCascadingMetaData.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,17 @@
88

99
import java.lang.reflect.TypeVariable;
1010
import java.util.Collections;
11+
import java.util.List;
12+
import java.util.Map;
13+
import java.util.Optional;
1114
import java.util.Set;
1215

1316
import javax.validation.metadata.GroupConversionDescriptor;
1417

1518
import org.hibernate.validator.internal.engine.valueextraction.AnnotatedObject;
19+
import org.hibernate.validator.internal.engine.valueextraction.ArrayElement;
20+
import org.hibernate.validator.internal.engine.valueextraction.LegacyCollectionSupportValueExtractors;
21+
import org.hibernate.validator.internal.util.TypeVariables;
1622
import org.hibernate.validator.internal.util.logging.Log;
1723
import org.hibernate.validator.internal.util.logging.LoggerFactory;
1824

@@ -95,6 +101,34 @@ public boolean isContainer() {
95101
return false;
96102
}
97103

104+
public ContainerCascadingMetaData getLegacyContainerCascadingMetaData(Class<?> valueClass) {
105+
if ( List.class.isAssignableFrom( valueClass ) ) {
106+
return new ContainerCascadingMetaData( List.class, List.class.getTypeParameters()[0], List.class, List.class.getTypeParameters()[0],
107+
groupConversionHelper, LegacyCollectionSupportValueExtractors.LIST );
108+
}
109+
else if ( Map.class.isAssignableFrom( valueClass ) ) {
110+
return new ContainerCascadingMetaData( Map.class, Map.class.getTypeParameters()[1], Map.class, Map.class.getTypeParameters()[1],
111+
groupConversionHelper, LegacyCollectionSupportValueExtractors.MAP );
112+
}
113+
else if ( Iterable.class.isAssignableFrom( valueClass ) ) {
114+
return new ContainerCascadingMetaData( Iterable.class, Iterable.class.getTypeParameters()[0], Iterable.class, Iterable.class.getTypeParameters()[0],
115+
groupConversionHelper, LegacyCollectionSupportValueExtractors.ITERABLE );
116+
}
117+
else if ( Optional.class.isAssignableFrom( valueClass ) ) {
118+
return new ContainerCascadingMetaData( Optional.class, Optional.class.getTypeParameters()[0], Optional.class, Optional.class.getTypeParameters()[0],
119+
groupConversionHelper, LegacyCollectionSupportValueExtractors.OPTIONAL );
120+
}
121+
else if ( valueClass.isArray() ) {
122+
TypeVariable<?> typeParameter = new ArrayElement( valueClass );
123+
124+
return new ContainerCascadingMetaData( valueClass, typeParameter,
125+
TypeVariables.getContainerClass( typeParameter ), TypeVariables.getActualTypeParameter( typeParameter ),
126+
groupConversionHelper, LegacyCollectionSupportValueExtractors.ARRAY );
127+
}
128+
129+
return null;
130+
}
131+
98132
@Override
99133
@SuppressWarnings("unchecked")
100134
public <T extends CascadingMetaData> T as(Class<T> clazz) {

‎engine/src/test/java/org/hibernate/validator/test/internal/engine/valueextraction/GenericModelLegacyCascadingTest.java

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
/*
2+
* Hibernate Validator, declare and validate application constraints
3+
*
4+
* License: Apache License, Version 2.0
5+
* See the license.txt file in the root directory or <http://www.apache.org/licenses/LICENSE-2.0>.
6+
*/
7+
package org.hibernate.validator.test.internal.engine.valueextraction;
8+
9+
import static org.hibernate.validator.testutil.ConstraintViolationAssert.assertThat;
10+
import static org.hibernate.validator.testutil.ConstraintViolationAssert.pathWith;
11+
import static org.hibernate.validator.testutil.ConstraintViolationAssert.violationOf;
12+
import static org.hibernate.validator.testutils.ValidatorUtil.getValidator;
13+
14+
import java.util.Arrays;
15+
import java.util.HashMap;
16+
import java.util.List;
17+
import java.util.Map;
18+
import java.util.Set;
19+
20+
import javax.validation.ConstraintViolation;
21+
import javax.validation.Valid;
22+
import javax.validation.Validator;
23+
import javax.validation.constraints.NotNull;
24+
25+
import org.hibernate.validator.internal.util.CollectionHelper;
26+
import org.hibernate.validator.testutil.TestForIssue;
27+
import org.hibernate.validator.testutils.CandidateForTck;
28+
import org.testng.annotations.Test;
29+
30+
@CandidateForTck
31+
public class GenericModelLegacyCascadingTest {
32+
33+
@Test
34+
@TestForIssue(jiraKey = "HV-1481")
35+
public void testCascadingOnObject() {
36+
Validator validator = getValidator();
37+
38+
GenericModelHolder<InvalidModel> holder = new GenericModelHolder<>();
39+
holder.setModel( new InvalidModel() );
40+
41+
Set<ConstraintViolation<GenericModelHolder<?>>> constraintViolations = validator.validate( holder );
42+
43+
assertThat( constraintViolations ).containsOnlyViolations(
44+
violationOf( NotNull.class )
45+
.withPropertyPath( pathWith()
46+
.property( "model" )
47+
.property( "notNullProperty" ) ) );
48+
}
49+
50+
@Test
51+
@TestForIssue(jiraKey = "HV-1481")
52+
public void testCascadingOnList() {
53+
Validator validator = getValidator();
54+
55+
GenericModelHolder<List<InvalidModel>> holder = new GenericModelHolder<>();
56+
holder.setModel( Arrays.asList( new InvalidModel() ) );
57+
58+
Set<ConstraintViolation<GenericModelHolder<?>>> constraintViolations = validator.validate( holder );
59+
60+
assertThat( constraintViolations ).containsOnlyViolations(
61+
violationOf( NotNull.class )
62+
.withPropertyPath( pathWith()
63+
.property( "model" )
64+
.property( "notNullProperty", true, null, 0, List.class, 0 ) ) );
65+
}
66+
67+
@Test
68+
@TestForIssue(jiraKey = "HV-1481")
69+
public void testCascadingOnMap() {
70+
Validator validator = getValidator();
71+
72+
Map<String, InvalidModel> map = new HashMap<>();
73+
map.put( "invalidModel", new InvalidModel() );
74+
75+
GenericModelHolder<Map<String, InvalidModel>> holder = new GenericModelHolder<>();
76+
holder.setModel( map );
77+
78+
Set<ConstraintViolation<GenericModelHolder<?>>> constraintViolations = validator.validate( holder );
79+
80+
assertThat( constraintViolations ).containsOnlyViolations(
81+
violationOf( NotNull.class )
82+
.withPropertyPath( pathWith()
83+
.property( "model" )
84+
.property( "notNullProperty", true, "invalidModel", null, Map.class, 1 ) ) );
85+
}
86+
87+
@Test
88+
@TestForIssue(jiraKey = "HV-1481")
89+
public void testCascadingOnIterable() {
90+
Validator validator = getValidator();
91+
92+
GenericModelHolder<Set<InvalidModel>> holder = new GenericModelHolder<>();
93+
holder.setModel( CollectionHelper.asSet( new InvalidModel() ) );
94+
95+
Set<ConstraintViolation<GenericModelHolder<?>>> constraintViolations = validator.validate( holder );
96+
97+
assertThat( constraintViolations ).containsOnlyViolations(
98+
violationOf( NotNull.class )
99+
.withPropertyPath( pathWith()
100+
.property( "model" )
101+
.property( "notNullProperty", true, null, null, Iterable.class, 0 ) ) );
102+
}
103+
104+
@Test
105+
@TestForIssue(jiraKey = "HV-1481")
106+
public void testCascadingOnArray() {
107+
Validator validator = getValidator();
108+
109+
GenericModelHolder<InvalidModel[]> holder = new GenericModelHolder<>();
110+
holder.setModel( new InvalidModel[]{ new InvalidModel() } );
111+
112+
Set<ConstraintViolation<GenericModelHolder<?>>> constraintViolations = validator.validate( holder );
113+
114+
assertThat( constraintViolations ).containsOnlyViolations(
115+
violationOf( NotNull.class )
116+
.withPropertyPath( pathWith()
117+
.property( "model" )
118+
.property( "notNullProperty", true, null, 0, Object[].class, null ) ) );
119+
}
120+
121+
private class GenericModelHolder<M> {
122+
123+
private M model;
124+
125+
@Valid
126+
public M getModel() {
127+
return model;
128+
}
129+
130+
public void setModel(M model) {
131+
this.model = model;
132+
}
133+
}
134+
135+
public class InvalidModel {
136+
137+
@NotNull
138+
public Object getNotNullProperty() {
139+
return null;
140+
}
141+
}
142+
}

0 commit comments

Comments
 (0)