allow disambiguating annotation on resource accessor methods

[gavinking]

UPDATE: this is now issue #476.

Here's a proposal that in my opinion solves #264, as well as providing an alternative to "configuration" as envisaged in issue #22.

What do ya'll think of this approach?

[njr-11]

This is an interesting approach, but I think it goes too far because these annotations enable you to obtain any DataSource/EntityManager, not just the one that is backing the repository. It isn't the place of Jakarta Data to provide access to other resources apart from maybe the one that it is using, which we already have via the resource accessor method pattern. I don't think we need to provide anything else here. @Resource and @PersistenceContext already exist in Jakarta EE and can be used to obtain resources in the ways in which they are already standardized to do so.

[gavinking]

I think it goes too far because these annotations enable you to obtain any DataSource/EntityManager, not just the one that is backing the repository.

But what I'm saying here is that the annotation would determine the resource that backs the repository.

i.e. the repository implementation itself would use those annotations to look up its datasource, or entity manager, or whatever.

[gavinking]

The issue is that currently, if I have, say, two persistence units in my EE application, we have no clear way to tell a repository which one to use.

[otaviojava]

Once we are changing this part.
IMHO:
The entire part of Resource accessor methods regarding methodology and dynamics should be included in the Jakarta Persistence Context.

As a reader, it sounds weird. I prefer to cover the Jakarta Persistence in a single place and not broad in the spec.

[gavinking]

The entire part of Resource accessor methods regarding methodology and dynamics should be included in the Jakarta Persistence Context.

@otaviojava I'm not sure what you mean. None of that section is specific to Jakarta Persistence or to relational databases. Mongo DB or whatever has some sort of native "connection" or "session" type.

JDBC is just an example.

[njr-11]

The issue is that currently, if I have, say, two persistence units in my EE application, we have no clear way to tell a repository which one to use.

Oh I see what you are trying to do here.
We do already have this, via the dataStore attribute of @Repository.

