fix for GRAILS-3209 "GORM inheritance related bug with id: this.id == null != this.getId() when accessed in method for a class that is subclassed"

Author: graemerocher

src/java/grails/util/GrailsNameUtils.java

@@ -27,6 +27,27 @@
  */
 public class GrailsNameUtils {
 
+    private static final String PROPERTY_SET_PREFIX = "set";
+    
+    /**
+     * Retrieves the name of a setter for the specified property name
+     * @param propertyName The property name
+     * @return The setter equivalent
+     */
+    public static String getSetterName(String propertyName) {
+        return PROPERTY_SET_PREFIX+propertyName.substring(0,1).toUpperCase()+ propertyName.substring(1);
+    }
+    
+    /**
+     * Calculate the name for a getter method to retrieve the specified property
+     * @param propertyName
+     * @return The name for the getter method for this property, if it were to exist, i.e. getConstraints
+     */
+    public static String getGetterName(String propertyName) {
+        return "get" + Character.toUpperCase(propertyName.charAt(0))
+            + propertyName.substring(1);
+    }
+    
     /**
      * Returns the class name for the given logical name and trailing name. For example "person" and "Controller" would evaluate to "PersonController"
      *

src/java/org/codehaus/groovy/grails/commons/GrailsClassUtils.java

@@ -14,6 +14,7 @@
  */
 package org.codehaus.groovy.grails.commons;
 
+import grails.util.GrailsNameUtils;
 import groovy.lang.*;
 
 import org.apache.commons.lang.StringUtils;
@@ -36,7 +37,7 @@
  */
 public class GrailsClassUtils {
 
-    private static final String PROPERTY_SET_PREFIX = "set";
+    
     public static final Map<Class<?>, Class<?>> PRIMITIVE_TYPE_COMPATIBLE_CLASSES = new HashMap<Class<?>, Class<?>>();
 
     /**
@@ -631,8 +632,7 @@ public static boolean isPublicStatic(Field f) {
      * @return The name for the getter method for this property, if it were to exist, i.e. getConstraints
      */
     public static String getGetterName(String propertyName) {
-        return "get" + Character.toUpperCase(propertyName.charAt(0))
-            + propertyName.substring(1);
+        return GrailsNameUtils.getGetterName(propertyName);
     }
 
     /**
@@ -812,7 +812,7 @@ public static String getLogicalPropertyName(String className, String trailingNam
      * @return The setter equivalent
      */
     public static String getSetterName(String propertyName) {
-        return PROPERTY_SET_PREFIX+propertyName.substring(0,1).toUpperCase()+ propertyName.substring(1);
+        return GrailsNameUtils.getSetterName(propertyName);
     }
 
     /**

src/java/org/codehaus/groovy/grails/compiler/injection/DefaultGrailsDomainClassInjector.java

@@ -198,15 +198,19 @@ private void injectVersionProperty(ClassNode classNode) {
         final boolean hasVersion = GrailsASTUtils.hasOrInheritsProperty(classNode, GrailsDomainClassProperty.VERSION);
 
         if (!hasVersion) {
-            classNode.addProperty(GrailsDomainClassProperty.VERSION, Modifier.PUBLIC, new ClassNode(Long.class), null, null, null);
+            ClassNode parent = GrailsASTUtils.getFurthestParent(classNode);
+            parent.addProperty(GrailsDomainClassProperty.VERSION, Modifier.PUBLIC, new ClassNode(Long.class), null, null, null);
         }
     }
 
     private void injectIdProperty(ClassNode classNode) {
         final boolean hasId = GrailsASTUtils.hasOrInheritsProperty(classNode, GrailsDomainClassProperty.IDENTITY);
 
         if (!hasId) {
-            classNode.addProperty(GrailsDomainClassProperty.IDENTITY, Modifier.PUBLIC, new ClassNode(Long.class), null, null, null);
+            // inject into furthest relative
+            ClassNode parent = GrailsASTUtils.getFurthestParent(classNode);
+
+            parent.addProperty(GrailsDomainClassProperty.IDENTITY, Modifier.PUBLIC, new ClassNode(Long.class), null, null, null);
         }
     }
 

src/java/org/codehaus/groovy/grails/compiler/injection/GrailsASTUtils.java

@@ -15,6 +15,7 @@
  */
 package org.codehaus.groovy.grails.compiler.injection;
 
+import grails.util.GrailsNameUtils;
 import org.apache.commons.lang.StringUtils;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.MethodNode;
@@ -44,11 +45,16 @@ public class GrailsASTUtils {
     public static boolean hasProperty(ClassNode classNode, String propertyName) {
         if (classNode == null || StringUtils.isBlank(propertyName))
             return false;
-        List properties = classNode.getProperties();
-        for (Iterator i = properties.iterator(); i.hasNext();) {
-            PropertyNode pn = (PropertyNode) i.next();
-            if (pn.getName().equals(propertyName) && !pn.isPrivate()) {
-                return true;
+
+        final MethodNode method = classNode.getMethod(GrailsNameUtils.getGetterName(propertyName), new Parameter[0]);
+        if(method != null) return true;
+        else {
+            List properties = classNode.getProperties();
+            for (Object property : properties) {
+                PropertyNode pn = (PropertyNode) property;
+                if (pn.getName().equals(propertyName) && !pn.isPrivate()) {
+                    return true;
+                }
             }
         }
         return false;
@@ -106,4 +112,12 @@ public static String getFullName(ClassNode classNode) {
       return classNode.getName();
     }
 
+    public static ClassNode getFurthestParent(ClassNode classNode) {
+        ClassNode parent = classNode.getSuperClass();
+        while (parent != null && !getFullName(parent).equals("java.lang.Object")) {
+            parent = parent.getSuperClass();
+            classNode = parent;
+        }
+        return classNode;
+    }
 }

src/test/org/codehaus/groovy/grails/orm/hibernate/IdInheritanceTests.groovy

@@ -0,0 +1,50 @@
+package org.codehaus.groovy.grails.orm.hibernate
+
+/**
+ * @author Graeme Rocher
+ * @since 1.1
+ */
+class IdInheritanceTests extends AbstractGrailsHibernateTests {
+
+    protected void onSetUp() {
+        gcl.parseClass('''
+import grails.persistence.*
+
+@Entity
+abstract class Parent {
+  String toString() {
+    return "${id}"
+  }
+}
+''')
+        gcl.parseClass('''
+import grails.persistence.*
+
+@Entity
+class Child extends Parent {
+
+  static transients = ['superId']
+
+  Long getSuperId() {
+    return super.id
+  }
+}
+
+''')
+    }
+
+
+  void testDirectAccess() {
+    def child = ga.getDomainClass("Child").newInstance()
+    child.save('flush': true)
+
+    assert 1 == child.id
+  }
+
+  void testInheritedMethodAccess() {
+    def child = ga.getDomainClass("Child").newInstance()
+    child.save('flush': true)
+
+    assert "1" == child.toString()
+  }
+}