jashkenas / backbone

Give your JS App some Backbone with Models, Views, Collections, and Events
http://backbonejs.org
MIT License
28.1k stars 5.39k forks source link

Save model before previous save issues POST(create) instead of PUT(update) request #345

Closed mattheworiordan closed 11 years ago

mattheworiordan commented 13 years ago

I've developed a nice rich application interface where users can add objects very quickly, and then start updating properties of those objects by simply tabbing to the relevant fields. The problem I am having is that sometimes the user beats the server to it's initial save and we end up saving two objects.

An example of how to recreate this problem is as follows:

  1. User clicks the Add person button, we add this to the DOM but don't save anything yet as we don't have any data yet. code sample --> person = new Person();
  2. User enters a value into the Name field, and tabs out of it (name field loses focus). This triggers a save so that we update the model on the server. As the model is new, Backbone.js will automatically issue a POST (create) request to the server. code sample --> person.set ({ name: 'John' }); code sample --> person.save(); // create new model
  3. User then very quickly types into the age field they have tabbed into, enters 20 and tabs to the next field (age therefore loses focus). This again triggers a save so that we update the model on the server. code sample --> person.set ({ age: 20 }); code sample --> person.save(); // update the model

So we would expect in this scenario one POST request to create the model, and one PUT requests to update the model.

However, if the first request is still being processed and we have not had a response before the code in point 3 above has run, then what we actually get is two POST requests and thus two objects created instead of one.

So my question is whether there is some best practice way of dealing with this problem and Backbone.js? Or, should Backbone.js have a queuing system for save actions so that one request is not sent until the previous request on that object has succeeded/failed? Or, alternatively should I build something to handle this gracefully by either sending only one create request instead of multiple update requests, perhaps use throttling of some sort, or check if the Backbone model is in the middle of a request and wait until that request is completed.

Your advice on how to deal with this issue would be appreciated.

And I'm happy to take a stab at implementing some sort of queuing system, although you may need to put up with my code which just won't be as well formed as the existing code base!

ghost commented 13 years ago

Presumably you get the ID from the server. How you using clientside UUID/Guids that way you can set the id immediately after save, and in future isNew() will evaluate to false

mattheworiordan commented 13 years ago

Hi Matt

Nice idea, but that would unfortunately require a change to the default database set up for Rails apps, so I'd prefer to work around that issue rather. See http://stackoverflow.com/questions/5886748/backbone-js-problem-when-saving-a-model-before-previous-save-issues-postcreate/5903652#comment-6802441 for a potential solution which I have not implemented yet, but will update this ticket & Stackoverflow if it works.

Matt

jashkenas commented 13 years ago

I don't think that core Backbone should default to queueing ... but this shouldn't be too hard to override for your app. All you have to do is override Backbone.sync to ensure that the prior XHR has finished, before launching the next one.

If you make a nice plugin out of it, it would be great material for the Backbone wiki.

mattheworiordan commented 13 years ago

I have written a patch which queues save requests, however it's not a plugin at this stage. If anyone likes the patch solution, then please drop me a line and I will see if I can tidy it up into a patch.

The patch is at https://gist.github.com/1037984

philfreo commented 12 years ago

I'm all about keeping Backbone simple, but Backbone's default behavior of calling POST multiple times on save()s while waiting on the first one here does seem pretty bad.

amccloud commented 12 years ago

@philfreo I've created a plugin that should prevent this behavior https://github.com/amccloud/backbone-safesync

zawaideh commented 11 years ago

I think a simple solution like the one suggested in http://stackoverflow.com/a/6122530/159376 would be ideal.

IMHO, this should be the default backbone behaviour.

braddunbar commented 11 years ago

@amccloud's plugin that abort's pending requests, while perfectly valid, is very different behavior from the deferred requests in the, also valid, solution recommended by @zawaideh. I tend to lean toward the abort behavior, but I'm not certain that either is a solid default that will work for 90% of cases (and more preferably 100%).

philfreo commented 11 years ago

@zawaideh I agree it's a good solution, except that it needs to still return the jqXHR in save()

lgsunnyvale commented 11 years ago

I'm going to try to solve this problem (and submit a pull request with tests). The central issue is we need to return a jqXHR-like object for a request that hasn't been made yet. The best approximate to return is a promise that includes an "abort" method.

The easiest thing to do would be to create a Backbone.$.Deferred, patch in an "abort" method and return that.

Unfortunately, Zepto doesn't support deferreds quite yet, although it looks like it is coming: https://github.com/madrobby/zepto/issues/353.

How best to proceed? Can I use Backbone.$.Deferred? Or should I try to create a small version of a promise within Backbone?

Please let me know and I'll get a pull request out asap. Thanks!

jashkenas commented 11 years ago

See my response here: https://github.com/documentcloud/backbone/pull/1325#issuecomment-11146707

... I'd be glad to look at a pull request that attempts to address those different use cases in a cohesive way ... but at the moment I think this is something that your own code is best equipped to handle.

richardscarrott commented 9 years ago

I've found it a little difficult implementing an ajax queue in Backbone because the 'method' is generated in save, i.e.

// from Backbone.Model.prototype.save
method = this.isNew() ? 'create' : (options.patch ? 'patch' : 'update');
if (method === 'patch') options.attrs = attrs;
xhr = this.sync(method, this, options);

So overriding Backbone.sync to queue requests would still issue two POSTS, e.g.

var model = new Backbone.Model();
model.url = '/products'
model.save({ foo: 'bar' }); // issues POST /products
model.save({ foo: 'baz' }); // issues POST to /products/{id} (NOTE: the url is correct because it's evaluated in Backbone.sync).

Expected:

var model = new Backbone.Model();
model.url = '/products'
model.save({ foo: 'bar' }); // issues POST /products
model.save({ foo: 'baz' }); // issues PUT to /products/{id}

Has anybody found a solution to this problem?

philfreo commented 9 years ago

@richardscarrott I use this https://gist.github.com/philfreo/fc068094f6e26d6995e2

richardscarrott commented 9 years ago

@philfreo - your gist would mean that the model is not set until any previous requests have complete... it makes sense to me that the queue would be handled by sync so the models & collections and therefore views can get on with their job leaving sync to worry about persisting the data in the background.

It can almost so easily be done if it wasn't for the fact the sync method (create, update etc.) is evaluated immediately.

richardscarrott commented 9 years ago

If anybody is interested I've managed to solve the problem by optimisitcally setting the models id in Backbone.sync and parsing it out with the server generated id beforeSend which also meant the queue could be handled at the jQuery level giving it more utility - https://github.com/richardscarrott/backbone-ajax-queue