-
Notifications
You must be signed in to change notification settings - Fork 930
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
Add all HTTP methods to route when the method is empty #2570
Conversation
Motivation: Check out this code: ```java sb.decorator((delegate, ctx, req) -> { System.err.println(2); return delegate.serve(ctx, req); }); sb.routeDecorator().pathPrefix("/").build((delegate, ctx, req) -> { System.err.println(1); return delegate.serve(ctx, req); }); sb.decoratorUnder("/", (delegate, ctx, req) -> { System.err.println(3); return delegate.serve(ctx, req); }); ``` The executed order of decorators are a little messed becuase `routeDecorator()` has all `HttpMethod`s set, but others don't. If we set `HttpMethods.knownMethods()` when there's no method set for a route, the execution order of decorator will be the order they inserted. Modifications: - Add `HttpMethod.knownMethods()` to the `Route` if it's empty. - However, the logger name and metric don't include it because it's verbose. Result: - Right decoration order.
core/src/main/java/com/linecorp/armeria/server/DefaultRoute.java
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #2570 +/- ##
============================================
- Coverage 73.4% 73.38% -0.03%
+ Complexity 10942 10933 -9
============================================
Files 957 958 +1
Lines 42146 42098 -48
Branches 5276 5261 -15
============================================
- Hits 30938 30894 -44
- Misses 8496 8505 +9
+ Partials 2712 2699 -13
Continue to review full report at Codecov.
|
core/src/main/java/com/linecorp/armeria/server/DefaultRoute.java
Outdated
Show resolved
Hide resolved
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, @minwoox !
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!
Motivation: Check out this code: ```java sb.decorator((delegate, ctx, req) -> { System.err.println(2); return delegate.serve(ctx, req); }); sb.routeDecorator().pathPrefix("/").build((delegate, ctx, req) -> { System.err.println(1); return delegate.serve(ctx, req); }); sb.decoratorUnder("/", (delegate, ctx, req) -> { System.err.println(3); return delegate.serve(ctx, req); }); ``` The executed order of decorators are a little messed becuase `routeDecorator()` has all `HttpMethod`s set, but others don't. If we set `HttpMethods.knownMethods()` when there's no method set for a route, the execution order of decorator will be the order they inserted. Modifications: - Add `HttpMethod.knownMethods()` to the `Route` if it's empty. - However, the logger name and metric don't include it because it's verbose. Result: - Right decoration order.
Motivation:
Check out this code:
The executed order of decorators are a little messed becuase
routeDecorator()
has allHttpMethod
s set, but others don't.If we set
HttpMethods.knownMethods()
when there's no method set for a route, the execution order of decorator will be the order they inserted.Modifications:
HttpMethod.knownMethods()
to theRoute
if it's empty.Result: