Merge pull request quarkusio#26491 from famod/spring-data-parent-id-t…

…yped

Fix spring-data-jpa field lookup in parameterized superclasses and for typed fields

extensions/spring-data-jpa/deployment/src/main/java/io/quarkus/spring/data/deployment/MethodNameParser.java

@@ -15,10 +15,10 @@
 import org.jboss.jandex.FieldInfo;
 import org.jboss.jandex.IndexView;
 import org.jboss.jandex.MethodInfo;
-import org.jboss.jandex.ParameterizedType;
 import org.jboss.jandex.Type;
-import org.jboss.jandex.Type.Kind;
+import org.jboss.jandex.TypeVariable;
 
+import io.quarkus.deployment.util.JandexUtil;
 import io.quarkus.panache.common.Sort;
 
 public class MethodNameParser {
@@ -351,6 +351,7 @@ private FieldInfo resolveNestedField(String repositoryMethodDescription, String
         ClassInfo parentClassInfo = this.entityClass;
         FieldInfo fieldInfo = null;
 
+        MutableReference<List<ClassInfo>> parentSuperClassInfos = new MutableReference<>();
         int fieldStartIndex = 0;
         while (fieldStartIndex < fieldPathExpression.length()) {
             if (fieldPathExpression.charAt(fieldStartIndex) == '_') {
@@ -359,7 +360,6 @@ private FieldInfo resolveNestedField(String repositoryMethodDescription, String
                     throw new UnableToParseMethodException(fieldNotResolvableMessage + offendingMethodMessage);
                 }
             }
-            MutableReference<List<ClassInfo>> parentSuperClassInfos = new MutableReference<>();
             // the underscore character is treated as reserved character to manually define traversal points.
             int firstSeparator = fieldPathExpression.indexOf('_', fieldStartIndex);
             int fieldEndIndex = firstSeparator == -1 ? fieldPathExpression.length() : firstSeparator;
@@ -385,17 +385,28 @@ private FieldInfo resolveNestedField(String repositoryMethodDescription, String
             }
             fieldPathBuilder.append(fieldInfo.name());
             if (!isHibernateProvidedBasicType(fieldInfo.type().name())) {
-                parentClassInfo = indexView.getClassByName(fieldInfo.type().name());
+                DotName parentClassName;
+                boolean typed = false;
+                if (fieldInfo.type().kind() == Type.Kind.TYPE_VARIABLE) {
+                    typed = true;
+                    parentClassName = getParentNameFromTypedFieldViaHierarchy(fieldInfo, mappedSuperClassInfos);
+                } else {
+                    parentClassName = fieldInfo.type().name();
+                }
+                parentClassInfo = indexView.getClassByName(parentClassName);
+                parentSuperClassInfos.set(null);
                 if (parentClassInfo == null) {
                     throw new IllegalStateException(
                             "Entity class " + fieldInfo.type().name() + " referenced by "
                                     + this.entityClass + "." + fieldPathBuilder
-                                    + " was not part of the Quarkus index. " + offendingMethodMessage);
+                                    + " was not part of the Quarkus index"
+                                    + (typed ? " or typed field could not be resolved properly. " : ". ")
+                                    + offendingMethodMessage);
                 }
+
             }
             fieldStartIndex = fieldEndIndex;
         }
-
         return fieldInfo;
     }
 
@@ -543,12 +554,7 @@ private List<ClassInfo> getSuperClassInfos(IndexView indexView, ClassInfo entity
                 mappedSuperClassInfoElements.add(superClass);
             }
 
-            if (superClassType.kind() == Kind.CLASS) {
-                superClassType = superClass.superClassType();
-            } else if (superClassType.kind() == Kind.PARAMETERIZED_TYPE) {
-                ParameterizedType parameterizedType = superClassType.asParameterizedType();
-                superClassType = parameterizedType.owner();
-            }
+            superClassType = superClass.superClassType();
         }
         return mappedSuperClassInfoElements;
     }
@@ -557,6 +563,53 @@ private boolean isHibernateProvidedBasicType(DotName dotName) {
         return DotNames.HIBERNATE_PROVIDED_BASIC_TYPES.contains(dotName);
     }
 
