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 builder for OAuth1aToken #2770

Merged
merged 5 commits into from
Jun 8, 2020
Merged

Conversation

minwoox
Copy link
Member

@minwoox minwoox commented Jun 3, 2020

Motivation:
Related #2680 (comment)
The current implementation for building OAuth 1a token isn't convenient.

Modifcations:

  • Add OAuth1aTokenBuilder.

Result:

  • You can now easily create OAuth1aToken.

Motivation:
Related line#2680 (comment)
The current implementation for building OAuth 1a token isn't convinient.

Modifcations:
- Add `OAuth1aTokenBuilder`.

Result:
- You can now easily create `OAuth1aToken`.
@minwoox minwoox added this to the 0.99.7 milestone Jun 3, 2020
@codecov
Copy link

codecov bot commented Jun 4, 2020

Codecov Report

Merging #2770 into master will increase coverage by 0.25%.
The diff coverage is 80.83%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2770      +/-   ##
============================================
+ Coverage     72.66%   72.92%   +0.25%     
- Complexity    11746    11901     +155     
============================================
  Files          1032     1043      +11     
  Lines         45883    46201     +318     
  Branches       5723     5765      +42     
============================================
+ Hits          33340    33690     +350     
+ Misses         9610     9562      -48     
- Partials       2933     2949      +16     
Impacted Files Coverage Δ Complexity Δ
...com/linecorp/armeria/common/auth/OAuth1aToken.java 67.56% <75.43%> (+5.20%) 15.00 <13.00> (-5.00) ⬆️
...ecorp/armeria/common/auth/OAuth1aTokenBuilder.java 85.24% <85.24%> (ø) 21.00 <21.00> (?)
...orp/armeria/server/auth/OAuth1aTokenExtractor.java 86.36% <100.00%> (ø) 6.00 <0.00> (ø)
...rmeria/client/endpoint/EndpointGroupException.java 33.33% <0.00%> (-22.23%) 2.00% <0.00%> (-1.00%)
...ava/com/linecorp/armeria/common/auth/AuthUtil.java 71.42% <0.00%> (-14.29%) 4.00% <0.00%> (-1.00%)
...corp/armeria/internal/common/TimeoutScheduler.java 89.89% <0.00%> (-5.76%) 38.00% <0.00%> (ø%)
...orp/armeria/client/DefaultClientBuilderParams.java 81.25% <0.00%> (-4.17%) 13.00% <0.00%> (-1.00%)
...ria/internal/server/annotation/AnnotationUtil.java 86.86% <0.00%> (-3.65%) 50.00% <0.00%> (-2.00%)
...ctuate/ArmeriaSpringActuatorAutoConfiguration.java 81.00% <0.00%> (-2.34%) 23.00% <0.00%> (+7.00%) ⬇️
...rmeria/internal/server/ResponseConversionUtil.java 84.00% <0.00%> (-1.86%) 11.00% <0.00%> (ø%)
... and 90 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 31f9f6b...4f54143. Read the comment docs.

@@ -125,13 +137,60 @@ public OAuth1aTokenBuilder version(String version) {
}

/**
* Sets additional (or user-defined) parameters.
* Sets additional parameters. If the specified {@code additionals} has pre-defined parameters,
* the parameters are set automatically to this builder and removed from the {@code additionals}.
*/
public OAuth1aTokenBuilder additionals(Map<String, String> additionals) {
Copy link
Member

Choose a reason for hiding this comment

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

This method is not only for additional parameters but any parameters including pre-defined ones. Perhaps we should rename it to putAll()? We could also add methods like put(CharSequence, CharSequence).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, added put() and putAll(). I didn't use CharSequence though because I feel it's too much.
If it's necessary, jut let me know. 😉

Copy link
Member

Choose a reason for hiding this comment

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

SGTM. Thanks, @minwoox

docs-client/package-lock.json Outdated Show resolved Hide resolved
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.

Nice work, @minwoox ❤️

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.

Thanks! @minwoox
Left some nits :-)

@trustin trustin merged commit 1ab50b1 into line:master Jun 8, 2020
@trustin
Copy link
Member

trustin commented Jun 8, 2020

Thanks a lot, @minwoox

@minwoox minwoox deleted the add_OAuth1aTokenBuilder branch June 8, 2020 06:29
@minwoox
Copy link
Member Author

minwoox commented Jun 8, 2020

Thanks for reviewing. 😉

fmguerreiro pushed a commit to fmguerreiro/armeria that referenced this pull request Sep 19, 2020
Motivation:

Related: line#2680 (comment)
The current implementation for building OAuth 1a token isn't convenient.

Modifcations:

- Add `OAuth1aTokenBuilder`.

Result:

- You can now easily create `OAuth1aToken`.
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.

3 participants