Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add option to use HTTP PATCH method for updates #796

Closed
wants to merge 1 commit into from

6 participants

@jasonkriss

I'm not sure if this is the best way to accomplish this, but I think it'd be nice to allow this option. Especially considering that PATCH will be the primary update method for Rails 4.

@bobbus

I think the point of PATCH is to only send the updated data, then I would think a proper implementation would only send modified attributes in the request. Sadly, the dirtiness behavior cannot let us do that for now.

@tchak

Sadly, the dirtiness behavior cannot let us do that for now.

@bobbus I think this is not true, we do have information about changed attributes.

@bobbus

@tchak , for now, I believe we only store the dirty state of a record, not the list of modified attributes.
You may be right as we can compare the last data received from the API and the record json serialization and figure out what did change, is it what you think about ?
I do not see another way to do it for now, if there is one it could help to fix #646

@tchak

@bobbus we do keep detailed infos for attributes changes, but there is no public api yet to get to it

https://github.com/emberjs/data/blob/master/packages/ember-data/lib/system/model/states.js#L183-L194

@bobbus

@tchak , thanks I thought this was lost after the sync() , will look further into this soon.

@jasonkriss

Good points. I think it'd be great to be able to only send the dirty attributes back to the server. That being said, I think PATCH is still valid with the current implementation. PUT is meant for complete replacement of a record and that's not what is happening with an update.

@igorT
Owner

We would love to have PATCH that would only send the changed attributes. I think the current internals support that but they will be refactored soon so I would hold off on that. I don't think that having a PATCH that behaves like a PUT would be that useful/would be confusing. If you need it for your use case, overwriting the RESTAdapter should work.

@igorT igorT closed this
@jasonkriss jasonkriss deleted the branch
@briangonzalez

@igorT What's the latest on this?

@valberg

