GRAILS-12010 - throw ConstraintException instead of IllegalStateExcep…

…tion
mfatihercik · Mar 10, 2015 · 49bcba2 · 49bcba2
1 parent 06b6b90
commit 49bcba2
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 89 deletions.
diff --git a/grails-test-suite-uber/src/test/groovy/grails/validation/ConstrainedPropertyTests.java b/grails-test-suite-uber/src/test/groovy/grails/validation/ConstrainedPropertyTests.java
@@ -2,10 +2,17 @@
 
 import grails.core.GrailsDomainClass;
 import grails.util.Holders;
+import grails.validation.exceptions.ConstraintException;
 import groovy.lang.GroovyClassLoader;
 import groovy.lang.GroovyObject;
 import groovy.lang.IntRange;
 import groovy.lang.ObjectRange;
+import junit.framework.TestCase;
+import org.grails.core.DefaultGrailsDomainClass;
+import org.grails.plugins.MockGrailsPluginManager;
+import org.grails.test.support.MockHibernatePluginHelper;
+import org.springframework.validation.BindException;
+import org.springframework.validation.Errors;
 
 import java.math.BigDecimal;
 import java.math.BigInteger;
@@ -14,14 +21,6 @@
 import java.util.Iterator;
 import java.util.Map;
 
-import junit.framework.TestCase;
-
-import org.grails.test.support.MockHibernatePluginHelper;
-import org.grails.core.DefaultGrailsDomainClass;
-import org.grails.plugins.MockGrailsPluginManager;
-import org.springframework.validation.BindException;
-import org.springframework.validation.Errors;
-
 public class ConstrainedPropertyTests extends TestCase {
 
     private int testValidatorValue = 0;
@@ -55,82 +54,6 @@ public void testGetSetURL() {
         assertTrue("should be an url", cp.isUrl());
     }
 
-    public void testIsEmailOnNonStringProperty() {
-        ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", Integer.class);
-        assertFalse(cp.isEmail());
-    }
-
-    public void testSetEmailOnForStringProperty() {
-        ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", Integer.class);
-        try {
-            cp.setEmail(true);
-            fail("The call to setEmail(true) on a ConstrainedProperty associated with an Integer property should throw an IllegalStateException.");
-        } catch (IllegalStateException expected) {
-            assertEquals("Email constraint can only be applied to String properties", expected.getMessage());
-        }
-    }
-
-    public void testIsCreditCardOnNonStringProperty() {
-        ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", Integer.class);
-        assertFalse(cp.isCreditCard());
-    }
-
-    public void testSetCreditCardOnForStringProperty() {
-        ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", Integer.class);
-        try {
-            cp.setCreditCard(true);
-            fail("The call to setCreditCard(true) on a ConstrainedProperty associated with an Integer property should throw an IllegalStateException.");
-        } catch (IllegalStateException expected) {
-            assertEquals("CreditCard constraint can only be applied to String properties", expected.getMessage());
-        }
-    }
-
-    public void testIsMatchesOnNonStringProperty() {
-        ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", Integer.class);
-        assertNull(cp.getMatches());
-    }
-
-    public void testSetMatchesOnForStringProperty() {
-        ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", Integer.class);
-        try {
-            cp.setMatches("some regex");
-            fail("The call to setMatches(...) on a ConstrainedProperty associated with an Integer property should throw an IllegalStateException.");
-        } catch (IllegalStateException expected) {
-            assertEquals("Matches constraint can only be applied to String properties", expected.getMessage());
-        }
-    }
-
-    public void testIsUrlOnNonStringProperty() {
-        ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", Integer.class);
-        assertFalse(cp.isUrl());
-    }
-
-    public void testSetUrlOnForStringProperty() {
-        ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", Integer.class);
-        try {
-            cp.setUrl(true);
-            fail("The call to setUrl(true) on a ConstrainedProperty associated with an Integer property should throw an IllegalStateException.");
-        } catch (IllegalStateException expected) {
-            assertEquals("URL constraint can only be applied to String properties", expected.getMessage());
-        }
-    }
-
-
-    public void testIsBlankOnNonStringProperty() {
-        ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", Integer.class);
-        assertFalse(cp.isBlank());
-    }
-
-    public void testSetBlankOnForStringProperty() {
-        ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", Integer.class);
-        try {
-            cp.setBlank(true);
-            fail("The call to setBlank(true) on a ConstrainedProperty associated with an Integer property should throw an IllegalStateException.");
-        } catch (IllegalStateException expected) {
-            assertEquals("Blank constraint can only be applied to String properties", expected.getMessage());
-        }
-    }
-
     public void testGetSetEmail() {
         ConstrainedProperty cp = new ConstrainedProperty(ConstrainedPropertyTests.class,"testProperty", String.class);
         cp.setEmail(true);

diff --git a/grails-validation/src/main/groovy/grails/validation/ConstrainedProperty.java b/grails-validation/src/main/groovy/grails/validation/ConstrainedProperty.java
@@ -574,7 +574,7 @@ public boolean isBlank() {
      */
     public void setBlank(boolean blank) {
         if (!isValidStringType()) {
-            throw new IllegalStateException("Blank constraint can only be applied to String properties");
+            throw new ConstraintException("Blank constraint can only be applied to String properties");
         }
 
         if (!blank) {
@@ -605,7 +605,7 @@ public boolean isEmail() {
      */
     public void setEmail(boolean email) {
         if (!isValidStringType()) {
-            throw new IllegalStateException("Email constraint can only be applied to String properties");
+            throw new ConstraintException("Email constraint can only be applied to String properties");
         }
 
         Constraint c = appliedConstraints.get(EMAIL_CONSTRAINT);
@@ -642,7 +642,7 @@ public boolean isCreditCard() {
      */
     public void setCreditCard(boolean creditCard) {
         if (!isValidStringType()) {
-            throw new IllegalStateException("CreditCard constraint can only be applied to String properties");
+            throw new ConstraintException("CreditCard constraint can only be applied to String properties");
         }
 
         Constraint c = appliedConstraints.get(CREDIT_CARD_CONSTRAINT);
@@ -676,7 +676,7 @@ public String getMatches() {
      */
     public void setMatches(String regex) {
         if (!isValidStringType()) {
-            throw new IllegalStateException("Matches constraint can only be applied to String properties");
+            throw new ConstraintException("Matches constraint can only be applied to String properties");
         }
 
         Constraint c = appliedConstraints.get(MATCHES_CONSTRAINT);
@@ -841,7 +841,7 @@ public boolean isUrl() {
      */
     public void setUrl(boolean url) {
         if (!isValidStringType()) {
-            throw new IllegalStateException("URL constraint can only be applied to String properties");
+            throw new ConstraintException("Url constraint can only be applied to String properties");
         }
 
         Constraint c = appliedConstraints.get(URL_CONSTRAINT);

diff --git a/grails-validation/src/test/groovy/grails/validation/ConstraintTypeMismatchSpec.groovy b/grails-validation/src/test/groovy/grails/validation/ConstraintTypeMismatchSpec.groovy
@@ -0,0 +1,46 @@
+package grails.validation
+
+import grails.validation.exceptions.ConstraintException
+import spock.lang.Specification
+import spock.lang.Unroll
+
+class ConstraintTypeMismatchSpec extends Specification {
+
+    @Unroll
+    void 'test calling #methodName on a ConstrainedProperty for a non-String property'() {
+        given:
+        def cp = new ConstrainedProperty(ConstraintTypeMismatchSpec, 'testProperty', Integer);
+
+        expect:
+        cp."$methodName"() == expectedResult
+
+        where:
+        methodName     | expectedResult
+        'isEmail'      | false
+        'isCreditCard' | false
+        'isUrl'        | false
+        'getMatches'   | null
+        'isUrl'        | false
+    }
+
+    @Unroll
+    void 'test calling set#constraintName(#argValue) on a ConstrainedProperty for a non-String property'() {
+        given:
+        def cp = new ConstrainedProperty(ConstraintTypeMismatchSpec, 'testProperty', Integer);
+
+        when:
+        cp."set${constraintName}"(argValue)
+
+        then:
+        ConstraintException ex = thrown()
+        "$constraintName constraint can only be applied to String properties" == ex.message
+
+        where:
+        constraintName | argValue
+        'Email'        | true
+        'CreditCard'   | true
+        'Url'          | true
+        'Matches'      | '.*'
+        'Blank'        | true
+    }
+}