Skip to content

Commit c63b86e

Browse files
marko-bekhtagsmet
authored andcommitted
HV-1363 Reworked the property filtering logic
1 parent e34276b commit c63b86e

34 files changed

+630
-450
lines changed

cdi/src/main/java/org/hibernate/validator/cdi/ValidationExtension.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,8 @@ private <T> void determineConstrainedMethods(AnnotatedType<T> type, BeanDescript
263263
for ( AnnotatedMethod<? super T> annotatedMethod : type.getMethods() ) {
264264
Method method = annotatedMethod.getJavaMember();
265265

266-
boolean isGetter = ReflectionHelper.isGetterMethod( method );
266+
//TODO: need to update the getter logic here:
267+
boolean isGetter = false/*ReflectionHelper.isGetterMethod( method )*/;
267268

268269
// obtain @ValidateOnExecution from the top-most method in the hierarchy
269270
Method methodForExecutableTypeRetrieval = replaceWithOverriddenOrInterfaceMethod( method, overriddenAndImplementedMethods );

engine/src/main/java/org/hibernate/validator/internal/cfg/context/TypeConstraintMappingContextImpl.java

+24-35
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,11 @@
1111

1212
import java.lang.annotation.ElementType;
1313
import java.lang.invoke.MethodHandles;
14-
import java.lang.reflect.Constructor;
15-
import java.lang.reflect.Field;
16-
import java.lang.reflect.Method;
1714
import java.security.AccessController;
1815
import java.security.PrivilegedAction;
1916
import java.util.Arrays;
2017
import java.util.List;
18+
import java.util.Optional;
2119
import java.util.Set;
2220

2321
import org.hibernate.validator.cfg.ConstraintDef;
@@ -35,20 +33,15 @@
3533
import org.hibernate.validator.internal.properties.Callable;
3634
import org.hibernate.validator.internal.properties.Constrainable;
3735
import org.hibernate.validator.internal.properties.Property;
38-
import org.hibernate.validator.internal.properties.javabean.JavaBeanExecutable;
39-
import org.hibernate.validator.internal.properties.javabean.JavaBeanField;
36+
import org.hibernate.validator.internal.properties.javabean.JavaBean;
4037
import org.hibernate.validator.internal.util.Contracts;
4138
import org.hibernate.validator.internal.util.ExecutableHelper;
4239
import org.hibernate.validator.internal.util.TypeResolutionHelper;
4340
import org.hibernate.validator.internal.util.logging.Log;
4441
import org.hibernate.validator.internal.util.logging.LoggerFactory;
45-
import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredConstructor;
46-
import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredField;
47-
import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredMethod;
48-
import org.hibernate.validator.internal.util.privilegedactions.GetMethodFromPropertyName;
4942
import org.hibernate.validator.internal.util.privilegedactions.NewInstance;
50-
import org.hibernate.validator.properties.GetterPropertyMatcher;
5143
import org.hibernate.validator.spi.group.DefaultGroupSequenceProvider;
44+
import org.hibernate.validator.spi.properties.GetterPropertyMatcher;
5245

5346
/**
5447
* Constraint mapping creational context which allows to configure the class-level constraints for one bean.
@@ -58,13 +51,15 @@
5851
* @author Hardy Ferentschik
5952
* @author Gunnar Morling
6053
* @author Kevin Pollet &lt;[email protected]&gt; (C) 2011 SERLI
54+
* @author Marko Bekhta
6155
*/
6256
public final class TypeConstraintMappingContextImpl<C> extends ConstraintMappingContextImplBase
6357
implements TypeConstraintMappingContext<C> {
6458

6559
private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() );
6660

6761
private final Class<C> beanClass;
62+
private final JavaBean javaBean;
6863

6964
private final Set<ExecutableConstraintMappingContextImpl> executableContexts = newHashSet();
7065
private final Set<PropertyConstraintMappingContextImpl> propertyContexts = newHashSet();
@@ -73,12 +68,10 @@ public final class TypeConstraintMappingContextImpl<C> extends ConstraintMapping
7368
private List<Class<?>> defaultGroupSequence;
7469
private Class<? extends DefaultGroupSequenceProvider<? super C>> defaultGroupSequenceProviderClass;
7570

76-
private final GetterPropertyMatcher getterPropertyMatcher;
77-
7871
TypeConstraintMappingContextImpl(DefaultConstraintMapping mapping, Class<C> beanClass, GetterPropertyMatcher getterPropertyMatcher) {
7972
super( mapping );
8073
this.beanClass = beanClass;
81-
this.getterPropertyMatcher = getterPropertyMatcher;
74+
this.javaBean = new JavaBean( getterPropertyMatcher, beanClass );
8275
mapping.getAnnotationProcessingOptions().ignoreAnnotationConstraintForClass( beanClass, Boolean.FALSE );
8376
}
8477

@@ -123,24 +116,24 @@ public PropertyConstraintMappingContext property(String property, ElementType el
123116
Contracts.assertNotNull( elementType, "The element type must not be null." );
124117
Contracts.assertNotEmpty( property, MESSAGES.propertyNameMustNotBeEmpty() );
125118

126-
Property member = getProperty(
119+
Optional<Property> member = getProperty(
127120
beanClass, property, elementType
128121
);
129122

130-
if ( member == null || member.getDeclaringClass() != beanClass ) {
123+
if ( !member.isPresent() || member.get().getDeclaringClass() != beanClass ) {
131124
throw LOG.getUnableToFindPropertyWithAccessException( beanClass, property, elementType );
132125
}
133-
134-
if ( configuredMembers.contains( member ) ) {
126+
Property constrainable = member.get();
127+
if ( configuredMembers.contains( constrainable ) ) {
135128
throw LOG.getPropertyHasAlreadyBeConfiguredViaProgrammaticApiException( beanClass, property );
136129
}
137130

138131
PropertyConstraintMappingContextImpl context = new PropertyConstraintMappingContextImpl(
139132
this,
140-
member
133+
constrainable
141134
);
142135

143-
configuredMembers.add( member );
136+
configuredMembers.add( constrainable );
144137
propertyContexts.add( context );
145138
return context;
146139
}
@@ -149,14 +142,13 @@ public PropertyConstraintMappingContext property(String property, ElementType el
149142
public MethodConstraintMappingContext method(String name, Class<?>... parameterTypes) {
150143
Contracts.assertNotNull( name, MESSAGES.methodNameMustNotBeNull() );
151144

152-
Method method = run( GetDeclaredMethod.action( beanClass, name, parameterTypes ) );
145+
Optional<Callable> method = javaBean.getCallableByNameAndParameters( name, parameterTypes );
153146

154-
if ( method == null || method.getDeclaringClass() != beanClass ) {
155-
throw LOG.getBeanDoesNotContainMethodException( beanClass, name, parameterTypes );
147+
if ( !method.isPresent() || method.get().getDeclaringClass() != beanClass ) {
148+
throw LOG.getBeanDoesNotContainMethodException( javaBean, name, parameterTypes );
156149
}
157150

158-
Callable callable = JavaBeanExecutable.of( getterPropertyMatcher, method );
159-
151+
Callable callable = method.get();
160152
if ( configuredMembers.contains( callable ) ) {
161153
throw LOG.getMethodHasAlreadyBeenConfiguredViaProgrammaticApiException(
162154
beanClass,
@@ -173,16 +165,16 @@ public MethodConstraintMappingContext method(String name, Class<?>... parameterT
173165

174166
@Override
175167
public ConstructorConstraintMappingContext constructor(Class<?>... parameterTypes) {
176-
Constructor<C> constructor = run( GetDeclaredConstructor.action( beanClass, parameterTypes ) );
168+
Optional<Callable> constructor = javaBean.getConstructorByParameters( parameterTypes );
177169

178-
if ( constructor == null || constructor.getDeclaringClass() != beanClass ) {
170+
if ( !constructor.isPresent() || constructor.get().getDeclaringClass() != beanClass ) {
179171
throw LOG.getBeanDoesNotContainConstructorException(
180-
beanClass,
172+
javaBean,
181173
parameterTypes
182174
);
183175
}
184176

185-
Callable callable = JavaBeanExecutable.of( getterPropertyMatcher, constructor );
177+
Callable callable = constructor.get();
186178

187179
if ( configuredMembers.contains( callable ) ) {
188180
throw LOG.getConstructorHasAlreadyBeConfiguredViaProgrammaticApiException(
@@ -263,9 +255,9 @@ protected ConstraintType getConstraintType() {
263255
* @param property The property name without "is", "get" or "has". Cannot be {@code null} or empty.
264256
* @param elementType The element type. Either {@code ElementType.FIELD} or {@code ElementType METHOD}.
265257
*
266-
* @return the property which matches the name and type or {@code null} if no such property exists.
258+
* @return the property which matches the name and type or {@link Optional#empty()} if no such property exists.
267259
*/
268-
private Property getProperty(Class<?> clazz, String property, ElementType elementType) {
260+
private Optional<Property> getProperty(Class<?> clazz, String property, ElementType elementType) {
269261
Contracts.assertNotNull( clazz, MESSAGES.classCannotBeNull() );
270262

271263
if ( property == null || property.length() == 0 ) {
@@ -277,13 +269,10 @@ private Property getProperty(Class<?> clazz, String property, ElementType elemen
277269
}
278270

279271
if ( ElementType.FIELD.equals( elementType ) ) {
280-
Field field = run( GetDeclaredField.action( clazz, property ) );
281-
return field == null ? null : new JavaBeanField( field );
272+
return javaBean.getFieldPropertyByName( property );
282273
}
283274
else {
284-
Method method = GetMethodFromPropertyName.action( clazz, getterPropertyMatcher, property ).run();
285-
286-
return method == null ? null : JavaBeanExecutable.of( getterPropertyMatcher, method ).as( Property.class );
275+
return javaBean.getCallablePropertyByName( property );
287276
}
288277
}
289278

engine/src/main/java/org/hibernate/validator/internal/engine/ValidatorFactoryImpl.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@
6060
import org.hibernate.validator.internal.util.privilegedactions.NewInstance;
6161
import org.hibernate.validator.internal.util.stereotypes.Immutable;
6262
import org.hibernate.validator.internal.util.stereotypes.ThreadSafe;
63-
import org.hibernate.validator.properties.GetterPropertyMatcher;
63+
import org.hibernate.validator.spi.properties.GetterPropertyMatcher;
6464
import org.hibernate.validator.spi.cfg.ConstraintMappingContributor;
6565
import org.hibernate.validator.spi.scripting.ScriptEvaluatorFactory;
6666

engine/src/main/java/org/hibernate/validator/internal/metadata/BeanMetaDataManager.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
import org.hibernate.validator.internal.util.TypeResolutionHelper;
3838
import org.hibernate.validator.internal.util.classhierarchy.ClassHierarchyHelper;
3939
import org.hibernate.validator.internal.util.stereotypes.Immutable;
40-
import org.hibernate.validator.properties.GetterPropertyMatcher;
40+
import org.hibernate.validator.spi.properties.GetterPropertyMatcher;
4141

4242
/**
4343
* This manager is in charge of providing all constraint related meta data

engine/src/main/java/org/hibernate/validator/internal/metadata/provider/AnnotationMetaDataProvider.java

+44-64
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import java.lang.reflect.Field;
2525
import java.lang.reflect.Member;
2626
import java.lang.reflect.Method;
27-
import java.lang.reflect.Modifier;
2827
import java.lang.reflect.Parameter;
2928
import java.lang.reflect.ParameterizedType;
3029
import java.lang.reflect.Type;
@@ -37,8 +36,10 @@
3736
import java.util.HashSet;
3837
import java.util.List;
3938
import java.util.Map;
39+
import java.util.Optional;
4040
import java.util.Set;
4141
import java.util.stream.Collectors;
42+
import java.util.stream.Stream;
4243

4344
import javax.validation.GroupSequence;
4445
import javax.validation.Valid;
@@ -66,6 +67,7 @@
6667
import org.hibernate.validator.internal.properties.Callable;
6768
import org.hibernate.validator.internal.properties.Constrainable;
6869
import org.hibernate.validator.internal.properties.Property;
70+
import org.hibernate.validator.internal.properties.javabean.JavaBean;
6971
import org.hibernate.validator.internal.properties.javabean.JavaBeanExecutable;
7072
import org.hibernate.validator.internal.properties.javabean.JavaBeanField;
7173
import org.hibernate.validator.internal.util.CollectionHelper;
@@ -74,12 +76,11 @@
7476
import org.hibernate.validator.internal.util.annotation.ConstraintAnnotationDescriptor;
7577
import org.hibernate.validator.internal.util.logging.Log;
7678
import org.hibernate.validator.internal.util.logging.LoggerFactory;
77-
import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredConstructors;
78-
import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredFields;
79-
import org.hibernate.validator.internal.util.privilegedactions.GetDeclaredMethods;
79+
import org.hibernate.validator.internal.util.privilegedactions.GetJavaBeanExecutableProperty;
80+
import org.hibernate.validator.internal.util.privilegedactions.GetJavaBeanFieldProperty;
8081
import org.hibernate.validator.internal.util.privilegedactions.GetMethods;
8182
import org.hibernate.validator.internal.util.privilegedactions.NewInstance;
82-
import org.hibernate.validator.properties.GetterPropertyMatcher;
83+
import org.hibernate.validator.spi.properties.GetterPropertyMatcher;
8384
import org.hibernate.validator.spi.group.DefaultGroupSequenceProvider;
8485

8586
/**
@@ -138,23 +139,33 @@ public <T> BeanConfiguration<T> getBeanConfiguration(Class<T> beanClass) {
138139
* @return Retrieves constraint related meta data from the annotations of the given type.
139140
*/
140141
private <T> BeanConfiguration<T> retrieveBeanConfiguration(Class<T> beanClass) {
141-
Set<ConstrainedElement> constrainedElements = getFieldMetaData( beanClass );
142-
constrainedElements.addAll( getMethodMetaData( beanClass ) );
143-
constrainedElements.addAll( getConstructorMetaData( beanClass ) );
142+
JavaBean javaBean = new JavaBean( getterPropertyMatcher, beanClass );
144143

144+
Stream<ConstrainedElement> classLevelStream;
145145
//TODO GM: currently class level constraints are represented by a PropertyMetaData. This
146146
//works but seems somewhat unnatural
147147
Set<MetaConstraint<?>> classLevelConstraints = getClassLevelConstraints( beanClass );
148148
if ( !classLevelConstraints.isEmpty() ) {
149-
ConstrainedType classLevelMetaData =
149+
classLevelStream = Stream.of(
150150
new ConstrainedType(
151151
ConfigurationSource.ANNOTATION,
152152
beanClass,
153153
classLevelConstraints
154-
);
155-
constrainedElements.add( classLevelMetaData );
154+
)
155+
);
156+
}
157+
else {
158+
classLevelStream = Stream.empty();
156159
}
157160

161+
Set<ConstrainedElement> constrainedElements = Stream.concat(
162+
classLevelStream,
163+
javaBean.getAllConstrainables()
164+
.map( this::toConstrainedElement )
165+
.filter( Optional::isPresent )
166+
.map( Optional::get )
167+
).collect( Collectors.toSet() );
168+
158169
return new BeanConfiguration<>(
159170
ConfigurationSource.ANNOTATION,
160171
beanClass,
@@ -164,6 +175,19 @@ private <T> BeanConfiguration<T> retrieveBeanConfiguration(Class<T> beanClass) {
164175
);
165176
}
166177

178+
private Optional<ConstrainedElement> toConstrainedElement(Constrainable constrainable) {
179+
if ( constrainable instanceof JavaBeanField ) {
180+
if ( annotationProcessingOptions.areMemberConstraintsIgnoredFor( constrainable ) ) {
181+
return Optional.empty();
182+
}
183+
return Optional.of( findPropertyMetaData( constrainable.as( Property.class ) ) );
184+
}
185+
else if ( constrainable instanceof JavaBeanExecutable ) {
186+
return Optional.of( findExecutableMetaData( constrainable.as( Callable.class ) ) );
187+
}
188+
throw new IllegalStateException( String.format( "Received unknown constrainable '%s' of type %s", constrainable.getName(), constrainable.getClass() ) );
189+
}
190+
167191
private List<Class<?>> getDefaultGroupSequence(Class<?> beanClass) {
168192
GroupSequence groupSequenceAnnotation = beanClass.getAnnotation( GroupSequence.class );
169193
return groupSequenceAnnotation != null ? Arrays.asList( groupSequenceAnnotation.value() ) : null;
@@ -218,25 +242,8 @@ private Set<MetaConstraint<?>> getClassLevelConstraints(Class<?> clazz) {
218242
return classLevelConstraints;
219243
}
220244

221-
private Set<ConstrainedElement> getFieldMetaData(Class<?> beanClass) {
222-
Set<ConstrainedElement> propertyMetaData = newHashSet();
223-
224-
for ( Field field : run( GetDeclaredFields.action( beanClass ) ) ) {
225-
Property property = new JavaBeanField( field );
226-
// HV-172
227-
if ( Modifier.isStatic( field.getModifiers() ) ||
228-
annotationProcessingOptions.areMemberConstraintsIgnoredFor( property ) ||
229-
field.isSynthetic() ) {
230-
231-
continue;
232-
}
233-
234-
propertyMetaData.add( findPropertyMetaData( field, property ) );
235-
}
236-
return propertyMetaData;
237-
}
238-
239-
private ConstrainedProperty findPropertyMetaData(Field field, Property property) {
245+
private ConstrainedProperty findPropertyMetaData(Property property) {
246+
Field field = run( GetJavaBeanFieldProperty.action( property.as( JavaBeanField.class ) ) );
240247
Set<MetaConstraint<?>> constraints = convertToMetaConstraints(
241248
findConstraints( field, ElementType.FIELD, property ),
242249
property
@@ -269,44 +276,17 @@ private Set<MetaConstraint<?>> convertToMetaConstraints(List<ConstraintDescripto
269276
return constraints;
270277
}
271278

272-
private Set<ConstrainedExecutable> getConstructorMetaData(Class<?> clazz) {
273-
Executable[] declaredConstructors = run( GetDeclaredConstructors.action( clazz ) );
274-
275-
return getMetaData( declaredConstructors );
276-
}
277-
278-
private Set<ConstrainedExecutable> getMethodMetaData(Class<?> clazz) {
279-
Executable[] declaredMethods = run( GetDeclaredMethods.action( clazz ) );
280-
281-
return getMetaData( declaredMethods );
282-
}
283-
284-
private Set<ConstrainedExecutable> getMetaData(Executable[] executableElements) {
285-
Set<ConstrainedExecutable> executableMetaData = newHashSet();
286-
287-
for ( Executable executable : executableElements ) {
288-
// HV-172; ignoring synthetic methods (inserted by the compiler), as they can't have any constraints
289-
// anyway and possibly hide the actual method with the same signature in the built meta model
290-
if ( Modifier.isStatic( executable.getModifiers() ) || executable.isSynthetic() ) {
291-
continue;
292-
}
293-
294-
executableMetaData.add( findExecutableMetaData( executable ) );
295-
}
296-
297-
return executableMetaData;
298-
}
299-
300279
/**
301280
* Finds all constraint annotations defined for the given method or constructor.
302281
*
303-
* @param executable The executable element to check for constraints annotations.
282+
* @param callable The executable element to check for constraints annotations.
304283
*
305284
* @return A meta data object describing the constraints specified for the
306-
* given element.
285+
* given element.
307286
*/
308-
private ConstrainedExecutable findExecutableMetaData(Executable executable) {
309-
Callable callable = JavaBeanExecutable.of( getterPropertyMatcher, executable );
287+
private ConstrainedExecutable findExecutableMetaData(Callable callable) {
288+
Executable executable = run( GetJavaBeanExecutableProperty.action( callable.as( JavaBeanExecutable.class ) ) );
289+
310290
List<ConstrainedParameter> parameterConstraints = getParameterMetaData( executable, callable );
311291

312292
Map<ConstraintType, List<ConstraintDescriptorImpl<?>>> executableConstraints = findConstraints(
@@ -834,7 +814,7 @@ private TypeArgumentExecutableParameterLocation(Executable executable, int index
834814

835815
@Override
836816
public ConstraintLocation toConstraintLocation(GetterPropertyMatcher getterPropertyMatcher) {
837-
return ConstraintLocation.forParameter( JavaBeanExecutable.of( getterPropertyMatcher, executable ), index );
817+
return ConstraintLocation.forParameter( JavaBean.toJavaBeanExecutable( getterPropertyMatcher, executable ), index );
838818
}
839819
}
840820

@@ -860,7 +840,7 @@ private TypeArgumentReturnValueLocation(Executable executable) {
860840

861841
@Override
862842
public ConstraintLocation toConstraintLocation(GetterPropertyMatcher getterPropertyMatcher) {
863-
return ConstraintLocation.forReturnValue( JavaBeanExecutable.of( getterPropertyMatcher, executable ) );
843+
return ConstraintLocation.forReturnValue( JavaBean.toJavaBeanExecutable( getterPropertyMatcher, executable ) );
864844
}
865845
}
866846

0 commit comments

Comments
 (0)