I have extended DS.RESTSerializer and DS.RESTAdapter like this (props to http://discuss.emberjs.com/t/saving-models-with-only-changed-attributes-relationships/3568):

export default DS.RESTSerializer.extend({
  serialize: function (record, options) {
    var self = this;
    var json = {};
    var inFlightAttrs = Object.keys(record.get('_inFlightAttributes'));

    if(options && options.includeId) {
      if(record.get('id')) {
        json[this.get('primaryKey')] = record.get('id');
      }
    }

    record.eachAttribute(function (key, attribute) {
      if(inFlightAttrs.indexOf(key) != -1) {
        self.serializeAttribute(record, json, key, attribute);
      }
    });

    record.eachRelationship(function (key, relationship) {
      if(relationship.kind == 'belongsTo') {
        self.serializeBelongsTo(record, json, relationship);
      } else if(relationship.kind == 'hasMany') {
        self.serializeHasMany(record, json, relationship);
      }
    });

    return json;
  }
});
export default DS.RESTAdapter.extend({
  updateRecord: function(store, type, record) {
    var json = {};
    var serializer = store.serializerFor(type.typeKey);
    serializer.serializeIntoHash(json, type, record);
    var id = record.get('id');
    return this.ajax(this.buildURL(type.typeKey, id), "PATCH", { data: json });
  },
});

It works, and only sends those attributes that have been changed. I'm not sure if there are any pitfalls though.

It would definitely be awesome to support doing partial updates using PATCH. Maybe it could be an option one can give to .save()? Like record.save(partial=true) and then it will use PATCH instead of PUT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 11, 2013
  1. @jasonkriss
This page is out of date. Refresh to see the latest.
View
14 packages/ember-data/lib/adapters/rest_adapter.js
@@ -63,6 +63,7 @@ var get = Ember.get, set = Ember.set, merge = Ember.merge;
DS.RESTAdapter = DS.Adapter.extend({
bulkCommit: false,
since: 'since',
+ updateViaPATCH: false,
serializer: DS.RESTSerializer,
@@ -153,13 +154,15 @@ DS.RESTAdapter = DS.Adapter.extend({
},
updateRecord: function(store, type, record) {
- var id = get(record, 'id');
- var root = this.rootForType(type);
+ var id = get(record, 'id'),
+ root = this.rootForType(type),
+ requestMethod = get(this, 'updateViaPATCH') ? "PATCH" : "PUT";
+
var data = {};
data[root] = this.serialize(record);
- this.ajax(this.buildURL(root, id), "PUT", {
+ this.ajax(this.buildURL(root, id), requestMethod, {
data: data,
context: this,
success: function(json) {
@@ -179,7 +182,8 @@ DS.RESTAdapter = DS.Adapter.extend({
}
var root = this.rootForType(type),
- plural = this.pluralize(root);
+ plural = this.pluralize(root),
+ requestMethod = get(this, 'updateViaPATCH') ? "PATCH" : "PUT";
var data = {};
data[plural] = [];
@@ -187,7 +191,7 @@ DS.RESTAdapter = DS.Adapter.extend({
data[plural].push(this.serialize(record, { includeId: true }));
}, this);
- this.ajax(this.buildURL(root, "bulk"), "PUT", {
+ this.ajax(this.buildURL(root, "bulk"), requestMethod, {
data: data,
context: this,
success: function(json) {
View
67 packages/ember-data/tests/unit/rest_adapter_test.js
@@ -172,6 +172,33 @@ test("updating a person makes a PUT to /people/:id with the data hash", function
equal(get(person, 'name'), "Brohuda Brokatz", "the hash should be updated");
});
+test("updates can optionally be made via the PATCH request method", function() {
+ set(adapter, 'updateViaPATCH', true);
+
+ store.load(Person, { id: 1, name: "Yehuda Katz" });
+
+ person = store.find(Person, 1);
+
+ expectState('new', false);
+ expectState('loaded');
+ expectState('dirty', false);
+
+ set(person, 'name', "Brohuda Brokatz");
+
+ expectState('dirty');
+ store.commit();
+ expectState('saving');
+
+ expectUrl("/people/1", "the plural of the model name with its ID");
+ expectType("PATCH");
+
+ ajaxHash.success({ person: { id: 1, name: "Brohuda Brokatz" } });
+ expectState('saving', false);
+
+ equal(person, store.find(Person, 1), "the same person is retrieved by the same ID");
+ equal(get(person, 'name'), "Brohuda Brokatz", "the hash should be updated");
+});
+
test("updates are not required to return data", function() {
store.load(Person, { id: 1, name: "Yehuda Katz" });
@@ -738,6 +765,46 @@ test("updating several people (with bulkCommit) makes a PUT to /people/bulk with
equal(carl, store.find(Person, 2), "the same person is retrieved by the same ID");
});
+test("updating several people (with bulkCommit) can optionally use the PATCH request method", function() {
+ set(adapter, 'bulkCommit', true);
+ set(adapter, 'updateViaPATCH', true);
+
+ store.loadMany(Person, [
+ { id: 1, name: "Yehuda Katz" },
+ { id: 2, name: "Carl Lerche" }
+ ]);
+
+ var yehuda = store.find(Person, 1);
+ var carl = store.find(Person, 2);
+
+ people = [ yehuda, carl ];
+
+ expectStates('new', false);
+ expectStates('loaded');
+ expectStates('dirty', false);
+
+ set(yehuda, 'name', "Brohuda Brokatz");
+ set(carl, 'name', "Brocarl Brolerche");
+
+ expectStates('dirty');
+ store.commit();
+ expectStates('saving');
+
+ expectUrl("/people/bulk", "the collection at the plural of the model name");
+ expectType("PATCH");
+ expectData({ people: [{ id: 1, name: "Brohuda Brokatz" }, { id: 2, name: "Brocarl Brolerche" }] });
+
+ ajaxHash.success({ people: [
+ { id: 1, name: "Brohuda Brokatz" },
+ { id: 2, name: "Brocarl Brolerche" }
+ ]});
+
+ expectStates('saving', false);
+
+ equal(yehuda, store.find(Person, 1), "the same person is retrieved by the same ID");
+ equal(carl, store.find(Person, 2), "the same person is retrieved by the same ID");
+});
+
test("bulk updates can sideload data", function() {
set(adapter, 'bulkCommit', true);
Something went wrong with that request. Please try again.