Transport for application/graphql contentType #2592
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit adds 'application/graphql' transport. It is based on POST metod and has only the 'query' part in it's body. See: https://graphql.org/learn/serving-over-http/#post-request and it's comment about this content-type. An example of correct application/graphql query is: ``` curl 'http://host/graphql' -d '{time{now}}' -H "Content-Type: application/graphql" ``` Some clients prefix body with 'query=': ``` -d 'query={time{now}}' ``` Some clients html encode body payload: ``` -d 'query=%7Btime%7Bnow%7D%7D' ``` We cleanup both in cleanupBody() method. Tests are in http_graphql_test.go file.
GRAPHQL transport (like GET, POST and MULTIPART transports) can have specific response headers added. This commit adds tests for it and changes doRequest() method so that we can set inbound Content-Type. Graphql transport uses 'application/graphql' content-type and not 'application/json'.
RatkoR
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull requests has three commits:
8cac3b5 adds new GRAPHQL transport
a270f35 adds header tests for this new transport
829596f adds GRAPHQL transport to the documentation
It is split into three commits to be easier to look at changes.
GRAPHQL transport is described here: https://graphql.org/learn/serving-over-http/#post-request nothing about it is on https://graphql.github.io/graphql-over-http/draft/.
I created a new transport (http_graphql.go) to minimize impact on existing gqlgen implementations. I could have added it to POST transport, but that would be riskier and an error there could make all gql servers inoperable. With it's own transport, developers can eneble or disable it, while keeping POST transport working as before.
From observing our graphql clients that use application/graphql content-type, some add
query=at body start, some html escape body contents and some don't. I don't know what is correct, but in order for us to migrate to gqlgen, we have to handle all those clients successfully. That's why cleanupBody() does some body transformations for this transport.I did not add GRAPHQL transport to the default server to keep existing gqlgen instances as are. Those interested in application/graphql can add it themselves as I noted in migration-0.11.md.
Closes #1129
Comments are welcome.
I have: