FISH-758 Fault Tolerance Annotations Not Applied to Rest Client Interfaces #5315
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Got Retry and other annotations to get detected for Rest Client. Asynchronous throws an exception however during deployment so is commented out, and not sure how to tackle Fallback due to its special nature of not allowing targets of TYPE (class). Signed-off-by: Andrew Pielage <[email protected]>
…r with CDI interception rules
Signed-off-by: Andrew Pielage <[email protected]>
pdudits
suggested changes
Jun 23, 2021
| interceptorType.configureAnnotatedType() | ||
| .remove(annotation -> annotation instanceof Priority) | ||
| .add(new PriorityLiteral(priorityOverride.get())); | ||
| void enableInterceptor(@Observes AfterTypeDiscovery afterTypeDiscovery) { |
There was a problem hiding this comment.
This is too much code to just believe in without a unit test ;)
Binary search is of course the correct approach since we already know that the list is sorted. But we're also looking at one time insertion in a table that is usually handful of items.
Personally I'd settle for linear scan of the list to find first interceptor with higher or equal priority and ours just before it. But the unit test requirement still holds ;)
...rance/src/main/java/fish/payara/microprofile/faulttolerance/cdi/FaultToleranceExtension.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Andrew Pielage <[email protected]>
Signed-off-by: Andrew Pielage <[email protected]>
Signed-off-by: Andrew Pielage <[email protected]>
|
Jenkins test please |
pdudits
approved these changes
Jun 23, 2021
JamesHillyard
pushed a commit
to JamesHillyard/Payara
that referenced
this pull request
Oct 28, 2021
FISH-758 Fault Tolerance Annotations Not Applied to Rest Client Interfaces
JamesHillyard
added a commit
to JamesHillyard/Payara
that referenced
this pull request
Dec 22, 2021
This reverts commit 80590c9 Signed-off-by: JamesHillyard <[email protected]>
JamesHillyard
pushed a commit
to JamesHillyard/Payara
that referenced
this pull request
Dec 23, 2021
FISH-758 Fault Tolerance Annotations Not Applied to Rest Client Interfaces Signed-off-by: JamesHillyard <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Bug fix.
The MicroProfile Rest Client spec has an untested requirement that FT annotations get applied to Rest Client interfaces.
https://github.com/eclipse/microprofile-rest-client/blob/master/spec/src/main/asciidoc/integration.asciidoc#microprofile-fault-tolerance
Weld wasn't picking up our synthetic "not quite a stereotype" Fault Tolerance interceptor as an interceptor for the individual FT annotations, as it attempts to match annotations to interceptors via this method -
FaultToleranceonly matchesFaultTolerance, notRetryand onlyRetry.The way around this was to go back to the old style of having an interceptor for each individual FT annotation, which becomes a problem when it comes to attempting to resolve
Fallbackas you can't write an interceptor for this in the usual way (marking a class withFallback&Interceptorand adding anAroundInvokemethod).Alternatively, we could do as we've done here and register the interceptor programmatically as an interceptor for the FT annotations.
Important Info
Blockers
None.
Testing
New tests
IT added to Payara Samples.
Testing Performed
Ran the FT TCK.
Ran Rudy's example test which demonstrates the
Retryannotation working on a Rest Client interface.Testing Environment
Windows 10, JDK 8
WSL OpenSUSE Leap 15.2, JDK 8
Documentation
N/A
Notes for Reviewers
I feel like this block is both over and under-engineered and I'm missing a much simpler way of determining the priority of each interceptor - this solution seems fragile.
https://github.com/payara/Payara/compare/master...Pandrex247:FISH-758?expand=1#diff-d1250f7d0ca1e02fa4c81c96aa7ef9d2c7045b6e4f7bdab566ea282ad3c60940R149-R248