Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

HV-421: Implemented checking of parameter constraints in type hierarc…

…hies
  • Loading branch information...
commit be407914826c4e91efe1f651596490025d3e0e9e 1 parent 3474fdd
@gunnarmorling authored
View
113 hibernate-validator/src/main/java/org/hibernate/validator/metadata/BeanMetaDataImpl.java
@@ -32,6 +32,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
+import javax.validation.ConstraintDefinitionException;
import javax.validation.GroupDefinitionException;
import javax.validation.GroupSequence;
import javax.validation.Valid;
@@ -48,6 +49,7 @@
import static org.hibernate.validator.util.CollectionHelper.newArrayList;
import static org.hibernate.validator.util.CollectionHelper.newHashMap;
+import static org.hibernate.validator.util.CollectionHelper.newHashSet;
import static org.hibernate.validator.util.ReflectionHelper.newInstance;
/**
@@ -85,6 +87,14 @@
private Map<Class<?>, Map<Method, MethodMetaData>> methodMetaConstraints = new HashMap<Class<?>, Map<Method, MethodMetaData>>();
/**
+ * Contains meta data for all method's of this type (including the method's
+ * from its super types). Used only at construction time to determine whether
+ * there are any illegal parameter constraints for overridden methods in a
+ * inheritance tree.
+ */
+ private final Set<MethodMetaData> allMethods = newHashSet();
+
+ /**
* List of cascaded members.
*/
private List<Member> cascadedMembers = new ArrayList<Member>();
@@ -199,6 +209,58 @@ public BeanMetaDataImpl(Class<T> beanClass,
addCascadedMember( member );
}
}
+
+ checkParameterConstraints();
+ }
+
+ /**
+ * Checks that there are no invalid parameter constraints defined at this
+ * type's methods. The following rules apply:
+ * <ul>
+ * <li>Only the root method of an overridden method in an inheritance
+ * hierarchy may be annotated with parameter constraints in order to avoid
+ * the strengthening of a method's preconditions by additional parameter
+ * constraints defined at sub-types. If the root method itself has no
+ * parameter constraints, also no parameter constraints may be added in
+ * sub-types.</li>
+ * <li>If there are multiple root methods for an method in an inheritance
+ * hierarchy (e.g. by implementing two interfaces defining the same method)
+ * no parameter constraints for this method are allowed at all in order to
+ * avoid a strengthening of a method's preconditions in parallel types.</li>
+ * </ul>
+ */
+ private void checkParameterConstraints() {
+
+ for ( MethodMetaData oneMethod : getMethodsWithParameterConstraints( allMethods ) ) {
+
+ Set<MethodMetaData> methodsWithSameSignature = getMethodsWithSameSignature( allMethods, oneMethod );
+ Set<MethodMetaData> methodsWithSameSignatureAndParameterConstraints = getMethodsWithParameterConstraints(
+ methodsWithSameSignature
+ );
+
+ if ( methodsWithSameSignatureAndParameterConstraints.size() > 1 ) {
+ throw 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
+ );
+ }
+
+ for ( MethodMetaData oneMethodWithSameSignature : methodsWithSameSignature ) {
+ if ( !oneMethod.getMethod().getDeclaringClass()
+ .isAssignableFrom( oneMethodWithSameSignature.getMethod().getDeclaringClass() ) ) {
+ throw 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
+ );
+ }
+ }
+
+ }
+
+
}
public Class<T> getBeanClass() {
@@ -336,6 +398,8 @@ private void addMetaConstraint(Class<?> clazz, BeanMetaConstraint<T, ? extends A
private void addMethodMetaConstraint(Class<?> clazz, MethodMetaData methodMetaData) {
+ allMethods.add( methodMetaData );
+
Map<Method, MethodMetaData> constraintsOfClass = methodMetaConstraints.get( clazz );
if ( constraintsOfClass == null ) {
@@ -654,7 +718,8 @@ else if ( constraintHelper.isMultiValueConstraint( annotationType ) ) {
*
* @param method The method to check for constraints annotations.
*
- * @return A list of constraint descriptors for all constraint specified for the given field or method.
+ * @return A meta data object describing the constraints specified for the
+ * given method.
*/
private MethodMetaData getMethodMetaData(Method method) {
@@ -735,6 +800,52 @@ private ConstraintOrigin determineOrigin(Class<?> clazz) {
}
}
+ /**
+ * Returns a set with those methods from the given pile of methods that have
+ * the same method as the specified one. If the given method itself is part
+ * of the specified pile of methods, it also will be contained in the result
+ * set.
+ *
+ * @param methods The methods to search in.
+ * @param methodToCheck The method to compare against.
+ *
+ * @return A set with methods with the same signature as the given one. May
+ * be empty, but never null.
+ */
+ private Set<MethodMetaData> getMethodsWithSameSignature(Iterable<MethodMetaData> methods, MethodMetaData methodToCheck) {
+
+ Set<MethodMetaData> theValue = newHashSet();
+
+ for ( MethodMetaData oneMethod : methods ) {
+ if ( ReflectionHelper.haveSameSignature( oneMethod.getMethod(), methodToCheck.getMethod() ) ) {
+ theValue.add( oneMethod );
+ }
+ }
+ return theValue;
+ }
+
+ /**
+ * Returns a set with those methods from the given pile of methods that have
+ * at least one constrained parameter or at least one parameter annotated
+ * with {@link Valid}.
+ *
+ * @param methods The methods to search in.
+ *
+ * @return A set with constrained methods. May be empty, but never null.
+ */
+ private Set<MethodMetaData> getMethodsWithParameterConstraints(Iterable<MethodMetaData> methods) {
+
+ Set<MethodMetaData> theValue = newHashSet();
+
+ for ( MethodMetaData oneMethod : methods ) {
+ if ( oneMethod.hasParameterConstraints() ) {
+ theValue.add( oneMethod );
+ }
+ }
+
+ return theValue;
+ }
+
@Override
public String toString() {
final StringBuilder sb = new StringBuilder();
View
69 hibernate-validator/src/main/java/org/hibernate/validator/metadata/MethodMetaData.java
@@ -22,6 +22,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
/**
* Represents a method of a Java type and all its associated meta-data relevant
@@ -40,6 +41,8 @@
private final boolean isCascading;
+ private final boolean hasParameterConstraints;
+
public MethodMetaData(
Method method,
List<BeanMetaConstraint<?, ? extends Annotation>> constraints,
@@ -58,6 +61,17 @@ public MethodMetaData(
this.parameterMetaData = parameterMetaData;
this.constraints = constraints;
this.isCascading = isCascading;
+
+ boolean foundParameterConstraint = false;
+
+ for ( Entry<Integer, ParameterMetaData> oneParameter : parameterMetaData.entrySet() ) {
+ if ( oneParameter.getValue().isCascading() || oneParameter.getValue().iterator().hasNext() ) {
+ foundParameterConstraint = true;
+ break;
+ }
+ }
+
+ this.hasParameterConstraints = foundParameterConstraint;
}
/**
@@ -99,11 +113,64 @@ public boolean isCascading() {
return isCascading;
}
+ /**
+ * Whether this method has at least one cascaded parameter or at least one
+ * parameter with constraints.
+ *
+ * @return <code>True</code>, if this method has at least one cascading or
+ * constrained parameter, <code>false</code> otherwise.
+ */
+ public boolean hasParameterConstraints() {
+ return hasParameterConstraints;
+ }
+
@Override
public String toString() {
return "MethodMetaData [method=" + method + ", parameterMetaData="
+ parameterMetaData + ", constraints=" + constraints
- + ", isCascading=" + isCascading + "]";
+ + ", isCascading=" + isCascading + ", hasParameterConstraints="
+ + hasParameterConstraints + "]";
+ }
+
+ /**
+ * It is expected that there is exactly one instance of this type for a
+ * given method in a type system. This method is therefore only based on the
+ * hash code defined by the represented {@link Method}.
+ */
+ @Override
+ public int hashCode() {
+ final int prime = 31;
+ int result = 1;
+ result = prime * result + ( ( method == null ) ? 0 : method.hashCode() );
+ return result;
+ }
+
+ /**
+ * It is expected that there is exactly one instance of this type for a
+ * given method in a type system. This method is therefore only based on
+ * the equality as defined by the represented {@link Method}.
+ */
+ @Override
+ public boolean equals(Object obj) {
+ if ( this == obj ) {
+ return true;
+ }
+ if ( obj == null ) {
+ return false;
+ }
+ if ( getClass() != obj.getClass() ) {
+ return false;
+ }
+ MethodMetaData other = (MethodMetaData) obj;
+ if ( method == null ) {
+ if ( other.method != null ) {
+ return false;
+ }
+ }
+ else if ( !method.equals( other.method ) ) {
+ return false;
+ }
+ return true;
}
}
View
22 ...e-validator/src/test/java/org/hibernate/validator/test/engine/methodlevel/MethodLevelValidationTest.java
@@ -379,28 +379,6 @@ public void validFromOverriddenMethodIsEvaluated() {
}
@Test
- public void constraintsAtOverridingMethodAreEvaluated() {
-
- try {
-
- customerRepository.foo( Long.valueOf( 0 ) );
- fail( "Expected MethodConstraintViolationException wasn't thrown." );
- }
- catch ( MethodConstraintViolationException e ) {
-
- assertEquals( e.getConstraintViolations().size(), 1 );
-
- MethodConstraintViolation<?> constraintViolation = e.getConstraintViolations().iterator().next();
- assertEquals( constraintViolation.getMessage(), "must be greater than or equal to 1" );
- assertEquals( constraintViolation.getMethod().getName(), "foo" );
- assertEquals( constraintViolation.getMethod().getDeclaringClass(), CustomerRepository.class );
- assertEquals( constraintViolation.getParameterIndex(), Integer.valueOf( 0 ) );
- assertEquals( constraintViolation.getKind(), Kind.PARAMETER );
- assertEquals( constraintViolation.getRootBeanClass(), CustomerRepositoryImpl.class );
- }
- }
-
- @Test
public void parameterValidationOfParameterlessMethod() {
customerRepository.boz();
}
View
2  ...-validator/src/test/java/org/hibernate/validator/test/engine/methodlevel/service/CustomerRepository.java
@@ -45,7 +45,7 @@
void cascadingParameter(@NotNull @Valid Customer param1, @NotNull @Valid Customer param2);
- void foo(@Min(1) Long id);
+ void foo(Long id);
void bar(Customer customer);
View
87 hibernate-validator/src/test/java/org/hibernate/validator/test/metadata/BeanMetaDataImplTest.java
@@ -18,7 +18,10 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.Method;
+import javax.validation.ConstraintDefinitionException;
import javax.validation.constraints.Min;
+import javax.validation.constraints.NotNull;
+import javax.validation.constraints.Size;
import org.testng.annotations.Test;
@@ -95,6 +98,33 @@ public void cascadingConstraintAtMethodReturnValue() throws Exception {
return new BeanMetaDataImpl<T>( clazz, new ConstraintHelper(), new BeanMetaDataCache() );
}
+ @Test(
+ expectedExceptions = ConstraintDefinitionException.class,
+ expectedExceptionsMessageRegExp = "Only the root method of an overridden method in an inheritance hierarchy may be annotated with parameter constraints\\. The following.*"
+ )
+ public void parameterConstraintsAddedInSubTypeCausesDefinitionException() {
+
+ setupBeanMetaData( FooExtImpl.class );
+ }
+
+ @Test(
+ expectedExceptions = ConstraintDefinitionException.class,
+ expectedExceptionsMessageRegExp = "Only the root method of an overridden method in an inheritance hierarchy may be annotated with parameter constraints, but there are.*"
+ )
+ public void constraintStrengtheningInSubTypeCausesDefinitionException() {
+
+ setupBeanMetaData( BarExtImpl.class );
+ }
+
+ @Test(
+ expectedExceptions = ConstraintDefinitionException.class,
+ expectedExceptionsMessageRegExp = "Only the root method of an overridden method in an inheritance hierarchy may be annotated with parameter constraints\\. The following.*"
+ )
+ public void parameterConstraintsInHierarchyWithMultipleRootMethodsCausesDefinitionException() {
+
+ setupBeanMetaData( BazImpl.class );
+ }
+
private void assertSize(Iterable<?> iterable, int expectedCount) {
int i = 0;
@@ -105,4 +135,61 @@ private void assertSize(Iterable<?> iterable, int expectedCount) {
assertEquals( i, expectedCount, "Actual size of iterable [" + iterable + "] differed from expected size." );
}
+
+ public static interface Foo {
+
+ void foo(String s);
+ }
+
+ public static interface FooExt extends Foo {
+
+ /**
+ * Adds constraints to an un-constrained method from a super-type, which is not allowed.
+ */
+ void foo(@NotNull String s);
+ }
+
+ public static class FooExtImpl implements FooExt {
+
+ public void foo(String s) {
+ }
+ }
+
+ public static interface Bar {
+
+ void bar(@NotNull String s);
+ }
+
+ public static interface BarExt extends Bar {
+
+ /**
+ * Adds constraints to a constrained method from a super-type, which is not allowed.
+ */
+ void bar(@Size(min = 3) String s);
+ }
+
+ public static class BarExtImpl implements BarExt {
+
+ public void bar(String s) {
+ }
+ }
+
+ public static interface Baz1 {
+
+ void baz(String s);
+ }
+
+ public static interface Baz2 {
+
+ void baz(@Size(min = 3) String s);
+ }
+
+ public static class BazImpl implements Baz1, Baz2 {
+
+ /**
+ * Implements a method defined by two interfaces, with di a super-type, which is not allowed.
+ */
+ public void baz(String s) {
+ }
+ }
}
Please sign in to comment.
Something went wrong with that request. Please try again.