Quarkus: multi layered entity inheritance not supported by JpaRepository

[mickroll]

Describe the bug
Imagine a simple JpaRepo:

public interface ExampleRepo extends JpaRepository<ExampleEntity, Long> {
    Optional<ExampleEntity> findByNameAndId(String name, Long id);
}

Now dependending on the amount of layers between ExampleEntity and the definition of id, this repository works or doesn't work:

• works:

@MappedSuperclass public abstract class Base1 { @Id Long id; }
@Entity public class ExampleEntity extends Base1 { String name; }

• doesn't work:

@MappedSuperclass public abstract class Base1 { @Id id }
@MappedSuperclass public abstract class Base2 extends Base1 { }
@Entity public class ExampleEntity extends Base2 { String name; }

Expected behavior

• id - property is found by JpaReposiory, regardless of hierarchy size.

Actual behavior

• build error io.quarkus.spring.data.deployment.UnableToParseMethodException: Entity ExampleEntity does not contain a field named: Id. Offending method is findByNameAndId
  
  
  
  • repo looks for property Id (note the uppercase "i") instead of id
  
  

To Reproduce
See archive: entity_hierarchy_demonstrator.zip

Environment (please complete the following information):

• Output of uname -a or ver: win 10
• Output of java -version: 11
• GraalVM version (if different from Java): /
• Quarkus version or git rev: 1.8.3.Final
• Build tool (ie. output of mvnw --version or gradlew --version): Apache Maven 3.6.3

/cc @famod

[famod]

/cc @geoand

[famod]

@mickroll The reproducer does not contain a pom.xml. Can you check please?

[geoand]

@aureamunoz is this something you'd be interested in tackling?

[famod]

repo looks for property Id (note the uppercase "i") instead of id

A quick look at the code reveals that this does not seem to be the root problem.

The exception message is created here:
https://github.com/quarkusio/quarkus/blob/1.8.3.Final/extensions/spring-data-jpa/deployment/src/main/java/io/quarkus/spring/data/deployment/MethodNameParser.java#L197
I'd say it should use fieldName, not part.
See also: https://github.com/quarkusio/quarkus/blob/1.8.3.Final/extensions/spring-data-jpa/deployment/src/main/java/io/quarkus/spring/data/deployment/MethodNameParser.java#L190-L194 (notice lowerFirstLetter())

[mickroll]

@mickroll The reproducer does not contain a pom.xml. Can you check please?

forgot to create that, will do on monday

[famod]

I was curious so I simply took the quickstart pom and adjusted it a little: spring-data-jpa-issue-12753.zip

Stacktrace:

Build failure: Build failed due to errors
        [error]: Build step io.quarkus.spring.data.deployment.SpringDataJPAProcessor#build threw an exception: io.quarkus.spring.data.deployment.UnableToParseMethodException: Entity com.example.persistence.inheritance.ExampleTwoLayerEntity does not contain a field named: Id. Offending method is findByNameAndId
        at io.quarkus.spring.data.deployment.MethodNameParser.parse(MethodNameParser.java:212)
        at io.quarkus.spring.data.deployment.generate.DerivedMethodsAdder.add(DerivedMethodsAdder.java:77)
        at io.quarkus.spring.data.deployment.generate.SpringDataRepositoryCreator.implementCrudRepository(SpringDataRepositoryCreator.java:112)
        at io.quarkus.spring.data.deployment.SpringDataJPAProcessor.implementCrudRepositories(SpringDataJPAProcessor.java:239)
        at io.quarkus.spring.data.deployment.SpringDataJPAProcessor.build(SpringDataJPAProcessor.java:104)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:566)
        at io.quarkus.deployment.ExtensionLoader$2.execute(ExtensionLoader.java:936)
        at io.quarkus.builder.BuildContext.run(BuildContext.java:277)
        at org.jboss.threads.ContextClassLoaderSavingRunnable.run(ContextClassLoaderSavingRunnable.java:35)
        at org.jboss.threads.EnhancedQueueExecutor.safeRun(EnhancedQueueExecutor.java:2046)
        at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.doRunTask(EnhancedQueueExecutor.java:1578)
        at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1452)
        at java.base/java.lang.Thread.run(Thread.java:834)
        at org.jboss.threads.JBossThread.run(JBossThread.java:479)

[tkalmar]

That is what we noticed in the debugger. Working case generates id non working case (2 levels of inheritance) generates Id in the hql query.

[famod]

I found the problem and might be able to create a PR on sunday evening.

But in case you @aureamunoz want to do it here is the root cause:
This is wrong: https://github.com/quarkusio/quarkus/blob/1.8.3.Final/extensions/spring-data-jpa/deployment/src/main/java/io/quarkus/spring/data/deployment/MethodNameParser.java#L590

For each loop it calls this:

ClassInfo superClass = indexView.getClassByName(entityClass.superName());

but entityClass never changes.
This is correct:

ClassInfo superClass = indexView.getClassByName(superClassType.name());

(also leading to a simplification further down)

/cc @geoand

[famod]

@tkalmar This is because it ends up on the wrong code path:
https://github.com/quarkusio/quarkus/blob/1.8.3.Final/extensions/spring-data-jpa/deployment/src/main/java/io/quarkus/spring/data/deployment/MethodNameParser.java#L195-L196
fieldInfo should not be null here but as a consequence of the problem I described in my previous comment it in fact is null.

[famod]

Btw, another adjustment to the reproducer (application.properties were missing): spring-data-jpa-issue-12753-2.zip

[mickroll]

I was curious so I simply took the quickstart pom and adjusted it a little: spring-data-jpa-issue-12753.zip

Thanks for that!
And for the fix, of course. That was fast 😎 🚀

@geoand Do you think this fix can make it into the next quarkus release? That would be great!

[geoand]

Thanks for the contribution!

It will most likely be part of Quarkus 1.9.1