Properties with only getters shouldn't be constrained. Fixes #9754

Author: graemerocher

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

@@ -137,19 +137,17 @@ class DomainClassValidationSpec extends Specification {
         def constraints = getAssociatedDomainClassFromApplication(new MyDomainClass()).getConstrainedProperties()
 
         then:
-        constraints.size() == 5
+        constraints.size() == 4
         constraints.containsKey 'name'
         constraints.containsKey 'town'
         constraints.containsKey 'age'
         constraints.containsKey 'someProperty'
-        constraints.containsKey 'twiceAge' // only getter method
 
         and:
         constraints.name.appliedConstraints.size() == 2
         constraints.age.appliedConstraints.size() == 2
         constraints.town.appliedConstraints.size() == 1
         constraints.someProperty.appliedConstraints.size() == 1
-        constraints.twiceAge.appliedConstraints.size() == 1
 
         and:
         constraints.name.hasAppliedConstraint 'matches'
@@ -158,14 +156,12 @@ class DomainClassValidationSpec extends Specification {
         constraints.age.hasAppliedConstraint 'nullable'
         constraints.town.hasAppliedConstraint 'nullable'
         constraints.someProperty.hasAppliedConstraint 'nullable'
-        constraints.twiceAge.hasAppliedConstraint 'nullable'
 
         and: 'implicit defaultNullable is nullable:false'
         !constraints.name.nullable
         !constraints.age.nullable
         !constraints.town.nullable
         !constraints.someProperty.nullable
-        !constraints.twiceAge.nullable
     }
 
     @Ignore('defaultNullable is not supported yet')
@@ -216,9 +212,8 @@ class DomainClassValidationSpec extends Specification {
         expect: 'validation is executed and public properties/getters are marked nullable in errors'
         !obj.validate()
         obj.hasErrors()
-        obj.errors.errorCount == 2
+        obj.errors.errorCount == 1
         obj.errors['town']?.code == 'nullable' // public property
-        obj.errors['name']?.code == 'nullable' // only public getter method
         !obj.errors['surname'] // only protected getter method
         !obj.errors['email'] // only private getter method
     }
@@ -228,8 +223,7 @@ class DomainClassValidationSpec extends Specification {
         Map constraints = getAssociatedDomainClassFromApplication(new DomainClassGetters()).getConstrainedProperties()
 
         then: 'only public properties and public getters should be considered domainClass properties by default'
-        constraints.size() == 5
-        constraints.name
+        constraints.size() == 4
         constraints.town
         constraints.bar
         constraints.qux

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

@@ -18,6 +18,7 @@
 import grails.core.GrailsDomainClass;
 import grails.core.GrailsDomainClassProperty;
 import grails.io.IOUtils;
+import grails.persistence.PersistenceMethod;
 import grails.util.GrailsClassUtils;
 import grails.validation.Constrained;
 import grails.validation.ConstrainedProperty;
@@ -34,6 +35,8 @@
 import org.grails.core.artefact.DomainClassArtefactHandler;
 import org.grails.core.exceptions.GrailsConfigurationException;
 import org.grails.core.support.GrailsDomainConfigurationUtil;
+import org.springframework.beans.BeanUtils;
+import org.springframework.beans.CachedIntrospectionResults;
 
 import java.beans.BeanInfo;
 import java.beans.IntrospectionException;
@@ -43,13 +46,7 @@
 import java.lang.reflect.Field;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 
 /**
  * Default implementation of the {@link grails.validation.ConstraintsEvaluator} interface.
@@ -156,7 +153,7 @@ protected Map<String, Constrained> evaluateConstraints(Map<String, Constrained>
 
         boolean isDomainClass = DomainClassArtefactHandler.isDomainClass(theClass);
         Map<String, GrailsDomainClassProperty> domainClassPropertyMap = indexPropertiesByPropertyName(domainClassProperties);
-        Map<String, Method> constrainablePropertyMap = getConstrainablePropertyMap(theClass);
+        Map<String, Method> constrainablePropertyMap = getConstrainablePropertyMap(theClass, isDomainClass);
 
         for (String propertyName : constrainablePropertyMap.keySet()) {
             GrailsDomainClassProperty domainClassProperty = domainClassPropertyMap.get(propertyName);
@@ -177,6 +174,10 @@ protected Map<String, Constrained> evaluateConstraints(Map<String, Constrained>
                         }
                     }
                 }
+                else {
+                    // for domain class, a constraint of a property as only getter method is not supported
+                    continue;
+                }
             }
 
             // complete constraints not defined by user.
@@ -359,38 +360,42 @@ protected void applyDefaultNullableConstraint(Constrained constrained, boolean d
         }
     }
 
-    protected Map<String, Method> getConstrainablePropertyMap(Class theClass) {
-        List<String> ignoredProperties = new ArrayList<>();
+    protected Map<String, Method> getConstrainablePropertyMap(Class theClass, boolean isDomainClass) {
+        Set<String> ignoredProperties = new HashSet<>();
         ignoredProperties.add(GrailsDomainClassProperty.CLASS);
         ignoredProperties.add(GrailsDomainClassProperty.META_CLASS);
         ignoredProperties.add(GrailsDomainClassProperty.ERRORS);
-        if (DomainClassArtefactHandler.isDomainClass(theClass)) {
+
+        if (isDomainClass) {
             ignoredProperties.add(GrailsDomainConfigurationUtil.PROPERTIES_PROPERTY);
             ignoredProperties.add(GrailsDomainClassProperty.IDENTITY);
             ignoredProperties.add(GrailsDomainClassProperty.VERSION);
             ignoredProperties.add(GrailsDomainClassProperty.DIRTY_PROPERTY_NAMES);
             ignoredProperties.add(GrailsDomainClassProperty.DIRTY);
             ignoredProperties.add(GrailsDomainClassProperty.ATTACHED);
-        }
-
-        final Object transients = GrailsClassUtils.getStaticPropertyValue(theClass,
-                                                                          GrailsDomainClassProperty.TRANSIENT);
-        if(transients instanceof List) {
-            ignoredProperties.addAll((List) transients);
-        }
-        try {
-            final BeanInfo beanInfo = Introspector.getBeanInfo(theClass);
-            final PropertyDescriptor[] propertyDescriptors = beanInfo.getPropertyDescriptors();
+            final Object transients = GrailsClassUtils.getStaticPropertyValue(theClass,
+                    GrailsDomainClassProperty.TRANSIENT);
+            if(transients instanceof List) {
+                ignoredProperties.addAll((List) transients);
+            }
+            final PropertyDescriptor[] propertyDescriptors = BeanUtils.getPropertyDescriptors(theClass);
             for(PropertyDescriptor descriptor : propertyDescriptors) {
                 final Method readMethod = descriptor.getReadMethod();
-                if(readMethod != null && Modifier.isTransient(readMethod.getModifiers())) {
+
+                final Method writeMethod = descriptor.getWriteMethod();
+                if( readMethod == null ) {
                     ignoredProperties.add(descriptor.getName());
                 }
+                else if(writeMethod == null || (Modifier.isTransient(readMethod.getModifiers()))) {
+                    PersistenceMethod annotation = readMethod.getAnnotation(PersistenceMethod.class);
+                    if(annotation == null) {
+                        ignoredProperties.add(descriptor.getName());
+                    }
+                }
             }
-        } catch (IntrospectionException e) {
-            LOG.error("An error occurred introspecting properties", e);
         }
 
+
         Field[] declaredFields = theClass.getDeclaredFields();
         for(Field field : declaredFields) {
             if(Modifier.isTransient(field.getModifiers())) {