@Repository(dataStore = "persistence/MyPersistenceUnit")
public interface Employees {

[gavinking]

OK, yeah, I forgot about that when I was looking at #264, but the thing is that the @Repository approach doesn't let me specify CDI qualifier types. (Which was the main thing I was really targeting here.)

[gavinking]

(And I don't think it would be correct to just stick the CDI qualifiers on the repository itself.)

[njr-11]

How important is it to be able to obtain the resource via qualifiers? The Respository(dataStore= approach is more general and can work for any resource type, is self-documenting, and more concise. But you are right, it doesn't cover if the user wants to have the resource be located via qualifiers. Unfortunately, none of the qualifier patterns seem very intuitive.

[gavinking]

Well it's something we (JPA) have been getting pressure from Jakarta EE folks and some individuals about so apparently it matters to some people.

I think there's a desire to unify everything around a CDI-based approach and deemphasize the more "legacy" JNDI stuff. And I'm not going to argue against that.

[gavinking]

Is it a "have to have" 🤷‍♂️ well, for me personally, no, I guess??

[njr-11]

Well it's something we (JPA) have been getting pressure from Jakarta EE folks and some individuals about so apparently it matters to some people.

I think there's a desire to unify everything around a CDI-based approach and deemphasize the more "legacy" JNDI stuff. And I'm not going to argue against that.

It sounds like it's part of the CDI-centric push from Jakarta EE platform, which I agree in general is a good direction. But I don't think some of the specs we might utilize are far enough along with it for us to figure out what the best approach will be for Jakarta Data. For example, I don't see a spec-defined solution yet for a CDI alternative to resource references to JDBC DataSource. I would rather wait for these other specifications and Jakarta EE to get further along with a clearer picture of where it all ends up and then have Jakarta Data tie into that, especially when the approaches we currently would have with qualifiers are a bit awkward and not intuitive. I would propose that we go with what is currently in the spec here for 1.0 and assign this issue to Future, where we can revisit when there is better direction.

[gavinking]

I don't see a spec-defined solution yet for a CDI alternative to resource references to JDBC DataSource.

Just to be clear, this has been available and well-defined since CDI 1.0. The way to do it is to write:

@Produces @Resource(lookup="java:global/env/jdbc/CustomerDatasource")
@CustomerDatabase DataSource customerDatabase;

or:

@Produces @PersistenceContext(unitName="CustomerDatabase")
@CustomerDatabase EntityManager customerDatabasePersistenceContext;

anywhere that CDI will look.

And then inject with @Inject @CustomerDatabase DataSource ds.

This is currently in section 9.7.1 of the CDI 4 spec, but it's been there forever.

I'm pointing this out because some people don't seem to be aware that it exists, or for some reason view it as some sort of "workaround".

[gavinking]

I would propose that we go with what is currently in the spec here for 1.0 and assign this issue to Future, where we can revisit when there is better direction.

OK, so look, how about we close issue #264, since I think that the issue description is a bit outdated/confusing/not-quite-right, and I will open a new issue specifically describing the problem with CDI qualifiers, and laying out what I see as the range of possible solutions. (There aren't really many options, since interfaces are a bit limited in terms of places you can but an annotation.)

[njr-11]

I'm pointing this out because some people don't seem to be aware that it exists, or for some reason view it as some sort of "workaround".

I'm one of the people that thought of that as a workaround. I didn't realize that was the intended solution.

[gavinking]

I'm one of the people that thought of that as a workaround. I didn't realize that was the intended solution.

I mean, to me it's a great solution. I find it more natural to write the mapping from a string-based name to a typesafe annotation in Java, than to write annotations in, say persistence.xml. But people apparently want to write annotations in persistence.xml, so that's what we're going to let them do 🤷‍♂️🤷‍♂️

[njr-11]

I've done more investigation of this proposal and even got some of it sort of working, although this has also raised more concerns because I don't see it working in all cases.

First, some background. We currently let the user configure either a persistence unit reference or a data source resource reference, as you can do with @Repository(dataStore=...). We believe this is important because many users will not be JPA users and they shouldn't need to configure anything JPA specific to use Jakarta Data if they don't want to or know how to. What we have been doing is to take the data source resource reference and essentially write the JPA config for them. That works because a data source resource reference fits into that model. If you consider what happens with the resource accessor method (maybe better termed a resource supplier method for this usage) we now end up with a set of qualifiers and a resource type of DataSource.

One problem is how to supply that to the Jakarta Persistence provider. Even if the Data CDI extension is running at a point where it has access to do CDI.current().select(DataSource.class, qualifierAnnos), this doesn't help us any because Jakarta Persistence config doesn't accept DataSource instances. It takes JNDI names to resource references. At this point, I can locate the producer definition, grab the @Resource definition on it and look at the name() and use that, but only if name was specified. This actually worked when I tried it, sort of, because I did everything in a particular way. First, I actually specified a JNDI name value on @Resource. To me, it seems to defeat the purpose of being CDI-centric if you still need JNDI names. Second, the name I specified was intentionally in java:app so that the Jakarta Persistence provider would be able to look it up even when not running in the same module/component. I expect most users would want to omit the resource reference JNDI name value. When that happens, there are some defaults that apply, based on field name and so forth. Technically I could follow those same rules to figure out the name. But then there's another problem -- I expect these defaulted names are in java:comp and they won't always be accessible.

Another problem I worry about is CDI scopes. In this case of the hacky approach I described above, I swizzled it back to JNDI and the scope doesn't even apply. That seems wrong. If taking another approach, maybe with obtaining the EntityManager/EntityManagerFactory rather than DataSource, do we need to be concerned with it going out of scope and the Jakarta Data provider/Jakarta Persistence provider still trying to use it across the application? Maybe the users needs to always need to specify @ApplicationScoped. If they don't, this could be another place for things to go wrong.

Another issue is with obtaining EntityManager. I like how the existing approach can give you an EntityManagerFactory, not EntityManager. In the future I see this being quite useful if a Jakarta Data provider wants to in turn request a specific type of EntityManager from the Jakarta Persistence provider. I'm imagining a emf.getStatelessEntityManager method or something to that effect. If, on the other hand, we have a model where the user, via their repository, supplies the EntityManager and not the EntityManagerFactory, then the Jakarta Data provider is stuck with whatever sort of EntityManager the user's gave it.

To summarize, the proposed approach has some aspects that might be workable, but introduces complexity and unanswered questions. We shouldn't add it in version 1.0. Maybe there are ways we can improve on it and/or be able to resolve some of the issues related to it, and if so, maybe it will be something we can add later on.

[gavinking]

@njr-11 I don't quite understand what you're trying to do here.

Are you trying to obtain a DataSource and somehow pass that directly to JPA? That's not the way JPA is supposed t work. A JPA EntityManagerFactory has its own DataSource configured in persistence.xml, or programmatically via the new PersistenceConfiguration object. You don't pass a DataSource to an EMF.

With respect to CDI qualifiers:

• For a repository backed by JPA the idea is that these would be CDI qualifier annotations that identify a persistence unit, not a datasource.
• The case of qualifier annotations which identify a datasoure is for when you have a repository backed by direct JDBC access or whatever.

[gavinking]

That is to say:

@Repository
interface DocumentRepo {
    @DocumentDatabase 
    EntityManager entityManager()
}

Would by implemented by a class like this, say:

class DocumentRepo_ implements DocumentRepo {
    private final EntityManager entityManager;

    @Inject
    DocumentRepo_(@DocumentDatabase EntityManager entityManager) {
        this.entityManager = entityManager;
    }
    
    @Override
    EntityManager entityManager() { return entityManager; }
}

Or whatever. There's lot of possible variations on this.

Anyway the point is I don't see what a repository based on JPA would be messing around with the DataSource directly.

[njr-11]

One of the scenarios I was looking it is when a user writes a repository resource accessor/supplier method to produce a DataSource, associating that repository to a Jakarta Data provider that is backed by Jakarta Persistence. I noted that we can already handle the equivalent with @Repository(dataStore=...) but I do not see any good way of doing so for the resource accessor/supplier pattern. Maybe we need to specify that is an error path. Even if we do that, my other concerns still apply about the EntityManager.

[gavinking]

One of the scenarios I was looking it is when a user writes a repository resource accessor/supplier method to produce a DataSource, associating that repository to a Jakarta Data provider that is backed by Jakarta Persistence.
I noted that we can already handle the equivalent with @Repository(dataStore=...)

I suppose you're doing it by passing the datasource name via config property "jakarta.persistence.jtaDataSource" to Persistence.createEntityManagerFactory().

I believe that it's possible to directly pass an instance of DataSource using the property "jakarta.persistence.dataSource". That's how the Jakarta EE container passes the datasource to the JPA provider I believe. (Not really an expert on this particular aspect.)

Quote from the JPA spec:

The following additional standard properties are defined by this specification for the configuration of the entity manager factory:

• jakarta.persistence.jdbc.driver — value is
  the fully qualified name of the driver class.
• jakarta.persistence.jdbc.url — string
  corresponding to the driver-specific URL.
• jakarta.persistence.jdbc.user — value is the
  username used by database connection.
• jakarta.persistence.jdbc.password — value is
  the password for database connection validation.
• jakarta.persistence.dataSource — value is
  instance of javax.sql.DataSource to be used for the specified
  persistence unit.
• jakarta.persistence.validation.factory —
  value is instance of jakarta.validation.ValidatorFactory.

[gavinking]

If, on the other hand, we have a model where the user, via their repository, supplies the EntityManager and not the EntityManagerFactory, then the Jakarta Data provider is stuck with whatever sort of EntityManager the user's gave it.

So the current proposal is that the EntityManager and the EntityManagerFactory would both be registered in the bean container with the exact same qualifier annotations, so I don't really see a problem here.

[njr-11]

Nice - I should try out "jakarta.persistence.dataSource" and see how that works. That would reduce the issues with DataSource to being more similar to those with EntityManager, where I'm wondering what if it goes out of scope?