[Use Case]: There should be an EnityQualifier type of annotation to mark custom entity defining annotations

starksm64 commented 5 months ago

As a ...

[X] Application user/user of the configuration itself
[ ] API user (application developer)
[X] SPI user (container or runtime developer)
[X] Specification implementer

I need to be able to ...

Currently the notion of a custom entity defining annotation is one that ends with 'Entity'. This is not as typesafe as it should be as Entity is a very common notion and suffix. It would be better to have a meta annotation similar to the CDI Qualifier that allowed one to annotate a custom annotation to ensure the intent of its purpose.

Which enables me to ...

This allows annotation processors and CDI extensions, etc. to look for annotations marked with the entity qualifier annotation rather than trying to infer this purpose based on a naming pattern.

Additional information

Having an annotation like:

package jakarta.data.repository;

...

@Target(ANNOTATION_TYPE)
@Retention(RUNTIME)
@Documented
public @interface EntityQualifier {}

would allow one to more safely mark a custom annotation as an entity defining annotation type:

package ee.jakarta.tck.data.common.cdi;

import jakarta.data.repository.EntityQualifier;
...

@EntityQualifier
@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
public @interface PersonEntity {

}

otaviojava commented 5 months ago

Good point, to simplify to version 1.0.0. I would define entity a class that has either: jakarta.persistence.Entity or `jakarta.nosql.Entity annotation.

@njr-11 @gavinking

gavinking commented 5 months ago

I mean I don't hate the idea in principle, but such an annotation could not be defined by Jakarta Data, since neither Jakarta Persistence nor Jakarta NoSQL depends on Jakarta Data.

Note, however, that the notion of an entity-defining annotation is not directed toward extensibility by the application programmer. It's only for providers, who know how to recognize their own entity-defining annotations.

But sure, generic tools such as implementations of the query-by-method-name extension could benefit from such a meta-annotation, I agree. But it would have to be defined in, I dunno, jakarta.annotation or something.

gavinking commented 5 months ago

such an annotation could not be defined by Jakarta Data, since neither Jakarta Persistence nor Jakarta NoSQL depends on Jakarta Data.

Well, I suppose we could say that an entity defining annotation is jakarta.persistence.Entity, jakarta.nosql.Entity, or any annotation meta-annotated jakarta.data.EntityDefining. That would work.

otaviojava commented 5 months ago

such an annotation could not be defined by Jakarta Data, since neither Jakarta Persistence nor Jakarta NoSQL depends on Jakarta Data.

Well, I suppose we could say that an entity defining annotation is jakarta.persistence.Entity, jakarta.nosql.Entity, or any annotation meta-annotated jakarta.data.EntityDefining. That would work.

Thus, we need a TCK test for that, right? IMHO: I am afraid of the time.

gavinking commented 5 months ago

I don't think we need to do anything with respect to this for Jakarta Data 1.0.

otaviojava commented 5 months ago

I don't think we need to do anything with respect to this for Jakarta Data 1.0.

Yeap, that is my point:

Jakarta Data 1.0: Entity is jakarta.persistence.Entity or jakarta.nosql.Entity
Jakarta Future: we can fix it better.

gavinking commented 5 months ago

Jakarta Data 1.0: Entity is jakarta.persistence.Entity or jakarta.nosql.Entity

I'm not sure what you're saying here Otavio. Currently this spec says:

Jakarta Data does not define its own programming model for entities ... This section lays out the core requirements that an entity programming model must satisfy in order to be compatible with Jakarta Data, and for the defining provider to be considered a fully-compliant implementation of this specification.

Every entity programming model specifies an entity-defining annotation. For Jakarta Persistence, this is jakarta.persistence.Entity. For Jakarta NoSQL, it is jakarta.nosql.Entity. A Jakarta Data provider must provide repository implementations for entity classes bearing the entity-defining annotations it supports, and must ignore entity classes with entity-defining annotations it does not support.

This is perfectly fine, in my opinion.

Scott is right that this isn't so great for generic tools which wish to work generically for any entity programming model, but at this moment, Scott is the only being in the Universe who has such a tool, and so I think that's a use case we can reasonably leave for the Future.

gavinking commented 5 months ago

Currently the notion of a custom entity defining annotation is one that ends with 'Entity'.

@starksm64 do we really still say this somewhere? I can't find it.

otaviojava commented 5 months ago

@gavinking

A Jakarta Data provider might define a custom entity-defining annotation. The name of such a custom entity-defining annotation type must end with Entity. This convention allows any Jakarta Data provider to identify when a repository is associated with entity classes declared using an unrecognized entity-defining annotation.

This is the part that I would like to remove in version 1.0.

gavinking commented 5 months ago

Damnit I looked for that and could not find it.

I agree that's a bit rubbish. I'm not even sure what we need it for. Surely the provider can check that the repository does use entities it supports instead of looking for entities it doesnt support?

gavinking commented 5 months ago

I guess the thinking is that if provider A supports JPA entities and provider B supports both JPA and NoSQL entities, and I define a repository which depends on both JPA and NoSQL entities, then provider A would ignore it.

But I'm not sure that's very useful. The two providers are still going to fight over repos with only JPA entities, and isn't this why we have provider in @Repository? To disambiguate such cases?

njr-11 commented 5 months ago

Surely the provider can check that the repository does use entities or supports instead of looking for entities it doesnt support?

I'm pretty sure that part is there to allow providers to distinguish entities without an entity annotation -- specifically records (these cannot have the jakarta.persistence.Entity annotation) -- from entities with custom entity annotations from a different provider.

otaviojava commented 5 months ago

Surely the provider can check that the repository does use entities or supports instead of looking for entities it doesnt support?

I'm pretty sure that part is there to allow providers to distinguish entities without an entity annotation -- specifically records (these cannot have the jakarta.persistence.Entity annotation) -- from entities with custom entity annotations from a different provider.

I got it, @njr-11. Are you ok with removing this version?

gavinking commented 5 months ago

@njr-11 I see.

I guess it's just that the naming convention thing there is pretty fragile, and would be better-addressed with a meta-annotation as Scott says.

In the meantime there's always the fallback to explicitly specifying the provider in case of ambiguity.

gavinking commented 5 months ago

I'm pretty sure that part is there to allow providers to distinguish entities without an entity annotation -- specifically records (these cannot have the jakarta.persistence.Entity annotation) -- from entities with custom entity annotations from a different provider.

Another reason I think this is a bit fragile is that it doesn't help at all if both providers allow un-annotated entities.

So in your use case, I would say it's anyway more robust, and more explicit, to introduce your own entity-defining annotation, rather than saying that the records are un-annotated.

jakartaee / data