kurko / ember-json-api

JSON API adapter for Ember Data.
MIT License
254 stars 62 forks source link

Use PATCH instead of PUT for updating records #3

Closed gr0uch closed 9 years ago

gr0uch commented 11 years ago

Currently, the adapter still follows the default REST adapter behaviour when updating records, which is to PUT the entire resource. A smarter, better approach would be to only serialize changes, and send the request as a PATCH.

The request might look something like this:

PATCH /animals/123
Content-Type: application/json-patch+json

[
  {"op": "replace", "path": "/animals/0/name", "value": "Tomster"},
  {"op": "replace", "path": "/animals/0/links/friends", "value": ["124"]}
]
wrenoud commented 10 years ago

I think you mentioned this over in #19 wrt the spec update, but it may be easier to use the new breaking changes that allow for only including changed attributes in PUT, per updating attributes.

gr0uch commented 10 years ago

The JSON API authors have mentioned that PUT is almost never what you want to do. I'm keeping this open, since PATCH support is very nice to have.

ColtonProvias commented 10 years ago

It should be mentioned that JSON API now lists PUT as an option for updating resources. PATCH is now further down on the page as an optional implementation for servers.

http://jsonapi.org/format/#crud

andrewbranch commented 9 years ago

Looks like PUT is no longer mentioned in the spec. What’s the status of this?

jonkoops commented 9 years ago

I've found some additional information about PUT requests in the FAQ:

Where's PUT? Can I use method X to do Y?

JSON API does not currently specify the use of the PUT method for any purpose.

Servers may complement the base specification by providing extra capabilities and alternative ways of requesting certain operations (e.g., resource creation via PUT in addition to POST).

andrewbranch commented 9 years ago

So shouldn’t this change be incorporated into #71? I can look into it if it’s not already in progress elsewhere.

kurko commented 9 years ago

Yes, I think it should @andrewbranch.

andrewbranch commented 9 years ago

@kurko Already done :)

jonkoops commented 9 years ago

Closing this issue since the discussed code has been merged into master.