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

Provide a better way to send with application/x-www-form-urlencoded #1705

Closed
shafreenAnfar opened this issue Jul 27, 2021 · 5 comments · Fixed by ballerina-platform/module-ballerina-http#797
Assignees
Labels
module/http Team/PCM Protocol connector packages related issues Type/Improvement

Comments

@shafreenAnfar
Copy link
Contributor

Description:
At the moment we need to do the following to send a request with application/x-www-form-urlencoded.

http:Client clientEp = check new ("https://sample.com");
string payload = string `key1=value1&key2=value2`;
string encodedPayload = check url:encode(payload, "UTF-8");
http:Response response = check checkclientEp->post("/update", encodedPayload, mediaType=mime:APPLICATION_FORM_URLENCODED);

It requires too much code and we need a better way.

@shafreenAnfar
Copy link
Contributor Author

This can be easily improved as follows.

http:Response response = check snowpeak->post("/update", 
    {
        "key1": "value1",
        "key2": "value2"
    }, 
    mediaType=mime:APPLICATION_FORM_URLENCODED
);

Then the underline implementation can do the needful and send it to the back-end.

@shafreenAnfar
Copy link
Contributor Author

shafreenAnfar commented Jan 17, 2022

I think we can improve the implementation a bit.

map<string> pairs = {
  "key1": "value1",
  "key2": "value2"
};
http:Response response = check snowpeak->post("/update", pairs);

If the code is like the above, the user do not need to explicitly mention the media type. We can do a type checking for map<string> and default it to x-www-form-urlencoded.

This also means the below types also will fall under x-www-form-urlencoded instead of json.

readonly & json j = {key1: "foo", key2: "bar"};
record {|
    string key1;
    string key2;
|} r = {key1: "foo", key2: "bar"};
record {|
    string...;
|} r = {"key1": "foo", "key2": "bar"};

80% of the time json would include more than string type. Therefore, it shouldn't be a problem.

This would also make the inbound and outbound implementations identical.

@shafreenAnfar
Copy link
Contributor Author

shafreenAnfar commented Jan 19, 2022

The same applies for the service side as well. At the moment a service that consumes and produces x-www-form-urlencoded looks like the below.

// this maps to x-www-form-urlencoded
service / on new http:Listener(9000) {
    resource function get foo(@http:Payload map<string> keyvalue) 
            returns @http:Payload { mediaType: mime:APPLICATION_FORM_URLENCODED} map<string> {
        
    }
}

And if we do the improvement as mentioned in the previous comment. It would look like the below.

service / on new http:Listener(9000) {
    resource function get foo(@http:Payload map<string> keyvalue) returns map<string> {      
    }
}

@shafreenAnfar
Copy link
Contributor Author

shafreenAnfar commented Jan 19, 2022

Currently, the default mapping for map<string> is application/json. Therefore, when generating OpenAPI for x-www-form-urlecnoded the service should be as follows. Basically we need to override the default content-type.

// this maps to x-www-form-urlencoded
service / on new http:Listener(9000) {
    resource function get foo(@http:Payload { mediaType: mime:APPLICATION_FORM_URLENCODED} map<string> keyvalue) 
            returns @http:Payload { mediaType: mime:APPLICATION_FORM_URLENCODED} map<string> {
        
    }
}

@chamil321
Copy link
Contributor

Fixed via ballerina-platform/module-ballerina-http#968

Binding and the conversion happens based on the diagram of #2530 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/http Team/PCM Protocol connector packages related issues Type/Improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants