From 1a32fc763c86e69b088896fa9ae230472cdf3189 Mon Sep 17 00:00:00 2001 From: Falko Modler Date: Fri, 1 Jul 2022 00:43:27 +0200 Subject: [PATCH] Fix spring-data-jpa field lookup in parameterized superclasses and for typed fields --- .../data/deployment/MethodNameParser.java | 77 ++++++++++++++++--- .../spring/data/jpa/AbstractPhoneEntity.java | 12 +++ .../data/jpa/AbstractTypedIdEntity.java | 22 ++++++ .../quarkus/it/spring/data/jpa/PhoneCall.java | 17 ++-- .../it/spring/data/jpa/PhoneCallId.java | 14 ++++ .../spring/data/jpa/PhoneCallRepository.java | 4 +- .../it/spring/data/jpa/PhoneCallResource.java | 4 +- .../it/spring/data/jpa/PhoneNumberId.java | 7 +- 8 files changed, 126 insertions(+), 31 deletions(-) create mode 100644 integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/AbstractPhoneEntity.java create mode 100644 integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/AbstractTypedIdEntity.java create mode 100644 integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallId.java diff --git a/extensions/spring-data-jpa/deployment/src/main/java/io/quarkus/spring/data/deployment/MethodNameParser.java b/extensions/spring-data-jpa/deployment/src/main/java/io/quarkus/spring/data/deployment/MethodNameParser.java index 0be30eee83b5e..c5886556091dd 100644 --- a/extensions/spring-data-jpa/deployment/src/main/java/io/quarkus/spring/data/deployment/MethodNameParser.java +++ b/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> 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> 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 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 { @EmbeddedId ID id; } + // class Bar extends FOO { } + // class Baz extends Bar { } + // @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 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 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 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 { private T reference; diff --git a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/AbstractPhoneEntity.java b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/AbstractPhoneEntity.java new file mode 100644 index 0000000000000..e67bd3dd0a0cf --- /dev/null +++ b/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 extends AbstractTypedIdEntity { + + protected AbstractPhoneEntity(ID id) { + super(id); + } +} diff --git a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/AbstractTypedIdEntity.java b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/AbstractTypedIdEntity.java new file mode 100644 index 0000000000000..d105ceea4978a --- /dev/null +++ b/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 { + + @EmbeddedId + private ID id; + + protected AbstractTypedIdEntity(ID id) { + this.id = id; + } + + public ID getId() { + return id; + } +} diff --git a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCall.java b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCall.java index ae6d95ab7c73d..a3c5deef117b5 100644 --- a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCall.java +++ b/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 { 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() { diff --git a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallId.java b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallId.java new file mode 100644 index 0000000000000..5322c29edb989 --- /dev/null +++ b/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); + } +} diff --git a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallRepository.java b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallRepository.java index 51fbb08c61763..24424e88d9cdb 100644 --- a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallRepository.java +++ b/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 { +public interface PhoneCallRepository extends JpaRepository { PhoneCall findByIdAreaCode(String areaCode); @Query("select p.id from PhoneCall p") - Set findAllIds(); + Set findAllIds(); @Query("select p.callAgent from PhoneCall p") Set findAllCallAgents(); diff --git a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallResource.java b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallResource.java index 5fba7b4150df3..0a27f60c2e8c2 100644 --- a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneCallResource.java +++ b/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 allIds() { + public Set allIds() { return repository.findAllIds(); } diff --git a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneNumberId.java b/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneNumberId.java index 901f4395a344b..c87dcae5aeed4 100644 --- a/integration-tests/spring-data-jpa/src/main/java/io/quarkus/it/spring/data/jpa/PhoneNumberId.java +++ b/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;