Skip to content

Commit e74474d

Browse files
marko-bekhtamarko-bekhta
committed
HV-1831 WIP
Signed-off-by: marko-bekhta <[email protected]>
1 parent 48bfe78 commit e74474d

File tree

3 files changed

+111
-32
lines changed

3 files changed

+111
-32
lines changed

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

Lines changed: 71 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,16 @@
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;
1312
import java.util.Set;
1413

14+
import org.hibernate.validator.internal.engine.valueextraction.ValueExtractorDescriptor;
1515
import org.hibernate.validator.internal.metadata.aggregated.BeanMetaData;
1616
import org.hibernate.validator.internal.metadata.aggregated.CascadingMetaData;
1717
import org.hibernate.validator.internal.metadata.aggregated.ContainerCascadingMetaData;
18+
import org.hibernate.validator.internal.metadata.aggregated.PotentiallyContainerCascadingMetaData;
1819
import org.hibernate.validator.internal.metadata.facets.Cascadable;
1920
import org.hibernate.validator.internal.properties.Signature;
2021
import org.hibernate.validator.internal.util.CollectionHelper;
@@ -29,11 +30,11 @@ public class PredefinedScopeProcessedBeansTrackingStrategy implements ProcessedB
2930

3031
public PredefinedScopeProcessedBeansTrackingStrategy(Map<Class<?>, BeanMetaData<?>> rawBeanMetaDataMap) {
3132
// 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.
33+
// There is a good chance we will need a structure with the whole hierarchy of constraint classes.
34+
// That's something we could add to PredefinedScopeBeanMetaDataManager, as we are already doing similar things
35+
// there (see the ClassHierarchyHelper.getHierarchy call).
36+
// In the predefined scope case, we will have the whole hierarchy of constrained classes passed to
37+
// PredefinedScopeBeanMetaDataManager.
3738

3839
this.trackingEnabledForBeans = CollectionHelper.toImmutableMap(
3940
new TrackingEnabledStrategyBuilder( rawBeanMetaDataMap ).build()
@@ -45,10 +46,21 @@ public PredefinedScopeProcessedBeansTrackingStrategy(Map<Class<?>, BeanMetaData<
4546
private static class TrackingEnabledStrategyBuilder {
4647
private final Map<Class<?>, BeanMetaData<?>> rawBeanMetaDataMap;
4748
private final Map<Class<?>, Boolean> classToBeanTrackingEnabled;
49+
// Map values are a set of subtypes for the key class, including "self" i.e. the "key":
50+
private final Map<Class<?>, Set<Class<?>>> subtypesMap;
4851

4952
TrackingEnabledStrategyBuilder(Map<Class<?>, BeanMetaData<?>> rawBeanMetaDataMap) {
5053
this.rawBeanMetaDataMap = rawBeanMetaDataMap;
5154
this.classToBeanTrackingEnabled = CollectionHelper.newHashMap( rawBeanMetaDataMap.size() );
55+
this.subtypesMap = CollectionHelper.newHashMap( rawBeanMetaDataMap.size() );
56+
for ( Class<?> beanClass : rawBeanMetaDataMap.keySet() ) {
57+
for ( Class<?> otherBeanClass : rawBeanMetaDataMap.keySet() ) {
58+
if ( beanClass.isAssignableFrom( otherBeanClass ) ) {
59+
subtypesMap.computeIfAbsent( beanClass, k -> new HashSet<>() )
60+
.add( otherBeanClass );
61+
}
62+
}
63+
}
5264
}
5365

5466
public Map<Class<?>, Boolean> build() {
@@ -95,8 +107,16 @@ public Map<Class<?>, Boolean> build() {
95107
// -----
96108
// A, B, C have cycles; D does not have a cycle.
97109
//
110+
//
111+
// We also need to account for the case when the subtype is used at runtime that may change the cycles:
112+
// 4) A -> B -> C -> D
113+
// And C1 extends C where C1 -> A
114+
// Hence, at runtime we "may" get:
115+
// A -> B -> C1 -> D
116+
// ^ |
117+
// | |
118+
// -----------
98119
private boolean determineTrackingRequired(Class<?> beanClass, Set<Class<?>> beanClassesInPath) {
99-
100120
final Boolean isBeanTrackingEnabled = classToBeanTrackingEnabled.get( beanClass );
101121
if ( isBeanTrackingEnabled != null ) {
102122
// It was already determined for beanClass.
@@ -157,36 +177,59 @@ private boolean determineTrackingRequired(Class<?> beanClass, Set<Class<?>> bean
157177

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

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 );
194+
if ( beanMetaData == null || !beanMetaData.hasCascadables() ) {
195+
continue;
196+
}
197+
198+
for ( Cascadable cascadable : beanMetaData.getCascadables() ) {
199+
final CascadingMetaData cascadingMetaData = cascadable.getCascadingMetaData();
200+
if ( cascadingMetaData.isContainer() ) {
201+
final ContainerCascadingMetaData containerCascadingMetaData = (ContainerCascadingMetaData) cascadingMetaData;
202+
if ( containerCascadingMetaData.getEnclosingType() instanceof ParameterizedType parameterizedType ) {
203+
for ( Type typeArgument : parameterizedType.getActualTypeArguments() ) {
204+
if ( typeArgument instanceof Class<?> typeArgumentClass ) {
205+
directCascadedBeanClasses.add( typeArgumentClass );
206+
}
207+
else {
208+
throw new UnsupportedOperationException( "Only ParameterizedType values of type Class are supported" );
209+
}
176210
}
177-
else {
178-
throw new UnsupportedOperationException( "Only ParameterizedType values of type Class are supported" );
211+
}
212+
else {
213+
// If we do not have the type arguments then we can go though the value extractors,
214+
// as they are required to define the `@ExtractedValue(type = ???)` ...
215+
// this way we should get the type we want:
216+
for ( ValueExtractorDescriptor valueExtractorCandidate : containerCascadingMetaData.getValueExtractorCandidates() ) {
217+
valueExtractorCandidate.getExtractedType().ifPresent( directCascadedBeanClasses::add );
179218
}
180219
}
181220
}
221+
else if ( cascadingMetaData instanceof PotentiallyContainerCascadingMetaData potentiallyContainerCascadingMetaData ) {
222+
// if it's a potentially container cascading one, we are "in trouble" as thing can be "almost anything".
223+
// TODO: would it be enough to just take the type as defined ?
224+
directCascadedBeanClasses.add( (Class<?>) cascadable.getCascadableType() );
225+
// TODO: or be much more cautious and just assume that it can be "anything":
226+
directCascadedBeanClasses.add( Object.class );
227+
}
182228
else {
183-
throw new UnsupportedOperationException( "Non-parameterized containers are not supported yet." );
229+
// TODO: For now, assume non-container Cascadables are always beans. Truee???
230+
directCascadedBeanClasses.add( (Class<?>) cascadable.getCascadableType() );
184231
}
185232
}
186-
else {
187-
// TODO: For now, assume non-container Cascadables are always beans. Truee???
188-
directCascadedBeanClasses.add( (Class<?>) cascadable.getCascadableType() );
189-
}
190233
}
191234
return directCascadedBeanClasses;
192235
}

‎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)