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

Enhancement Proposal: ResourceInfo.getAnnotation(Class annotationClass) #1292

Open
mkarg opened this issue Nov 20, 2024 · 8 comments · May be fixed by #1303
Open

Enhancement Proposal: ResourceInfo.getAnnotation(Class annotationClass) #1292

mkarg opened this issue Nov 20, 2024 · 8 comments · May be fixed by #1303

Comments

@mkarg
Copy link
Contributor

mkarg commented Nov 20, 2024

I hereby propose the addition of the following method:

public <A extends Annotation> A ResourceInfo.getResourceAnnotation(Class<A> annotationClass)

The idea is that this method returns the annotation instance of the given annotationClass with full respect to the rules found in spec chapter 3.6. This effectively means, this method returns either the super class annotation, or the interface annotation, or the resource class annotation, or the resource method annotation, depending on whether any JAX-RS annotation is found in the particular deeper levels.

The benefit of this method is that there is no need for filters / interceptors to re-implement this algorithm again and again for custom annotations, in case the annotation author wants to apply the same rules as found in chapter 3.6 (hence: the new annotation shall "look and feel" like an official JAX-RS annotation). Implementations could make the already existing algorithm publicly available here, so it is guaranteed that the exact same treatment happens for custom annotations and for official annotations.

@mkarg
Copy link
Contributor Author

mkarg commented Nov 25, 2024

Kindly asking for more opinions, in particular from committers.

If nobody vetos ResourceInfo.getResourceAnnotations upfront, I will provide a PR and compliant implementation (PR for Jersey) in the next weeks.

@mkarg
Copy link
Contributor Author

mkarg commented Dec 17, 2024

There had been not upfront vetos, so I assume everybody is fine with adopting this enhancement. If not, please clearly tell me NOW. Thanks.

@hantsy
Copy link

hantsy commented Jan 4, 2025

When moving to CDI, I hope the annotation operations can be by CDI utilities directly. Currently, almost every spec has some util to handle the reflection-related operations.

@jamezp
Copy link
Contributor

jamezp commented Jan 4, 2025

Should there ba a getResourceClassAnnotations() and/or getResourceMethodAnnotations()?

@chkal
Copy link
Contributor

chkal commented Jan 4, 2025

@mkarg Thanks for bringing this up. I like the idea. We actually had to reimplement the annotation inheritance rules for the MVC spec implementation as well. So +1 for this idea. Just one though: Shouldn't it be getResourceAnnotation instead of getResourceAnnotations (so singular instead of plural)?

Should there ba a getResourceClassAnnotations() and/or getResourceMethodAnnotations()?

I personally don't see a real benefit of having these. Especially separate ones for class- and method-level annotations.

@mkarg
Copy link
Contributor Author

mkarg commented Jan 4, 2025

Should there ba a getResourceClassAnnotations() and/or getResourceMethodAnnotations()?

Never had the actual need this separation.

@mkarg
Copy link
Contributor Author

mkarg commented Jan 4, 2025

Shouldn't it be getResourceAnnotation instead of getResourceAnnotations (so singular instead of plural)?

Correct, I will fix fixed the original description.

@mkarg mkarg linked a pull request Jan 6, 2025 that will close this issue
@mkarg
Copy link
Contributor Author

mkarg commented Jan 6, 2025

As nobody vetoed, I have sketched a first draft. I think it makes sense to continue our discussion there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants