Skip to content

HV-1363 support for non-standard Java beans #938

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed

HV-1363 support for non-standard Java beans #938

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9b26462
HV-1363 Build abstraction over reflection in ConstraintLocation and r…
marko-bekhta Mar 10, 2018
dd51f80
fixup! HV-1363 Build abstraction over reflection in ConstraintLocatio…
gsmet May 7, 2018
e34276b
HV-1363 Moved getter detection logic to separate class
marko-bekhta Mar 11, 2018
c63b86e
HV-1363 Reworked the property filtering logic
marko-bekhta Mar 18, 2018
4cb8d13
HV-1363 Added ability to configure property filter for ValidatorFactory
marko-bekhta Mar 18, 2018
1b55886
HV-1363 Updated ValidationExtension to use new property filtering
marko-bekhta Mar 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 15 additions & 26 deletions engine/src/main/java/org/hibernate/validator/internal/cfg/context/ConfiguredConstraint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@
import java.lang.annotation.ElementType;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.reflect.Executable;
import java.lang.reflect.Field;
import java.lang.reflect.Member;
import java.lang.reflect.Method;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.security.AccessController;
Expand All @@ -24,7 +20,9 @@
import org.hibernate.validator.cfg.AnnotationDef;
import org.hibernate.validator.cfg.ConstraintDef;
import org.hibernate.validator.internal.metadata.location.ConstraintLocation;
import org.hibernate.validator.internal.util.ExecutableHelper;
import org.hibernate.validator.internal.properties.Callable;
import org.hibernate.validator.internal.properties.Property;
import org.hibernate.validator.internal.properties.javabean.JavaBeanField;
import org.hibernate.validator.internal.util.annotation.AnnotationDescriptor;
import org.hibernate.validator.internal.util.annotation.ConstraintAnnotationDescriptor;
import org.hibernate.validator.internal.util.logging.Log;
Expand Down Expand Up @@ -58,42 +56,33 @@ static <A extends Annotation> ConfiguredConstraint<A> forType(ConstraintDef<?, A
return new ConfiguredConstraint<>( constraint, ConstraintLocation.forClass( beanType ), ElementType.TYPE );
}

