Jpamodelgen fixes for panache

[FroMage]

Please @gavinking can you review this?

• Support ORM/HR+Panache types such as anything that extends PanacheEntityBase or PanacheRepositoryBase to make sure to use their session getter type, but never generate a DAO because Quarkus will call the generated static methods. Also support annotations on native methods.
• Support Uni<Mutiny.Session> session types
• Support session getters with a default implementation, in which case never store the session in a field but always call the getter. This is mostly because reactive sessions can't be stored in a CDI context and need to be obtained for each call.
• Fix update/delete method return types that have to return Uni<Integer|Void> for HR
• When ORM/HR+Panache is detected, for types that are not Panache entities or repositories, default to the Quarkus way of obtaining a session unless there's a session getter declared. The default way being @Inject EntityManager for ORM and SessionOperations.getSession() for HR. Note that this doesn't respect non-default PU for ORM, but we can improve later, if we have an entity defined (for Panache, we can override the PU per entity), and meanwhile users can always define a session getter to override. HR/Panache doesn't support non-default PU so this is not an issue.

Also, do you want docs for the changes? Tests?

[hibernate-github-bot[bot]]

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ All commit messages should start with a JIRA issue key matching pattern HHH-\d+     ↳ Offending commits: [24f58e03b12a3e81b2703e569903b7f20ab87208, f139c0497d0a4bd8b7d3651f75617e3ff984f5b5, a0972fa8b3c61522cd04c3b75c0bbfe5e7c850dd, 031dbb600f70e1d60a84c1b4d023bc80174f152a, 7e243edd13b622cabbedc871131d5ee6ede52575, ed67d7bbdad622ecbf6fcc1dd257f77be169ce94, 587f2a258f97f467ec2753fe0ce041cb1ecdd69e, 883f66e175f6d2f0c51646e8e1a2c36a812d14ad, fd09c63d59ef24a66e42fbffe440c0fa95fd5ed7, 197f9a5d21d50c30524da1e4918cc2b9503bf5c3, a2ab45e7e3bfce2cc0e4f48a37e3a96cdf7c3819]

› This message was automatically generated.

[gavinking]

OK, on it.

[FroMage]

I think I've treated all the remarks. But for tests, I now realise that the detection allows for either Quarkus ORM or HR, but not both (because that is not supported and both flavours are mutually exclusive), so I'm not sure (especially with Gradle) how to declare a dependency on those libs. Also, it appears that the tests such as DaoTest only checks that the metamodel is generated, not that it has this or that method or structure, am I missing something?

[gavinking]

so I'm not sure (especially with Gradle) how to declare a dependency on those libs.

ugh.

Also, it appears that the tests such as DaoTest only checks that the metamodel is generated, not that it has this or that method or structure, am I missing something?

For now that's all it does indeed. We could probably improve that by adding some code which calls the generated code. But the fact that it is accepted by the compiler is already a fairly demanding test, though not perfect of course.

[FroMage]

Also, it appears that the tests such as DaoTest only checks that the metamodel is generated, not that it has this or that method or structure, am I missing something?

For now that's all it does indeed. We could probably improve that by adding some code which calls the generated code. But the fact that it is accepted by the compiler is already a fairly demanding test, though not perfect of course.

What's supposed to be the result of this test:

public interface Dao {
    @HQL("select upper('Hibernate')")
    TypedQuery<String> getName();
}

Because it appears to result in:

@StaticMetamodel(Dao.class)
@Generated("org.hibernate.jpamodelgen.JPAMetaModelEntityProcessor")
public abstract class Dao_ {
}

And… I mean… it does compile, sure… 😂 but is it really supposed to be empty? 🤔

[FroMage]

so I'm not sure (especially with Gradle) how to declare a dependency on those libs.

I've figured it out thanks to Steve.

[sebersole]

Nice!

[gavinking]

And… I mean… it does compile, sure… 😂 but is it really supposed to be empty? 🤔

Um, no, that does not look right. 😬

[FroMage]

I've figured it out thanks to Steve

Famous Last Words 🤦

Apparently checkerframework is something that's required, but it doesn't automatically applies to my new source sets and I'm getting a compilation error that it's looking for checker-qual.jar.

[gavinking]

@FroMage what's the status of this?

[FroMage]

Blocked on the Gradle build of the tests that require quarkus, due to checkerframework :( I can push what I have, but it doesn't build.

[FroMage]

You just had to update that project, @gavinking, you couldn't just leave it be for the 6 months it took me to do this PR 😂

Well, I've rebased, I'll sort out the checkstyle issues and add tests for the non-panache fixes as well as the panache stuff.

[FroMage]

Actually @gavinking I'm not sure how to get these checkstyle errors from formatting. When I run gradlew hibernate-jpamodelgen:check I only get errors due to } else { but not whitespace in parens, as you've suggested in your review.

What's the magic command line I have to run to report all those errors?

[gavinking]

Check style doesn't enforce it.

[FroMage]

Yeah, so, where does this info come from?

[gavinking]

From looking at the rest of the codebase.

[FroMage]

From looking at the rest of the codebase.

Mimicry-based development 🤦

I'll… I'll try to blend in, I guess…

[gavinking]

You just had to update that project, @gavinking, you couldn't just leave it be for the 6 months it took me to do this PR 😂

Y'know, I totally left it alone for five months but that just wasn't enough apparently :)

[FroMage]

Yeah, I have zero excuse :(

[FroMage]

OK, rebased on master, reformatted, includes tests for every feature/commit, tests now run as part of regular tests, and checkstyle passes.

So, IMO the only remaining thing might be to document some of those things?

[FroMage]

Rebased again.

[FroMage]

I can rebase again, but we should really merge this because I'm not going to keep doing that every day ;)