-
Notifications
You must be signed in to change notification settings - Fork 3k
Interrogate candidate resources/return types to stop REST server endpoint indexer from scanning REST clients #48481
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
Conversation
…oint indexer from scanning REST clients As detailed in quarkusio#48464 certail methods cause the server endpoint indexer to scan _all_ REST client methods, which can lead to errors if any client method violates the requirements for resource methods. This fixes this in two ways: ### 1. Detect `java.lang.Object` returning methods and attempt further deduction. Since the crux of the issue is `java.lang.Object` infecting the candidate list (which causes any query of does X derive Y to always result in true) this step tries to reduce the false positives here. The only method I could implement was for Kotlin suspend methods, since by view of Java they always return `Object`. If the method is a suspend method we extract the real return type. Unfortunately while this reduces the pollution of the list dramatically, any method must be allowed to return `Object` (or Kotlin `Any`) which may dynamically return a sub-resource. ### 2. Check candidates for annotations from one of the Q-REST, MP-REST, or JAX-RS REST client pacakges. Since we can’t always ensure the return type list is “free” of `Object`, the candidates are filtered based on if the class or method uses any annotatoins from the spec client packages. Reasonably this _should_ filter the majority of client interfaces because they generally have at least one client annotation (e.g. `RegisterRestClient`). —- These issuees are tested were tested in our, fairly large set of projects, which all use Kotlin. and even include some methods returning `Any`. with no client interfaces being sent to the server indexer. This wont fully ensure all client interfaces won’t ever be scanned by the server indexer, but the likelihood has been reduced dramatically. To get a REST client scanned by the server indexer you would need to: 1. Write a Java method returning `java.lang.Object` or `Kotlin.Any`, this could be anywhere in the project. 2. Write an interface with no client specific annotations and register it programmatically. In this scenario, the liklihood of an issue arising is further reduced by the fact that errors are only raised when a method on the client interface fails to validate as a server method as well. From my investigation this seems very unlikely. E.g., in the REST client that triggered the error in our project, the method used an extra parameter annotated with `@NotBody` together with the `@ClientHeaders` annotation to add special client request headers. Now, the presence of either of thse annotations causes the interface to be filtered before being scanned.
bf3e51e
to
2c6ed6f
Compare
@geoand I took a stab at it. It passes the local tests and I tested it in our code base to verify it solved the issues. We'll see if it passes the integration tests. There are no tests, I didn't immediately know where/how to add them. The triggering scenario (Kotlin Any/suspend method plus invalid server resource method in a client) can be created as a test to verify it's not producing errors incorrectly but a step further would probably be better; something like running the REST processor and comparing the classes that were indexed vs a known list of what should have been indexed. As I said originally, I have very little time right now to spend on this so I couldn't spend the time figuring the testing out. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @kdubb!
Status for workflow
|
As detailed in #48464 certain methods cause the server endpoint indexer to scan all REST client methods, which can lead to errors if any client method violates the requirements for resource methods.
This fixes this in two ways:
1. Detect
java.lang.Object
returning methods and attempt further deduction.Since the crux of the issue is
java.lang.Object
infecting the candidate list (which causes any query of does X derive Y to always result in true) this step tries to reduce the false positives here. The only method I could implement was for Kotlin suspend methods, since by view of Java they always returnObject
. If the method is a suspend method we extract the real return type.Unfortunately while this reduces the pollution of the list dramatically, any method must be allowed to return
Object
(or KotlinAny
) which may dynamically return a sub-resource.2. Check candidates for annotations from one of the Q-REST, MP-REST, or JAX-RS REST client pacakges.
Since we can’t always ensure the return type list is “free” of
Object
, the candidates are filtered based on if the class or method uses any annotatoins from the spec client packages. Reasonably this should filter the majority of client interfaces because they generally have at least one client annotation (e.g.RegisterRestClient
).—-
These issuees are tested were tested in our, fairly large set of projects, which all use Kotlin. and even include some methods returning
Any
. with no client interfaces being sent to the server indexer.This wont fully ensure all client interfaces won’t ever be scanned by the server indexer, but the likelihood has been reduced dramatically. To get a REST client scanned by the server indexer you would need to:
java.lang.Object
orKotlin.Any
, this could be anywhere in the project.In this scenario, the liklihood of an issue arising is further reduced by the fact that errors are only raised when a method on the client interface fails to validate as a server method as well. From my investigation this seems very unlikely. E.g., in the REST client that triggered the error in our project, the method used an extra parameter annotated with
@NotBody
together with the@ClientHeaders
annotation to add special client request headers. Now, the presence of either of thse annotations causes the interface to be filtered before being scanned.