Skip to content

Commit d2e9beb

Browse files
marko-bekhtamarko-bekhta
committed
HV-1831 WIP
Signed-off-by: marko-bekhta <marko.prykladna@gmail.com>
1 parent 48bfe78 commit d2e9beb

File tree

3 files changed

+102
-33
lines changed

3 files changed

+102
-33
lines changed

‎engine/src/main/java/org/hibernate/validator/internal/engine/tracking/PredefinedScopeProcessedBeansTrackingStrategy.java

Lines changed: 62 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
import java.lang.reflect.ParameterizedType;
88
import java.lang.reflect.Type;
9-
import java.util.Collections;
109
import java.util.HashMap;
1110
import java.util.HashSet;
1211
import java.util.Map;
@@ -15,6 +14,7 @@
1514
import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData;
1615
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaData;
1716
import org.hibernate.validator.internal.metadata.aggregated.ContainerCascadingMetaData;
17+
import org.hibernate.validator.internal.metadata.aggregated.PotentiallyContainerCascadingMetaData;
1818
import org.hibernate.validator.internal.metadata.facets.Cascadable;
1919
import org.hibernate.validator.internal.properties.Signature;
2020
import org.hibernate.validator.internal.util.CollectionHelper;
@@ -29,11 +29,11 @@ public class PredefinedScopeProcessedBeansTrackingStrategy implements ProcessedB
2929

3030
public PredefinedScopeProcessedBeansTrackingStrategy(Map<Class<?>, BeanMetaData<?>> rawBeanMetaDataMap) {
3131
// TODO: build the maps from the information inside the beanMetaDataManager
32-
// There is a good chance we will need a structure with the whole hierarchy of constraint classes.
33-
// That's something we could add to PredefinedScopeBeanMetaDataManager, as we are already doing similar things
34-
// there (see the ClassHierarchyHelper.getHierarchy call).
35-
// In the predefined scope case, we will have the whole hierarchy of constrained classes passed to
36-
// PredefinedScopeBeanMetaDataManager.
32+
// There is a good chance we will need a structure with the whole hierarchy of constraint classes.
33+
// That's something we could add to PredefinedScopeBeanMetaDataManager, as we are already doing similar things
34+
// there (see the ClassHierarchyHelper.getHierarchy call).
35+
// In the predefined scope case, we will have the whole hierarchy of constrained classes passed to
36+
// PredefinedScopeBeanMetaDataManager.
3737

3838
this.trackingEnabledForBeans = CollectionHelper.toImmutableMap(
3939
new TrackingEnabledStrategyBuilder( rawBeanMetaDataMap ).build()
@@ -45,10 +45,21 @@ public PredefinedScopeProcessedBeansTrackingStrategy(Map<Class<?>, BeanMetaData<
4545
private static class TrackingEnabledStrategyBuilder {
4646
private final Map<Class<?>, BeanMetaData<?>> rawBeanMetaDataMap;
4747
private final Map<Class<?>, Boolean> classToBeanTrackingEnabled;
48+
// Map values are a set of subtypes for the key class, including "self" i.e. the "key":
49+
private final Map<Class<?>, Set<Class<?>>> subtypesMap;
4850

4951
TrackingEnabledStrategyBuilder(Map<Class<?>, BeanMetaData<?>> rawBeanMetaDataMap) {
5052
this.rawBeanMetaDataMap = rawBeanMetaDataMap;
5153
this.classToBeanTrackingEnabled = CollectionHelper.newHashMap( rawBeanMetaDataMap.size() );
54+
this.subtypesMap = CollectionHelper.newHashMap( rawBeanMetaDataMap.size() );
55+
for ( Class<?> beanClass : rawBeanMetaDataMap.keySet() ) {
56+
for ( Class<?> otherBeanClass : rawBeanMetaDataMap.keySet() ) {
57+
if ( beanClass.isAssignableFrom( otherBeanClass ) ) {
58+
subtypesMap.computeIfAbsent( beanClass, k -> new HashSet<>() )
59+
.add( otherBeanClass );
60+
}
61+
}
62+
}
5263
}
5364

5465
public Map<Class<?>, Boolean> build() {
@@ -95,8 +106,16 @@ public Map<Class<?>, Boolean> build() {
95106
// -----
96107
// A, B, C have cycles; D does not have a cycle.
97108
//
109+
//
110+
// We also need to account for the case when the subtype is used at runtime that may change the cycles:
111+
// 4) A -> B -> C -> D
112+
// And C1 extends C where C1 -> A
113+
// Hence, at runtime we "may" get:
114+
// A -> B -> C1 -> D
115+
// ^ |
116+
// | |
117+
// -----------
98118
private boolean determineTrackingRequired(Class<?> beanClass, Set<Class<?>> beanClassesInPath) {
99-
100119
final Boolean isBeanTrackingEnabled = classToBeanTrackingEnabled.get( beanClass );
101120
if ( isBeanTrackingEnabled != null ) {
102121
// It was already determined for beanClass.
@@ -157,36 +176,50 @@ private boolean determineTrackingRequired(Class<?> beanClass, Set<Class<?>> bean
157176

158177
// TODO: is there a more concise way to do this?
159178
private <T> Set<Class<?>> getDirectCascadedBeanClasses(Class<T> beanClass) {
160-
final BeanMetaData<?> beanMetaData = rawBeanMetaDataMap.get( beanClass );
161-
162-
if ( beanMetaData == null || !beanMetaData.hasCascadables() ) {
163-
return Collections.emptySet();
179+
final Set<Class<?>> directCascadedBeanClasses = new HashSet<>();
180+
// At runtime, if we are not looking at the root bean the actual value of a cascadable
181+
// can be either the same `beanClass` or one of its subtypes... since subtypes can potentially add
182+
// more constraints we want to iterate through the subclasses (for which there is some metadata defined)
183+
// and include the info from them too.
184+
Set<Class<?>> classes = subtypesMap.get( beanClass );
185+
if ( classes == null ) {
186+
// It may be that some bean property without any constraints is marked for cascading validation,
187+
// In that case the metadata entry will be missing from the map, but we
188+
return Set.of();
164189
}
190+
for ( Class<?> otherBeanClass : classes ) {
191+
final BeanMetaData<?> beanMetaData = rawBeanMetaDataMap.get( otherBeanClass );
165192

166-
final Set<Class<?>> directCascadedBeanClasses = new HashSet<>();
167-
for ( Cascadable cascadable : beanMetaData.getCascadables() ) {
168-
final CascadingMetaData cascadingMetaData = cascadable.getCascadingMetaData();
169-
if ( cascadingMetaData.isContainer() ) {
170-
final ContainerCascadingMetaData containerCascadingMetaData = (ContainerCascadingMetaData) cascadingMetaData;
171-
if ( containerCascadingMetaData.getEnclosingType() instanceof ParameterizedType ) {
172-
ParameterizedType parameterizedType = (ParameterizedType) containerCascadingMetaData.getEnclosingType();
173-
for ( Type typeArgument : parameterizedType.getActualTypeArguments() ) {
174-
if ( typeArgument instanceof Class ) {
175-
directCascadedBeanClasses.add( (Class<?>) typeArgument );
176-
}
177-
else {
178-
throw new UnsupportedOperationException( "Only ParameterizedType values of type Class are supported" );
193+
if ( beanMetaData == null || !beanMetaData.hasCascadables() ) {
194+
continue;
195+
}
196+
197+
for ( Cascadable cascadable : beanMetaData.getCascadables() ) {
198+
final CascadingMetaData cascadingMetaData = cascadable.getCascadingMetaData();
199+
if ( cascadingMetaData.isContainer() ) {
200+
final ContainerCascadingMetaData containerCascadingMetaData = (ContainerCascadingMetaData) cascadingMetaData;
201+
if ( containerCascadingMetaData.getEnclosingType() instanceof ParameterizedType parameterizedType ) {
202+
for ( Type typeArgument : parameterizedType.getActualTypeArguments() ) {
203+
if ( typeArgument instanceof Class ) {
204+
directCascadedBeanClasses.add( (Class<?>) typeArgument );
205+
}
206+
else {
207+
throw new UnsupportedOperationException( "Only ParameterizedType values of type Class are supported" );
208+
}
179209
}
180210
}
211+
else {
212+
throw new UnsupportedOperationException( "Non-parameterized containers are not supported yet." );
213+
}
214+
}
215+
else if ( cascadingMetaData instanceof PotentiallyContainerCascadingMetaData ) {
216+
// if it's a potentially container cascading one ...
181217
}
182218
else {
183-
throw new UnsupportedOperationException( "Non-parameterized containers are not supported yet." );
219+
// TODO: For now, assume non-container Cascadables are always beans. Truee???
220+
directCascadedBeanClasses.add( (Class<?>) cascadable.getCascadableType() );
184221
}
185222
}
186-
else {
187-
// TODO: For now, assume non-container Cascadables are always beans. Truee???
188-
directCascadedBeanClasses.add( (Class<?>) cascadable.getCascadableType() );
189-
}
190223
}
191224
return directCascadedBeanClasses;
192225
}

‎engine/src/test/java/org/hibernate/validator/test/internal/engine/tracking/ProcessedBeansTrackingCycles5Test.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,18 @@
44
*/
55
package org.hibernate.validator.test.internal.engine.tracking;
66

7+
import static org.hibernate.validator.testutil.ConstraintViolationAssert.assertThat;
8+
79
import java.util.List;
10+
import java.util.Set;
811

912
import jakarta.validation.Valid;
1013
import jakarta.validation.Validator;
1114
import jakarta.validation.constraints.NotNull;
1215

1316
import org.hibernate.validator.testutils.ValidatorUtil;
1417

18+
import org.testng.annotations.DataProvider;
1519
import org.testng.annotations.Test;
1620

1721
/**
@@ -28,11 +32,17 @@
2832
*/
2933
public class ProcessedBeansTrackingCycles5Test {
3034

31-
@Test
32-
public void testSerializeHibernateEmail() throws Exception {
33-
Validator validator = ValidatorUtil.getValidator();
35+
@DataProvider(name = "validators")
36+
public Object[][] createValidators() {
37+
return new Object[][] {
38+
{ ValidatorUtil.getValidator() },
39+
{ ValidatorUtil.getPredefinedValidator( Set.of( Parent.class, ChildWithNoCycles.class, Child.class ) ) }
40+
};
41+
}
3442

35-
validator.validate( new Parent() );
43+
@Test(dataProvider = "validators")
44+
public void testCycle(Validator validator) {
45+
assertThat( validator.validate( new Parent() ) ).isNotEmpty();
3646
}
3747

3848
private static class Parent {
@@ -41,6 +51,11 @@ private static class Parent {
4151
private String property;
4252

4353
private List<@Valid ChildWithNoCycles> children;
54+
55+
public Parent() {
56+
this.property = null;
57+
this.children = List.of( new ChildWithNoCycles(), new Child( this ) );
58+
}
4459
}
4560

4661
private static class ChildWithNoCycles {
@@ -53,5 +68,9 @@ private static class Child extends ChildWithNoCycles {
5368

5469
@Valid
5570
private Parent parent;
71+
72+
public Child(Parent parent) {
73+
this.parent = parent;
74+
}
5675
}
5776
}

‎engine/src/test/java/org/hibernate/validator/testutils/ValidatorUtil.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.lang.reflect.InvocationHandler;
1010
import java.lang.reflect.Proxy;
1111
import java.util.Locale;
12+
import java.util.Set;
1213

1314
import jakarta.validation.Configuration;
1415
import jakarta.validation.Validation;
@@ -23,10 +24,12 @@
2324

2425
import org.hibernate.validator.HibernateValidator;
2526
import org.hibernate.validator.HibernateValidatorConfiguration;
27+
import org.hibernate.validator.PredefinedScopeHibernateValidator;
2628
import org.hibernate.validator.constraintvalidation.HibernateConstraintValidatorContext;
2729
import org.hibernate.validator.internal.engine.DefaultClockProvider;
2830
import org.hibernate.validator.internal.engine.constraintvalidation.ConstraintValidatorContextImpl;
2931
import org.hibernate.validator.internal.engine.path.ModifiablePath;
32+
import org.hibernate.validator.internal.metadata.core.ConstraintHelper;
3033
import org.hibernate.validator.messageinterpolation.ExpressionLanguageFeatureLevel;
3134
import org.hibernate.validator.testutil.DummyTraversableResolver;
3235
import org.hibernate.validator.testutil.ValidationInvocationHandler;
@@ -59,6 +62,20 @@ public static Validator getValidator() {
5962
return configuration.buildValidatorFactory().getValidator();
6063
}
6164

65+
public static Validator getPredefinedValidator(Set<Class<?>> beans) {
66+
return getPredefinedValidator( beans, ConstraintHelper.getBuiltinConstraints() );
67+
}
68+
69+
public static Validator getPredefinedValidator(Set<Class<?>> beans, Set<String> constraints) {
70+
return Validation.byProvider( PredefinedScopeHibernateValidator.class )
71+
.configure()
72+
.traversableResolver( new DummyTraversableResolver() )
73+
.builtinConstraints( constraints )
74+
.initializeBeanMetaData( beans )
75+
.buildValidatorFactory()
76+
.getValidator();
77+
}
78+
6279
/**
6380
* Returns the {@code Configuration} object for Hibernate Validator. This method also sets the default locale to
6481
* english.

0 commit comments

Comments
 (0)