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

Support application/x-www-form-urlencoded content-type for transforms #8097

Closed
tirumaraiselvan opened this issue Jan 28, 2022 · 7 comments
Closed
Assignees
Labels
k/enhancement New feature or improve an existing feature

Comments

@tirumaraiselvan
Copy link
Contributor

tirumaraiselvan commented Jan 28, 2022

Currently, payload transforms in REST connectors have content-type: application/json. We should support application/x-www-form-urlencoded as well. This is described in brief in the initial RFC: https://github.com/hasura/graphql-engine/blob/master/rfcs/transforms.md (see Example 5)

Open questions:

  1. Should we change the body API to reflect form urlencoded natively? (i.e. key value pairs)
  2. Should we automatically escape values or delegate it to the user to use the escape() function?

Note: The docs wrongly suggests that x-www-form-urlencoded is already supported: https://hasura.io/docs/latest/graphql/core/actions/transforms.html#content-type

@tirumaraiselvan tirumaraiselvan added the k/enhancement New feature or improve an existing feature label Jan 28, 2022
@motleydev
Copy link
Contributor

FWIW, I think Postman's pre-existing thought work on this could be a good source of inspiration.

CleanShot 2022-01-28 at 10 29 00

CleanShot 2022-01-28 at 10 29 47

How often does a person modify BOTH query params AND the body of the request? Does it warrant taking up separate space or could it be condensed into a tabbed interface to require less overall scrolling?

@martin-hasura

@solomon-b
Copy link
Contributor

solomon-b commented Feb 9, 2022

Current Transform Schema

As of PR #3475 in the mono repo, the syntax for body and header transforms is as follows:

	   "request_transform": {
	     "request_method": "GET",
	     "body": {
	       "action": "remove"
	     },
	     "request_headers": { "foo": "bar", "content-type": "application/json"},
	     "engine": "kriti"
	   } 
	   "request_transform": {
	     "request_method": "GET",
	     "body": {
	       "action": "transform",
               "template": "{{ $body.foo }}"
	     },
	     "request_headers": { "foo": "bar", "content-type": "application/json"},
	     "engine": "kriti"
	   } 

Additionally, the content_type transform has been removed. All field transformations are now independent and declarative. What the client submits, is what the client gets.

Important Properties of this design

The lack of implicit back-end behavior is a very desirable property to maintain. It simplifies the technical implementation, removes edge cases (and the need to test them), and it leads to overall less surprising behavior for users. It also allows users to generate atypical requests so long as they conform to the HTTP Spec.

From the console user's perspective we can recover all the convenience of implicit back-end behaviors by moving those behaviors into explicit front-end behaviors. For example, if we want to change the Content-Type automatically based on a setting in the body action, then we visibly change the Content-Type in the request_headers section of the web form in response to the user modifying the body section of the form.

Proposal for x-www-form-urlencoded bodies

Based on @tirumaraiselvan's suggestion in our slack discussion, I propose we add a new action type to body transforms. This transform would be called makeFormUrlEncoded (or some better keyword) and it would add a formTemplate field to the body envelope:

	   "request_transform": {
	     "request_method": "GET",
	     "body": {
	       "action": "makeFormUrlEncoded",
                "formTemplate": {
                  "param1": "{{$body.input.value1}}",
                  "{{ $body.input.value3 }}": "{{$body.input.value2}}"
                } 
	     },
	     "request_headers": { "foo": "bar", "content-type": "application/x-www-form-urlencoded"},
	     "engine": "kriti"
	   } 

The formTemplate field will take an object made up of key value pairs which will be interpreted as Kriti String Templates and then used to construct a syntactically valid x-www-form-urlencoded body. The front-end (or an end user calling the metadata API directly) will need to set the Content-Type explicitly in the request_headers transform field, otherwise it will be absent.

This design will not break the existing schema agreed upon for PR #3475 and it will maintain all the desirable properties outlined in the previous heading.

@tirumaraiselvan
Copy link
Contributor Author

tirumaraiselvan commented Feb 10, 2022

@solomon-b I like the above proposal with respect to handling x-www-form-urlencoded body.

The only thing I am not sure about is using request_headers instead of add_headers and remove_headers. We already accept request headers from the default configuration (i.e. without transforms). The user need not specify ALL static headers again in request_headers. Moreover, the user might prefer to have certain default headers to be sent as well (e.g. User-Agent). Header transforms should be used only if some dynamism/transformation is involved with headers per request. For this, isn't add_headers and remove_headers better?

@sordina
Copy link
Contributor

sordina commented Mar 15, 2022

API support is now in server since the latest beta. Console work is in-progress.

@johndevs
Copy link

johndevs commented May 10, 2022

@sordina I am using v2.6.1 and CLI, and the content_type transform does not seem to be applied?

$ hasura metadata apply
INFO Metadata applied                             
$ hasura metadata diff
INFO Showing diff between project and server...   
actions.yaml

actions.my_action.definition.request_transform
  + one map entry added:
    content_type: application/x-www-form-urlencoded

@abhi40308
Copy link
Member

hey @johndevs with the new api update, you could change the content-type of request body within the body field itself, we have it here in existing documentation, and we're in process of updating the docs to make it clear with examples.
You can for example give the body field as:

"action": "x_www_form_urlencoded", 
"form_template": {
     "foo": "{{ $body.hello }}",
     "bar": "baz"
}

@rikinsk
Copy link
Member

rikinsk commented Jun 8, 2022

closed via b09bb60

@rikinsk rikinsk closed this as completed Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
k/enhancement New feature or improve an existing feature
Projects
None yet
Development

No branches or pull requests

7 participants