Fix tests to allow getterOnly constrained. Fix GrailsClassUtils.isGetter to check for boolean

Author: jameskleeh

grails-core/src/main/groovy/grails/util/GrailsClassUtils.java

@@ -940,7 +940,7 @@ public static boolean isPropertyInherited(Class clz, String propertyName) {
      * @return true if the method is a property getter
      */
     public static boolean isPropertyGetter(Method method) {
-        return !Modifier.isStatic(method.getModifiers()) && Modifier.isPublic(method.getModifiers()) && isGetter(method.getName(), method.getParameterTypes());
+        return !Modifier.isStatic(method.getModifiers()) && Modifier.isPublic(method.getModifiers()) && isGetter(method.getName(), method.getReturnType(), method.getParameterTypes());
     }
 
     /**
@@ -1022,16 +1022,30 @@ public static String getSetterName(String propertyName) {
      * @param name The name of the method
      * @param args The arguments
      * @return true if it is a javabean property getter
+     * @deprecated use {@link #isGetter(String, Class, Class[])} instead because this method has a defect for "is.." method with Boolean return types.
      */
     public static boolean isGetter(String name, Class<?>[] args) {
+        return isGetter(name, boolean.class, args);
+    }
+
+    /**
+     * Returns true if the name of the method specified and the number of arguments make it a javabean property getter.
+     * The name is assumed to be a valid Java method name, that is not verified.
+     *
+     * @param name The name of the method
+     * @param returnType The return type of the method
+     * @param args The arguments
+     * @return true if it is a javabean property getter
+     */
+    public static boolean isGetter(String name, Class returnType, Class<?>[] args) {
         if (!StringUtils.hasText(name) || args == null)return false;
         if (args.length != 0)return false;
 
         if (name.startsWith("get")) {
             name = name.substring(3);
             if (isPropertyMethodSuffix(name)) return true;
         }
-        else if (name.startsWith("is")) {
+        else if (name.startsWith("is") && returnType == boolean.class) {
             name = name.substring(2);
             if (isPropertyMethodSuffix(name)) return true;
         }

grails-core/src/main/groovy/org/grails/compiler/injection/GrailsASTUtils.java

@@ -263,7 +263,7 @@ public static MethodNode addDelegateInstanceMethod(ClassNode classNode, Expressi
         if (classNode.hasDeclaredMethod(methodName, copyParameters(parameterTypes, genericsPlaceholders))) {
             return null;
         }
-        String propertyName = GrailsClassUtils.getPropertyForGetter(methodName);
+        String propertyName = GrailsClassUtils.getPropertyForGetter(methodName, declaredMethod.getReturnType().getTypeClass());
         if (propertyName != null && parameterTypes.length == 0 && classNode.hasProperty(propertyName)) {
             return null;
         }

grails-core/src/main/groovy/org/grails/core/metaclass/BaseApiProvider.java

@@ -131,7 +131,7 @@ private boolean isNotExcluded(Method method, final int modifiers) {
         boolean isStatic = Modifier.isStatic(modifiers);
 
         // skip plain setters/getters by default for instance methods (non-static)
-        if (!isStatic && (GrailsClassUtils.isSetter(name, method.getParameterTypes()) || GrailsClassUtils.isGetter(name, method.getParameterTypes()))) {
+        if (!isStatic && (GrailsClassUtils.isSetter(name, method.getParameterTypes()) || GrailsClassUtils.isGetter(name, method.getReturnType(), method.getParameterTypes()))) {
             return false;
         }
 

grails-core/src/main/groovy/org/grails/core/util/ClassPropertyFetcher.java

@@ -130,7 +130,7 @@ public void doWith(CachedMethod method) throws IllegalArgumentException,
                     return;
                 }
 
-                String propertyName = GrailsClassUtils.getPropertyForGetter(method.getName());
+                String propertyName = GrailsClassUtils.getPropertyForGetter(method.getName(), method.getReturnType());
                 if(propertyName == null || propertyName.indexOf('$') != -1) {
                     return;
                 }

grails-test-suite-uber/src/test/groovy/grails/validation/CommandObjectConstraintGettersSpec.groovy

@@ -52,17 +52,18 @@ class CommandObjectConstraintGettersSpec extends Specification {
     }
 
 
-    void 'ensure only public non-static properties with getter and setter are constrained properties'() {
+    void 'ensure only public non-static properties with getter are constrained properties'() {
         MethodPropertiesCommand command = new MethodPropertiesCommand()
 
         when: 'empty command with method properties is validated'
         command.validate()
 
         //anotherPublicProperty did not error because it is boolean which defaults to false
-        then: 'only public with getter and setter should fail'
+        then: 'only public with getter should fail'
         command.hasErrors()
         command.errors['booleanPublicProperty']?.code == 'nullable'
-        command.errors.getErrorCount() == 1
+        command.errors['getterOnly']?.code == 'nullable'
+        command.errors.getErrorCount() == 2
     }
 
 
@@ -71,9 +72,10 @@ class CommandObjectConstraintGettersSpec extends Specification {
         Map constrainedProperties = MethodPropertiesCommand.getConstraintsMap()
 
         then: 'only public property with getter and setter should fail'
-        constrainedProperties.size() == 2
+        constrainedProperties.size() == 3
         constrainedProperties.containsKey('anotherPublicProperty')
         constrainedProperties.containsKey('booleanPublicProperty')
+        constrainedProperties.containsKey('getterOnly')
     }
 
     // COMMAND OBJECT WITH SUPER CLASS
@@ -104,7 +106,7 @@ class CommandObjectConstraintGettersSpec extends Specification {
     }
 
 
-    void 'ensure only public non-static inherited properties with getter and setter are constrained properties'() {
+    void 'ensure only public non-static inherited properties with getter are constrained properties'() {
         InheritedMethodPropertiesCommand command = new InheritedMethodPropertiesCommand()
         when: 'empty command with method properties is validated'
         command.validate()
@@ -113,18 +115,20 @@ class CommandObjectConstraintGettersSpec extends Specification {
         then: 'only public with getter and setter should fail'
         command.hasErrors()
         command.errors['booleanPublicProperty']?.code == 'nullable'
-        command.errors.getErrorCount() == 1
+        command.errors['getterOnly']?.code == 'nullable'
+        command.errors.getErrorCount() == 2
     }
 
 
-    void 'ensure constrained inherited method properties are only public ones with both getter and setter'() {
+    void 'ensure constrained inherited method properties are only public ones with getter'() {
         when: 'constrained properties map is get from child class'
         Map constrainedProperties = InheritedMethodPropertiesCommand.getConstraintsMap()
 
         then: 'only public with getter and setter should be there'
-        constrainedProperties.size() == 2
+        constrainedProperties.size() == 3
         constrainedProperties.containsKey('anotherPublicProperty')
         constrainedProperties.containsKey('booleanPublicProperty')
+        constrainedProperties.containsKey('getterOnly')
     }
 
     // COMMAND OBJECT WITH TRAIT
@@ -155,7 +159,7 @@ class CommandObjectConstraintGettersSpec extends Specification {
     }
 
 
-    void 'ensure only public non-static properties from trait with getter and setter are constrained properties'() {
+    void 'ensure only public non-static properties from trait with getter are constrained properties'() {
         TraitMethodPropertiesCommand command = new TraitMethodPropertiesCommand()
         when: 'empty command with simple properties is validated'
         command.validate()
@@ -164,23 +168,25 @@ class CommandObjectConstraintGettersSpec extends Specification {
         then: 'all should fail on nullable constraint'
         command.hasErrors()
         command.errors['booleanPublicProperty']?.code == 'nullable'
-        command.errors.getErrorCount() == 1
+        command.errors['getterOnly']?.code == 'nullable'
+        command.errors.getErrorCount() == 2
     }
 
 
-    void 'ensure constrained method properties from trait are only public ones with both getter and setter'() {
+    void 'ensure constrained method properties from trait are only public ones with getter'() {
         when: 'constrained properties map is get'
         Map constrainedProperties = TraitMethodPropertiesCommand.getConstraintsMap()
 
         then: 'only 4 defined public properties are there'
-        constrainedProperties.size() == 2
+        constrainedProperties.size() == 3
         constrainedProperties.containsKey('anotherPublicProperty')
         constrainedProperties.containsKey('booleanPublicProperty')
+        constrainedProperties.containsKey('getterOnly')
     }
 
     // BOOL METHODS COMMAND OBJECT
 
-    void 'ensure only public non-static bool properties with getter and setter are constrained properties'() {
+    void 'ensure only public non-static bool properties with getter are constrained properties'() {
         BoolMethodPropertiesCommand command = new BoolMethodPropertiesCommand()
         when: 'empty command with method properties is validated'
         command.validate()
@@ -189,23 +195,25 @@ class CommandObjectConstraintGettersSpec extends Specification {
         then: 'only public with getter and setter should fail'
         command.hasErrors()
         command.errors['booleanPublicProperty']?.code == 'nullable'
-        command.errors.getErrorCount() == 1
+        command.errors['getterOnly']?.code == 'nullable'
+        command.errors.getErrorCount() == 2
     }
 
 
-    void 'ensure constrained bool method properties are only public ones with both getter and setter'() {
+    void 'ensure constrained bool method properties are only public ones with getter'() {
         when: 'constrained properties map is get'
         Map constrainedProperties = BoolMethodPropertiesCommand.getConstraintsMap()
 
         then: 'only public property with getter and setter should fail'
-        constrainedProperties.size() == 2
+        constrainedProperties.size() == 3
         constrainedProperties.containsKey('anotherPublicProperty')
         constrainedProperties.containsKey('booleanPublicProperty')
+        constrainedProperties.containsKey('getterOnly')
     }
 
     // BOOL COMMAND OBJECT WITH SUPER CLASS
 
-    void 'ensure only public non-static inherited bool properties with getter and setter are constrained properties'() {
+    void 'ensure only public non-static inherited bool properties with getter are constrained properties'() {
         InheritedBoolMethodPropertiesCommand command = new InheritedBoolMethodPropertiesCommand()
         when: 'empty command with method properties is validated'
         command.validate()
@@ -214,23 +222,25 @@ class CommandObjectConstraintGettersSpec extends Specification {
         then: 'only public with getter and setter should fail'
         command.hasErrors()
         command.errors['booleanPublicProperty']?.code == 'nullable'
-        command.errors.getErrorCount() == 1
+        command.errors['getterOnly']?.code == 'nullable'
+        command.errors.getErrorCount() == 2
     }
 
-    void 'ensure constrained inherited bool method properties are only public ones with both getter and setter'() {
+    void 'ensure constrained inherited bool method properties are only public ones with getter'() {
         when: 'constrained properties map is get from child class'
         Map constrainedProperties = InheritedBoolMethodPropertiesCommand.getConstraintsMap()
 
         then: 'only public with getter and setter should be there'
-        constrainedProperties.size() == 2
+        constrainedProperties.size() == 3
         constrainedProperties.containsKey('anotherPublicProperty')
         constrainedProperties.containsKey('booleanPublicProperty')
+        constrainedProperties.containsKey('getterOnly')
     }
 
     // BOOL COMMAND OBJECT WITH TRAIT
 
 
-    void 'ensure only public non-static bool properties from trait with getter and setter are constrained properties'() {
+    void 'ensure only public non-static bool properties from trait with getter are constrained properties'() {
         TraitBoolMethodPropertiesCommand command = new TraitBoolMethodPropertiesCommand()
         when: 'empty command with simple properties is validated'
         command.validate()
@@ -239,17 +249,19 @@ class CommandObjectConstraintGettersSpec extends Specification {
         then: 'all should fail on nullable constraint'
         command.hasErrors()
         command.errors['booleanPublicProperty']?.code == 'nullable'
-        command.errors.getErrorCount() == 1
+        command.errors['getterOnly']?.code == 'nullable'
+        command.errors.getErrorCount() == 2
     }
 
-    void 'ensure constrained bool method properties from trait are only public ones with both getter and setter'() {
+    void 'ensure constrained bool method properties from trait are only public ones with getter'() {
         when: 'constrained properties map is get'
         Map constrainedProperties = TraitBoolMethodPropertiesCommand.getConstraintsMap()
 
         then: 'only 4 defined public properties are there'
-        constrainedProperties.size() == 2
+        constrainedProperties.size() == 3
         constrainedProperties.containsKey('anotherPublicProperty')
         constrainedProperties.containsKey('booleanPublicProperty')
+        constrainedProperties.containsKey('getterOnly')
     }
 }
 

grails-validation/src/main/groovy/org/grails/validation/DefaultConstraintEvaluator.java

@@ -409,12 +409,8 @@ else if(writeMethod == null || (Modifier.isTransient(readMethod.getModifiers()))
             for (Method method : clazz.getDeclaredMethods()) {
                 if (GrailsClassUtils.isPropertyGetter(method)) {
                     String propertyName = GrailsClassUtils.getPropertyForGetter(method.getName(), method.getReturnType());
-                    if (!ignoredProperties.contains(propertyName)) {
-                        System.out.println(method.getName());
-                        PropertyDescriptor descriptor = BeanUtils.getPropertyDescriptor(theClass, propertyName);
-                        if (descriptor != null && descriptor.getWriteMethod() != null) {
-                            propertyMap.put(propertyName, method);
-                        }
+                    if (propertyName != null && !ignoredProperties.contains(propertyName)) {
+                        propertyMap.put(propertyName, method);
                     }
                 }
             }