Author: epbernard Date: 2009-05-05 08:40:55 -0400 (Tue, 05 May 2009) New Revision: 16510 Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ExecutionContext.java validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ValidatorImpl.java validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/DefaultTraversableResolver.java validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/JPATraversableResolver.java validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/SingleThreadCachedTraversableResolver.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/bootstrap/ValidationTest.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/CachedTraversableResolverTest.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/SnifferTraversableResolver.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/TraversableResolverTest.java Log: BVAL-152 HV-160 TraversableResolver should differentiate reachability and cascadability Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ExecutionContext.java =================================================================== --- validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ExecutionContext.java 2009-05-05 12:36:30 UTC (rev 16509) +++ validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ExecutionContext.java 2009-05-05 12:40:55 UTC (rev 16510) @@ -289,7 +289,7 @@ return false; } - return traversableResolver.isTraversable( + return traversableResolver.isReachable( peekCurrentBean(), peekProperty(), getRootBeanClass(), @@ -299,12 +299,24 @@ } public boolean isCascadeRequired(Member member) { - return traversableResolver.isTraversable( - peekCurrentBean(), - peekProperty(), - getRootBeanClass(), - peekParentPath(), - member instanceof Field ? ElementType.FIELD : ElementType.METHOD + final ElementType type = member instanceof Field ? ElementType.FIELD : ElementType.METHOD; + final String parentPath = peekParentPath(); + final Class<T> rootBeanType = getRootBeanClass(); + final String traversableProperty = peekProperty(); + final Object traversableobject = peekCurrentBean(); + return traversableResolver.isReachable( + traversableobject, + traversableProperty, + rootBeanType, + parentPath, + type + ) + && traversableResolver.isCascadable( + traversableobject, + traversableProperty, + rootBeanType, + parentPath, + type ); } 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-05-05 12:36:30 UTC (rev 16509) +++ validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/ValidatorImpl.java 2009-05-05 12:40:55 UTC (rev 16510) @@ -260,7 +260,6 @@ */ private <T> boolean validateConstraintsForCurrentGroup(ExecutionContext<T> executionContext, BeanMetaData<T> beanMetaData) { boolean validationSuccessful = true; - //FIXME isValidationRequired for all constraints in a given context May need to divide by elementType for ( MetaConstraint<T, ?> metaConstraint : beanMetaData.geMetaConstraintList() ) { executionContext.pushProperty( metaConstraint.getPropertyName() ); if ( executionContext.isValidationRequired( metaConstraint ) ) { @@ -272,10 +271,11 @@ return validationSuccessful; } + //this method must always be called after validateConstraints for the same context + //TODO define a validate that calls validateConstraints and then validateCascadedConstraints private <T> void validateCascadedConstraints(ExecutionContext<T> context) { List<Member> cascadedMembers = getBeanMetaData( context.peekCurrentBeanType() ) .getCascadedMembers(); - //FIXME isValidationRequired for all constraints in a given context May need to divide by elementType for ( Member member : cascadedMembers ) { Type type = ReflectionHelper.typeOf( member ); context.pushProperty( ReflectionHelper.getPropertyName( member ) ); Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/DefaultTraversableResolver.java =================================================================== --- validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/DefaultTraversableResolver.java 2009-05-05 12:36:30 UTC (rev 16509) +++ validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/DefaultTraversableResolver.java 2009-05-05 12:40:55 UTC (rev 16510) @@ -38,12 +38,6 @@ detectJPA(); } - public boolean isTraversable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { - return jpaTraversableResolver == null || jpaTraversableResolver.isTraversable( - traversableObject, traversableProperty, rootBeanType, pathToTraversableObject, elementType - ); - } - /** * Tries to load detect and load JPA. */ @@ -88,4 +82,16 @@ ); } } + + public boolean isReachable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + return jpaTraversableResolver == null || jpaTraversableResolver.isReachable( + traversableObject, traversableProperty, rootBeanType, pathToTraversableObject, elementType + ); + } + + public boolean isCascadable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + return jpaTraversableResolver == null || jpaTraversableResolver.isCascadable( + traversableObject, traversableProperty, rootBeanType, pathToTraversableObject, elementType + ); + } } Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/JPATraversableResolver.java =================================================================== --- validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/JPATraversableResolver.java 2009-05-05 12:36:30 UTC (rev 16509) +++ validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/JPATraversableResolver.java 2009-05-05 12:40:55 UTC (rev 16510) @@ -23,11 +23,16 @@ /** * @author Hardy Ferentschik + * @author Emmanuel Bernard */ public class JPATraversableResolver implements TraversableResolver { - public boolean isTraversable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + public boolean isReachable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { return traversableObject == null || Persistence.getPersistenceUtil().isLoaded( traversableObject, traversableProperty ); } + + public boolean isCascadable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + return true; + } } Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/SingleThreadCachedTraversableResolver.java =================================================================== --- validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/SingleThreadCachedTraversableResolver.java 2009-05-05 12:36:30 UTC (rev 16509) +++ validator/trunk/hibernate-validator/src/main/java/org/hibernate/validation/engine/resolver/SingleThreadCachedTraversableResolver.java 2009-05-05 12:40:55 UTC (rev 16510) @@ -14,110 +14,123 @@ */ public class SingleThreadCachedTraversableResolver implements TraversableResolver { private TraversableResolver delegate; - private Map<LazyHolder, LazyHolder> lazys = new HashMap<LazyHolder, LazyHolder>(); private Map<TraversableHolder, TraversableHolder> traversables = new HashMap<TraversableHolder, TraversableHolder>(); public SingleThreadCachedTraversableResolver(TraversableResolver delegate) { this.delegate = delegate; } - public boolean isTraversable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { - Boolean delegateResult = null; - - //if traversableObject is null we can't determine lazyness - if (traversableObject != null) { - LazyHolder currentLH = new LazyHolder( traversableObject, traversableProperty ); - LazyHolder cachedLH = lazys.get( currentLH ); - if (cachedLH == null) { - delegateResult = delegate.isTraversable( - traversableObject, - traversableProperty, - rootBeanType, - pathToTraversableObject, - elementType ); - currentLH.isTraversable = delegateResult; - lazys.put( currentLH, currentLH ); - cachedLH = currentLH; - } - if ( ! cachedLH.isTraversable ) return false; + public boolean isReachable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + TraversableHolder currentLH = new TraversableHolder( traversableObject, traversableProperty, rootBeanType, pathToTraversableObject, elementType ); + TraversableHolder cachedLH = traversables.get( currentLH ); + if (cachedLH == null) { + currentLH.isReachable = delegate.isReachable( + traversableObject, + traversableProperty, + rootBeanType, + pathToTraversableObject, + elementType ); + traversables.put( currentLH, currentLH ); + cachedLH = currentLH; } - - - TraversableHolder currentTH = new TraversableHolder( rootBeanType, pathToTraversableObject, elementType ); - TraversableHolder cachedTH = traversables.get(currentTH); - if ( cachedTH == null ) { - if (delegateResult == null) { - delegateResult = delegate.isTraversable( + else if ( cachedLH.isReachable == null ) { + cachedLH.isReachable = delegate.isReachable( traversableObject, traversableProperty, rootBeanType, pathToTraversableObject, elementType ); - } - currentTH.isTraversable = delegateResult; - traversables.put( currentTH, currentTH ); - cachedTH = currentTH; } - return cachedTH.isTraversable; + return cachedLH.isReachable; } - private static class LazyHolder { - private final Object traversableObject; - private final String traversableProperty; - private final int hashCode; - private boolean isTraversable; - - private LazyHolder(Object traversableObject, String traversableProperty) { - this.traversableObject = traversableObject; - this.traversableProperty = traversableProperty == null ? "" : traversableProperty; - hashCode = this.traversableObject.hashCode() + this.traversableProperty.hashCode(); + public boolean isCascadable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + TraversableHolder currentLH = new TraversableHolder( traversableObject, traversableProperty, rootBeanType, pathToTraversableObject, elementType ); + TraversableHolder cachedLH = traversables.get( currentLH ); + if (cachedLH == null) { + currentLH.isCascadable = delegate.isCascadable( + traversableObject, + traversableProperty, + rootBeanType, + pathToTraversableObject, + elementType ); + traversables.put( currentLH, currentLH ); + cachedLH = currentLH; } - - @Override - public int hashCode() { - return hashCode; + else if ( cachedLH.isCascadable == null ) { + cachedLH.isCascadable = delegate.isCascadable( + traversableObject, + traversableProperty, + rootBeanType, + pathToTraversableObject, + elementType ); } - - @Override - public boolean equals(Object obj) { - if ( ! (obj instanceof LazyHolder) ) { - return false; - } - LazyHolder that = (LazyHolder) obj; - return traversableObject == that.traversableObject - && traversableProperty.equals( that.traversableProperty ); - } + return cachedLH.isCascadable; } private static class TraversableHolder { + private final Object traversableObject; + private final String traversableProperty; private final Class<?> rootBeanType; private final String pathToTraversableObject; private final ElementType elementType; private final int hashCode; - private boolean isTraversable; + private Boolean isReachable; + private Boolean isCascadable; - private TraversableHolder(Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + + private TraversableHolder(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + this.traversableObject = traversableObject; + this.traversableProperty = traversableProperty; this.rootBeanType = rootBeanType; this.pathToTraversableObject = pathToTraversableObject == null ? "" : pathToTraversableObject; this.elementType = elementType; - hashCode = this.rootBeanType.hashCode() + this.pathToTraversableObject.hashCode() + this.elementType.hashCode(); + this.hashCode = buildHashCode(); } @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + + TraversableHolder that = ( TraversableHolder ) o; + + if ( elementType != that.elementType ) { + return false; + } + if ( !pathToTraversableObject.equals( that.pathToTraversableObject ) ) { + return false; + } + if ( !rootBeanType.equals( that.rootBeanType ) ) { + return false; + } + if ( traversableObject != null ? !traversableObject.equals( that.traversableObject ) : that.traversableObject != null ) { + return false; + } + if ( !traversableProperty.equals( that.traversableProperty ) ) { + return false; + } + + return true; + } + + @Override public int hashCode() { return hashCode; } - @Override - public boolean equals(Object obj) { - if ( ! (obj instanceof TraversableHolder) ) { - return false; - } - TraversableHolder that = (TraversableHolder) obj; - return rootBeanType == that.rootBeanType - && pathToTraversableObject.equals( that.pathToTraversableObject ) - && elementType == that.elementType; + public int buildHashCode() { + int result = traversableObject != null ? traversableObject.hashCode() : 0; + result = 31 * result + traversableProperty.hashCode(); + result = 31 * result + rootBeanType.hashCode(); + result = 31 * result + pathToTraversableObject.hashCode(); + result = 31 * result + elementType.hashCode(); + return result; } } } Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/bootstrap/ValidationTest.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/bootstrap/ValidationTest.java 2009-05-05 12:36:30 UTC (rev 16509) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/bootstrap/ValidationTest.java 2009-05-05 12:40:55 UTC (rev 16510) @@ -273,9 +273,13 @@ configuration = Validation.byDefaultProvider().configure(); configuration.traversableResolver( new TraversableResolver() { - public boolean isTraversable(Object o, String s, Class<?> aClass, String s1, ElementType elementType) { + public boolean isReachable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { return false; } + + public boolean isCascadable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + return false; + } } ); factory = configuration.buildValidatorFactory(); Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/CachedTraversableResolverTest.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/CachedTraversableResolverTest.java 2009-05-05 12:36:30 UTC (rev 16509) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/CachedTraversableResolverTest.java 2009-05-05 12:40:55 UTC (rev 16510) @@ -57,19 +57,24 @@ } private static class AskOnceTR implements TraversableResolver { - private Set<Holder> asked = new HashSet<Holder>(); + private Set<Holder> askedReach = new HashSet<Holder>(); + private Set<Holder> askedCascade = new HashSet<Holder>(); - public Set<Holder> getAsked() { - return asked; - } - - public boolean isTraversable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + private boolean isTraversable(Set<Holder> asked, Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { Holder h = new Holder(traversableObject, traversableProperty); - if (asked.contains( h )) throw new IllegalStateException( "Called twice" ); + if ( asked.contains( h )) throw new IllegalStateException( "Called twice" ); asked.add( h ); return true; } + public boolean isReachable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + return isTraversable( askedReach, traversableObject, traversableProperty, rootBeanType, pathToTraversableObject,elementType ); + } + + public boolean isCascadable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + return isTraversable( askedCascade, traversableObject, traversableProperty, rootBeanType, pathToTraversableObject,elementType ); + } + public static class Holder { Object NULL = new Object(); Object to; Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/SnifferTraversableResolver.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/SnifferTraversableResolver.java 2009-05-05 12:36:30 UTC (rev 16509) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/SnifferTraversableResolver.java 2009-05-05 12:40:55 UTC (rev 16510) @@ -9,23 +9,30 @@ * @author Emmanuel Bernard */ public class SnifferTraversableResolver implements TraversableResolver { - Set<String> paths = new HashSet<String>(); - Set<Call> calls = new HashSet<Call>(); + Set<String> reachPaths = new HashSet<String>(); + Set<String> cascadePaths = new HashSet<String>(); + Set<Call> reachCalls = new HashSet<Call>(); + Set<Call> cascadeCalls = new HashSet<Call>(); public SnifferTraversableResolver(Suit suit) { - calls.add( new Call(suit, "size", Suit.class, "", ElementType.FIELD ) ); - calls.add( new Call(suit, "trousers", Suit.class, "", ElementType.FIELD ) ); - calls.add( new Call(suit.getTrousers(), "length", Suit.class, "trousers", ElementType.FIELD ) ); - calls.add( new Call(suit, "jacket", Suit.class, "", ElementType.METHOD ) ); - calls.add( new Call(suit.getJacket(), "width", Suit.class, "jacket", ElementType.METHOD ) ); + reachCalls.add( new Call(suit, "size", Suit.class, "", ElementType.FIELD ) ); + reachCalls.add( new Call(suit, "trousers", Suit.class, "", ElementType.FIELD ) ); + cascadeCalls.add( new Call(suit, "trousers", Suit.class, "", ElementType.FIELD ) ); + reachCalls.add( new Call(suit.getTrousers(), "length", Suit.class, "trousers", ElementType.FIELD ) ); + reachCalls.add( new Call(suit, "jacket", Suit.class, "", ElementType.METHOD ) ); + cascadeCalls.add( new Call(suit, "jacket", Suit.class, "", ElementType.METHOD ) ); + reachCalls.add( new Call(suit.getJacket(), "width", Suit.class, "jacket", ElementType.METHOD ) ); } - public Set<String> getPaths() { - return paths; + public Set<String> getReachPaths() { + return reachPaths; } - //TODO add test with correct paths and types to make sure the impl does not mess it up - public boolean isTraversable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + public Set<String> getCascadePaths() { + return cascadePaths; + } + + public boolean isTraversable(Set<Call> calls, Set<String> paths, Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { String path = ""; if (! (pathToTraversableObject == null || pathToTraversableObject.length() == 0 ) ) { path = pathToTraversableObject + "."; @@ -39,6 +46,14 @@ return true; } + public boolean isReachable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + return isTraversable( reachCalls, reachPaths, traversableObject, traversableProperty, rootBeanType, pathToTraversableObject, elementType ); + } + + public boolean isCascadable(Object traversableObject, String traversableProperty, Class<?> rootBeanType, String pathToTraversableObject, ElementType elementType) { + return isTraversable( cascadeCalls, cascadePaths, traversableObject, traversableProperty, rootBeanType, pathToTraversableObject, elementType ); + } + private static final class Call { private Object traversableObject; private String traversableProperty; Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/TraversableResolverTest.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/TraversableResolverTest.java 2009-05-05 12:36:30 UTC (rev 16509) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validation/engine/resolver/TraversableResolverTest.java 2009-05-05 12:40:55 UTC (rev 16510) @@ -1,9 +1,5 @@ package org.hibernate.validation.engine.resolver; -import java.lang.annotation.ElementType; -import java.util.HashSet; -import java.util.Set; -import javax.validation.TraversableResolver; import javax.validation.Validation; import javax.validation.ValidatorFactory; import javax.validation.Validator; @@ -12,8 +8,6 @@ import org.testng.annotations.Test; import static org.testng.Assert.*; -import org.hibernate.validation.engine.HibernateValidatorConfiguration; - /** * @author Emmanuel Bernard */ @@ -36,6 +30,7 @@ //Raises an IllegalStateException if something goes wrong v.validate( suit, Default.class, Cloth.class ); - assertEquals( 5, resolver.getPaths().size() ); + assertEquals( 5, resolver.getReachPaths().size() ); + assertEquals( 2, resolver.getCascadePaths().size() ); } } \ No newline at end of file