Skip to content

Make AnnotatedService public#5628

Merged
trustin merged 26 commits intoline:mainfrom
chickenchickenlove:240422-test
May 7, 2024
Merged

Make AnnotatedService public#5628
trustin merged 26 commits intoline:mainfrom
chickenchickenlove:240422-test

Conversation

@chickenchickenlove
Copy link
Copy Markdown
Contributor

@chickenchickenlove chickenchickenlove commented Apr 22, 2024

Motivation:

  • In the past, AnnotatedService provided specialized information for annotated services such as Method, service instance, and the default status, and so on. however, users could not access this information because it was declared with a package-private access modifier, so it was not part of the public API.

Modifications:

  • Split AnnotatedService into two parts: AnnotatedService (interface) and DefaultAnnotatedService (implementation).
  • AnnotatedService : The methods that can be used by users through the Public API are declared in the AnnotatedService interface.
  • DefaultAnnotatedService: Methods intended for internal use are declared in DefaultAnnotatedService.

Result:

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 22, 2024

@trustin trustin added new feature sprint Issues for OSS Sprint participants labels Apr 23, 2024
Comment thread core/src/main/java/com/linecorp/armeria/server/DefaultServiceConfigSetters.java Outdated
Comment thread core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java Outdated
Comment thread core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java Outdated
Comment thread core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java Outdated
Comment thread core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java Outdated
Comment thread core/src/main/java/com/linecorp/armeria/server/DefaultServiceConfigSetters.java Outdated
Comment thread core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java Outdated
Comment thread core/src/main/java/com/linecorp/armeria/server/annotation/AnnotatedService.java Outdated
chickenchickenlove and others added 15 commits April 28, 2024 19:29
…tion/AnnotatedDocServicePlugin.java

Co-authored-by: Ikhun Um <[email protected]>
…tion/DefaultAnnotatedService.java

Co-authored-by: Ikhun Um <[email protected]>
…tion/AnnotatedDocServicePlugin.java

Co-authored-by: Ikhun Um <[email protected]>
…tion/AnnotatedDocServicePlugin.java

Co-authored-by: Ikhun Um <[email protected]>
Copy link
Copy Markdown
Member

@trustin trustin left a 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!

Comment thread core/src/main/java/com/linecorp/armeria/server/DefaultServiceConfigSetters.java Outdated
@chickenchickenlove chickenchickenlove requested a review from trustin May 2, 2024 01:12
@ikhoon ikhoon added this to the 1.29.0 milestone May 2, 2024
Copy link
Copy Markdown
Contributor

@ikhoon ikhoon left a 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. 🙇‍♂️🙇‍♂️

Copy link
Copy Markdown
Member

@trustin trustin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

@jrhee17 jrhee17 left a 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 ! 🙇 👍 🙇

Copy link
Copy Markdown
Contributor

@minwoox minwoox left a 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! 👍 👍 👍

@trustin trustin merged commit 7d562ce into line:main May 7, 2024
@trustin
Copy link
Copy Markdown
Member

trustin commented May 7, 2024

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.

@chickenchickenlove
Copy link
Copy Markdown
Contributor Author

Thanks for your comment.
I updated the PR description following your guide. 🙇‍♂️

Dogacel pushed a commit to Dogacel/armeria that referenced this pull request Jun 8, 2024
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 <[email protected]>
Co-authored-by: Trustin Lee <[email protected]>
Co-authored-by: jrhee17 <[email protected]>
Co-authored-by: minux <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new feature sprint Issues for OSS Sprint participants

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make AnnotatedService public

5 participants