Provide clarifications on content type and serialization #83

sjparsons · 2020-04-24T20:19:26Z

This PR introduces some new rules in addition to addressing clarifications on the behavior of servers and clients with response to content types.

Most notably:

Specifies that servers MUST support JSON serialization and the associated application/graphl+json content type.
Describes how servers and clients should interact with respect to content types.
Separates examples in both GET and POST requests to their own section heading

Add clarifications about how servers and clients should handle serialization format and content types.

README.md

sjparsons · 2020-04-28T21:09:05Z

README.md

+
+If another content type is preferable to a client, it MAY include an `Accept` HTTP header listing other acceptable content types in order of preference. In this case a client SHOULD include `application/graphql+json` in the list, according to their preferred priority.
+
+If no `Accept` header is given, the server MUST respond with a content type of `application/graphql+json`.


Perhaps weaken this statement and add non-normative note suggesting that this default be application/graphql+json to support legacy clients, to allow more wider implementations.

ghmcadams · 2020-05-01T04:40:06Z

README.md

-Additionally, GraphQL MAY be used in combination with other HTTP request methods.
+A server MUST accept either GET requests or POST requests.
+
+A server SHOULD handle both GET and POST requests.


There is ambiguity here in that one line states that a server MUST accept either GET or POST requests and then that it should handle both.

It also seemed strange to me.

What is the difference between accept and handle?

ghmcadams · 2020-05-01T04:42:07Z

README.md

 ## GET

-For HTTP GET requests, the query parameters should be provided in the query component of the request URL in the form of
+For HTTP GET requests, the query parameters MUST be provided in the query component of the request URL in the form of


I totally agree that this should be changed to MUST. I cannot imagine what could be done with a GET request that contained no operation.

ghmcadams · 2020-05-01T04:46:15Z

README.md

+
+Servers MUST return an _Error response_ if a GET request includes a `query` and `operationName` (if present) describing an operation other than a query, such as a mutation or subscription.
+
+Clients MUST NOT send GET requests for non-query operations.


Are these lines similar in meaning?

GET requests must ONLY be used for executing queries and MUST NOT be used for mutations or subscriptions.

and

Clients MUST NOT send GET requests for non-query operations.

If not, should we provide an example of a non-query operation?

ghmcadams · 2020-05-01T05:37:58Z

README.md

-the value of `Content-Type` header.
+A GraphQL POST request instructs the server to perform a query, mutation or subscription operation. A GraphQL POST request should have a body which contains values of the request parameters encoded according to the value of `Content-Type` header of the request.
+
+When sending a POST request to a GraphQL server a client MUST include a body.


This line seems like it might be contradictory to the one above it. The first one says A GraphQL POST request should have a body..., but this one says MUST. Perhaps they could be combined?

ralfhandl · 2020-05-01T05:45:25Z

README.md

