-
Notifications
You must be signed in to change notification settings - Fork 925
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 support for passing json values for header and payload #643
Add support for passing json values for header and payload #643
Conversation
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 good, thanks! Only request is to not throw the jackson exception, and instead wrap it in an IllegalArgumentException
.
* @return this same Builder instance. | ||
* @throws JsonProcessingException if json value has invalid structure | ||
*/ | ||
public Builder withHeader(String headerClaimsJson) throws JsonProcessingException { |
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.
Instead of throwing an exception from jackson, let's catch and throw an IllegalArgumentException
with the nested cause, to be consistent with the other methods and not leak the jackson dependency.
* or if the values are not of a supported type. | ||
* @throws JsonProcessingException if json value has invalid structure | ||
*/ | ||
public Builder withPayload(String payloadClaimsJson) throws IllegalArgumentException, JsonProcessingException { |
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.
Same thing here re. the jackson exception.
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 good, thanks!
Changes
Relates to #622
Added support for json values to be pass to the
withHeader
&withPayload
apis. This should de-couple the library to be only used with java Map and allow easy integration with other languages on JVM.