-
Notifications
You must be signed in to change notification settings - Fork 85
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 anydata
support for setPayload
methods
#2135
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2135 +/- ##
=========================================
Coverage 80.22% 80.23%
Complexity 669 669
=========================================
Files 408 408
Lines 22926 22926
Branches 5271 5271
=========================================
+ Hits 18393 18395 +2
+ Misses 3396 3395 -1
+ Partials 1137 1136 -1 ☔ View full report in Codecov by Sentry. |
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.
LGTM
0c9bbbe
ballerina/http_request.bal
Outdated
@@ -512,13 +512,15 @@ public class Request { | |||
|
|||
# Sets the request payload. This method overrides any existing content-type by passing the content-type | |||
# as an optional parameter. If the content type parameter is not provided then the default value derived | |||
# from the payload will be used as content-type only when there are no existing content-type header. | |||
# from the payload will be used as content-type only when there are no existing content-type header. If | |||
# the payload is non-json typed value then the value is converted to json using the `toJson` method. |
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.
Not entirely an accurate description for xml, right?
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.
Yes, will update it
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.
Updated via #2135
ballerina/http_request.bal
Outdated
# | ||
# + payload - Payload can be of type `string`, `xml`, `json`, `byte[]`, `stream<byte[], io:Error?>` | ||
# + payload - Payload can be of type `anydata`, `stream<byte[], io:Error?>` |
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.
Would we have to explicitly mention xml, json, etc. as examples? Just thinking of someone getting started with Ballerina.
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.
We usually don't suggest users to use request/response objects with setPayload methods unless it is an advanced scenario like multipart. May be we can include a more detailed description in the spec
Quality Gate passedIssues Measures |
Purpose
Fixes: ballerina-platform/ballerina-library#6954
The
setPayload
withanydata
uses thesetJsonPayload
with thetoJson
value of theanydata
.Example
Checklist
Updated the specChecked the impact on OpenAPI generation