+For HTTP GET requests, the query parameters MUST be provided in the query component of the request URL in the form of
 `key=value` pairs with `&` symbol as a separator and both the key and value should have their "reserved" characters percent-encoded as specified in [section 2 of RFC3986](https://tools.ietf.org/html/rfc3986#section-2).
-The unencoded value of the `variables` parameter should be represented as a JSON-encoded string.
+The unencoded value of the `variables` parameter MUST be represented as a URL-encoded JSON string.


Suggested change

The unencoded value of the `variables` parameter MUST be represented as a URL-encoded JSON string.

The value of the `variables` parameter MUST be represented as a URL-encoded JSON object, with a name-value pair for each variable and its value.

This is what the example shows: the unencoded/decoded value is a JSON object, the actual/encoded value is a URL-encoded JSON object. No double-encoding.

nit pick (not blocking): JSON is a string (the N is for notation).

I'm still confused by The unencoded value ... represented as a URL-encoded ....

Does this mean that I

first have to translate the variables object into JSON,

then URL-encode that JSON to get an un-encoded parameter value,

and then again percent-encode the un-encoded parameter value before sending it in the HTTP request?

I hope percent/URL-encoding happens only once.

README.md

sjparsons · 2020-06-30T21:55:32Z

I intend to circle back to this and merge this PR before the July meeting.

andimarek · 2020-08-14T22:47:38Z

README.md


-The GraphQL HTTP server SHOULD handle the HTTP GET and POST methods.
-Additionally, GraphQL MAY be used in combination with other HTTP request methods.
+A server MUST accept either GET requests or POST requests.


I have never encountered a server which doesn't support POST.
I think it should be MUST accept POST

ghmcadams · 2021-02-02T18:46:08Z

There are some unaddressed comments on this PR and some significant time has passed. This PR will be merged soon. Please comment as desired.

PowerKiKi · 2021-02-03T00:23:44Z

README.md

+A server MUST accept either GET requests or POST requests.
+
+A server SHOULD handle both GET and POST requests.


Suggested change

A server MUST accept either GET requests or POST requests.

A server SHOULD handle both GET and POST requests.

A server MUST handle POST requests, and MAY handle other HTTP methods, such as GET.

README.md

Co-authored-by: Adrien Crivelli <adrien.crivelli@gmail.com> Co-authored-by: mmatsa <mmatsa@users.noreply.github.com>

linux-foundation-easycla · 2021-02-17T17:09:39Z

✅ Gabriel McAdams (28f82d4, 312a0de, 0c9f922)

❌ The commit (635a689 ,8ab3573419fbb3d636e1fb7c3e165fd8b7b246a3 ,e143f3c2acca5810ace555a75914b0ee55c1f0bd) is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

README.md

Co-authored-by: Adrien Crivelli <adrien.crivelli@gmail.com> Co-authored-by: Ivan Maximov <sungam3r@yandex.ru>

ghmcadams · 2021-02-17T17:21:51Z

@sjparsons Could you please resolve these conflicts and address the remaining comments?

ghmcadams · 2021-05-30T15:54:49Z

@sjparsons Could you review this PR to determine if it is still needed? If so, could you modify it to edit the spec file rather than the readme file?

ghmcadams · 2021-05-31T20:55:51Z

Attempted to revive with #161

Serialization format and content type

635a689

Add clarifications about how servers and clients should handle serialization format and content types.

sjparsons mentioned this pull request Apr 24, 2020

Actions from Mar 24 meeting #76

Closed

7 tasks

Make language around header more precise

8ab3573

sjparsons mentioned this pull request Apr 24, 2020

Tests to build #75

Merged

update the intro content-type section

e143f3c

spawnia reviewed Apr 25, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

sungam3r reviewed Apr 25, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

sungam3r reviewed Apr 25, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

mmatsa reviewed Apr 28, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

sjparsons commented Apr 28, 2020

View reviewed changes

ghmcadams reviewed May 1, 2020

View reviewed changes

ralfhandl reviewed May 1, 2020

View reviewed changes

spawnia reviewed May 1, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

sjparsons added this to the July 2020 milestone Jun 30, 2020

spawnia mentioned this pull request Jul 30, 2020

Implement PSR-7 RequestInterface support webonyx/graphql-php#598

Merged

sjparsons modified the milestones: July 2020, August 2020 Aug 14, 2020

andimarek reviewed Aug 14, 2020

View reviewed changes

sjparsons modified the milestones: August 2020, September 2020 Aug 25, 2020

sjparsons mentioned this pull request Aug 25, 2020

How to identify GraphQL requests #120

Closed

Base automatically changed from master to main February 2, 2021 20:06

PowerKiKi reviewed Feb 3, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

PowerKiKi reviewed Feb 3, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

PowerKiKi reviewed Feb 3, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

Apply suggestions from code review

28f82d4

Co-authored-by: Adrien Crivelli <adrien.crivelli@gmail.com> Co-authored-by: mmatsa <mmatsa@users.noreply.github.com>

ghmcadams reviewed Feb 17, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

ghmcadams and others added 2 commits February 17, 2021 09:11

Update README.md

312a0de

Apply suggestions from code review

0c9f922

Co-authored-by: Adrien Crivelli <adrien.crivelli@gmail.com> Co-authored-by: Ivan Maximov <sungam3r@yandex.ru>

ghmcadams added a commit to ghmcadams/graphql-over-http that referenced this pull request May 31, 2021

An attempt to revive graphql#83 while modifying as little as possible

db0dfd1

ghmcadams mentioned this pull request May 31, 2021

Provide clarifications on content type and serialization #161

Merged

ghmcadams closed this May 31, 2021

ghmcadams added a commit to ghmcadams/graphql-over-http that referenced this pull request Jun 1, 2021

An attempt to revive graphql#83 while modifying as little as possible

b7d02ec


		If another content type is preferable to a client, it MAY include an `Accept` HTTP header listing other acceptable content types in order of preference. In this case a client SHOULD include `application/graphql+json` in the list, according to their preferred priority.

		If no `Accept` header is given, the server MUST respond with a content type of `application/graphql+json`.


		Servers MUST return an _Error response_ if a GET request includes a `query` and `operationName` (if present) describing an operation other than a query, such as a mutation or subscription.

		Clients MUST NOT send GET requests for non-query operations.

	The unencoded value of the `variables` parameter MUST be represented as a URL-encoded JSON string.
	The value of the `variables` parameter MUST be represented as a URL-encoded JSON object, with a name-value pair for each variable and its value.

		A server MUST accept either GET requests or POST requests.

		A server SHOULD handle both GET and POST requests.

Provide clarifications on content type and serialization #83

Provide clarifications on content type and serialization #83

Uh oh!

Conversation

sjparsons commented Apr 24, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sjparsons commented Jun 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghmcadams commented Feb 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

linux-foundation-easycla bot commented Feb 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ghmcadams commented Feb 17, 2021

Uh oh!

ghmcadams commented May 30, 2021

Uh oh!

ghmcadams commented May 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

linux-foundation-easycla bot commented Feb 17, 2021 •

edited

Loading