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

REST PATCH json protocol #145

Closed
Dragomir-Ivanov opened this issue Oct 2, 2017 · 7 comments
Closed

REST PATCH json protocol #145

Dragomir-Ivanov opened this issue Oct 2, 2017 · 7 comments

Comments

@Dragomir-Ivanov
Copy link
Contributor

Currently rest-layer implements HTTP PATCH method in a very crude way, i.e. it only replaces top level fields. How about adding something more advanced like: http://jsonpatch.com/
https://tools.ietf.org/html/rfc6902

It can deeply patch JSON documents, and is supported by many languages, Go included.

@rs rs added the enhancement label Oct 2, 2017
@Dragomir-Ivanov
Copy link
Contributor Author

Another resource on the topic:
http://williamdurand.fr/2014/02/14/please-do-not-patch-like-an-idiot/

@rs
Copy link
Owner

rs commented Oct 3, 2017

I didn't know about this. Sounds like we could include support for that in the rest package without breaking compat as it would rely on content negotiation.

@smyrman thoughts?

@smyrman
Copy link
Collaborator

smyrman commented Oct 4, 2017

I think it's a good idea if someone want to implement it relying on content negotiation 👍

@Dragomir-Ivanov
Copy link
Contributor Author

Dragomir-Ivanov commented Nov 25, 2018

I want to start working on this, however we need to make a plan. I want to add this, because patching large documents is not convenient if we pass the document left and right constantly. Using JSON-Patch is much more convenient. So my plan is:

  1. Modify func itemPatch so it will work the old way, unless it receives HTTP Header of Content-Type: application/json-patch+json.
  2. Do the ETag logic.
  3. Apply the patch from the HTTP body to the original document, and use this as payload.
  4. Use the standard rest-layer logic for patching.
  5. Return HTTP response code of 204 (No Content) if the patching was successful and ETag was present in the PATCH request. This way we will not need to return the full document, because the client has the old one + the patch, and can reconstruct updated one. In this case, we also set Content-Location as a response HTTP header.
  6. If ETag was not present in the PATCH request we return the full document with HTTP response code of 200, as of now.

Guys apart from supplying the right Content-Type: what kind of content negotiation you have in mind?
Also I am planning to use this Go package: https://github.com/evanphx/json-patch

@smyrman
Copy link
Collaborator

smyrman commented Nov 26, 2018

For point 5, I think it would be better to make it explicit. Yes, you could argue it's possible to derive the new object on the client when supplying the E-tag for concurrency control, but it may still be convenient to get a response.

Maybe a header X-No-Body. I think that can be a separate PR and apply to all of POST, PUT and PATCH.

@Dragomir-Ivanov
Copy link
Contributor Author

Dragomir-Ivanov commented Nov 26, 2018

Actually there is a standard way of doing these ( I think ).
https://greenbytes.de/tech/webdav/draft-snell-http-prefer-02.html#rfc.section.4
https://greenbytes.de/tech/webdav/rfc7240.html#return

Basically the client sets HTTP header of Prefer:, with some specified value.

EDIT: Okay it is PROPOSED and not yet standardized:
https://tools.ietf.org/html/rfc7240#section-4.2

EDIT2: It seems that M$ has another view how this should work:
https://msdn.microsoft.com/en-us/library/hh537533.aspx

@smyrman
Copy link
Collaborator

smyrman commented Nov 26, 2018

Guys apart from supplying the right Content-Type what kind of content negotiation you have in mind?

Negotiation is probably not the right word; checking the Content-Type is what I meant.

Dragomir-Ivanov added a commit to Dragomir-Ivanov/rest-layer that referenced this issue Nov 28, 2018
Dragomir-Ivanov added a commit to Dragomir-Ivanov/rest-layer that referenced this issue Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants