Provide a way to find dynamic decorators handling the current request #5670
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
Motivation: `HttpService.as(Class)` can be used to unwrap and find an instance of a decorator or a service. `ServiceRequestContext.config().service().as(Class)` can't find all decorators because decoraters set with `ServerBuilder.decorator()` don't statically wrap the services. I propose to add `ServiceRequestContext.findService(Class)` for finding both dynamic and static decorators handling a request. Modifcations: - Add `ServiceRequestContext.findService(Class)` that gets all service chain from `InitialDispatcherService` and finds the specific service. Result: You can easily find both dynamic and static decorators that handle a request by using `ServiceRequestContext.findService(Class)`.
ikhoon
commented
May 9, 2024
| @@ -162,7 +163,7 @@ public String toString() { | |||
| .toString(); | |||
| } | |||
|
|
|||
| private static class InitialDispatcherService extends SimpleDecoratingHttpService { | |||
| public static final class InitialDispatcherService extends SimpleDecoratingHttpService { | |||
There was a problem hiding this comment.
I kept InitialDispatcherService as the nested class because InitialDispatcherService and RouteDecoratingService are tightly coupled.
trustin
reviewed
May 9, 2024
core/src/main/java/com/linecorp/armeria/internal/server/RouteDecoratingService.java
Outdated
Show resolved
Hide resolved
jrhee17
approved these changes
May 10, 2024
core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/ServiceRequestContext.java
Outdated
Show resolved
Hide resolved
…tServiceRequestContext.java Co-authored-by: jrhee17 <guins_j@guins.org>
…ontext.java Co-authored-by: jrhee17 <guins_j@guins.org>
minwoox
reviewed
May 14, 2024
core/src/main/java/com/linecorp/armeria/internal/server/DefaultServiceRequestContext.java
Show resolved
Hide resolved
ikhoon
added a commit
to ikhoon/armeria
that referenced
this pull request
Jun 7, 2024
Motivation: This PR is follow-up of line#5632 (comment) and line#5670 that apply `ctx.findService()` to find `CorsService` added with route decorators. Modifications: - Use `ctx.findService()` to find `CorsService` in the service chain of a request Result: CorsServerErrorHandler now correctly handle response exceptions even if `CorsService` added as a route decorator. (Not need to metion in the release notes)
trustin
pushed a commit
that referenced
this pull request
Jun 7, 2024
Motivation: This PR is follow-up of #5632 (comment) and #5670 that `ctx.findService()` should be used to find `CorsService` added with route decorators. Modifications: - Use `ctx.findService()` to find `CorsService` in the service chain of a request Result: CorsServerErrorHandler now correctly handles response exceptions even if `CorsService` is added as a route decorator. (Not need to be mentioned in the release notes)
Dogacel
pushed a commit
to Dogacel/armeria
that referenced
this pull request
Jun 8, 2024
…line#5670) Motivation: `HttpService.as(Class)` can be used to unwrap and find an instance of a decorator or a service. `ServiceRequestContext.config().service().as(Class)` can't find all decorators because decorators set with `ServerBuilder.decorator()` don't statically wrap the services. I propose to add `ServiceRequestContext.findService(Class)` for finding both dynamic and static decorators handling a request. Modifications: - Add `ServiceRequestContext.findService(Class)` that gets all service chain from `InitialDispatcherService` and finds the specific service. Result: You can now easily find both dynamic and static decorators that handle a request by using `ServiceRequestContext.findService(Class)`.
Dogacel
pushed a commit
to Dogacel/armeria
that referenced
this pull request
Jun 8, 2024
…#5739) Motivation: This PR is follow-up of line#5632 (comment) and line#5670 that `ctx.findService()` should be used to find `CorsService` added with route decorators. Modifications: - Use `ctx.findService()` to find `CorsService` in the service chain of a request Result: CorsServerErrorHandler now correctly handles response exceptions even if `CorsService` is added as a route decorator. (Not need to be mentioned in the release notes)
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.
Motivation:
HttpService.as(Class)can be used to unwrap and find an instance of a decorator or a service.ServiceRequestContext.config().service().as(Class)can't find all decorators because decorators set withServerBuilder.decorator()don't statically wrap the services.I propose to add
ServiceRequestContext.findService(Class)for finding both dynamic and static decorators handling a request.Modifications:
ServiceRequestContext.findService(Class)that gets all service chain fromInitialDispatcherServiceand finds the specific service.Result:
You can now easily find both dynamic and static decorators that handle a request by using
ServiceRequestContext.findService(Class).