You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bval.apache.org by mb...@apache.org on 2010/10/08 21:10:45 UTC

svn commit: r1005969 - in /incubator/bval/sandbox/lang3-work/bval-jsr303d/src: main/java/org/apache/bval/constraints/dynamic/ main/java/org/apache/bval/jsr303/dynamic/ test/java/org/apache/bval/constraints/dynamic/ test/java/org/apache/bval/jsr303/dyna...

Author: mbenson
Date: Fri Oct  8 19:10:45 2010
New Revision: 1005969

URL: http://svn.apache.org/viewvc?rev=1005969&view=rev
Log:
refactor constraint appender support hierarchy; make defaultConstraintAppender extend AbstractConstraintAppender; much support for ignoring constraints we have already seen; finally, retain empty values constraints

Modified:
    incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractConstraintAppender.java
    incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractGenericConstraintAppender.java
    incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractGenericMultivaluedConstraintAppender.java
    incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractValuesConstraintAppender.java
    incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/jsr303/dynamic/DefaultConstraintAppender.java
    incubator/bval/sandbox/lang3-work/bval-jsr303d/src/test/java/org/apache/bval/constraints/dynamic/ValuesConstraintValidationTest.java
    incubator/bval/sandbox/lang3-work/bval-jsr303d/src/test/java/org/apache/bval/jsr303/dynamic/DynamicValidationTest.java

Modified: incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractConstraintAppender.java
URL: http://svn.apache.org/viewvc/incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractConstraintAppender.java?rev=1005969&r1=1005968&r2=1005969&view=diff
==============================================================================
--- incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractConstraintAppender.java (original)
+++ incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractConstraintAppender.java Fri Oct  8 19:10:45 2010
@@ -17,6 +17,7 @@
 package org.apache.bval.constraints.dynamic;
 
 import java.lang.annotation.Annotation;
+import java.util.Collection;
 
 import javax.validation.Payload;
 
@@ -36,6 +37,33 @@ public abstract class AbstractConstraint
     protected static final GroupsComputer GROUPS_COMPUTER = new GroupsComputer();
 
     /**
+     * {@inheritDoc}
+     */
+    public final boolean append(Annotation constraint, Collection<Annotation> collection) {
+        return collection.contains(constraint) || appendSimple(constraint, collection) || doAppend(constraint, collection);
+    }
+
+    /**
+     * Simply append the constraint to the collection.
+     * 
+     * @param constraint
+     * @param collection
+     * @return whether the constraint was applied successfully.
+     */
+    protected boolean appendSimple(Annotation constraint, Collection<Annotation> collection) {
+        return collection.add(constraint);
+    }
+
+    /**
+     * Implement a non-simple append process.
+     * 
+     * @param constraint
+     * @param collection
+     * @return whether the constraint was applied successfully.
+     */
+    protected abstract boolean doAppend(Annotation constraint, Collection<Annotation> collection);
+
+    /**
      * Get the validation message of the specified constraint.
      * 
      * @param constraint

Modified: incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractGenericConstraintAppender.java
URL: http://svn.apache.org/viewvc/incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractGenericConstraintAppender.java?rev=1005969&r1=1005968&r2=1005969&view=diff
==============================================================================
--- incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractGenericConstraintAppender.java (original)
+++ incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractGenericConstraintAppender.java Fri Oct  8 19:10:45 2010
@@ -25,8 +25,8 @@ import org.apache.bval.jsr303.dynamic.Co
 import org.apache.bval.jsr303.groups.Groups;
 
 /**
- * Convenience superclass for a {@link ConstraintAppender} that deals
- * exclusively with a single constraint annotation type.
+ * Convenience superclass for a {@link ConstraintAppender} that deals exclusively with a single constraint annotation
+ * type.
  * 
  * @param <A>
  *            constraint type
@@ -51,13 +51,6 @@ public abstract class AbstractGenericCon
     }
 
     /**
-     * {@inheritDoc}
-     */
-    public final boolean append(Annotation constraint, Collection<Annotation> collection) {
-        return appendSimple(constraint, collection) || doAppend(constraint, collection);
-    }
-
-    /**
      * Implement a non-simple append process.
      * 
      * @param constraint
@@ -65,6 +58,7 @@ public abstract class AbstractGenericCon
      * @return whether the constraint was applied successfully.
      */
     @SuppressWarnings("unchecked")
+    @Override
     protected boolean doAppend(Annotation constraint, Collection<Annotation> collection) {
         return constraintType.isInstance(constraint) && appendTyped((A) constraint, collection);
     }
@@ -79,9 +73,8 @@ public abstract class AbstractGenericCon
     protected abstract boolean appendTyped(A constraint, Collection<Annotation> collection);
 
     /**
-     * If permitted, simply append the constraint to the collection. Default
-     * implementation assumes that this operation would be precluded by an
-     * existing constraint of this type with overlapping groups.
+     * {@inheritDoc} This implementation assumes that this operation would be precluded by an existing constraint of
+     * this type with overlapping groups.
      * 
      * @param constraint
      * @param collection
@@ -89,14 +82,11 @@ public abstract class AbstractGenericCon
      */
     protected boolean appendSimple(Annotation constraint, Collection<Annotation> collection) {
         if (!ConstraintAnnotationAttributes.GROUPS.isDeclaredOn(constraint.annotationType())) {
-            //non-constraint; don't know how to handle:
+            // non-constraint; don't know how to handle:
             return false;
         }
         Groups groups = GROUPS_COMPUTER.computeGroups(getGroups(constraint));
         for (Annotation a : collection) {
-            if (constraint.equals(a)) {
-                return false;
-            }
             if (constraintType.isInstance(a)) {
                 if (Collections.disjoint(groups.getGroups(), GROUPS_COMPUTER.computeGroups(getGroups(a)).getGroups())) {
                     continue;
@@ -104,7 +94,7 @@ public abstract class AbstractGenericCon
                 return false;
             }
         }
-        return collection.add(constraint);
+        return super.appendSimple(constraint, collection);
     }
 
 }

Modified: incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractGenericMultivaluedConstraintAppender.java
URL: http://svn.apache.org/viewvc/incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractGenericMultivaluedConstraintAppender.java?rev=1005969&r1=1005968&r2=1005969&view=diff
==============================================================================
--- incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractGenericMultivaluedConstraintAppender.java (original)
+++ incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractGenericMultivaluedConstraintAppender.java Fri Oct  8 19:10:45 2010
@@ -105,8 +105,13 @@ public abstract class AbstractGenericMul
             foundSingles |= constraintType.isInstance(a);
         }
         if (multipleConstraintType.isInstance(constraint)) {
-            return !foundSingles && collection.add(constraint);
+            if (!foundSingles) {
+                collection.add(constraint);
+                return true;
+            }
+            return false;
         }
+        //if we get here, it must be of our single type
         return super.appendSimple(constraint, collection);
     }
 }

Modified: incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractValuesConstraintAppender.java
URL: http://svn.apache.org/viewvc/incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractValuesConstraintAppender.java?rev=1005969&r1=1005968&r2=1005969&view=diff
==============================================================================
--- incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractValuesConstraintAppender.java (original)
+++ incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/constraints/dynamic/AbstractValuesConstraintAppender.java Fri Oct  8 19:10:45 2010
@@ -93,7 +93,7 @@ public abstract class AbstractValuesCons
         final Set<V> values = new LinkedHashSet<V>();
         final Set<Class<? extends Payload>> payload = new HashSet<Class<? extends Payload>>();
         final Set<String> message = new LinkedHashSet<String>();
