Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Replace Optional<List<T>> parameter to ObjectProvider<T> #5527

Closed
kth496 opened this issue Mar 25, 2024 · 3 comments · Fixed by #5573
Closed

Replace Optional<List<T>> parameter to ObjectProvider<T> #5527

kth496 opened this issue Mar 25, 2024 · 3 comments · Fixed by #5573
Labels
cleanup
Milestone
1.28.0

Comments

@kth496
Copy link
Contributor

kth496 commented Mar 25, 2024

Motivation

Currently, some codebases continue to utilize the Optional<List> style. However, it is clearer to simply use an empty collection to signify that a parameter is absent. Since version 4.3, the Spring framework has offered the ObjectProvider, which is effective for managing the optionality of injection points. Conveniently, Armeria also leverages ObjectProvider.
(e.g. ArmeriaSpringActuatorAutoConfiguration.java)

Suggestion

Replace Optional<List<T>> parameter to ObjectProvider<T>
@jrhee17
Copy link
Contributor

jrhee17 commented Mar 26, 2024

Sure, sounds good 👍

@kratosmy
Copy link
Contributor

kratosmy commented Apr 1, 2024

Hi, @jrhee17 , could I take on this?

@jrhee17
Copy link
Contributor

jrhee17 commented Apr 1, 2024

Sure, go for it

This was referenced Apr 2, 2024
Closed
Merged
jrhee17 pushed a commit that referenced this issue Apr 11, 2024
@kratosmy
Replace Optional with Spring's ObjectProvider to manage the optionali… (
8d4ddea 
#5573)

Motivation:

Optional<T> is not as compatible as ObjectProvider<T>, which is included
in Spring Framework since Spring 4.3, so use ObjectProvider would
improve the programmatic resolution of Beans.

Modifications:

Two file are affected, AbstractArmeriaAutoConfiguration.java and
ArmeriaReactiveWebServerFactoryAutoConfiguration.java

Result:

Closes #5527 .
@jrhee17 jrhee17 added this to the 1.28.0 milestone Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
cleanup
Projects
None yet
Milestone
1.28.0
3 participants
@jrhee17 @kth496 @kratosmy