Fix isGetterOrSetter from incorrectly returning true for methods that start with set that aren't setters, Ex: setupData. Apply transactional behavior to getter methods when a setter is not found. Fixes apache/grails-data-mapping#1013 for GORM 6.0.x

Author: jameskleeh

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

@@ -1457,19 +1457,25 @@ public static void processVariableScopes(SourceUnit source, ClassNode classNode,
             scopeVisitor.visitMethod(methodNode);
         }
     }
-    
-    public static boolean isSetterOrGetterMethod(MethodNode md) {
+
+    public static boolean isGetterMethod(MethodNode md) {
         String methodName = md.getName();
-        
-        if((methodName.startsWith("set") && methodName.length() > 3) && md.getParameters() != null && md.getParameters().length == 1) {
-            return true;
-        }
-        
-        if(((methodName.startsWith("get") && methodName.length() > 3) || (methodName.startsWith("is") && methodName.length() > 2)) && (md.getParameters()==null || md.getParameters().length == 0)) {
-            return true;
-        }
-        
-        return false;
+
+        return (((methodName.startsWith("get") && methodName.length() > 3) || (methodName.startsWith("is") && methodName.length() > 2)) && (md.getParameters()==null || md.getParameters().length == 0));
+    }
+
+    public static boolean isSetterMethod(MethodNode md) {
+        String methodName = md.getName();
+
+        return ((methodName.startsWith("set") &&
+                methodName.length() > 3) &&
+                Character.isUpperCase(methodName.substring(3).charAt(0)) &&
+                md.getParameters() != null &&
+                md.getParameters().length == 1);
+    }
+
+    public static boolean isSetterOrGetterMethod(MethodNode md) {
+        return isGetterMethod(md) || isSetterMethod(md);
     }
 
     /**

grails-core/src/main/groovy/org/grails/transaction/transform/TransactionalTransform.groovy

@@ -18,6 +18,8 @@ package org.grails.transaction.transform
 
 import grails.compiler.DelegatingMethod
 import grails.transaction.Rollback
+import grails.util.GrailsClassUtils
+import grails.util.GrailsNameUtils
 import groovy.transform.TypeChecked
 import org.codehaus.groovy.ast.stmt.Statement
 import org.codehaus.groovy.ast.tools.GenericsUtils
@@ -114,7 +116,17 @@ class TransactionalTransform implements ASTTransformation{
     public void weaveTransactionalBehavior(SourceUnit source, ClassNode classNode, AnnotationNode annotationNode) {
         weaveTransactionManagerAware(source, classNode)
         List<MethodNode> methods = new ArrayList<MethodNode>(classNode.getMethods());
-        
+
+        List<String> setterMethodNames = []
+        Iterator<MethodNode> methodNodeIterator = methods.iterator()
+        while (methodNodeIterator.hasNext()) {
+            MethodNode md = methodNodeIterator.next()
+            if (isSetterMethod(md)) {
+                setterMethodNames.add(md.name)
+                methodNodeIterator.remove()
+            }
+        }
+
         for (MethodNode md in methods) {
             String methodName = md.getName()
             int modifiers = md.modifiers
@@ -132,8 +144,15 @@ class TransactionalTransform implements ASTTransformation{
                 }
 
                 if(METHOD_NAME_EXCLUDES.contains(methodName)) continue
-                
-                if(isSetterOrGetterMethod(md)) continue
+
+                if (isGetterMethod(md)) {
+                    final String propertyName = GrailsClassUtils.getPropertyForGetter(md.name, md.returnType.typeClass)
+                    final String setterName = GrailsNameUtils.getSetterName(propertyName)
+
+                    //If a setter exists for the getter, don't apply the transformation
+                    if (setterMethodNames.contains(setterName)) continue
+                }
+
 
                 // don't apply to methods added by traits
                 if(hasAnnotation(md, org.codehaus.groovy.transform.trait.Traits.TraitBridge.class)) continue

grails-core/src/test/groovy/grails/transaction/TransactionalTransformSpec.groovy

@@ -906,7 +906,83 @@ class SomeClass {
         ex.message.contains 'Cannot find matching method java.lang.String#lastName()'
 
     }
+
+
+    void "test transactional behavior is applied to getter methods without a setter"() {
+        given:
+        def someClass = new GroovyShell().evaluate('''
+package demo
+    
+@grails.transaction.Transactional
+class SomeClass {
+
+    public void setAge(String name) {
+        
+    }
+
+    public String getAge() {
+        'valid'
+    }
+
+    public String getPhone() {
+        'valid'
+    }
 }
+
+new SomeClass()
+''')
+
+        final transactionManager = getPlatformTransactionManager()
+        someClass.transactionManager = transactionManager
+
+        when:
+        someClass.setAge('x')
+
+        then:
+        transactionManager.transactionStarted == false
+
+        when:
+        someClass.getAge()
+
+        then:
+        transactionManager.transactionStarted == false
+
+        when:
+        someClass.getPhone()
+
+        then:
+        transactionManager.transactionStarted == true
+    }
+
+    void "test transactional behavior is applied to methods that aren't setters but start with set"() {
+        given:
+        def someClass = new GroovyShell().evaluate('''
+package demo
+
+    
+@grails.transaction.Transactional
+class SomeClass {
+
+    public void setupSessionAfterLogin(String username) {
+    }
+
+}
+
+new SomeClass()
+''')
+
+        final transactionManager = getPlatformTransactionManager()
+        someClass.transactionManager = transactionManager
+
+        when:
+        someClass.setupSessionAfterLogin()
+
+        then:
+        transactionManager.transactionStarted == true
+    }
+}
+
+
 @Transactional
 class TransactionalTransformSpecService implements InitializingBean {
     String name