Author: hardy.ferentschik Date: 2009-02-24 15:36:01 -0500 (Tue, 24 Feb 2009) New Revision: 16027 Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ValidatorImpl.java validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/util/ReflectionHelper.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/ValidatorImplTest.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/groups/GroupTest.java Log: Various bug fixes for validateValue and validateProperty Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ValidatorImpl.java =================================================================== --- validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ValidatorImpl.java 2009-02-24 20:02:35 UTC (rev 16026) +++ validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ValidatorImpl.java 2009-02-24 20:36:01 UTC (rev 16027) @@ -119,6 +119,13 @@ * {@inheritDoc} */ public <T> Set<ConstraintViolation<T>> validateProperty(T object, String propertyName, Class<?>... groups) { + + sanityCheckPropertyPath( propertyName ); + + if ( object == null ) { + throw new IllegalArgumentException( "Validated object cannot be null." ); + } + List<ConstraintViolationImpl<T>> failingConstraintViolations = new ArrayList<ConstraintViolationImpl<T>>(); validateProperty( object, new PropertyIterator( propertyName ), failingConstraintViolations, groups ); return new HashSet<ConstraintViolation<T>>( failingConstraintViolations ); @@ -128,6 +135,13 @@ * {@inheritDoc} */ public <T> Set<ConstraintViolation<T>> validateValue(Class<T> beanType, String propertyName, Object value, Class<?>... groups) { + + sanityCheckPropertyPath( propertyName ); + + if ( beanType == null ) { + throw new IllegalArgumentException( "The bean type cannot be null." ); + } + List<ConstraintViolationImpl<T>> failingConstraintViolations = new ArrayList<ConstraintViolationImpl<T>>(); validateValue( beanType, value, new PropertyIterator( propertyName ), failingConstraintViolations, groups ); return new HashSet<ConstraintViolation<T>>( failingConstraintViolations ); @@ -140,6 +154,12 @@ return getBeanMetaData( clazz ).getBeanDescriptor(); } + private void sanityCheckPropertyPath(String propertyName) { + if ( propertyName == null || propertyName.length() == 0 ) { + throw new IllegalArgumentException( "Invalid property path." ); + } + } + private Class<?>[] validateGroupVararg(Class<?>[] groups) { if ( groups == null ) { throw new IllegalArgumentException( "null passed as group name" ); @@ -196,14 +216,14 @@ List<Class<?>> defaultGroupSequence = beanMetaData.getDefaultGroupSequence(); if ( log.isDebugEnabled() && defaultGroupSequence.size() > 0 ) { log.debug( - "Executing redefined Default group for bean {} as sequence {}", + "Executing re-defined Default group for bean {} as sequence {}", beanMetaData.getBeanClass().getName(), defaultGroupSequence ); } for ( Class<?> defaultSequenceMember : defaultGroupSequence ) { executionContext.setCurrentGroup( defaultSequenceMember ); - boolean validationSuccessful = validateConstraintsForBean( executionContext, beanMetaData ); + boolean validationSuccessful = validateConstraintsForCurrentGroup( executionContext, beanMetaData ); if ( !validationSuccessful ) { if ( log.isDebugEnabled() ) { log.debug( @@ -216,11 +236,20 @@ } } else { - validateConstraintsForBean( executionContext, beanMetaData ); + validateConstraintsForCurrentGroup( executionContext, beanMetaData ); } } - private <T> boolean validateConstraintsForBean(ExecutionContext<T> executionContext, BeanMetaData<T> beanMetaData) { + /** + * Validates all constraints for the given bean using the current group set in the execution context. + * + * @param executionContext The execution context. + * @param beanMetaData The bean metadata object for the bean to validate. + * + * @return <code>true</code> if the validation was successful (meaning no constraint violations), <code>false</code> + * otherwise. + */ + private <T> boolean validateConstraintsForCurrentGroup(ExecutionContext<T> executionContext, BeanMetaData<T> beanMetaData) { boolean validationSuccessful = true; for ( MetaConstraint metaConstraint : beanMetaData.geMetaConstraintList() ) { executionContext.pushProperty( metaConstraint.getPropertyName() ); @@ -314,15 +343,17 @@ } private <T> void validateProperty(T object, PropertyIterator propertyIter, List<ConstraintViolationImpl<T>> failingConstraintViolations, Class<?>... groups) { - if ( object == null ) { - throw new IllegalArgumentException( "Validated object cannot be null" ); - } + @SuppressWarnings("unchecked") final Class<T> beanType = ( Class<T> ) object.getClass(); Set<MetaConstraint> metaConstraints = new HashSet<MetaConstraint>(); - collectMetaConstraintsForPath( beanType, propertyIter, metaConstraints ); + Object hostingBean = collectMetaConstraintsForPath( beanType, object, propertyIter, metaConstraints ); + if ( hostingBean == null ) { + throw new IllegalArgumentException( "Invalid property path." ); + } + if ( metaConstraints.size() == 0 ) { return; } @@ -337,7 +368,7 @@ continue; } ExecutionContext<T> context = new ExecutionContext<T>( - object, messageInterpolator, constraintValidatorFactory + object, hostingBean, messageInterpolator, constraintValidatorFactory ); context.pushProperty( propertyIter.getOriginalProperty() ); metaConstraint.validateConstraint( object.getClass(), context ); @@ -352,7 +383,7 @@ private <T> void validateValue(Class<T> beanType, Object value, PropertyIterator propertyIter, List<ConstraintViolationImpl<T>> failingConstraintViolations, Class<?>... groups) { Set<MetaConstraint> metaConstraints = new HashSet<MetaConstraint>(); - collectMetaConstraintsForPath( beanType, propertyIter, metaConstraints ); + collectMetaConstraintsForPath( beanType, null, propertyIter, metaConstraints ); if ( metaConstraints.size() == 0 ) { return; @@ -386,21 +417,24 @@ /** * Collects all <code>MetaConstraint</code>s which match the given path relative to the specified root class. * <p> - * This method does not traverse an actual object, but rather tries to resolve the porperty generically. + * This method is called recursively. * </p> - * <p> - * This method is called recursively. Only if there is a valid 'validation path' through the object graph - * a constraint descriptor will be returned. - * </p> * * @param clazz the class type to check for constraints. - * @param propertyIter an instance of <code>PropertyIterator</code> + * @param value While resolving the property path this instance points to the current object. Might be <code>null</code>. + * @param propertyIter an instance of <code>PropertyIterator</code> in order to iterate the items of the original property path. * @param metaConstraints Set of <code>MetaConstraint</code>s to collect all matching constraints. + * + * @return Returns the bean hosting the constraints which match the specified property path. */ - private void collectMetaConstraintsForPath(Class<?> clazz, PropertyIterator propertyIter, Set<MetaConstraint> metaConstraints) { + private Object collectMetaConstraintsForPath(Class<?> clazz, Object value, PropertyIterator propertyIter, Set<MetaConstraint> metaConstraints) { propertyIter.split(); if ( !propertyIter.hasNext() ) { + if ( !ReflectionHelper.containsMember( clazz, propertyIter.getHead() ) ) { + throw new IllegalArgumentException( "Invalid property path." ); + } + List<MetaConstraint> metaConstraintList = getBeanMetaData( clazz ).geMetaConstraintList(); for ( MetaConstraint metaConstraint : metaConstraintList ) { if ( metaConstraint.getPropertyName().equals( propertyIter.getHead() ) ) { @@ -413,17 +447,23 @@ for ( Member m : cascadedMembers ) { if ( ReflectionHelper.getPropertyName( m ).equals( propertyIter.getHead() ) ) { Type type = ReflectionHelper.typeOf( m ); - + value = value == null ? null : ReflectionHelper.getValue( m, value ); if ( propertyIter.isIndexed() ) { type = ReflectionHelper.getIndexedType( type ); + value = value == null ? null : ReflectionHelper.getIndexedValue( + value, propertyIter.getIndex() + ); if ( type == null ) { continue; } } - collectMetaConstraintsForPath( ( Class<?> ) type, propertyIter, metaConstraints ); + collectMetaConstraintsForPath( + ( Class<?> ) type, value, propertyIter, metaConstraints + ); } } } + return value; } /** Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/util/ReflectionHelper.java =================================================================== --- validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/util/ReflectionHelper.java 2009-02-24 20:02:35 UTC (rev 16026) +++ validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/util/ReflectionHelper.java 2009-02-24 20:36:01 UTC (rev 16027) @@ -234,8 +234,6 @@ * * @return The bean method name with the "is" or "get" prefix stripped off, <code>null</code> * the method name id not according to the JavaBeans standard. - * - * @todo reference the JavaBean naming conventions spec here */ public static String getPropertyName(Member member) { String name = null; @@ -253,7 +251,6 @@ name = Introspector.decapitalize( methodName.substring( 3 ) ); } } - return name; } @@ -489,4 +486,32 @@ } return null; } + + /** + * Checks whether the specified class contains a field or member which matches a given property. + * + * @param clazz The class to check. + * @param property The property name. + * + * @return Returns <code>true</code> if the cass contains a field or member for the specified property, <code> + * false</code> otherwise. + */ + public static boolean containsMember(Class clazz, String property) { + try { + clazz.getField( property ); + return true; + } + catch ( NoSuchFieldException e ) { + ; // ignore + } + + try { + clazz.getMethod( "get" + property.substring( 0, 1 ).toUpperCase() + property.substring( 1 ) ); + return true; + } + catch ( NoSuchMethodException e ) { + ; // ignore + } + return false; + } } Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/ValidatorImplTest.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/ValidatorImplTest.java 2009-02-24 20:02:35 UTC (rev 16026) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/ValidatorImplTest.java 2009-02-24 20:36:01 UTC (rev 16027) @@ -102,7 +102,7 @@ } @Test - @SuppressWarnings( "NullArgumentToVariableArgMethod") + @SuppressWarnings("NullArgumentToVariableArgMethod") public void testPassingNullAsGroup() { Validator validator = TestUtil.getValidator(); Customer customer = new Customer(); @@ -176,7 +176,7 @@ } @Test - public void testValidationMethod() { + public void testMultipleValidationMethods() { Validator validator = TestUtil.getValidator(); Address address = new Address(); @@ -224,15 +224,15 @@ Set<ConstraintViolation<Customer>> constraintViolations = validator.validate( customer ); assertEquals( "Wrong number of constraints", 0, constraintViolations.size() ); - Order order1 = new Order(); - customer.addOrder( order1 ); + Order order = new Order(); + customer.addOrder( order ); constraintViolations = validator.validate( customer ); ConstraintViolation constraintViolation = constraintViolations.iterator().next(); assertEquals( "Wrong number of constraints", 1, constraintViolations.size() ); assertEquals( "Wrong message", "may not be null", constraintViolation.getMessage() ); assertEquals( "Wrong root entity", customer, constraintViolation.getRootBean() ); - assertEquals( "Wrong value", order1.getOrderNumber(), constraintViolation.getInvalidValue() ); + assertEquals( "Wrong value", order.getOrderNumber(), constraintViolation.getInvalidValue() ); assertEquals( "Wrong propertyName", "orderList[0].orderNumber", constraintViolation.getPropertyPath() ); } @@ -286,8 +286,6 @@ public void testValidateValue() { Validator validator = TestUtil.getValidator(); - Order order = new Order(); - Set<ConstraintViolation<Customer>> constraintViolations = validator.validateValue( Customer.class, "orderList[0].orderNumber", null ); @@ -297,13 +295,115 @@ assertEquals( "Wrong number of constraints", 1, constraintViolations.size() ); assertEquals( "Wrong message", "may not be null", constraintViolation.getMessage() ); assertEquals( "Wrong root entity", null, constraintViolation.getRootBean() ); - assertEquals( "Wrong value", order.getOrderNumber(), constraintViolation.getInvalidValue() ); + assertEquals( "Wrong value", null, constraintViolation.getInvalidValue() ); assertEquals( "Wrong propertyName", "orderList[0].orderNumber", constraintViolation.getPropertyPath() ); constraintViolations = validator.validateValue( Customer.class, "orderList[0].orderNumber", "1234" ); assertEquals( "Wrong number of constraints", 0, constraintViolations.size() ); } + @Test + public void testValidateValueWithInvalidPropertyPath() { + Validator validator = TestUtil.getValidator(); + + try { + validator.validateValue( Customer.class, "", null ); + fail(); + } + catch ( IllegalArgumentException e ) { + assertEquals( "Invalid property path.", e.getMessage() ); + } + + try { + validator.validateValue( Customer.class, "foobar", null ); + fail(); + } + catch ( IllegalArgumentException e ) { + assertEquals( "Invalid property path.", e.getMessage() ); + } + + try { + validator.validateValue( Customer.class, "orderList[0].foobar", null ); + fail(); + } + catch ( IllegalArgumentException e ) { + assertEquals( "Invalid property path.", e.getMessage() ); + } + } + + @Test + public void testValidateProperty() { + Validator validator = TestUtil.getValidator(); + + Customer customer = new Customer(); + Order order = new Order(); + customer.addOrder( order ); + + Set<ConstraintViolation<Customer>> constraintViolations = validator.validateProperty( + customer, "orderList[0].orderNumber" + ); + assertEquals( "Wrong number of constraints", 1, constraintViolations.size() ); + + ConstraintViolation constraintViolation = constraintViolations.iterator().next(); + assertEquals( "Wrong number of constraints", 1, constraintViolations.size() ); + assertEquals( "Wrong message", "may not be null", constraintViolation.getMessage() ); + assertEquals( "Wrong root entity", customer, constraintViolation.getRootBean() ); + assertEquals( "Wrong value", order.getOrderNumber(), constraintViolation.getInvalidValue() ); + assertEquals( "Wrong propertyName", "orderList[0].orderNumber", constraintViolation.getPropertyPath() ); + + order.setOrderNumber( 1234 ); + constraintViolations = validator.validateProperty( customer, "orderList[0].orderNumber" ); + assertEquals( "Wrong number of constraints", 0, constraintViolations.size() ); + + try { + constraintViolations = validator.validateProperty( customer, "orderList[1].orderNumber" ); + } + catch ( IllegalArgumentException e ) { + assertEquals( "Invalid property path.", e.getMessage() ); + } + } + + @Test + public void testValidatePropertyWithInvalidPropertyPath() { + Validator validator = TestUtil.getValidator(); + + Customer customer = new Customer(); + Order order = new Order(); + customer.addOrder( order ); + + try { + validator.validateProperty( customer, "orderList[1].orderNumber" ); + fail(); + } + catch ( IllegalArgumentException e ) { + assertEquals( "Invalid property path.", e.getMessage() ); + } + + try { + validator.validateProperty( customer, "" ); + fail(); + } + catch ( IllegalArgumentException e ) { + assertEquals( "Invalid property path.", e.getMessage() ); + } + + try { + validator.validateProperty( customer, "foobar" ); + fail(); + } + catch ( IllegalArgumentException e ) { + assertEquals( "Invalid property path.", e.getMessage() ); + } + + try { + validator.validateProperty( customer, "orderList[0].foobar" ); + fail(); + } + catch ( IllegalArgumentException e ) { + assertEquals( "Invalid property path.", e.getMessage() ); + } + } + /** * HV-108 */ Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/groups/GroupTest.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/groups/GroupTest.java 2009-02-24 20:02:35 UTC (rev 16026) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/groups/GroupTest.java 2009-02-24 20:36:01 UTC (rev 16027) @@ -17,7 +17,6 @@ */ package org.hibernate.validation.engine.groups; -import java.util.HashSet; import java.util.Set; import javax.validation.ConstraintViolation; import javax.validation.ValidationException;