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

ClientProxy getClass().getAnnotations() does not return actual annotations on the class #30327

Closed
ssaip opened this issue Jan 12, 2023 · 8 comments
Labels
area/arc Issue related to ARC (dependency injection) kind/question Further information is requested

Comments

@ssaip
Copy link

ssaip commented Jan 12, 2023

Describe the bug

I'm not sure if this is indeed a bug, or a limitition, but it is something that I came across right now and wonder if there is a way to make this work with beans that are proxied/subclassed (like @ApplicationScoped).

Due to the nature of subclassing managed beans, getAnnotations() does not yield all present annotations (unless they are marked as @Inherited)

because of that, certain code that depend on runtime annotations like

Optional.ofNullable(getClass().getAnnotation(Priority.class))
            .map(Priority::value)
            .orElse(DEFAULT_PRIORITY);

(taken from org.eclipse.microprofile.rest.client.ext.ResponseExceptionMapper) does not work on quarkus beans unless they are marked as @Singleton.

Expected behavior

getAnnotations() should work on proxies as well.

If this is technically not possible, the documentation should be updated to include this (and other known) limitations of using @ApplicationScoped over @Singleton

Actual behavior

getAnnotations() only returns @Inherited annotations

How to Reproduce?

Sample project: https://github.com/ssaip/quarkus-getannotations/blob/main/src/test/java/com/example/AnnotatedClassTest.java

Output of uname -a or ver

No response

Output of java -version

Java version: 11.0.17, vendor: Azul Systems, Inc.

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.15.3.Final

Build tool (ie. output of mvnw --version or gradlew --version)

apache-maven-3.8.6

Additional information

No response

@ssaip ssaip added the kind/bug Something isn't working label Jan 12, 2023
@geoand geoand added area/arc Issue related to ARC (dependency injection) and removed triage/needs-triage labels Jan 12, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Jan 12, 2023

/cc @manovotn (arc), @mkouba (arc)

@geoand geoand added kind/question Further information is requested and removed kind/bug Something isn't working labels Jan 12, 2023
@mkouba
Copy link
Contributor

mkouba commented Jan 12, 2023

It's not a bug per se but a known limitation.

If you really need to inspect annotations via reflection: if clazz instanceof io.quarkus.arc.ClientProxy (-> generated client proxy) or clazz instanceof io.quarkus.arc.Subclass (-> generated intercepted subclass) then use the clazz.getSuperclass() instead.

@ssaip
Copy link
Author

ssaip commented Jan 12, 2023

@mkouba I understand that, but maybe the proxy/subclass can be enhanced to delegate the getAnnotations() call to the getSuperClass().getAnnotations() instead, so that code depending on it would work on proxies/subclasses.

@mkouba
Copy link
Contributor

mkouba commented Jan 12, 2023

@mkouba I understand that, but maybe the proxy/subclass can be enhanced to delegate the getAnnotations() call to the getSuperClass().getAnnotations() instead, so that code depending on it would work on proxies/subclasses.

I think that we would have to copy all annotations instead, i.e. delegation is not possible. And that's something we would like to avoid.

@manovotn
Copy link
Contributor

@mkouba I understand that, but maybe the proxy/subclass can be enhanced to delegate the getAnnotations() call to the getSuperClass().getAnnotations() instead, so that code depending on it would work on proxies/subclasses.

Note that you can fiddle with annotations programmatically (which isn't uncommon in case of CDI) meaning that the view you get from reflection isn't always complete.
To get a full view, you'd need to inspect given class at build time via Quarkus extension.

@ssaip
Copy link
Author

ssaip commented Jan 12, 2023

Note that you can fiddle with annotations programmatically

@manovotn unfortunately this may not be an option, as the code is not always under control.
I found this issue because I was registering an @ApplicationScoped errorhandler for a microprofile client with @Priority(1) and the order in which the handlers are called is determined by the ResponseExceptionMapper:

    default int getPriority() {
        return Optional.ofNullable(getClass().getAnnotation(Priority.class))
            .map(Priority::value)
            .orElse(DEFAULT_PRIORITY);
    }

I could override this method in my errorhandler (which is not very pretty), but I reckon there may also be other third-party libraries that rely on annotations

To get a full view, you'd need to inspect given class at build time via Quarkus extension.

This is something that I expect the framework to take care of for me, hence the issue.

@manovotn
Copy link
Contributor

Technically, you can unwrap the proxy to see the actual bean (and use reflection on it), but that's impl dependent - for instance different between Quarkus and Weld.
See https://github.com/quarkusio/quarkus/blob/main/extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/ClientProxyUnwrapper.java

This is something that I expect the framework to take care of for me, hence the issue.

What I am trying to say here is that the reflection is not good enough as a tool to actually see all annotations no matter what the framework does for it - at least in terms of CDI beans and CDI-related annotations.

@manovotn
Copy link
Contributor

Looking at CDI specification, it states the following:

The behavior of all methods declared by java.lang.Object, except for toString(), is undefined for a client proxy. Portable applications should not invoke any method declared by java.lang.Object, except for toString(), on a client proxy.

Therefore, invoking getClass() on the proxy is undefined.
I don't think the spec says anything about subclasses (as those you need for interception for instance) but the situation is very much the same there.

I'll close this issue as it's not a bug and there are workarounds to unwrap the proxy and get to actual bean class instance.
I also don't really see how else to improve this apart from copying annotations to subclasses which is overhead and looks fragile. Besides, it will still suffer from the same issues I described above - the reflection not containing any programmatically adjusted annotations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) kind/question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants