Skip to content
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

New method ResourceInfo.getAnnotation() #1303

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mkarg
Copy link
Contributor

@mkarg mkarg commented Jan 6, 2025

This PR is a draft for further discussion of ResourceInfo.getAnnotation().

It is not intended for code review, voting, or merge. Please use it solely for discussion, unless we agreed upon the final API and have added TCK tests.

Closes #1292

@mkarg
Copy link
Contributor Author

mkarg commented Jan 6, 2025

Kindly requesting to continue our discussion here. :-)

In particular the question is: What to do with custom annotations like @Tracing (where possible the annotation author wants to treat it as a JAX-RS annotation) or @RolesAllowed (where there is a published spec mandating inheritance always, independent of JAX-RS annotation). What first came to my mind (and is found in this first draft) is to have an option for custom annotations: JAX_RS (treat it as it would be a JAX-RS annotation), INHERIT (performan inheritance always) or NONE (never inherit it).

WDYT?

@chkal
Copy link
Contributor

chkal commented Jan 7, 2025

Shouldn't a Jakarta REST API for retrieving the effective annotations for a given resource method always use the Jakarta REST annotation inheritance rules? Just thinking out loudly. I'm not 100% myself sure yet.

@mkarg
Copy link
Contributor Author

mkarg commented Jan 7, 2025

Shouldn't a JAX-RS API for retrieving the effective annotations for a given resource method always use the JAX-RS annotation inheritance rules? Just thinking out loudly. I'm not 100% myself sure yet.

You missed the point that the customization of the algorithm is for custom annotations only. For JAX-RS annotations certainly always the JAX-RS algorithm is used. This customization is needed as e. g. @RolesAllowed is commonly used with JAX-RS, but is not a JAX-RS annotation itself, and mandates a different "always-inherit" policy.

@jamezp
Copy link
Contributor

jamezp commented Jan 7, 2025

Shouldn't a JAX-RS API for retrieving the effective annotations for a given resource method always use the JAX-RS annotation inheritance rules? Just thinking out loudly. I'm not 100% myself sure yet.

You missed the point that the customization of the algorithm is for custom annotations only. For JAX-RS annotations certainly always the JAX-RS algorithm is used. This customization is needed as e. g. @RolesAllowed is commonly used with JAX-RS, but is not a JAX-RS annotation itself, and mandates a different "always-inherit" policy.

I'm not too sure how I feel about that. The enum feels a bit clumsy, but I might not fully understand the use-case. While I can appreciate that @RolesAllowed is no inherited via the annotation, there's nothing really stopping a user from doing ResourceInfo.getResourceAnnotation(RolesAllowed.class, CustomAnnotationsInheritancePolicy.JAX_RS). It feels out of scope for the Jakarta REST API.

Note, I'm not convinced one way or the other, this is just my initial take on it.

I also wonder if the return type should be something like List<A> for repeatable annotations if we're going to allow any annotation to be resolved.

I'm also not to clear on the meaning on CustomAnnotationsInheritancePolicy.INHERIT. It states:

Inherit custom annotations but do not treat them as JAX-RS annotation.

What does that mean? Is there an expectation that annotations on interfaces get included? Which annotation wins in a case where both an interface and implementation are annotated?

@mkarg
Copy link
Contributor Author

mkarg commented Jan 11, 2025

Regarding singular / plural: Agreed, we should go with plural.

Whether or not we have to deal with custom algorithms at all: I agree that is something missing in the JRE in fact. It would simply be good to put the algorithm at the @interface declaration, as it hasn't to do with us at all, neither with "us" as the JAX-RS spec authors, nor with "us" as the filter programmers. But as the JRE does not provide it, this is unfortuntately not an option for us here (and I doubt that we can convince Brian Goetz from adding this any time soon).

I also dislike the clumsy enum, but if we omit this parameter, then we have an odd situation: A filter programmer that needs to query some JAX-RS-ish custom annotation plus some non-JAX-RS-ish custom annotation needs to go two distinct roads in his code. So even if we dislike the enum as spec authors, from the view of a filter programmer it feels much better to have getAnnotations(MyJaxRsIshAnnotation.class, JAX_RS) and getAnnotations(RolesAllowed.class, INHERIT) instead of writing a long and error-prone code to deal with @RolesAllowed-ish algorithm in each and every filter. The alternative would be having two distinct methods: getAnnotationUsingJaxRsAlgorithm() and getAnnotationUsingCustomAlgorithm(INHERIT), which feels in no way better (or ignore custom algorithms and end up with only half the benefit for the filter programmer).

INHERIT: Note that this PR is just still a draft to outline the problem to solve. I used INHERIT to make clear that once we deal with non-JAX-RS-ish inheritance policies, we have to provide them somehow. INHERIT in this case is just an example taken from the @RolesAllowed spec which implies that "the neares wins". Whether or not interfaces are contained depends on the actual outome of our current discussion.

I hope that sheds some light, if not please ask! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api enhancement New feature or request spec tck
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement Proposal: ResourceInfo.getAnnotation(Class annotationClass)
3 participants