quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.44k stars 2.58k forks source link

Have a way to get access to EntityManager even without entities #7148

Open maxandersen opened 4 years ago

maxandersen commented 4 years ago

Description JPA and Hibernate APIs can still be used and be useful without entities( query raw data and using native sql) but for now we don't support @Inject of JPA apis when no entities.

At the moment having to tell users to create a dummy entity to get access to a stateless session is pretty suboptimal :)

Implementation ideas have a quarkus.jpa.enable=true|false|auto property to allow to set to true.

What do you say @Sanne / @gsmet ? :)

gastaldi commented 4 years ago

It would be even better if you could @Inject EntityManager in projects without entities without setting any extra property. The error like one reported in #7280 is annoying for users

machi1990 commented 4 years ago

It would be even better if you could @Inject EntityManager in projects without entities without setting any extra property. The error like one reported in #7280 is annoying for users

@gastaldi +1, I can prepare a PR for this.

cc @Sanne

Sanne commented 4 years ago

wondering a couple of things:

  1. regarding the proposed quarkus.jpa.enable=true|false|auto : which extension is responsible for intepreting this jpa group configuration? That's not the prefix we normally use to configure Hibernate.

Besides, having to configure such things IMO defeats the point as you'd get out of the flow of iterative development in live reload: think about the case in which you're introducing Hibernate ORM into a project - IMO such experience needs to be polished and is more important than to support entityless-ORMs.

  1. While a StatelessSession could be useful, a JPA EntityManager is mostly useless. Should this allow to inject the first and still not the latter?

  2. I believe a possible improvement for use cases like #7280 would be to have a non-functional EntityManager injected: so to allow the injection to happen (wich is useful for iterative coding in live reload mode) but can't actually use it. This would allow for example to inject and EM even when the datasource wasn't configured yet. Should we scrap the idea of such a non-functional EM, or how would it interact with using an entity-les StatelessSession ?

TBH I wonder how messy this gets. It's certainly far simpler to explain that Hibernate won't be started when there are no entities :stuck_out_tongue_winking_eye:

maxandersen commented 4 years ago

How "useless' is that that injected EntityManager ? Can I get a statelessSession from it ?

IMO such experience needs to be polished and is more important than to support entityless-ORMs. I guess what you mean is that you want to ensure the following:

1) create quarkus project 2) run devmode 3) add-extension jpa 4) add datasource info 5) add entity 6) write code that access entitymanager / statelesssession 7) goto 5

Without having it fail hard at step #3 ?

I fully agree to that and in my world view that mimicks the reason why I don't want users having to do step #5 before they can do #6

We'll still require step #4 but that i feel is something that is possible for us to have go error messages and easy for users to grok that acccess data requires a datasource configured.

maxandersen commented 4 years ago

see #8348 for issue where this would be relevant for Panache too

renegrob commented 3 years ago

I'm having the same issue with a module that defines a Singleton that offers some generic operations. The entities are defined in the more specific modules. A module/library must not always have a single use.

gavinking commented 3 years ago

Very tangential to this and sorry for hijacking, but I think it's about time we introduced clean new Session and StatelessSession APIs for people who want access to Hibernate features that aren't available in JPA. The current APIs have 15 years of evolution written all over them and it's time to make a break. The new APIs in HR are much cleaner, IMO.

geoand commented 1 year ago

@maxandersen @Sanne as promised, a first version of StatelessSession support based on the now merged ORM 6 work is can be found here.

I have not made any changes to conditions under which Hibernate is booted in Quarkus, I just added support for @Inject StatelessSession in the same manner as @Inject Session is implemented.

maxandersen commented 1 year ago

StatelessSession implemented but not the part of starting hibernate without entities.

geoand commented 1 year ago

Did we ever get some documentation for StatelessSession?

geoand commented 1 year ago

@Sanne I think the proposal above from @gastaldi (to activate Hibernate when the EntityManager or now the StatelessSession is injected) makes a lot of sense (and was also brought up by @maxandersen lately).

Any reason why we should not do it?

Sanne commented 1 year ago

Sure let's do it.

I have some minor concerns, which I'll write down so we can think about it, but don't consider these blocking: I hope we can deal with them or accept them as lesser inconvenience.

1# detection based on injection won't give people a consistent behaviour with programmatic lookup. Not sure how far that's a problem, but say other extensions attempt to lookup the Session programmatically (some do), stuff might randmoly break or magically get fixed as a side effect of such an annotation being added(removed) by user code.

2# I don't remember if ORM is now able to boot w/o entities.

3# There's for sure going to be some sideffect on other extension's expectations.

geoand commented 1 year ago

#1 is concerning indeed... How about if we start out with just taking @StatelessSession into account, as that is almost certainly not used anywhere programmatically in extensions.

WDYT?

Sanne commented 1 year ago

I wouldn't worry too much and just do it - we'll never know otherwise what problems it might cause.

I pushed back about doing such things earlier as the ORM6 migration would have been complex enough w/o having to think about additional side-effects, but that's done... it's a good time to address these aspects too.

geoand commented 1 year ago

👌

maxandersen commented 1 year ago

Isn't #1 minor issue as most will have entities anyway that will still be able to enable ?

I agree with sanne - we should just do it and see it :)

geoand commented 1 year ago

Yeah, I'll take care of it soon

geoand commented 1 year ago

Of course it turns out to be easier said then done...

Getting the injection points using BeanRegistrationPhaseBuildItem leads to various build cycles that I doubt we can easily resolve (because we are looking for injection points in order to then turn out and configure beans). @mkouba is there a way we can get an "approximation" of the injection points using some Jandex util or something that won't cause build cycles when used in methods that also produce bean related build items?

mkouba commented 1 year ago

Getting the injection points using BeanRegistrationPhaseBuildItem leads to various build cycles that I doubt we can easily resolve (because we are looking for injection points in order to then turn out and configure beans).

How are those beans configured? It should work fine for synthetic beans. But I suppose that it wouldn't be possible to turn those beans into synthetic ones?

is there a way we can get an "approximation" of the injection points using some Jandex util or something that won't cause build cycles when used in methods that also produce bean related build items?

I don't know of any Jandex util but you can query the BeanArchiveIndexBuildItem#getImmutableIndex() for all @Inject annotations (injected fields and initializers) and fields annotated with qualifiers. But that's not all - we also don't require @Inject for single constructors.

geoand commented 1 year ago

I am not the maintainer of this extension, but I don't think all the beans here can be synthetic.

Yeah, I am aware of the lack of inject for constructors, hence the question about the existence of the utility

mkouba commented 1 year ago

Yeah, I am aware of the lack of inject for constructors, hence the question about the existence of the utility

We don't have such a utility because it's not the "rigtht" thing to do ;-)

geoand commented 1 year ago

Yeah, I am aware of what's right and what's not. But the overlap with practicality is not always complete

gian1200 commented 3 months ago

@maxandersen @Sanne as promised, a first version of StatelessSession support based on the now merged ORM 6 work is can be found here.

I have not made any changes to conditions under which Hibernate is booted in Quarkus, I just added support for @Inject StatelessSession in the same manner as @Inject Session is implemented.

After reading https://github.com/quarkusio/quarkus/pull/8861 and https://github.com/quarkusio/quarkus/pull/31392, I have a question regarding StatelessSession. Does it mean we can now insert A LOT of entities to the DB (Primary key generated in DB) without worrying about the generated id? We don't need it in the Quarkus App (current persist(Iterable<Entity>) is expensive for that reason).

Currently, our workaround is to iterate the list of entities, split them in big "batches" (as big as we consider them "safe and quick to insert") and use "createNativeQuery". We dynamically build and run something like INSERT INTO schema.table (col1, col2) VALUES (?, ?), (?,?), (?,?)... (PostgreSQL) as many times as necessary.

Since we use PanacheRepository, we do getEntityManager().createNativeQuery(strQueryInsert). Is this going to be available there, too? Something like getStatelessManager().createNativeQuery(strQueryInsert), is the @Inject StatelessSession mandatory? (Maybe we should do the same in our app for EntityManager and are doing it wrong (?)).

Ideally we'd like to just call something like statelessPersist(Iterable<Entity>), persist(Iterable<Entity>, false) or equivalent directly from PanacheRepository and let the framework handle the rest.

If required, I can open a separate issue.

Update: or maybe reopen https://github.com/quarkusio/quarkus/issues/8348 with this into consideration?

maxandersen commented 3 months ago

you can consider to use batch inserts in hibernate then doing persist(iterable) should perform well if done cleanly.

but yes, you should be able to do a inserts a bit more "raw" with stateless session.

Sanne commented 2 months ago

I think you have a good point about some use cases not needing the Id to be returned, especially when inserting large batches of data.

This has bit myself in the past; normally this can be worked around by using a different ID generation strategy or one of our id generation optimizers, but I think you're right that, especially the Stateless API, could benefit from an improved alternative.

Yes @gian1200 , could you open a new issue? Although it would be best to open this one on the Hibernate JIRA or discuss it on the Hibernate zulip chat.

@gavinking @beikov opinions ^ ?