-        final Strategy strategy;
+        Strategy strategy;
 
         /**
          * Create a new AbstractCumulativeValues instance.
@@ -126,8 +126,7 @@ public abstract class AbstractValuesCons
          * 
          * @param constraint
          * @param applyingModification
-         *            whether the constraint being applied is (one of) the
-         *            constraint(s) that began this append process
+         *            whether the constraint being applied is (one of) the constraint(s) that began this append process
          */
         public void apply(A constraint, boolean applyingModification) {
             // when we are applying the modification, the meaning of the rule
@@ -136,19 +135,21 @@ public abstract class AbstractValuesCons
 
             Rule rule = getRule(constraint);
             Groups chain = GROUPS_COMPUTER.computeGroups(getGroups(constraint));
-
             Strategy strategy = getStrategy(constraint);
-            if (strategy == Strategy.REPLACE) {
-                for (Group group : chain.getGroups()) {
-                    includedValuesByGroup.remove(group);
-                    excludedValuesByGroup.remove(group);
-                }
-            }
 
             List<V> constraintValue = Arrays.asList(getConstraintValue(constraint));
             for (Group group : chain.getGroups()) {
-                CumulativeValues includes = includedValuesByGroup.get(group);
-                CumulativeValues excludes = excludedValuesByGroup.get(group);
+                CumulativeValues includes = null;
+                CumulativeValues excludes = null;
+                if (strategy == Strategy.REPLACE) {
+                    //keep null
+                    includedValuesByGroup.remove(group);
+                    excludedValuesByGroup.remove(group);
+                } else {
+                    //get existing values
+                    includes = includedValuesByGroup.get(group);
+                    excludes = excludedValuesByGroup.get(group);
+                }
 
                 // inclusions may be inclusions OR negative exclusions, but not
                 // both
@@ -162,7 +163,11 @@ public abstract class AbstractValuesCons
                         excludes.values.removeAll(constraintValue);
                     }
 
-                    if (!useNegativeExclusion) {
+                    if (useNegativeExclusion) {
+                        if (applyingModification) {
+                            excludes.strategy = Strategy.MERGE;
+                        }
+                    } else {
                         if (includes == null) {
                             includes = new CumulativeValues(strategy);
                             includedValuesByGroup.put(group, includes);
@@ -181,7 +186,11 @@ public abstract class AbstractValuesCons
                         }
                         includes.values.removeAll(constraintValue);
                     }
-                    if (!useNegativeInclusion) {
+                    if (useNegativeInclusion) {
+                        if (applyingModification) {
+                            includes.strategy = Strategy.MERGE;
+                        }
+                    } else {
                         if (excludes == null) {
                             excludes = new CumulativeValues(strategy);
                             excludedValuesByGroup.put(group, excludes);
@@ -284,7 +293,7 @@ public abstract class AbstractValuesCons
         groups.add(group.getGroup());
         Set<Class<? extends Payload>> payloads = new LinkedHashSet<Class<? extends Payload>>(values.payload);
         Set<String> messages = new LinkedHashSet<String>(values.message);
-        if (!values.values.isEmpty()) {
+        if (!values.values.isEmpty() || values.strategy == Strategy.REPLACE) {
             // merge groups with same values and strategy:
             for (Iterator<Map.Entry<Group, CumulativeValues>> iter = valuesMap.entrySet().iterator(); iter.hasNext();) {
                 Map.Entry<Group, CumulativeValues> e = iter.next();
@@ -313,8 +322,7 @@ public abstract class AbstractValuesCons
     }
 
     /**
-     * In a given set of groups, find the first one that does not extend any
-     * other group in the set.
+     * In a given set of groups, find the first one that does not extend any other group in the set.
      * 
      * @param groups
      * @return Group

Modified: incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/jsr303/dynamic/DefaultConstraintAppender.java
URL: http://svn.apache.org/viewvc/incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/jsr303/dynamic/DefaultConstraintAppender.java?rev=1005969&r1=1005968&r2=1005969&view=diff
==============================================================================
--- incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/jsr303/dynamic/DefaultConstraintAppender.java (original)
+++ incubator/bval/sandbox/lang3-work/bval-jsr303d/src/main/java/org/apache/bval/jsr303/dynamic/DefaultConstraintAppender.java Fri Oct  8 19:10:45 2010
@@ -19,17 +19,14 @@ package org.apache.bval.jsr303.dynamic;
 import java.lang.annotation.Annotation;
 import java.util.Collection;
 
-import org.apache.commons.lang3.AnnotationUtils;
+import org.apache.bval.constraints.dynamic.AbstractConstraintAppender;
 
 /**
- * Default {@link ConstraintAppender} implementation. Adds unique constraints to
- * the collection.
- *
- * @see Annotation#equals(Object)
- *
+ * Default {@link ConstraintAppender} implementation. Adds unique constraints to the collection.
+ * 
  * @version $Rev$ $Date$
  */
-public final class DefaultConstraintAppender implements ConstraintAppender {
+public final class DefaultConstraintAppender extends AbstractConstraintAppender {
     /**
      * Static instance.
      */
@@ -44,13 +41,9 @@ public final class DefaultConstraintAppe
     /**
      * {@inheritDoc}
      */
-    public boolean append(Annotation constraint, Collection<Annotation> collection) {
-        for (Annotation element : collection) {
-            if (AnnotationUtils.equals(constraint, element)) {
-                return false;
-            }
-        }
-        return collection.add(constraint);
+    @Override
+    protected boolean doAppend(Annotation constraint, Collection<Annotation> collection) {
+        return false;
     }
 
 }

Modified: incubator/bval/sandbox/lang3-work/bval-jsr303d/src/test/java/org/apache/bval/constraints/dynamic/ValuesConstraintValidationTest.java
URL: http://svn.apache.org/viewvc/incubator/bval/sandbox/lang3-work/bval-jsr303d/src/test/java/org/apache/bval/constraints/dynamic/ValuesConstraintValidationTest.java?rev=1005969&r1=1005968&r2=1005969&view=diff
==============================================================================
--- incubator/bval/sandbox/lang3-work/bval-jsr303d/src/test/java/org/apache/bval/constraints/dynamic/ValuesConstraintValidationTest.java (original)
+++ incubator/bval/sandbox/lang3-work/bval-jsr303d/src/test/java/org/apache/bval/constraints/dynamic/ValuesConstraintValidationTest.java Fri Oct  8 19:10:45 2010
@@ -36,14 +36,15 @@ import org.apache.bval.jsr303.DefaultMes
 import org.apache.bval.jsr303.dynamic.ConstraintAppender;
 import org.apache.bval.jsr303.dynamic.DynamicValidatorContext;
 import org.apache.bval.jsr303.dynamic.DynamicValidatorFactory;
+import org.apache.bval.jsr303.dynamic.StubConstraint;
 import org.apache.commons.proxy2.stub.AnnotationFactory;
 import org.apache.commons.proxy2.stub.StubConfigurer;
 import org.junit.Before;
 import org.junit.Test;
 
 /**
- * Test the {@link Values} and {@link Values.Labeled} constraints, as well as
- * their provided {@link ConstraintValidator}s and {@link ConstraintAppender}s.
+ * Test the {@link Values} and {@link Values.Labeled} constraints, as well as their provided {@link ConstraintValidator}
+ * s and {@link ConstraintAppender}s.
  * 
  * @version $Rev$ $Date$
  */
@@ -229,6 +230,40 @@ public class ValuesConstraintValidationT
     }
 
     @Test
+    public void testValuesConstraintDuplication() {
+        testBean.fooBarOrBaz = "foo";
+        assertEquals(0, validator.validateProperty(testBean, "fooBarOrBaz").size());
+        StubConstraint.Builder builder = StubConstraint.build();
+        validatorContext.constrain(TestBean.class, "fooBarOrBaz", builder.new CustomConfig<Values>() {
+
+            @Override
+            protected void customConfigure(Values stub) {
+                when(stub.value()).thenReturn("foo", "bar", "baz");
+            }
+        }.build());
+        assertEquals(0, validator.validateProperty(testBean, "fooBarOrBaz").size());
+
+        validatorContext.constrain(TestBean.class, "fooBarOrBaz", builder.new CustomConfig<Values>() {
+
+            @Override
+            protected void customConfigure(Values stub) {
+                when(stub.value()).thenReturn(/* nothing */);
+            }
+        }.build());
+        assertEquals(1, validator.validateProperty(testBean, "fooBarOrBaz").size());
+
+        // verify that constraint merging did not eliminate the empty values entirely when re-applying:
+        validatorContext.constrain(TestBean.class, "fooBarOrBaz", builder.new CustomConfig<Values>() {
+
+            @Override
+            protected void customConfigure(Values stub) {
+                when(stub.value()).thenReturn(/* nothing */);
+            }
+        }.build());
+        assertEquals(1, validator.validateProperty(testBean, "fooBarOrBaz").size());
+    }
+
+    @Test
     public void testLabeledValuesConstraintAccumulation() {
         testBean.annotationOrConstructor = ElementType.ANNOTATION_TYPE;
         assertEquals(0, validator.validate(testBean).size());

Modified: incubator/bval/sandbox/lang3-work/bval-jsr303d/src/test/java/org/apache/bval/jsr303/dynamic/DynamicValidationTest.java
URL: http://svn.apache.org/viewvc/incubator/bval/sandbox/lang3-work/bval-jsr303d/src/test/java/org/apache/bval/jsr303/dynamic/DynamicValidationTest.java?rev=1005969&r1=1005968&r2=1005969&view=diff
==============================================================================
--- incubator/bval/sandbox/lang3-work/bval-jsr303d/src/test/java/org/apache/bval/jsr303/dynamic/DynamicValidationTest.java (original)
+++ incubator/bval/sandbox/lang3-work/bval-jsr303d/src/test/java/org/apache/bval/jsr303/dynamic/DynamicValidationTest.java Fri Oct  8 19:10:45 2010
@@ -53,7 +53,7 @@ import org.apache.commons.proxy2.stub.St
 
 /**
  * Test dynamic bean validation.
- *
+ * 
  * @version $Rev$ $Date$
  */
 public class DynamicValidationTest extends ValidationTest {
@@ -67,9 +67,12 @@ public class DynamicValidationTest exten
     @Override
     public void setUp() throws Exception {
         DynamicValidatorFactory factory =
-            Validation.byProvider(ApacheValidationProvider.class).configure().addProperty(
-                ApacheValidatorConfiguration.Properties.VALIDATOR_FACTORY_CLASSNAME,
-                DynamicValidatorFactory.class.getName()).buildValidatorFactory().unwrap(DynamicValidatorFactory.class);
+            Validation
+                .byProvider(ApacheValidationProvider.class)
+                .configure()
+                .addProperty(ApacheValidatorConfiguration.Properties.VALIDATOR_FACTORY_CLASSNAME,
+                    DynamicValidatorFactory.class.getName()).buildValidatorFactory()
+                .unwrap(DynamicValidatorFactory.class);
         ((DefaultMessageInterpolator) factory.getMessageInterpolator()).setLocale(Locale.ENGLISH);
         validatorContext = factory.usingContext();
         super.setUp();
@@ -134,8 +137,8 @@ public class DynamicValidationTest exten
         Set<ConstraintViolation<Author>> errors = validator.validate(author);
         assertTrue(errors.isEmpty());
 
-        validatorContext.constrain(Author.class, "addresses[].addressline2", annotationFactory
-            .create(new StubConfigurer<Pattern>() {
+        validatorContext.constrain(Author.class, "addresses[].addressline2",
+            annotationFactory.create(new StubConfigurer<Pattern>() {
                 /**
                  * {@inheritDoc}
                  */
@@ -159,6 +162,48 @@ public class DynamicValidationTest exten
         assertEquals(2, errors.size());
     }
 
+    public void testAutoMergeEquivalentDynamicConstraints() {
+        Author author = new Author();
+        author.setLastName("Baudelaire");
+        author.setFirstName("Chuck");
+        Book book = new Book();
+        book.setAuthor(author);
+        book.setTitle("foo");
+        Set<ConstraintViolation<Book>> errors = validator.validate(book, Book.All.class);
+        assertTrue(errors.isEmpty());
+
+        StubConstraint.Builder builder = StubConstraint.build().withGroups(First.class);
+        validatorContext.constrain(Book.class, "title", builder.new CustomConfig<Pattern>() {
+
+            @Override
+            protected void customConfigure(Pattern stub) {
+                when(stub.regexp()).thenReturn("[A-Za-z]*");
+            }
+        }.build());
+
+        errors = validator.validate(book, Book.All.class);
+        assertTrue(errors.isEmpty());
+
+        book.setTitle("1001 ways to whatever");
+        errors = validator.validate(book, Book.All.class);
+        assertEquals(1, errors.size());
+        ConstraintViolation<Book> vio = errors.iterator().next();
+        assertEquals(Pattern.class, vio.getConstraintDescriptor().getAnnotation().annotationType());
+        
+        validatorContext.constrain(Book.class, "title", builder.new CustomConfig<Pattern>() {
+
+            @Override
+            protected void customConfigure(Pattern stub) {
+                when(stub.regexp()).thenReturn("[A-Za-z]*");
+            }
+        }.build());
+
+        errors = validator.validate(book, Book.All.class);
+        assertEquals(1, errors.size());
+        vio = errors.iterator().next();
+        assertEquals(Pattern.class, vio.getConstraintDescriptor().getAnnotation().annotationType());
+    }
+
     public void testDynamicMetadataBean() {
         BeanDescriptor authorDescriptor = validator.getConstraintsForClass(Author.class);
         assertEquals(0, authorDescriptor.getConstraintDescriptors().size());
@@ -177,8 +222,8 @@ public class DynamicValidationTest exten
             assertFalse(Pattern.class.equals(desc.getAnnotation().annotationType()));
         }
 
-        validatorContext.constrain(Author.class, "addresses[].addressline2", annotationFactory
-            .create(new StubConfigurer<Pattern>() {
+        validatorContext.constrain(Author.class, "addresses[].addressline2",
+            annotationFactory.create(new StubConfigurer<Pattern>() {
                 /**
                  * {@inheritDoc}
                  */