[hibernate-commits] Hibernate SVN: r19019 - in validator/trunk/hibernate-validator/src: test/java/org/hibernate/validator/engine/groups and 1 other directory.

Author: hardy.ferentschik Date: 2010-03-18 12:36:03 -0400 (Thu, 18 Mar 2010) New Revision: 19019 Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validator/engine/groups/Group.java validator/trunk/hibernate-validator/src/main/java/org/hibernate/validator/engine/groups/GroupChainGenerator.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupChainGeneratorTest.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupChainTest.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupsTest.java validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/Try.java Log: HV-288 - fixed GroupChainGenerator to also take into account inherited groups. Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validator/engine/groups/Group.java =================================================================== --- validator/trunk/hibernate-validator/src/main/java/org/hibernate/validator/engine/groups/Group.java 2010-03-18 14:02:38 UTC (rev 19018) +++ validator/trunk/hibernate-validator/src/main/java/org/hibernate/validator/engine/groups/Group.java 2010-03-18 16:36:03 UTC (rev 19019) @@ -80,9 +80,6 @@ @Override public String toString() { - return "Group{" + - "groups=" + group + - ", sequence=" + sequence + - '}'; + return "Group{" + "group=" + group.getName() + '}'; } } Modified: validator/trunk/hibernate-validator/src/main/java/org/hibernate/validator/engine/groups/GroupChainGenerator.java =================================================================== --- validator/trunk/hibernate-validator/src/main/java/org/hibernate/validator/engine/groups/GroupChainGenerator.java 2010-03-18 14:02:38 UTC (rev 19018) +++ validator/trunk/hibernate-validator/src/main/java/org/hibernate/validator/engine/groups/GroupChainGenerator.java 2010-03-18 16:36:03 UTC (rev 19019) @@ -27,7 +27,7 @@ import javax.validation.ValidationException; /** - * Used to determine the execution order. + * Helper class used to resolve groups and sequences into a single chain of groups which can then be validated. * * @author Hardy Ferentschik */ @@ -35,6 +35,13 @@ private final Map<Class<?>, List<Group>> resolvedSequences = new HashMap<Class<?>, List<Group>>(); + /** + * Generates a chain of groups to be validated given the specified validation groups. + * + * @param groups The groups specified at the validation call. + * + * @return an instance of {@code GroupChain} defining the order in which validation has to occur. + */ public GroupChain getGroupChainFor(Collection<Class<?>> groups) { if ( groups == null || groups.size() == 0 ) { throw new IllegalArgumentException( "At least one groups has to be specified." ); @@ -48,24 +55,34 @@ GroupChain chain = new GroupChain(); for ( Class<?> clazz : groups ) { - if ( clazz.getAnnotation( GroupSequence.class ) == null ) { + if ( isGroupSequence( clazz ) ) { + insertSequence( clazz, chain ); + } + else { Group group = new Group( clazz ); chain.insertGroup( group ); insertInheritedGroups( clazz, chain ); } - else { - insertSequence( clazz, chain ); - } } return chain; } + private boolean isGroupSequence(Class<?> clazz) { + return clazz.getAnnotation( GroupSequence.class ) != null; + } + + /** + * Recursively add inherited groups into the group chain. + * + * @param clazz The group interface + * @param chain The group chain we are currently building. + */ private void insertInheritedGroups(Class<?> clazz, GroupChain chain) { - for ( Class<?> extendedInterface : clazz.getInterfaces() ) { - Group group = new Group( extendedInterface ); + for ( Class<?> inheritedGroup : clazz.getInterfaces() ) { + Group group = new Group( inheritedGroup ); chain.insertGroup( group ); - insertInheritedGroups( extendedInterface, chain ); + insertInheritedGroups( inheritedGroup, chain ); } } @@ -76,10 +93,34 @@ } else { sequence = resolveSequence( clazz, new ArrayList<Class<?>>() ); + // we expand the inherited groups only after we determined whether the sequence is expandable + sequence = expandInhertitedGroups( sequence ); } chain.insertSequence( sequence ); } + private List<Group> expandInhertitedGroups(List<Group> sequence) { + List<Group> expandedGroup = new ArrayList<Group>(); + for ( Group group : sequence ) { + expandedGroup.add( group ); + addInheritedGroups( group, expandedGroup ); + } + return expandedGroup; + } + + private void addInheritedGroups(Group group, List<Group> expandedGroups) { + for ( Class<?> inheritedGroup : group.getGroup().getInterfaces() ) { + if ( isGroupSequence( inheritedGroup ) ) { + throw new GroupDefinitionException( + "Sequence definitions are not allowed as composing parts of a sequence." + ); + } + Group g = new Group( inheritedGroup, group.getSequence() ); + expandedGroups.add( g ); + addInheritedGroups( g, expandedGroups ); + } + } + private List<Group> resolveSequence(Class<?> group, List<Class<?>> processedSequences) { if ( processedSequences.contains( group ) ) { throw new GroupDefinitionException( "Cyclic dependency in groups definition" ); @@ -91,24 +132,36 @@ GroupSequence sequenceAnnotation = group.getAnnotation( GroupSequence.class ); Class<?>[] sequenceArray = sequenceAnnotation.value(); for ( Class<?> clazz : sequenceArray ) { - if ( clazz.getAnnotation( GroupSequence.class ) == null ) { - resolvedGroupSequence.add( new Group( clazz, group ) ); + if ( isGroupSequence( clazz ) ) { + List<Group> tmpSequence = resolveSequence( clazz, processedSequences ); + addGroups( resolvedGroupSequence, tmpSequence ); } else { - List<Group> tmpSequence = resolveSequence( clazz, processedSequences ); - addTmpSequence( resolvedGroupSequence, tmpSequence ); + List<Group> list = new ArrayList<Group>(); + list.add( new Group( clazz, group ) ); + addGroups( resolvedGroupSequence, list ); } } resolvedSequences.put( group, resolvedGroupSequence ); return resolvedGroupSequence; } - private void addTmpSequence(List<Group> resolvedGroupSequence, List<Group> tmpSequence) { - for ( Group tmpGroup : tmpSequence ) { - if ( resolvedGroupSequence.contains( tmpGroup ) && resolvedGroupSequence.indexOf( tmpGroup ) < resolvedGroupSequence.size() - 1 ) { - throw new GroupDefinitionException( "Unable to expand group sequence." ); + private void addGroups(List<Group> resolvedGroupSequence, List<Group> groups) { + for ( Group tmpGroup : groups ) { + if ( resolvedGroupSequence.contains( tmpGroup ) && resolvedGroupSequence.indexOf( tmpGroup ) < resolvedGroupSequence + .size() - 1 ) { + throw new GroupDefinitionException( "Unable to expand group sequence." ); } resolvedGroupSequence.add( tmpGroup ); } } + + @Override + public String toString() { + final StringBuilder sb = new StringBuilder(); + sb.append( "GroupChainGenerator" ); + sb.append( "{resolvedSequences=" ).append( resolvedSequences ); + sb.append( '}' ); + return sb.toString(); + } } Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupChainGeneratorTest.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupChainGeneratorTest.java 2010-03-18 14:02:38 UTC (rev 19018) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupChainGeneratorTest.java 2010-03-18 16:36:03 UTC (rev 19019) @@ -21,13 +21,16 @@ import java.util.Iterator; import java.util.List; import java.util.Set; +import javax.validation.GroupDefinitionException; +import javax.validation.GroupSequence; import javax.validation.ValidationException; import javax.validation.groups.Default; -import static org.testng.Assert.assertEquals; import org.testng.annotations.BeforeTest; import org.testng.annotations.Test; +import static org.testng.Assert.assertEquals; + /** * @author Hardy Ferentschik */ @@ -97,17 +100,28 @@ assertEquals( count, 2, "Wrong number of groups" ); } - private int countGroups(GroupChain chain) { - Iterator<Group> groupIterator = chain.getGroupIterator(); - int count = 0; - while ( groupIterator.hasNext() ) { - groupIterator.next(); - count++; - } - return count; + @Test(expectedExceptions = GroupDefinitionException.class) + public void testGroupDefiningSequencePartOfGroupComposingSequence() { + Set<Class<?>> groups = new HashSet<Class<?>>(); + groups.add( Sequence1.class ); + generator.getGroupChainFor( groups ); } + @Test(expectedExceptions = GroupDefinitionException.class) + public void testUnexpandableSequence() { + Set<Class<?>> groups = new HashSet<Class<?>>(); + groups.add( Sequence3.class ); + generator.getGroupChainFor( groups ); + } + @Test + public void testExpandableSequenceWithInheritance() { + Set<Class<?>> groups = new HashSet<Class<?>>(); + groups.add( Sequence4.class ); + generator.getGroupChainFor( groups ); + } + + @Test public void testSequenceResolution() { Set<Class<?>> groups = new HashSet<Class<?>>(); groups.add( Address.Complete.class ); @@ -118,4 +132,40 @@ assertEquals( sequence.get( 0 ).getGroup(), Default.class, "Wrong group" ); assertEquals( sequence.get( 1 ).getGroup(), Address.HighLevelCoherence.class, "Wrong group" ); } + + private int countGroups(GroupChain chain) { + Iterator<Group> groupIterator = chain.getGroupIterator(); + int count = 0; + while ( groupIterator.hasNext() ) { + groupIterator.next(); + count++; + } + return count; + } + + + interface GroupA extends Default { + } + + interface GroupB { + } + + interface GroupC extends Sequence2 { + } + + @GroupSequence({ GroupA.class, GroupC.class }) + interface Sequence1 { + } + + @GroupSequence({ GroupB.class, GroupA.class }) + interface Sequence2 { + } + + @GroupSequence({ Sequence2.class, GroupB.class }) + interface Sequence3 { + } + + @GroupSequence({ Sequence2.class, GroupA.class }) + interface Sequence4 { + } } Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupChainTest.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupChainTest.java 2010-03-18 14:02:38 UTC (rev 19018) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupChainTest.java 2010-03-18 16:36:03 UTC (rev 19019) @@ -90,7 +90,7 @@ @Test - public void testAssertDefaulGroupSequenceIsExpandableWithDefaultAtBeginningOfSequence() { + public void testAssertDefaultGroupSequenceIsExpandableWithDefaultAtBeginningOfSequence() { // create a dummy sequence Group a = new Group( GroupA.class, TestSequence.class ); Group b = new Group( GroupB.class, TestSequence.class ); Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupsTest.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupsTest.java 2010-03-18 14:02:38 UTC (rev 19018) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/GroupsTest.java 2010-03-18 16:36:03 UTC (rev 19019) @@ -32,6 +32,9 @@ */ public class GroupsTest { + /** + * HV-288 + */ @Test public void testGroupInheritance() { Validator validator = TestUtil.getValidator(); @@ -40,6 +43,6 @@ tryMe.field3 = "bar"; Set<ConstraintViolation<Try>> violations = validator.validate( tryMe, Try.GlobalCheck.class ); - //assertCorrectConstraintViolationMessages(violations, "field1"); + assertCorrectConstraintViolationMessages(violations, "field1"); } } \ No newline at end of file Modified: validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/Try.java =================================================================== --- validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/Try.java 2010-03-18 14:02:38 UTC (rev 19018) +++ validator/trunk/hibernate-validator/src/test/java/org/hibernate/validator/engine/groups/Try.java 2010-03-18 16:36:03 UTC (rev 19019) @@ -33,7 +33,6 @@ @NotNull(message = "field3", groups = OtherComponent.class) public String field3; - public interface BaseComponent { }