-
Notifications
You must be signed in to change notification settings - Fork 924
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
Make AnnotatedService public #5628
Conversation
core/src/main/java/com/linecorp/armeria/internal/server/annotation/DefaultAnnotatedService.java
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/DefaultServiceConfigSetters.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedServiceElement.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/internal/server/annotation/DefaultAnnotatedService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/internal/server/annotation/DefaultAnnotatedService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/internal/server/annotation/DefaultAnnotatedService.java
Outdated
Show resolved
Hide resolved
...src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedDocServicePlugin.java
Outdated
Show resolved
Hide resolved
...src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedDocServicePlugin.java
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/internal/server/annotation/DefaultAnnotatedService.java
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/DefaultServiceConfigSetters.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/internal/server/annotation/DefaultAnnotatedService.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/internal/server/annotation/DefaultAnnotatedService.java
Show resolved
Hide resolved
...src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedDocServicePlugin.java
Outdated
Show resolved
Hide resolved
…tion/AnnotatedDocServicePlugin.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tion/DefaultAnnotatedService.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…onfigSetters.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…onfigSetters.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tatedService.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tion/AnnotatedDocServicePlugin.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
…tion/AnnotatedDocServicePlugin.java Co-authored-by: Ikhun Um <ih.pert@gmail.com>
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.
Almost there - Let's go!
core/src/main/java/com/linecorp/armeria/internal/common/util/ServiceNamingUtil.java
Outdated
Show resolved
Hide resolved
...src/main/java/com/linecorp/armeria/internal/server/annotation/AnnotatedDocServicePlugin.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/DefaultServiceConfigSetters.java
Outdated
Show resolved
Hide resolved
core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java
Outdated
Show resolved
Hide resolved
…onfigSetters.java Co-authored-by: Trustin Lee <t@motd.kr>
…erviceNamingUtil.java Co-authored-by: Trustin Lee <t@motd.kr>
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.
Nice work! Thanks, @chickenchickenlove. 🙇♂️🙇♂️
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.
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.
Pushed a small commit with minor cleanups. Thanks @chickenchickenlove ! 🙇 👍 🙇
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.
Looks really great! Thanks, @chickenchickenlove! 👍 👍 👍
core/src/main/java/com/linecorp/armeria/internal/server/annotation/DefaultAnnotatedService.java
Outdated
Show resolved
Hide resolved
…tion/DefaultAnnotatedService.java
Please make sure the PR description is up to date and has no grammar errors or incorrect class names next time. Also, please do not mix up motivation, modification and result. For example, what you initially wrote at the motivation section is the result of your work. |
Thanks for your comment. |
Motivation: - Related issue: line#5382 - It'd be nice if a user can access the information provided by `AnnotatredService`, but it is not part of the public API. ### Modifications: - Split `AnnotatedService` into `AnnotatedService` (interface) and `DefaultAnnotatedService` (implementation). ### Result: - Closes line#5382 Before: ```java ServiceRequestContext serviceRequestContext = ServiceRequestContext.current(); AnnotatedService service = serviceRequestContext.config().service().as(AnnotatedService.class); // This doesn't compile. // service.method(); // service.defaultStatus(); ``` After: ```java ServiceRequestContext serviceRequestContext = ServiceRequestContext.current(); AnnotatedService service = serviceRequestContext.config().service().as(AnnotatedService.class); // This compiles and provides useful properties. service.method(); service.defaultStatus(); ``` --------- Co-authored-by: Ikhun Um <ih.pert@gmail.com> Co-authored-by: Trustin Lee <t@motd.kr> Co-authored-by: jrhee17 <guins_j@guins.org> Co-authored-by: minux <songmw725@gmail.com>
Motivation:
AnnotatedService
provided specialized information for annotated services such asMethod
,service instance
, and thedefault status
, and so on. however, users could not access this information because it was declared with apackage-private
access modifier, so it was not part of the public API.Modifications:
AnnotatedService
into two parts:AnnotatedService
(interface) andDefaultAnnotatedService
(implementation).AnnotatedService
: The methods that can be used by users through the Public API are declared in theAnnotatedService
interface.DefaultAnnotatedService
: Methods intended for internal use are declared inDefaultAnnotatedService
.Result:
AnnotatedService
public #5382AnnotatedService
available through a public API.