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

Proposal: Data-binding for application/x-www-form-urlencoded #2530

Closed
shafreenAnfar opened this issue Dec 21, 2021 · 6 comments
Closed

Proposal: Data-binding for application/x-www-form-urlencoded #2530

shafreenAnfar opened this issue Dec 21, 2021 · 6 comments
Labels
module/http Status/Implemented Implemented proposals Team/PCM Protocol connector packages related issues Type/Proposal

Comments

@shafreenAnfar
Copy link
Contributor

shafreenAnfar commented Dec 21, 2021

Summary

application/x-www-form-urlencoded is common MIME type in use. It allows you to send key/value pairs in the payload. This proposal is to provide data binding for such payload. Both on listener side as well as the client side.

Goals

  • Improve user experience when dealing with application/x-www-form-urlencoded

Motivation

As mentioned in the summary section application/x-www-form-urlencoded is used to send key/value pairs in the entity body. However, Ballerina does not provide a direct data binding for such payload. Therefore, users have to either do string manipulation or use http:Request object's getFormParams() function. But it is not the best user experience. This is applicable for both the listener side as well as the client side.

User experience can be radically improved by introducing data binding for such payloads as discussed in the next section.

Description

Since application/x-www-form-urlencoded is about key/value pairs, the appropriate Ballerina data binding is map<string>. This is applicable for both listener and client side. However, users could use map<string> for json payloads as well. Following table shows the mapping and the payload.

Ballerina Type Media Type Payload
map<string> application/x-www-form-urlencoded key1=value1&key2=value2
map<string> application/json { "key1": "value1", "key2": "value2" }

Since map<string> maps into two different types of payloads, implementation must try parsing the payload in both ways to make sure the payload cannot be mapped to map<string>. However, map<string> is quite common in the application/x-www-form-urlencoded scenario than the application/json scenario. Therefore, implementation always must try first parsing the payload for application/x-www-form-urlencoded and if it fails must try parsing the payload for application/json.

Listener

On the listener side users should be able to do following when dealing with such payload.

service / on new http:Listener(9090) {
    resource function post key\-value(@http:Payload map<string> pairs) returns string {
        // do something
    }
}

As mentioned earlier, since two types of payloads could map into map<string>, as a performance optimization users can be specific as follows.

service / on new http:Listener(9090) {
    resource function post key\-value(@http:Payload { mediaType: "application/www-x-form-urlencoded"} map<string> pairs) returns string {
        // do something
    }
}

Client

On the client side the user experience should be as follows.

http:Client foo = check new("localhost:9090/bar");
map<string> pairs = check foo->get("/zee");

As on the listener side, there is no room for performance optimization on the client side. This is because on the client side there is no place to mention what type payload the user is expecting.

Anyway, it is worth noting that as mentioned earlier 80% case would be the users are trying to map application/x-www-form-urlencoded to map<string>. Therefore, there is very little gains to make with such optimizations.

@shafreenAnfar
Copy link
Contributor Author

shafreenAnfar commented Jan 11, 2022

After having a discussion with @chamil321 @ayeshLK @lnash94, we decided to do a slight improvement to the proposal. We decided to update the mapping as follows for inbound requests.

Ballerina Type Media Type Payload
map<string> application/x-www-form-urlencoded key1=value1&key2=value2
json application/json { "key1": "value1", "key2": "value2" }

@shafreenAnfar
Copy link
Contributor Author

shafreenAnfar commented Jan 12, 2022

I think we can look at the problem in a different angle as well. We can look at it as inbound and outbound. This proposal is related to inbound messages. The inbound message can be branched as follows.

Screen Shot 2022-01-12 at 10 37 36 PM

This basically means we are not limiting a given Ballerina type to a specific mime type. For instance map<string> could be used for application/json as well as application/x-www-form-urlencoded.

@nadeeshaan
Copy link

One question regarding the unknown content-type,
When we say that it is going to map to a known content type, do we infer that it would be mapped to a fixed canonical content-type (for instance application/octect-stream) or is it dynamic - in the sense, we are going to utilize other message properties and map to one of the available content types.

@shafreenAnfar shafreenAnfar changed the title Proposal: Data-binding for application/www-x-form-urlencoded Proposal: Data-binding for application/x-www-form-urlencoded Jan 17, 2022
@shafreenAnfar
Copy link
Contributor Author

@nadeeshaan It is dynamic and will be inferred by looking at the Ballerina type.

@shafreenAnfar shafreenAnfar added the Status/Active Proposals that are under review label Feb 23, 2022
@chamil321
Copy link
Contributor

chamil321 commented Apr 7, 2022

The existing implementation solely depend on the ballerina type and does not check the content type except for the map<string> type. The proposed architecture changes the logic to do the conversion based on the the content type. If the content type is unknown or not a standard one, then conversion is done based on the type.

Case 1: text/plain payload casting to a json

    resource function 'default body3(http:Caller caller, @http:Payload json person) returns error? {
        jsonval1 = check person.name;
    }   

Error : error: {ballerina}JSONOperationError {"message":"JSON value is not a mapping"}

Conclusion: String to Json casting is avoided. Check the table to understand the allowed payloads for each type

@chamil321
Copy link
Contributor

Fixed as per the table #2700 (comment). Hence closed

@chamil321 chamil321 added Status/Implemented Implemented proposals and removed Status/Active Proposals that are under review labels Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/http Status/Implemented Implemented proposals Team/PCM Protocol connector packages related issues Type/Proposal
Projects
None yet
Development

No branches or pull requests

3 participants