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.

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 Koltlin nullable type support to AnnotatedService #4225

Merged
merged 6 commits into from
Jun 8, 2022

Conversation

ks-yim
Copy link
Contributor

@ks-yim ks-yim commented Apr 25, 2022

Motivation:

AnnotatedService requires @Nullable annotations for nullable resolved values even when it is used with Kotlin which offers nice null-safety with nullable types.

Modifications:

  • Add isMarkedNullable method to KotlinUtil.
  • Make AnnotatedValueResolver#Builder detect Kotlin nullable types of Field and Parameter and allow null values for them.

Result:

@codecov
Copy link

codecov bot commented Apr 25, 2022

Codecov Report

Merging #4225 (edb06ab) into master (da376b5) will decrease coverage by 0.00%.
The diff coverage is 59.09%.

@@             Coverage Diff              @@
##             master    #4225      +/-   ##
============================================
- Coverage     73.34%   73.34%   -0.01%     
- Complexity    16952    16978      +26     
============================================
  Files          1451     1450       -1     
  Lines         63982    64104     +122     
  Branches       8026     8061      +35     
============================================
+ Hits          46928    47017      +89     
- Misses        12964    12991      +27     
- Partials       4090     4096       +6     
Impacted Files Coverage Δ
...rmeria/internal/common/kotlin/ArmeriaKotlinUtil.kt 35.29% <31.81%> (-6.38%) ⬇️
...armeria/internal/server/annotation/KotlinUtil.java 71.76% <71.42%> (-0.04%) ⬇️
...rnal/server/annotation/AnnotatedValueResolver.java 84.65% <93.33%> (+0.45%) ⬆️
...ecorator/RateLimitingDecoratorFactoryFunction.java 0.00% <0.00%> (-66.67%) ⬇️
...ver/throttling/RateLimitingThrottlingStrategy.java 40.00% <0.00%> (-30.00%) ⬇️
...armeria/internal/common/stream/SubscriberUtil.java 72.22% <0.00%> (-11.12%) ⬇️
...ecorp/armeria/dropwizard/ArmeriaServerFactory.java 70.27% <0.00%> (-8.07%) ⬇️
...in/java/example/armeria/grpc/HelloServiceImpl.java 79.24% <0.00%> (-5.67%) ⬇️
...eria/internal/common/stream/StreamMessageUtil.java 61.11% <0.00%> (-5.56%) ⬇️
...necorp/armeria/server/grpc/GrpcServiceBuilder.java 75.00% <0.00%> (-4.84%) ⬇️
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da376b5...edb06ab. Read the comment docs.

@ikhoon ikhoon added this to the 1.17.0 milestone Apr 25, 2022
Copy link
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.

Overall, looking nice!

Copy link
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, @ks-yim! 💯❤️

@ks-yim
Copy link
Contributor Author

ks-yim commented Apr 28, 2022

Thanks, it would be nice if we improve the doc, too. I will work on that too.

Copy link
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.

Great news for Kotlin users 🙇

Copy link
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.

Thanks! 😄

@trustin trustin merged commit 007f5bf into line:master Jun 8, 2022
@ks-yim ks-yim deleted the feature/kotlin-nullable-type branch June 9, 2022 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Kotlin nullable type support for AnnotatedService arguments
4 participants