search

api-platform / core

The server component of API Platform: hypermedia and GraphQL APIs in minutes
https://api-platform.com
MIT License
2.4k stars 855 forks source link

JSON PATCH support #759

Open dkarlovi opened 7 years ago

dkarlovi commented 7 years ago

147 suggested adding a PATCH support for partials updates, it was closed as PUT already does that.

How about adding a JSON PATCH support? Difference here is you're able to do operations WITH entities, not only ON entites, quoted from Why PATCH is Good for Your HTTP API

PATCH is atomic, which means that if you need to do some complex changes to the state of a resource, you can do it with confidence. It also means that you can create synthetic resources and PATCH them if you need to orchestrate changes to the state of several resources.

dunglas commented 7 years ago

It would definitely be a great improvement to have support for JSON PATCH.

dunglas commented 7 years ago

On the other end, JSON Merge Patch will be easier to implement, and easier to use client-side.

teohhanhui commented 7 years ago

http://erosb.github.io/post/json-patch-vs-merge-patch/

asimonf commented 7 years ago

@dunglas given that the content-type of both methods is different, couldn't both be implemented? I understand that Merge Patch has limitations, but for many cases, it's simplicity could be great for certain applications.

What I mean is, I don't see why we should have to pick one or the other. They are just different formats and both could be supported. If that's ok with you, I might have a go at trying to implement merge patch, time permitting.

dunglas commented 7 years ago

Yew we can support both. It should be possible to support JSON MERGE PATCH with just a few modifications to the current system.

teohhanhui commented 7 years ago

Implementing JSON Patch will force us to improve our design. And it will allow us to deprecate the current partial PUT semantics (which is wrong). :smile:

asimonf commented 7 years ago

@teohhanhui I don't see why both (JSON MERGE and JSON PATCH) can't be implemented. I don't disagree with you and I'm sure if a PR appears for JSON PATCH, @dunglas would be willing to merge assuming it passes review. In my use-case, the simplicity of JSON MERGE trumps the flexibility of JSON PATCH.

teohhanhui commented 7 years ago

I'm okay with having both. :)

On 27 Feb 2017 22:19, "Alberto Simon" notifications@github.com wrote:

@teohhanhui https://github.com/teohhanhui I don't see why both (JSON MERGE and JSON PATCH) can't be implemented. I don't disagree with you and I'm sure if a PR appears for JSON PATCH, @dunglas https://github.com/dunglas would be willing to merge assuming it passes review. In my use-case, the simplicity of JSON MERGE trumps the flexibility of JSON PATCH.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/api-platform/core/issues/759#issuecomment-282732998, or mute the thread https://github.com/notifications/unsubscribe-auth/AAhf615IQF9pQkU8tZdIzn-8SG20cehuks5rgttugaJpZM4KBoyo .

teohhanhui commented 7 years ago

Also, I think we can deprecate the partial PUT once we have JSON Merge Patch. Because it already covers the existing semantics.

On 28 Feb 2017 11:53, "Teoh Han Hui" teohhanhui@gmail.com wrote:

I'm okay with having both. :)

On 27 Feb 2017 22:19, "Alberto Simon" notifications@github.com wrote:

@teohhanhui https://github.com/teohhanhui I don't see why both (JSON MERGE and JSON PATCH) can't be implemented. I don't disagree with you and I'm sure if a PR appears for JSON PATCH, @dunglas https://github.com/dunglas would be willing to merge assuming it passes review. In my use-case, the simplicity of JSON MERGE trumps the flexibility of JSON PATCH.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/api-platform/core/issues/759#issuecomment-282732998, or mute the thread https://github.com/notifications/unsubscribe-auth/AAhf615IQF9pQkU8tZdIzn-8SG20cehuks5rgttugaJpZM4KBoyo .

StephenOTT commented 7 years ago

anyone actively working on this?

dunglas commented 7 years ago

AFAIK, no. But we should probably consider using JSON Merge Patch instead: https://tools.ietf.org/html/rfc7386

StephenOTT commented 7 years ago

@dunglas can this issue be closed in favour of https://github.com/api-platform/core/pull/1175 ?

teohhanhui commented 7 years ago

JSON API is only one media type. This is about PATCH support for all of the supported media types, and I'd think especially JSON-LD / Hydra.

dkarlovi commented 5 years ago

@dunglas IMO this shouldn't be closed in favor of JSON Merge since Merge is only a partial PUT.

JSON PATCH is a diff to a JSON document so, if a document represents a collection of your resources, you could:

  1. append a new resource into a collection
  2. remove a resource from a collection
  3. re-order (some) collection items without touching (or mentioning) the rest

It solves the partial collection management problem well and IMO it should be kept as a future option.

bendavies commented 5 years ago

this seems a step too far at the moment. for the moment, why can we not just make PUT mean PUT semantically, rather than PATCH? is is difficult?

EDIT: from slack

it seems not that hard? we know what attributes are writable for the PUT from serializer, so we can just

  1. treat missing as null. fill missing as null if not provided, or
  2. treat missing as error. throw error as not all required attributes were provided.
teohhanhui commented 5 years ago

@bendavies Yes, but we need to make it opt-in for BC. And when we have proper JSON Patch support, we can deprecate not using the correct semantics.

alanpoulain commented 4 years ago

JSON Merge Patch support has landed in 2.5.

teohhanhui commented 4 years ago

Let's keep this open as we only support JSON Merge Patch, not JSON Patch.

Muspi commented 3 years ago

Hi, how do you manage partial Validation constraints with PATCH actions?

I'm facing some problems to achieve this, but I do not have solution.

I opened an issue here: https://github.com/api-platform/api-platform/issues/1637

DurandA commented 2 years ago

I am attempting to implement a subset of JSON Patch/RFC 6902 in order to partially update collections. See discussion https://github.com/api-platform/core/discussions/4882.