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

add deferred request to models or collections for race condutions #1567

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions backbone.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,12 @@
// began.
_pending: null,

// Deferred request is a request that is waiting for
// response of currently running model request.
// Initial value is false which means
// there is not an currently running request.
_deferredRequest: false,

// The default name for the JSON `id` attribute is `"id"`. MongoDB and
// CouchDB users may want to set this to `"_id"`.
idAttribute: 'id',
Expand Down Expand Up @@ -571,6 +577,12 @@
// This should be overridden in most cases.
model: Model,

// Deferred request is a request that is waiting for
// response of currently running collection request.
// Initial value is false which means
// there is not an currently running request.
_deferredRequest: false,

// Initialize is an empty function by default. Override it with your own
// initialization logic.
initialize: function(){},
Expand Down Expand Up @@ -1387,6 +1399,33 @@
params.processData = false;
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pseudo-code for incoming commits

    // detach update and create. create is like below
    if (!options.data && model && (method === 'create')) {
        // not sending pseudo-id for decoupling from server
        params.data = JSON.stringify(model.toJSON());
        // for calling Backbone.sync with delete method until real id is coming
        model.set({id:model.cid}, {silent:true});
    }

var success = options.success;
options.success = function(){
var dr = model._deferredRequest;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pseudo-code for incoming commits

    if (!_.isEmpty(dr) && arguments[0].id && model.id === model.cid) {
        dr[1].set({id:arguments[0].id}, {silent:true})
    }

// running request responsed,
// there is no need to defer another request now
model._deferredRequest = false;

// if there is a request that deferred until response for this model
if (!_.isEmpty(dr)) {
model.sync(dr[0], dr[1], dr[2]);
}
if (success) success.apply(null, arguments);
};

// if there is a running request
if (_.isArray(model._deferredRequest)) {
// defer this request until response of running request.
// if there is another deferred request,
//override previous deferred request
model._deferredRequest = [method, model, options];
// FIXME: return sth look like xhr
return true;
}

// last requests until response will defer
// but now model._deferredRequest is empty array
model._deferredRequest = [];
// Make the request, allowing the user to override any Ajax options.
return Backbone.ajax(_.extend(params, options));
};
Expand All @@ -1400,6 +1439,7 @@
Backbone.wrapError = function(onError, originalModel, options) {
return function(model, resp) {
resp = model === originalModel ? resp : model;
resp._deferredRequest = false;
if (onError) {
onError(originalModel, resp, options);
} else {
Expand Down
23 changes: 23 additions & 0 deletions test/sync.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ $(document).ready(function() {
library = new Library();
Backbone.ajax = function(obj) {
lastRequest = obj;
obj.success(obj.data);
};
library.create(attrs, {wait: false});
},
Expand Down Expand Up @@ -142,6 +143,28 @@ $(document).ready(function() {
equal(lastRequest.url, '/one/two');
});

test("sync: Defer requests until response", 5, function() {
Backbone.ajax = function(obj){
lastRequest = obj;
// not calling obj.success causes running first request forever
};
var model = new Backbone.Model({id:'1'});
model.url = '/test';
// first long running request that is not deferred
model.save({deferred:false});
// first deferred request
model.save({deferred:true, deferralId:1});
strictEqual(model._deferredRequest[1].get('deferred'), true);
equal(model._deferredRequest[1].get('deferralId'), 1);
// second request that override first deferred request
model.save({deferred:true, deferralId:2});
equal(model._deferredRequest[1].get('deferralId'), 2);
// last deferred request that override previous deferred requests
model.destroy();
equal(model._deferredRequest[0], 'delete');
equal(model._deferredRequest[1].get('deferralId'), 2);
});

test("#1052 - `options` is optional.", 0, function() {
var model = new Backbone.Model();
model.url = '/test';
Expand Down