fix for GRAILS-2656

git-svn-id: https://svn.codehaus.org/grails/trunk@6816 1cfb16fd-6d17-0410-8ff1-b7e8e1e2867d

Author: graeme

src/commons/org/codehaus/groovy/grails/validation/BlankConstraint.java

@@ -64,9 +64,9 @@ protected boolean processValidateWithVetoing(Object target, Object propertyValue
             if(!blank) {
                 Object[] args = new Object[] { constraintPropertyName, constraintOwningClass };
                 super.rejectValue( target,errors, ConstrainedProperty.DEFAULT_BLANK_MESSAGE_CODE, ConstrainedProperty.BLANK_CONSTRAINT, args );
+                // empty string is catched by 'blank' constraint, no addition validation needed
+                return true;
             }
-            // empty string is catched by 'blank' constraint, no addition validation needed
-            return true;
         }
         return false;
     }

src/commons/org/codehaus/groovy/grails/validation/NullableConstraint.java

@@ -63,9 +63,9 @@ protected boolean processValidateWithVetoing(Object target, Object propertyValue
             if(!nullable) {
                 Object[] args = new Object[] { constraintPropertyName, constraintOwningClass};
                 super.rejectValue(target, errors, ConstrainedProperty.DEFAULT_NULL_MESSAGE_CODE, ConstrainedProperty.NULLABLE_CONSTRAINT,args );
+                // null value is catched by 'blank' constraint, no addition validation needed
+                return true;
             }
-            // null value is catched by 'blank' constraint, no addition validation needed
-            return true;
         }
         return false;
     }

test/commons/org/codehaus/groovy/grails/validation/BlankConstraintTests.java

@@ -23,11 +23,6 @@ public void testValidate() {
                 "someData"
         );
 
-        testConstraintPassedAndVetoed(
-                getConstraint( "testString", Boolean.TRUE ),
-                ""
-        );
-
         testConstraintPassed(
                 getConstraint( "testString", Boolean.TRUE ),
                 "someData"

test/commons/org/codehaus/groovy/grails/validation/NullableConstraint2Tests.java

@@ -23,11 +23,6 @@ public void testValidation() {
                 "test"
         );
 
-        testConstraintPassedAndVetoed(
-                getConstraint( "testString", Boolean.TRUE ),
-                null
-        );
-
         testConstraintPassed(
                 getConstraint( "testString", Boolean.TRUE ),
                 ""

test/groovy/org/codehaus/groovy/grails/validation/VetoingNullableBehaviourTests.groovy

@@ -0,0 +1,114 @@
+package org.codehaus.groovy.grails.validation
+
+import org.codehaus.groovy.grails.orm.hibernate.AbstractGrailsHibernateTests
+import org.springframework.validation.BindingResult
+
+/**
+ * @author Graeme Rocher
+ * @since 1.0
+ * 
+ * Created: Mar 20, 2008
+ */
+class VetoingNullableBehaviourTests extends AbstractGrailsHibernateTests{
+
+    protected void onSetUp() {
+        gcl.parseClass('''
+import org.apache.commons.lang.StringUtils
+
+class VetoingNullableBehaviour {
+    Long id
+    Long version
+
+    String name
+
+    static constraints = {
+        name(nullable:true, validator:{ val ->
+            if(val) {
+                return Character.isUpperCase(val.toCharArray()[0])
+            }
+        })
+    }
+}
+
+
+class VetoingNullableBehaviourBook {  
+    Long id
+    Long version
+    Boolean online
+    String onlineFormatDescription
+
+	/*
+	Premise here is that onlineFormatDescription is only required IF online == true
+	*/
+	static constraints = {
+		online(nullable: false)
+        onlineFormatDescription(nullable: true, blank: true, validator: onlineValidator)
+	}
+
+    static onlineValidator = {val, obj ->
+        if (obj.properties['online'] == true && StringUtils.isBlank(val)) {
+            return ['blank']
+        }
+    }
+
+}
+
+''')
+    }
+
+
+
+    void testVetoingConstraint() {
+        def test =  ga.getDomainClass("VetoingNullableBehaviour").newInstance()
+
+        
+
+        assert test.validate()
+
+        test.name = "fred"
+
+        assert !test.validate()
+
+        test.name = "Fred"
+
+        assert test.validate()
+    }
+
+	//this test will fail, but should pass.  it never gets to the custom validator because the nullable constraint stops further validation
+    void testOnlineFormatDescriptionValidates_null() {
+        def book = ga.getDomainClass("VetoingNullableBehaviourBook").newInstance()
+        book.online = true
+        book.onlineFormatDescription = null
+
+		book.validate()
+        assertFieldHasError(book.errors, 'onlineFormatDescription')
+    }
+
+	//this test will fail, but should pass.  it never gets to the custom validator because the blank constraint stops further validation
+    void testOnlineFormatDescriptionValidates_blank() {
+        def book = ga.getDomainClass("VetoingNullableBehaviourBook").newInstance()
+        book.online = true
+        book.onlineFormatDescription = ''
+
+        book.validate()
+        assertFieldHasError(book.errors, 'onlineFormatDescription')
+    }
+
+	//this test will pass
+    void testOnlineFormatDescriptionValidates_valid() {
+        def book = ga.getDomainClass("VetoingNullableBehaviourBook").newInstance()
+        book.online = true
+        book.onlineFormatDescription = 'Title'
+        book.validate()
+        assertFieldDoesNotHasError(book.errors, 'onlineFormatDescription')
+    }
+
+
+    protected assertFieldHasError(BindingResult bindErrors, String field) {
+        assertTrue("Error not found for field ${field}, errors were: ${bindErrors}", bindErrors.getFieldError(field) != null)
+    }
+
+    protected assertFieldDoesNotHasError(BindingResult bindErrors, String field) {
+        assertTrue("Error not found for field ${field}, errors were: ${bindErrors}", bindErrors.getFieldError(field) == null)
+    }
+}