-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Panache 2 #46096
Comments
/cc @loicmathieu (panache) |
This looks like a great improvement over the current situation! Thanks a lot for all this work. I think the challenge will be to make it look simple -- it's definitely simpler than what we have now, but could feel overwhelming for new users. With that in mind, here are a few suggestions... Mostly cosmetic/naming ones.
Do we really want to force users to make that "managed blocking" choice? I suspect some will not need the You could maybe have a Another suggestion, if you're going to have many declinations of the same class, you may want to go for a more "discoverable" structure with nested classes like this: @MappedSuperclass
public class PanacheEntity {
@Id
@GeneratedValue
public Long id;
@MappedSuperclass
public static class ManagedBlocking extends PanacheEntity {
// Relevant methods...
}
@MappedSuperclass
public static class StatelessBlocking extends PanacheEntity {
// Relevant methods...
}
@MappedSuperclass
public static class ManagedReactive extends PanacheEntity {
// Relevant methods...
}
@MappedSuperclass
public static class StatelessReactive extends PanacheEntity {
// Relevant methods...
}
} Then: public class MyEntity extends PanacheEntity.ManagedBlocking { This has the advantage of not making it seem that an entity is "stateless" -- that adjective mostly makes sense when applied to the session, so
We may want to shorten these
I'd suggest leaving out unnecessary @Entity
public class MyEntity extends PanacheManagedBlockingEntity {
public String name;
public interface Queries extends PanacheManagedBlockingRepository<MyEntity> {
default List<MyEntity> findByName(String name) {
return find("name", name);
}
default long deleteByName(String name) {
return delete("name", name);
}
}
} That being said, I suspect this code, in Kotlin, with single-expression functions, could feel absolutely wonderful... Relatedly, if we're going to call these "repositories"... shouldn't we suggest naming conventions that better match that? E.g. @Entity
public class MyEntity extends PanacheManagedBlockingEntity {
public String name;
public interface Repo extends PanacheManagedBlockingRepository<MyEntity> {
default List<MyEntity> findByName(String name) {
return find("name", name);
}
default long deleteByName(String name) {
return delete("name", name);
}
}
} Then code could access it with
Following my suggestions above: @Entity
public class MyEntity extends PanacheManagedBlockingEntity {
public String name;
public interface Repo {
@Find
List<MyEntity> findByName(String name);
@Delete
long deleteByName(String name);
}
}
I fear this will be a lot of freedom, and will end up being confusing... Don't get me wrong, you improved the situation. But now all the options are unified, it's very clear there is a lot of options :D I don't have a suggestion to reduce the chance of users being confused, though; this may be something we'll need to live with.
Shouldn't we expose the default repository with a default name, so that people who only care about the default repo can use it more easily? E.g. we could generate a
Great example to show how powerful this is, but... if it ends up in the docs, I think it could use some meaningful naming conventions/recommendations :)
Please, pretty please don't hardcode Panache class names in these PRs :X We'd really need this stuff to use the serviceloader so that Panache contributes whatever it needs to contribute. See https://hibernate.atlassian.net/browse/HHH-18159 , though that can be solved later -- I'd just like to avoid making the problem worse :) Also, just so you now, we now have a build of Quarkus running on every PR to maintenance branches (e.g. ORM 6.6 testing against Quarkus 3.15), so adding a dependency from ORM to Quarkus/Panache2 may not be necessary... Not sure.
For the record (and for others reading this), this is work in progress: |
Very interesting; I agree with @yrodiere that it first looks complicated. I noticed you use
Another solution is to use grouping, this is more verbose but easier to use (and on par with Mutiny that uses it a lot). So maybe I also need some time to investigate what would need to be done to port existing MongoDB with Panache to these concept so we it didn't stay behind. |
I was gonna suggest the same as @yrodiere, remove the “blocking” name everywhere. It’s less verbose and by having an reactive alternative, it suggests that the default one is blocking. And currently the blocking version of Panache also doesn’t use this term. Btw, great job and this was something I was looking forward for. And a question that comes to my mind: will this affect the resource usage of the app, since everything is included in 1 jar ? Let’s say that you don’t use any of the reactive classes, will Quarkus clean these up during the build phase ? |
Description
This all started in #36168 with adding support for ORM's type-safe annotated queries to Panache a lifetime ago. The original plan was to support them via
static native
methods on the entities and repositories.Since then, Jakarta Data (JD) got added, with their own type-safe annotations, but also their own repo type, orthogonal to the Panache repository classes. And with a focus on stateless sessions, which were not supported in ORM/Panache.
I pivoted to a different kind of API which would allow us to mix managed and stateless sessions as we wanted. This required a lot of trial and error. This is what I demoed to the team last summer.
Since then, #44473 started supporting mixing ORM and HR in the same application, allowing users to mix and match blocking and reactive operations using the same entities. Which, again, was absolutely not supported in ORM/HR/Panache.
Thus I pivoted once more to try to apply the same API to unify blocking and reactive in what became known as Panache 2.
Some of the problems with Panache 1
for
loops or in HR method chainingSelectionQuery
andMutationQuery
and others that may be worth exposing or switching toSort
andPage
types that since then got added to both ORM and JD (so now we have 3, great)Panache 2
Examples
An old-style managed blocking entity with type-unsafe operations
Same, but defining the operations in the repository
Most times, we should provide a type-safe API for our operations, rather than let any entity user do HQL.
Adding type-safe queries
So far, we're only using features on-par with Panache 1. Now let's translate our type-unsafe queries to type-safe queries:
Naturally, you can mix and match type-safe and type-unsafe queries.
Overriding the operation nature
We've only seen managed blocking operations, but let's see how we can override the type of operation while keeping the default mode.
Out of the box, you get
.managedBlocking()
,.managedReactive()
,.statelessBlocking()
and.statelessReactive()
operations on both the entity and the metamodel, so you can override the default mode of operation for your entity.Changing the default mode of operation
Naturally we can default to stateless and reactive:
Multiple operation modes
You can also add APIs for all the operation modes you want to support, in any kind of mix and match between stateless/managed, reactive/blocking, type-safe/type-unsafe, jakarta data, whatever… All these nested interfaces will get you a static method on the metamodel to access it if you don't want to inject it.
Status
Implementation ideas
No response
The text was updated successfully, but these errors were encountered: