Directly support CDI for injecting EntityManager and EntityManagerFactory

[arjantijms]

Jakarta Persistence predates CDI, and uses its own annotations to inject the EntityManager and EntityManagerFactory.

In order to make Jakarta EE more consistent, these should be directly injectable using CDI with an optional qualifier to specify the persistence unit name if needed.

[gavinking]

I'm not sure this is a real issue. The CDI spec (since the very first version) shows examples of using CDI annotations to inject the EntityManager.

In section 1.3.3, there is this example:

 public class Databases {
      @Produces @PersistenceContext(unitName="UserData")
      @Users EntityManager userDatabaseEntityManager;

      @Produces @PersistenceUnit(unitName="UserData")
      @Users EntityManagerFactory userDatabaseEntityManagerFactory;

      @Produces @PersistenceContext(unitName="DocumentData")
      @Documents EntityManager docDatabaseEntityManager;
  }

And then:

@Inject @Users EntityManager userDatabase;

Now, in principle, we could allow the CDI qualifier annotation to be specified in the persistence unit declaration in persistence.xml, but it's not completely clear to me that this would be much of an improvement.

[jgrassel]

I agree, I always felt like this was a glaring omission in the JPA specification.

[m-reza-rahman]

Would indeed be a good improvement. This could also take into account how transactions and concurrency is supposed to work outside of the better defined realm of EJB.

[arjantijms]

I'm not sure this is a real issue.

This issue (337) must be real, as I've opened it and people are responding to it. Okay, jokes aside, there are obviously workarounds for this (as there's with pretty much everything, thank you for designing both CDI and JPA in such way to make this possible).

But the idea is to not need all these workarounds or examples anymore, but just support it directly and natively.

Put differently, if CDI had happened to exist before JPA was designed, would @EntityManager have been used in it like it's now?

[gavinking]

But the idea is to not need all these workarounds or examples anymore, but just support it directly and natively.

I guess what I don't quite understand is what about the current design is it that you're worried is not native.

I mean, as far as I'm aware, Java EE containers today already implement @PersistenceContext and @PersistenceUnit injection using CDI, in terms of native CDI constructs. (Please correct me if I'm wrong.)

I'm guessing that you find it inelegant that the annotation to inject a persistence context is @PersistentContext(name="UserData") instead of @Inject @UserData. So, I assume, you're looking for a way to associate a CDI qualifier annotation with a persistence unit.

What's not clear to me is whether it's really much better to do that in persistence.xml, by writing:

<persistence-unit name="UserData">
     <cdi-qualifier>my.package.UserData</cdi-qualifier>
     ...

compared to writing the exact same mapping in Java:

@Produces @PersistenceUnit(unitName="UserData")
@UserData EntityManagerFactory userDatabaseEntityManagerFactory;

I mean, perhaps it's a bit better, but it's no more "native"; it's just a different place to specify the exact same information.

[m-reza-rahman]

We had this exact same conversation during JMS 2.0 and JMSContext injection via @Inject, @JMSConnectionFactory, etc. It is indeed syntactic sugar. However, it can be a lot more than that.

We actually specified the concurrency and transaction semantics of JMSContext when it is injected via CDI both inside and outside EJB (using just @Transactional on a CDI bean for example). This could be a great opportunity to do similar work for JPA. It would make using JPA outside of EJB in plain transactional CDI beans much more deterministic.

[gavinking]

To be clear, I'm not against adding something like <cdi-qualifier> to the persistence.xml format.

I just thought the issue description is a bit misleading, since the requested functionality already exists, you're merely asking for it to be exposed in XML instead of via annotations.

At least, that's what I understand the request to be. (I may be missing something.)

[m-reza-rahman]

At least, that's what I understand the request to be. (I may be missing something.)

There is scope to do some really valuable work here. For reference, please read this section of the JMS specification (I helped author it myself): https://jakarta.ee/specifications/messaging/3.1/jakarta-messaging-spec-3.1.html#injection-of-jmscontext-objects

[gavinking]

It would make using JPA outside of EJB in plain transactional CDI beans much more deterministic.

You mean like:

@Produces @TransactionScoped @PersistenceUnit(unitName="UserData")
@UserData EntityManagerFactory userDatabaseEntityManagerFactory;

I guess I must be missing something because this already seems well-defined?

[m-reza-rahman]

I guess I must be missing something because this already seems well-defined?

Give the spec section above a read please. I think it will be readily apparent what is missing for JPA. To be honest, when I helped author that, I had it in my mind to eventually bring it to JPA. Maybe this is finally that opportunity.

[gavinking]

You mean this language:

• If a method is called on an injected JMSContext when there is a Jakarta transaction (either bean-managed or container-managed), the scope of the JMSContext will be @TransactionScoped...
• If a method is called on an injected JMSContext when there is no Jakarta transaction then the scope of the JMSContext will be @RequestScoped...

So an injected JMSContext bean has no consistent, well-defined CDI scope, and its scope depends on whether a transaction context is active? And this is a completely ad hoc semantic tacked on to CDI by the platform spec? Instead of making use of native CDI constructs which make behavior like this totally achievable in an elegant way?

[gavinking]

I think it will be readily apparent what is missing for JPA.

I don't think we should do anything like that for JPA.

[m-reza-rahman]

You mean this language.

Yes, that's the correct section. How it is done honestly it is fine to revisit. The main point is that it is specified how JMS behaves outside of EJB in plain CDI - with transactions too. I think there is very much value to doing something similar or the same for JPA. It would make it much easier to take one more step away from EJB.

BTW, you are aware @Transactional and @TransantionScoped is defined in JTA right? Nothing really to do with the platform. We did this deliberately too. Any place CDI, JTA and JMS are available, you do rely on this is a totally portable way.

[gavinking]

It would make using JPA outside of EJB in plain transactional CDI beans much more deterministic.

You mean like:

@Produces @TransactionScoped @PersistenceUnit(unitName="UserData")
@UserData EntityManagerFactory userDatabaseEntityManagerFactory;

I guess I must be missing something because this already seems well-defined?

Note that what is nice here is that this declaration associates:

• the name of the persistence unit with
• a qualifier annotation, and
• an explicit scope/context.

Meaning that if you need a @RequestScoped persistence context in one place, a @TransactionScoped context in another place, and even something else (conversation scoped?) somewhere else, you can disambiguate these usages with separate qualifier annotations.

This is really very flexible, IMO.

[arjantijms]

It would make using JPA outside of EJB in plain transactional CDI beans much more deterministic.

You mean like:

@Produces @TransactionScoped @PersistenceUnit(unitName="UserData")
@UserData EntityManagerFactory userDatabaseEntityManagerFactory;

I guess I must be missing something because this already seems well-defined?

Everything is well-defined, obviously. That's not the problem. I mean, there are no portability concerns I'm trying to address here. The specs are watertight, and everything works.

What I'm trying to achieve is making CDI first class, instead of using things like producers to bridge CDI to other injection mechanisms.

E.g. without doing anything else in my application I'd like to be able to do:

    @Inject
    EntityManager entityManager;

And not:

    @PersistenceContext
    EntityManager entityManager;

Or with a qualifier provider by the specification:

    @Inject
    @PersistenceContext("SomePU")
    EntityManager entityManager;

And programmatically:

var x = beanManager.getBeans(EntityManager.class, PersistenceContext.Literal.of("SomePu");

The idea is to simply unify the model fully.

[arjantijms]

Meaning that if you need a @RequestScoped persistence context in one place, a @TransactionScoped context in another place, and even something else (conversation scoped?) somewhere else, you can disambiguate these usages with separate qualifier annotations.

This is really very flexible, IMO.

Obviously for more advanced use cases one can still uses producers. Having the default persistence context / entity manager injectable using CDI by default would not take that away, would it?

[m-reza-rahman]

This is really very flexible, IMO.

Flexible, sure. The problem is that it is asking a lot of the average developer to understand. That's a big part of why we went the direction we did with JMS. Basically developers can just @Inject and forget the rest, just as they do in EJB/JPA with @PersistenceContext.

[gavinking]

@arjantijms I mean I guess I'm just struggling a bit to see the concrete advantage of writing:

• @Inject instead of @PersistenceContext and
• @Inject @PersistenceContext("SomePU") instead of @PersistenceContext("SomePU").

I mean, I'm certainly not against letting you write that, of course. I just don't see it as particularly more "CDI native".

To my way of thinking, CDI isn't the @Inject annotation. CDI is the BeanManager doing the injection.

[gavinking]

Oh wait, perhaps the point I've missing is this: neither CDI nor JPA requires that the CDI container supports the @PersistenceContext annotation outside of the EE environment.

And so perhaps people can't rely on @PersistenceContext just working? Is that it?

[m-reza-rahman]

The idea is to simply unify the model fully.

Just to be clear, I agree this alone is also worth doing. It is similar to what we have done in Faces and what is now being done in REST. It's a lot simpler if all the injection related functionality is simply CDI.

[gavinking]

It's a lot simpler if all the injection related functionality is simply CDI.

Well I certainly agree 100% with that. As far as I know that's already the case in our products. I believe Weld takes care of injecting a @PersistenceContext. But don't quote me on that because I'm not completely certain of my facts here.

[arjantijms]

Well I certainly agree 100% with that. As far as I know that's already the case in our products. I believe Weld takes care of injecting a @PersistenceContext. But don't quote me on that because I'm not completely certain of my facts here.

There's functionality in Weld indeed if I'm not mistaken, but it's certainly not active by default. In Piranha for instance we're using Weld, but we still had to manually add support for injecting it via CDI, e.g.

https://github.com/piranhacloud/piranha/blob/current/extension/eclipselink/src/main/java/cloud/piranha/extension/eclipselink/EclipseLinkCdiExtension.java#L67

(we're trying to do as much as possible using CDI in Piranha, adding bridge functionality where needed)

So again, it works, but it would be a lot easier for both implementors and users if the spec defined it. One additional benefit there would be that for implementors supporting multiple implementations of CDI and JPA, at least for this particular functionality any permutation would just work together.

[gavinking]

@arjantijms yes, OK, that's reasonable.

[arjantijms]

@arjantijms I mean I guess I'm just struggling a bit to see the concrete advantage of writing:

• @Inject instead of @PersistenceContext and
• @Inject @PersistenceContext("SomePU") instead of @PersistenceContext("SomePU").

For users it's particularly the mental overhead. They can @Inject a large variety of beans that are vended by different specs. For instance, for Faces: https://arjan-tijms.omnifaces.org/p/jsf-23.html#1316

But when it comes to Jakarta Persistence, suddenly @Inject EntityManager will give "strange" errors. I've to tell those devs that Jakarta Persistence needs @PersistenceContext and not @Inject, and when they ask why... well there really isn't a good answer.

Sure, I can tell them they can create a producer, but then they immediately ask why they don't need to do that for Faces artefacts, or Servlet ones, or Security ones, etc.

All in all it gives the platform a somewhat sloppy feel. Things are different at various places in the platform, and basically for no reason other than either history, or that spec engineer X on project Y had different ideas or was against something for some reason, or didn't see the need for it, while spec engineer A on project B did see that need.

History or engineers having different opinions are obviously not a good reason for end users to do things differently and give them that extra mental overhead.

[scottmarlow]

https://issues.redhat.com/browse/WFLY-17996 has some discussion about using ("managed") transaction-scoped persistence contexts outside of EJB session beans, where the application library that might be called from EJB session beans or from non-EJB code.

With the non-EJB code case, could there be a way for Persistence (EE) containers [1] to track when CDI bean invocations start/end so that the same container requirements could apply to both EJB session beans (current support) but also could work with some new (to be introduced) CDI contract so that the persistence context is closed when the CDI bean invocation completes. Or something like that.

Persistence (EE) containers [1] - The Persistence Spec has requirements for the Jakarta EE Platform implementations that include how transaction-scoped persistence contexts are managed.

[scottmarlow]

And so perhaps people can't rely on @PersistenceContext just working? Is that it?

My previous comment points out an idea of what it could mean, not really sure if could actually work.

[m-reza-rahman]

This is what Spring does (which is actually very similar to the JMS context approach I mentioned earlier): https://github.com/spring-projects/spring-framework/blob/main/spring-orm/src/main/java/org/springframework/orm/jpa/SharedEntityManagerCreator.java

[scottmarlow]

This is what Spring does (which is actually very similar to the JMS context approach I mentioned earlier): https://github.com/spring-projects/spring-framework/blob/main/spring-orm/src/main/java/org/springframework/orm/jpa/SharedEntityManagerCreator.java

Thanks for pointing this out.

It looks like in some cases the Spring code lets the garbage collector deal with closing the entity manager(e.g. if a query is returned) unless I missed something. The Spring code linked does seem to implement the transaction-scoped persistence context in a similar when an active jta-tx is present. For the non-jta-tx case, we are different as Jakarta Persistence spec does not currently define what happens for certain cases (non session bean/web component), but Spring does define it as closing the underlying entitymanager per invocation (unless its the query case and entitymanager.close is never called).

Deciding what the per invocation statement means is also important. IMO, from an application perspective, I would like there to be a way to have the application use the same (underlying) EntityManager for non-jta-tx case until some context or event is fired indicating that the underlying EntityManager should be closed.

[m-reza-rahman]

Agreed. Personally I think the way JMS context does it is better.

[Emily-Jiang]

Catching up with this thread. I think what this issue is about:

• Not have to use a producer to produce the EntityManager
• Directly support injection via `@Inject @PersistenceUnit("some-unit") EntityManager

In this way, all other injection model in Jakarta EE should be deprecated and use @Inject instead. Jakarta REST has deprecated the injection model @Context and will drop it in the next release. I think JPA should consider the similar move by looking at how to simplifying the EntityManager and EntityManagerFactory injection. In this way, it can share the similar programming model such as JMS as pointed by Reza. Also, Jakarta Validation allows to inject Validator and ValidatorFactor. If JPA can follow the same trend, it will be great.

[m-reza-rahman]

Thanks Emily. Platform consistency (getting rid of one-off injection models and EJB in favor of CDI) is a great point indeed.

[gavinking]

Please see the proposal https://github.com/jakartaee/persistence/pull/460.