Skip to content

Commit

Permalink
HV-421: Throwing ConstraintDeclarationException due to illegal method…
Browse files Browse the repository at this point in the history
… parameter constraints lazily in order to allow for standard bean/property validation being performed on beans with illegal parameter constraints
  • Loading branch information
gunnarmorling committed Feb 16, 2011
1 parent a16b3fd commit 7c35dfd
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 3 deletions.
Expand Up @@ -840,6 +840,11 @@ private <T> void validateParametersInContext(MethodValidationContext<T> validati

BeanMetaData<T> beanMetaData = getBeanMetaData( validationContext.getRootBeanClass() );

//the method invocation can't be validated as this bean has one or more illegal parameter constraints
if ( beanMetaData.getParameterConstraintDefinitionException() != null ) {
throw beanMetaData.getParameterConstraintDefinitionException();
}

if ( beanMetaData.defaultGroupSequenceIsRedefined() ) {
groupChain.assertDefaultGroupSequenceIsExpandable( beanMetaData.getDefaultGroupSequence( object ) );
}
Expand Down
Expand Up @@ -22,6 +22,7 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.validation.ConstraintDefinitionException;
import javax.validation.metadata.BeanDescriptor;
import javax.validation.metadata.PropertyDescriptor;

Expand Down Expand Up @@ -115,4 +116,22 @@ public interface BeanMetaData<T> {
* as cascaded (@Valid).
*/
Set<PropertyDescriptor> getConstrainedProperties();

/**
* Returns a {@link ConstraintDefinitionException} describing an illegal
* method parameter constraint of the represented bean, if such an illegal
* constraint exists. This exception is created - but not thrown - when
* instantiating this meta data object. The rationale for this approach is
* that an illegal method parameter constraint shall not hinder standard
* validation of the represented bean as defined by the Bean Validation API.
* Only if actually a method validation is performed on that bean, this
* exception will be thrown by the validation engine.
*
* @return A {@link ConstraintDefinitionException} describing an illegal
* method parameter constraint of the represented bean, or
* <code>null</code> if the represented bean has no parameter
* constraints at all or only valid ones.
*/
ConstraintDefinitionException getParameterConstraintDefinitionException();

}
Expand Up @@ -128,6 +128,16 @@ public final class BeanMetaDataImpl<T> implements BeanMetaData<T> {
// Used to avoid ReflectionHelper#containsMember which is slow
private final Set<String> propertyNames = new HashSet<String>( 30 );

/**
* A definition exception in case the represented bean contains any illegal
* method parameter constraints. Such illegal parameter constraints shall
* not hinder standard bean/property validation of this type. Therefore this
* exception is created when instantiating this meta data object, but it
* will only be thrown by the validation engine when actually a method
* invocation is validated.
*/
private ConstraintDefinitionException parameterConstraintDefinitionException;

public BeanMetaDataImpl(Class<T> beanClass, ConstraintHelper constraintHelper, BeanMetaDataCache beanMetaDataCache) {
this(
beanClass,
Expand Down Expand Up @@ -214,8 +224,16 @@ public BeanMetaDataImpl(Class<T> beanClass,
}

/**
* <p>
* Checks that there are no invalid parameter constraints defined at this
* type's methods. The following rules apply:
* type's methods. This check will only populate
* {@link BeanMetaDataImpl#parameterConstraintDefinitionException}, but it
* won't be thrown here. This happens lazily, when a method validation is
* actually performed.
* </p>
* <p>
* The following rules apply:
* </p>
* <ul>
* <li>Only the root method of an overridden method in an inheritance
* hierarchy may be annotated with parameter constraints in order to avoid
Expand All @@ -239,22 +257,24 @@ private void checkParameterConstraints() {
);

if ( methodsWithSameSignatureAndParameterConstraints.size() > 1 ) {
throw new ConstraintDefinitionException(
parameterConstraintDefinitionException = new ConstraintDefinitionException(
"Only the root method of an overridden method in an inheritance hierarchy may be annotated with parameter constraints, " +
"but there are parameter constraints defined at all of the following overridden methods: " +
methodsWithSameSignatureAndParameterConstraints
);
return;
}

for ( MethodMetaData oneMethodWithSameSignature : methodsWithSameSignature ) {
if ( !oneMethod.getMethod().getDeclaringClass()
.isAssignableFrom( oneMethodWithSameSignature.getMethod().getDeclaringClass() ) ) {
throw new ConstraintDefinitionException(
parameterConstraintDefinitionException = new ConstraintDefinitionException(
"Only the root method of an overridden method in an inheritance hierarchy may be annotated with parameter constraints. " +
"The following method itself has no parameter constraints but it is not defined one a sub-type of " +
oneMethod.getMethod().getDeclaringClass() +
": " + oneMethodWithSameSignature
);
return;
}
}

Expand Down Expand Up @@ -332,6 +352,10 @@ public Set<PropertyDescriptor> getConstrainedProperties() {
return Collections.unmodifiableSet( new HashSet<PropertyDescriptor>( propertyDescriptors.values() ) );
}

public ConstraintDefinitionException getParameterConstraintDefinitionException() {
return parameterConstraintDefinitionException;
}

private void setDefaultGroupSequence(List<Class<?>> groupSequence) {
defaultGroupSequence = getValidDefaultGroupSequence( groupSequence );
}
Expand Down
Expand Up @@ -16,15 +16,19 @@
*/
package org.hibernate.validator.test.engine.methodlevel;

import java.util.Set;
import javax.validation.ConstraintDefinitionException;
import javax.validation.ConstraintViolation;
import javax.validation.constraints.NotNull;
import javax.validation.constraints.Size;

import org.testng.annotations.Test;

import org.hibernate.validator.metadata.BeanMetaDataImpl;

import static org.hibernate.validator.test.util.TestUtil.assertCorrectConstraintViolationMessages;
import static org.hibernate.validator.test.util.TestUtil.getMethodValidator;
import static org.hibernate.validator.test.util.TestUtil.getValidator;

/**
* Integration test for {@link ValidatorImpl} and {@link BeanMetaDataImpl} which
Expand Down Expand Up @@ -67,6 +71,24 @@ public void parameterConstraintsInHierarchyWithMultipleRootMethodsCausesDefiniti
);
}

@Test(
expectedExceptions = ConstraintDefinitionException.class,
expectedExceptionsMessageRegExp = "Only the root method of an overridden method in an inheritance hierarchy may be annotated with parameter constraints.*"
)
public void standardBeanValidationCanBePerformedOnTypeWithIllegalMethodParameterConstraints() {

QuxImpl qux = new QuxImpl();

//validating a property is fine
Set<ConstraintViolation<QuxImpl>> violations = getValidator().validate( qux );
assertCorrectConstraintViolationMessages( violations, "may not be null" );

//but method validation fails due to illegal parameter constraints being defined
getMethodValidator().validateAllParameters(
qux, QuxImpl.class.getDeclaredMethods()[0], new Object[] { }
);
}

public static interface Foo {

void foo(String s);
Expand Down Expand Up @@ -124,4 +146,28 @@ public void baz(String s) {
}
}

public static interface Qux {

@NotNull
public String getQux();

public void qux(String s);
}

public static interface QuxExt extends Qux {

public void qux(@NotNull String s);
}

public static class QuxImpl implements QuxExt {

public String getQux() {
return null;
}

public void qux(String s) {
}

}

}

0 comments on commit 7c35dfd

Please sign in to comment.