+    // Tries to get the parent (name) of a field that is typed, e.g.:
+    // class Foo<ID extends Serializable> { @EmbeddedId ID id; }
+    // class Bar<ID extends SomeMoreSpecificBaseId> extends FOO<ID> { }
+    // class Baz extends Bar<BazId> { }
+    // @Embeddable class BazId extends SomeMoreSpecificBaseId { @Column String a; @Column String b; }
+    //
+    // Without this method, type of Foo.id would be Serializable and therefore Foo.id.a (or b) would not be found.
+    //
+    // Note: This method is lenient in that it doesn't throw exceptions aggressively when an assumption is not met.
+    private DotName getParentNameFromTypedFieldViaHierarchy(FieldInfo fieldInfo, List<ClassInfo> parentSuperClassInfos) {
+        // find in the hierarchy the position of the class the fieldInfo belongs to
+        int superClassIndex = parentSuperClassInfos.indexOf(fieldInfo.declaringClass());
+        if (superClassIndex == -1) {
+            // field seems to belong to concrete entity class; no use narrowing it down via class hierarchy
+            return fieldInfo.type().name();
+        }
+        TypeVariable typeVariable = fieldInfo.type().asTypeVariable();
+        // entire hierarchy as list: entityClass, superclass of entityClass, superclass of the latter, ...
+        List<ClassInfo> classInfos = new ArrayList<>();
+        classInfos.add(entityClass);
+        classInfos.addAll(parentSuperClassInfos.subList(0, superClassIndex + 1));
+        // go down the hierarchy until ideally a concrete class is found that specifies the actual type of the field
+        for (int i = classInfos.size() - 1; i > 0; i--) {
+            ClassInfo currentClassInfo = classInfos.get(i);
+            ClassInfo childClassInfo = classInfos.get(i - 1);
+            int typeParameterIndex = currentClassInfo.typeParameters().indexOf(typeVariable);
+            if (typeParameterIndex >= 0) {
+                List<Type> resolveTypeParameters = JandexUtil.resolveTypeParameters(childClassInfo.name(),
+                        currentClassInfo.name(), indexView);
+                if (resolveTypeParameters.size() <= typeParameterIndex) {
+                    // edge case; subclass without or with incomplete parameterization? raw types?
+                    break;
+                }
+                Type type = resolveTypeParameters.get(typeParameterIndex);
+                if (type.kind() == Type.Kind.TYPE_VARIABLE) {
+                    // go on with the type variable from the child class (which is potentially more specific that the previous one)
+                    typeVariable = type.asTypeVariable();
+                } else if (type.kind() == Type.Kind.CLASS) {
+                    // ideal outcome: concrete class, doesn't get more specific than this
+                    return type.name();
+                }
+            }
+        }
+        // return the most specific type variable we were able to get
+        return typeVariable.name();
+    }
+
     private static class MutableReference<T> {
         private T reference;
 

integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/AbstractPhoneEntity.java

@@ -0,0 +1,12 @@
+package io.quarkus.it.spring.data.jpa;
+
+import javax.persistence.MappedSuperclass;
+
+// demonstrates that @MappedSuperclass detection works for more than one level and for parameterized types
+@MappedSuperclass
+public abstract class AbstractPhoneEntity<ID extends PhoneNumberId> extends AbstractTypedIdEntity<ID> {
+
+    protected AbstractPhoneEntity(ID id) {
+        super(id);
+    }
+}

integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/AbstractTypedIdEntity.java

@@ -0,0 +1,22 @@
+package io.quarkus.it.spring.data.jpa;
+
+import java.io.Serializable;
+
+import javax.persistence.EmbeddedId;
+import javax.persistence.MappedSuperclass;
+
+// example "base entity" using strongly typed ids (instead of just long)
+@MappedSuperclass
+public abstract class AbstractTypedIdEntity<ID extends Serializable> {
+
+    @EmbeddedId
+    private ID id;
+
+    protected AbstractTypedIdEntity(ID id) {
+        this.id = id;
+    }
+
+    public ID getId() {
+        return id;
+    }
+}

integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCall.java

@@ -2,30 +2,23 @@
 
 import javax.persistence.Column;
 import javax.persistence.Embeddable;
-import javax.persistence.EmbeddedId;
 import javax.persistence.Entity;
 import javax.persistence.Table;
 
 @Entity
 @Table(name = "phone_call")
-public class PhoneCall {
-
-    @EmbeddedId
-    private PhoneNumberId id;
+public class PhoneCall extends AbstractPhoneEntity<PhoneCallId> {
 
     private int duration;
 
     private CallAgent callAgent;
 
-    PhoneCall() {
-    }
-
-    public PhoneCall(PhoneNumberId id) {
-        this.id = id;
+    PhoneCall() { // only for hibernate
+        super(null);
     }
 
-    public PhoneNumberId getId() {
-        return id;
+    public PhoneCall(PhoneCallId id) {
+        super(id);
     }
 
     public int getDuration() {

integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallId.java

@@ -0,0 +1,14 @@
+package io.quarkus.it.spring.data.jpa;
+
+import javax.persistence.Embeddable;
+
+@Embeddable
+public class PhoneCallId extends PhoneNumberId {
+
+    PhoneCallId() {
+    }
+
+    public PhoneCallId(String areaCode, String number) {
+        super(areaCode, number);
+    }
+}

integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallRepository.java

@@ -7,12 +7,12 @@
 
 import io.quarkus.it.spring.data.jpa.PhoneCall.CallAgent;
 
-public interface PhoneCallRepository extends JpaRepository<PhoneCall, PhoneNumberId> {
+public interface PhoneCallRepository extends JpaRepository<PhoneCall, PhoneCallId> {
 
     PhoneCall findByIdAreaCode(String areaCode);
 
     @Query("select p.id from PhoneCall p")
-    Set<PhoneNumberId> findAllIds();
+    Set<PhoneCallId> findAllIds();
 
     @Query("select p.callAgent from PhoneCall p")
     Set<CallAgent> findAllCallAgents();

integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallResource.java

@@ -20,7 +20,7 @@ public class PhoneCallResource {
     @GET
     @Produces("application/json")
     public PhoneCall phoneCallById(@PathParam("areaCode") String areaCode, @PathParam("number") String number) {
-        return repository.findById(new PhoneNumberId(areaCode, number)).orElse(null);
+        return repository.findById(new PhoneCallId(areaCode, number)).orElse(null);
     }
 
     @Path("{areaCode}")
@@ -33,7 +33,7 @@ public PhoneCall phoneCallByAreaCode(@PathParam("areaCode") String areaCode) {
     @Path("ids")
     @GET
     @Produces("application/json")
-    public Set<PhoneNumberId> allIds() {
+    public Set<PhoneCallId> allIds() {
         return repository.findAllIds();
     }
 

integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneNumberId.java

@@ -4,10 +4,11 @@
 import java.util.Objects;
 
 import javax.persistence.Column;
-import javax.persistence.Embeddable;
+import javax.persistence.MappedSuperclass;
 
-@Embeddable
-public class PhoneNumberId implements Serializable {
+// this is a bit artificial, but next to PhoneCallId there could be e.g. a PhoneBookEntryId subclass
+@MappedSuperclass
+public abstract class PhoneNumberId implements Serializable {
 
     @Column(name = "area_code")
     private String areaCode;