static <A extends Annotation> ConfiguredConstraint<A> forProperty(ConstraintDef<?, A> constraint, Member member) {
if ( member instanceof Field ) {
return new ConfiguredConstraint<>(
constraint,
ConstraintLocation.forField( (Field) member ),
ElementType.FIELD
);
}
else {
return new ConfiguredConstraint<>(
static <A extends Annotation> ConfiguredConstraint<A> forProperty(ConstraintDef<?, A> constraint, Property property) {
return new ConfiguredConstraint<>(
constraint,
ConstraintLocation.forGetter( (Method) member ),
ElementType.METHOD
);
}
ConstraintLocation.forProperty( property ),
property instanceof JavaBeanField ? ElementType.FIELD : ElementType.METHOD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit surprised we don't also have an abstraction on ElementType. I think it's going to be in the way.

It's pretty obvious from the way we have to special case things whereas the ElementType could be returned by the Property object or the Callable in the case of a method/constructor.

Maybe we could call it ConstraintLocationType.

AFAICS, it's only exposed in BV in the TraversableResolver (and considering it only applies to Java Bean properties (either fields or getters), we could check that the ConstraintLocationType is valid and translate it then call a getElementType() method which would be present on JavaBeanField and JavaBeanGetter) and in ConstraintFinder (we probably could have a translation layer there too).

Did you think about it?

);
}

public static <A extends Annotation> ConfiguredConstraint<A> forParameter(ConstraintDef<?, A> constraint, Executable executable, int parameterIndex) {
public static <A extends Annotation> ConfiguredConstraint<A> forParameter(ConstraintDef<?, A> constraint, Callable callable, int parameterIndex) {
return new ConfiguredConstraint<>(
constraint, ConstraintLocation.forParameter( executable, parameterIndex ), ExecutableHelper.getElementType( executable )
constraint, ConstraintLocation.forParameter( callable, parameterIndex ), callable.isConstructor() ? ElementType.CONSTRUCTOR : ElementType.METHOD
);
}

public static <A extends Annotation> ConfiguredConstraint<A> forExecutable(ConstraintDef<?, A> constraint, Executable executable) {
public static <A extends Annotation> ConfiguredConstraint<A> forExecutable(ConstraintDef<?, A> constraint, Callable callable) {
return new ConfiguredConstraint<>(
constraint, ConstraintLocation.forReturnValue( executable ), ExecutableHelper.getElementType( executable )
constraint, ConstraintLocation.forReturnValue( callable ), callable.isConstructor() ? ElementType.CONSTRUCTOR : ElementType.METHOD
);
}

public static <A extends Annotation> ConfiguredConstraint<A> forCrossParameter(ConstraintDef<?, A> constraint, Executable executable) {
public static <A extends Annotation> ConfiguredConstraint<A> forCrossParameter(ConstraintDef<?, A> constraint, Callable callable) {
return new ConfiguredConstraint<>(
constraint, ConstraintLocation.forCrossParameter( executable ), ExecutableHelper.getElementType( executable )
constraint, ConstraintLocation.forCrossParameter( callable ), callable.isConstructor() ? ElementType.CONSTRUCTOR : ElementType.METHOD
);
}

public static <A extends Annotation> ConfiguredConstraint<A> forTypeArgument(ConstraintDef<?, A> constraint,ConstraintLocation delegate, TypeVariable<?> typeArgument, Type typeOfAnnotatedElement) {
public static <A extends Annotation> ConfiguredConstraint<A> forTypeArgument(ConstraintDef<?, A> constraint, ConstraintLocation delegate, TypeVariable<?> typeArgument, Type typeOfAnnotatedElement) {
return new ConfiguredConstraint<>(
constraint,
ConstraintLocation.forTypeArgument( delegate, typeArgument, typeOfAnnotatedElement ),
Expand Down
7 changes: 3 additions & 4 deletions ...org/hibernate/validator/internal/cfg/context/ConstructorConstraintMappingContextImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
*/
package org.hibernate.validator.internal.cfg.context;

import java.lang.reflect.Constructor;

import org.hibernate.validator.cfg.context.ConstructorConstraintMappingContext;
import org.hibernate.validator.internal.properties.Callable;

/**
* Constraint mapping creational context representing a constructor.
Expand All @@ -17,15 +16,15 @@
*/
class ConstructorConstraintMappingContextImpl extends ExecutableConstraintMappingContextImpl implements ConstructorConstraintMappingContext {

<T> ConstructorConstraintMappingContextImpl(TypeConstraintMappingContextImpl<T> typeContext, Constructor<T> constructor) {
<T> ConstructorConstraintMappingContextImpl(TypeConstraintMappingContextImpl<T> typeContext, Callable constructor) {
super( typeContext, constructor );
}

@Override
public ConstructorConstraintMappingContext ignoreAnnotations(boolean ignoreAnnotations) {
typeContext.mapping
.getAnnotationProcessingOptions()
.ignoreConstraintAnnotationsOnMember( executable, ignoreAnnotations );
.ignoreConstraintAnnotationsOnMember( callable, ignoreAnnotations );

return this;
}
Expand Down
4 changes: 2 additions & 2 deletions .../hibernate/validator/internal/cfg/context/CrossParameterConstraintMappingContextImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ final class CrossParameterConstraintMappingContextImpl

@Override
public CrossParameterConstraintMappingContext constraint(ConstraintDef<?, ?> definition) {
super.addConstraint( ConfiguredConstraint.forCrossParameter( definition, executableContext.getExecutable() ) );
super.addConstraint( ConfiguredConstraint.forCrossParameter( definition, executableContext.getCallable() ) );
return this;
}

@Override
public CrossParameterConstraintMappingContext ignoreAnnotations(boolean ignoreAnnotations) {
mapping.getAnnotationProcessingOptions().ignoreConstraintAnnotationsForCrossParameterConstraint(
executableContext.getExecutable(), ignoreAnnotations
executableContext.getCallable(), ignoreAnnotations
);
return this;
}
Expand Down
31 changes: 15 additions & 16 deletions .../org/hibernate/validator/internal/cfg/context/ExecutableConstraintMappingContextImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import static org.hibernate.validator.internal.util.CollectionHelper.newArrayList;

import java.lang.invoke.MethodHandles;
import java.lang.reflect.Executable;
import java.util.Collections;
import java.util.List;

Expand All @@ -24,7 +23,7 @@
import org.hibernate.validator.internal.metadata.raw.ConstrainedElement;
import org.hibernate.validator.internal.metadata.raw.ConstrainedExecutable;
import org.hibernate.validator.internal.metadata.raw.ConstrainedParameter;
import org.hibernate.validator.internal.util.ReflectionHelper;
import org.hibernate.validator.internal.properties.Callable;
import org.hibernate.validator.internal.util.TypeResolutionHelper;
import org.hibernate.validator.internal.util.logging.Log;
import org.hibernate.validator.internal.util.logging.LoggerFactory;
Expand All @@ -41,28 +40,28 @@ abstract class ExecutableConstraintMappingContextImpl {
private static final Log LOG = LoggerFactory.make( MethodHandles.lookup() );

protected final TypeConstraintMappingContextImpl<?> typeContext;
protected final Executable executable;
protected final Callable callable;
private final ParameterConstraintMappingContextImpl[] parameterContexts;
private ReturnValueConstraintMappingContextImpl returnValueContext;
private CrossParameterConstraintMappingContextImpl crossParameterContext;

protected ExecutableConstraintMappingContextImpl(TypeConstraintMappingContextImpl<?> typeContext, Executable executable) {
protected ExecutableConstraintMappingContextImpl(TypeConstraintMappingContextImpl<?> typeContext, Callable callable) {
this.typeContext = typeContext;
this.executable = executable;
this.parameterContexts = new ParameterConstraintMappingContextImpl[executable.getParameterTypes().length];
this.callable = callable;
this.parameterContexts = new ParameterConstraintMappingContextImpl[callable.getParameterTypes().length];
}

public ParameterConstraintMappingContext parameter(int index) {
if ( index < 0 || index >= executable.getParameterTypes().length ) {
throw LOG.getInvalidExecutableParameterIndexException( executable, index );
if ( index < 0 || index >= callable.getParameterTypes().length ) {
throw LOG.getInvalidExecutableParameterIndexException( callable, index );
}

ParameterConstraintMappingContextImpl context = parameterContexts[index];

if ( context != null ) {
throw LOG.getParameterHasAlreadyBeConfiguredViaProgrammaticApiException(
typeContext.getBeanClass(),
executable,
callable,
index
);
}
Expand All @@ -76,7 +75,7 @@ public CrossParameterConstraintMappingContext crossParameter() {
if ( crossParameterContext != null ) {
throw LOG.getCrossParameterElementHasAlreadyBeConfiguredViaProgrammaticApiException(
typeContext.getBeanClass(),
executable
callable
);
}

Expand All @@ -88,16 +87,16 @@ public ReturnValueConstraintMappingContext returnValue() {
if ( returnValueContext != null ) {
throw LOG.getReturnValueHasAlreadyBeConfiguredViaProgrammaticApiException(
typeContext.getBeanClass(),
executable
callable
);
}

returnValueContext = new ReturnValueConstraintMappingContextImpl( this );
return returnValueContext;
}

public Executable getExecutable() {
return executable;
public Callable getCallable() {
return callable;
}

public TypeConstraintMappingContextImpl<?> getTypeContext() {
Expand All @@ -108,7 +107,7 @@ public ConstrainedElement build(ConstraintHelper constraintHelper, TypeResolutio
ValueExtractorManager valueExtractorManager) {
return new ConstrainedExecutable(
ConfigurationSource.API,
executable,
callable,
getParameters( constraintHelper, typeResolutionHelper, valueExtractorManager ),
crossParameterContext != null ? crossParameterContext.getConstraints( constraintHelper, typeResolutionHelper, valueExtractorManager ) : Collections.<MetaConstraint<?>>emptySet(),
returnValueContext != null ? returnValueContext.getConstraints( constraintHelper, typeResolutionHelper, valueExtractorManager ) : Collections.<MetaConstraint<?>>emptySet(),
Expand All @@ -130,8 +129,8 @@ private List<ConstrainedParameter> getParameters(ConstraintHelper constraintHelp
constrainedParameters.add(
new ConstrainedParameter(
ConfigurationSource.API,
executable,
ReflectionHelper.typeOf( executable, i ),
callable,
callable.typeOfParameter( i ),
i
)
);
Expand Down
9 changes: 4 additions & 5 deletions ...java/org/hibernate/validator/internal/cfg/context/MethodConstraintMappingContextImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@
*/
package org.hibernate.validator.internal.cfg.context;

import java.lang.reflect.Method;

import org.hibernate.validator.cfg.context.MethodConstraintMappingContext;
import org.hibernate.validator.internal.properties.Callable;

/**
* Constraint mapping creational context representing a method.
Expand All @@ -17,15 +16,15 @@
*/
class MethodConstraintMappingContextImpl extends ExecutableConstraintMappingContextImpl implements MethodConstraintMappingContext {

MethodConstraintMappingContextImpl(TypeConstraintMappingContextImpl<?> typeContext, Method method) {
super( typeContext, method );
MethodConstraintMappingContextImpl(TypeConstraintMappingContextImpl<?> typeContext, Callable callable) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I don't have your vision about the full patch/future developments so these are real questions.

Is this really what we want to do or are we talking about JavaBeanExecutable?

I mean do we expect these things to be compatible with anything else than JavaBeanExecutable or is it specific to it?

Considering we are introducing the difference between Method and Constructor which is not present in Callable, I'm thinking we are talking about Java beans here.

To be honest, I'm wondering if we should introduce Method and Constructor as subinterfaces of Callable too.

Also wondering if we should create different objects for JavaBeanMethod and JavaBeanConstructor (which could inherit from an AbstractJavaBeanExecutable). They seem to be different beasts so I would have different objects for them (same as Getter and Field).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't think of any other kind of executables/callables at this point. But the idea was to not limit ourselves with something specific if the more generic thing could be used.

I'll get back to the Method/Constructor part after I'll go through all the other comments and remind myself of what's happening :)

super( typeContext, callable );
}

@Override
public MethodConstraintMappingContext ignoreAnnotations(boolean ignoreAnnotations) {
typeContext.mapping
.getAnnotationProcessingOptions()
.ignoreConstraintAnnotationsOnMember( executable, ignoreAnnotations );
.ignoreConstraintAnnotationsOnMember( callable, ignoreAnnotations );

return this;
}
Expand Down
15 changes: 7 additions & 8 deletions ...a/org/hibernate/validator/internal/cfg/context/ParameterConstraintMappingContextImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.hibernate.validator.internal.metadata.location.ConstraintLocation;
import org.hibernate.validator.internal.metadata.raw.ConfigurationSource;
import org.hibernate.validator.internal.metadata.raw.ConstrainedParameter;
import org.hibernate.validator.internal.util.ReflectionHelper;
import org.hibernate.validator.internal.util.TypeResolutionHelper;

/**
Expand All @@ -41,7 +40,7 @@ final class ParameterConstraintMappingContextImpl
ParameterConstraintMappingContextImpl(ExecutableConstraintMappingContextImpl executableContext, int parameterIndex) {
super(
executableContext.getTypeContext().getConstraintMapping(),
executableContext.executable.getGenericParameterTypes()[parameterIndex]
executableContext.callable.getGenericParameterTypes()[parameterIndex]
);

this.executableContext = executableContext;
Expand All @@ -58,7 +57,7 @@ public ParameterConstraintMappingContext constraint(ConstraintDef<?, ?> definiti
super.addConstraint(
ConfiguredConstraint.forParameter(
definition,
executableContext.getExecutable(),
executableContext.getCallable(),
parameterIndex
)
);
Expand All @@ -68,7 +67,7 @@ public ParameterConstraintMappingContext constraint(ConstraintDef<?, ?> definiti
@Override
public ParameterConstraintMappingContext ignoreAnnotations(boolean ignoreAnnotations) {
mapping.getAnnotationProcessingOptions().ignoreConstraintAnnotationsOnParameter(
executableContext.getExecutable(),
executableContext.getCallable(),
parameterIndex,
ignoreAnnotations
);
Expand Down Expand Up @@ -105,7 +104,7 @@ public ContainerElementConstraintMappingContext containerElementType() {
return super.containerElement(
this,
executableContext.getTypeContext(),
ConstraintLocation.forParameter( executableContext.getExecutable(), parameterIndex )
ConstraintLocation.forParameter( executableContext.getCallable(), parameterIndex )
);
}

Expand All @@ -114,19 +113,19 @@ public ContainerElementConstraintMappingContext containerElementType(int index,
return super.containerElement(
this,
executableContext.getTypeContext(),
ConstraintLocation.forParameter( executableContext.getExecutable(), parameterIndex ),
ConstraintLocation.forParameter( executableContext.getCallable(), parameterIndex ),
index,
nestedIndexes
);
}

public ConstrainedParameter build(ConstraintHelper constraintHelper, TypeResolutionHelper typeResolutionHelper,
ValueExtractorManager valueExtractorManager) {
Type parameterType = ReflectionHelper.typeOf( executableContext.getExecutable(), parameterIndex );
Type parameterType = executableContext.getCallable().typeOfParameter( parameterIndex );

return new ConstrainedParameter(
ConfigurationSource.API,
executableContext.getExecutable(),
executableContext.getCallable(),
parameterType,
parameterIndex,
getConstraints( constraintHelper, typeResolutionHelper, valueExtractorManager ),
Expand Down
Loading