Replace access to superclass fields in Panache entity classes with ca…

…lls to getter/setter in super

Instead of leaving the field access in place.

extensions/panache/panache-common/deployment/src/main/java/io/quarkus/panache/common/deployment/PanacheFieldAccessMethodVisitor.java

@@ -9,50 +9,79 @@
 public class PanacheFieldAccessMethodVisitor extends MethodVisitor {
 
     private final String methodName;
-    private String owner;
-    private String methodDescriptor;
-    private MetamodelInfo modelInfo;
+    private final String methodOwner;
+    private final String methodOwnerClassName;
+    private final String methodDescriptor;
+    private final MetamodelInfo modelInfo;
 
-    public PanacheFieldAccessMethodVisitor(MethodVisitor methodVisitor, String owner,
+    public PanacheFieldAccessMethodVisitor(MethodVisitor methodVisitor, String methodOwner,
             String methodName, String methodDescriptor,
             MetamodelInfo modelInfo) {
         super(Gizmo.ASM_API_VERSION, methodVisitor);
-        this.owner = owner;
+        this.methodOwner = methodOwner;
+        this.methodOwnerClassName = methodOwner.replace('/', '.');
         this.methodName = methodName;
         this.methodDescriptor = methodDescriptor;
         this.modelInfo = modelInfo;
     }
 
     @Override
-    public void visitFieldInsn(int opcode, String owner, String fieldName, String descriptor) {
-        String ownerName = owner.replace('/', '.');
-        if ((opcode == Opcodes.GETFIELD
-                || opcode == Opcodes.PUTFIELD)
+    public void visitFieldInsn(int opcode, String fieldOwner, String fieldName, String descriptor) {
+        String fieldOwnerName = fieldOwner.replace('/', '.');
+        if ( // we only care about non-static access
+        !(opcode == Opcodes.GETFIELD || opcode == Opcodes.PUTFIELD)
                 // if we're in the constructor, do not replace field accesses to this type and its supertypes
                 // otherwise we risk running setters that depend on initialisation
-                && (!this.methodName.equals("<init>")
-                        || !targetIsInHierarchy(this.owner.replace('/', '.'), ownerName))
-                && isEntityField(ownerName, fieldName)) {
-            String methodName;
-            String methodDescriptor;
-            if (opcode == Opcodes.GETFIELD) {
-                methodName = JavaBeanUtil.getGetterName(fieldName, descriptor);
-                methodDescriptor = "()" + descriptor;
-            } else {
-                methodName = JavaBeanUtil.getSetterName(fieldName);
-                methodDescriptor = "(" + descriptor + ")V";
+                || (this.methodName.equals("<init>")
+                        && targetIsInHierarchy(this.methodOwnerClassName, fieldOwnerName))) {
+            // In those cases, don't do anything.
+            super.visitFieldInsn(opcode, fieldOwner, fieldName, descriptor);
+            return;
+        }
+
+        String declaringEntityClassName = getDeclaringEntityClassName(fieldOwnerName, fieldName);
+        if (declaringEntityClassName == null) {
+            // Not an entity field, don't do anything.
+            super.visitFieldInsn(opcode, fieldOwner, fieldName, descriptor);
+            return;
+        }
+
+        String methodName;
+        String methodDescriptor;
+        if (opcode == Opcodes.GETFIELD) {
+            methodName = JavaBeanUtil.getGetterName(fieldName, descriptor);
+            methodDescriptor = "()" + descriptor;
+        } else {
+            methodName = JavaBeanUtil.getSetterName(fieldName);
+            methodDescriptor = "(" + descriptor + ")V";
+        }
+        if (methodName.equals(this.methodName)
+                && methodDescriptor.equals(this.methodDescriptor)
+                && targetIsInHierarchy(methodOwnerClassName, fieldOwnerName)) {
+            // The current method accessing the entity field is the corresponding getter/setter.
+
+            // If the current method is defined in the class that declares the field,
+            // we don't perform substitution at all.
+            if (declaringEntityClassName.equals(methodOwnerClassName)) {
+                super.visitFieldInsn(opcode, fieldOwner, fieldName, descriptor);
+                return;
             }
-            if (!owner.equals(this.owner)
-                    || !methodName.equals(this.methodName)
-                    || !methodDescriptor.equals(this.methodDescriptor)) {
-                super.visitMethodInsn(Opcodes.INVOKEVIRTUAL, owner, methodName, methodDescriptor, false);
-            } else {
-                // do not substitute to accessors inside its own accessor
-                super.visitFieldInsn(opcode, owner, fieldName, descriptor);
+            // Otherwise the current method is considered an override of
+            // the getter/setter which should be defined (manually or through bytecode generation)
+            // in the superclass that also declares the field.
+            // We will replace the field access with a call super.get*/super.set*.
+            EntityModel methodOwnerModel = modelInfo.getEntityModel(methodOwnerClassName);
+            // This should always be true since targetIsInHierarchy returned true, but let's be safe.
+            if (methodOwnerModel != null && methodOwnerModel.superClassName != null) {
+                super.visitMethodInsn(Opcodes.INVOKESPECIAL,
+                        methodOwnerModel.superClassName.replace('.', '/'),
+                        methodName, methodDescriptor, false);
+                return;
             }
-        } else {
-            super.visitFieldInsn(opcode, owner, fieldName, descriptor);
         }
+
+        // We found a relevant field access: replace it.
+        super.visitMethodInsn(Opcodes.INVOKEVIRTUAL, fieldOwner, methodName, methodDescriptor, false);
     }
 
     /**
@@ -73,15 +102,15 @@ private boolean targetIsInHierarchy(String currentClass, String targetClass) {
     /**
      * Checks that the given field belongs to an entity (any entity)
      */
-    boolean isEntityField(String className, String fieldName) {
-        EntityModel entityModel = modelInfo.getEntityModel(className);
+    String getDeclaringEntityClassName(String encounteredClassName, String fieldName) {
+        EntityModel entityModel = modelInfo.getEntityModel(encounteredClassName);
         if (entityModel == null)
-            return false;
+            return null;
         EntityField field = entityModel.fields.get(fieldName);
         if (field != null)
-            return true;
+            return encounteredClassName;
         if (entityModel.superClassName != null)
-            return isEntityField(entityModel.superClassName, fieldName);
-        return false;
+            return getDeclaringEntityClassName(entityModel.superClassName, fieldName);
+        return null;
     }
 }