diff --git a/.gitignore b/.gitignore index d1ac9ba..3249e0f 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ test/coverage/ node_modules/ bower_components/ test/coverage +.idea \ No newline at end of file diff --git a/README.md b/README.md index 64433cc..57d2bfa 100644 --- a/README.md +++ b/README.md @@ -159,9 +159,6 @@ Any models added to the collection will be synchronized to the provided Firebase using the Backbone binding will also receive `add`, `remove` and `changed` events on the collection as appropriate. -**BE AWARE!** If autoSync is set to true, you do not need to call any functions that will affect _remote_ data. If you call -`fetch()` on the collection, **the library will ignore it silently**. However, if autoSync is set to false, you can use `fetch()`. This is explained above in the autoSync section. - You should add and remove your models to the collection as you normally would, (via `add()` and `remove()`) and _remote_ data will be instantly updated. Subsequently, the same events will fire on all your other clients immediately. @@ -240,10 +237,9 @@ var todo = new Todo({ }); ``` -**BE AWARE!** You do not need to call any functions that will affect _remote_ data. If autoSync is enabled and you call -`save()` or `fetch()` on the model, **the library will ignore it silently**. +You do not need to call any functions that will affect _remote_ data when `autoSync` is enabled. Calling `fetch()` will simply fire the `sync` event. -If autoSync is enabled, you should modify your model as you normally would, (via `set()` and `destroy()`) and _remote_ data +If `autoSync` is enabled, you should modify your model as you normally would, (via `set()` and `destroy()`) and _remote_ data will be instantly updated. #### autoSync: true diff --git a/changelog.txt b/changelog.txt index e69de29..7b74bf6 100644 --- a/changelog.txt +++ b/changelog.txt @@ -0,0 +1,5 @@ +fixed - Improved asynchronous sync behavior. +feature - Added save and remove for autoSync models. +feature - Added fetch method for autoSync models. +fixed - Fixed issue with urlRoot property for models. +fixed - Fixed bug with comparator override. \ No newline at end of file diff --git a/src/backbonefire.js b/src/backbonefire.js index c26def7..4803699 100644 --- a/src/backbonefire.js +++ b/src/backbonefire.js @@ -30,8 +30,7 @@ */ Backbone.Firebase._determineAutoSync = function(model, options) { var proto = Object.getPrototypeOf(model); - return _.extend( - { + return _.extend({ autoSync: proto.hasOwnProperty('autoSync') ? proto.autoSync : true }, this, @@ -194,6 +193,52 @@ return !_.isObject(value) && value !== null; }; + /** + * A naive promise-like implementation. Requires a syncPromise object. + * + * syncPromise is an object that has three properties. + * - resolve (bool) - Has the data been retreived from the server? + * - success (bool) - Was the data retrieved successfully? + * - error (Error) - If there was an error, return the error object. + * + * This function relies on the syncPromise object being resolved from an + * outside source. When the "resolve" property has been set to true, + * the "success" and "error" functions will be evaluated. + */ + Backbone.Firebase._promiseEvent = function(params) { + // setup default values + var syncPromise = params.syncPromise; + var success = params.success; + var error = params.error; + var context = params.context || this; + var complete = params.complete; + + // set up an interval that checks to see if data has been synced from the server + var promiseInterval = setInterval(_.bind(function() { + // if the result has been retrieved from the server + if(syncPromise.resolve) { + + // on success fire off the event + if(syncPromise.success) { + success.call(context); + } + // on error fire off the returned error + else if(syncPromise.err) { + error.call(context, syncPromise.err); + } + + // fire off the provided completed event + if(complete) { + complete.call(context); + } + + // the "promise" has been resolved, clear the interval + clearInterval(promiseInterval); + } + }, context)); + + }; + /** * Model responsible for autoSynced objects * This model is never directly used. The Backbone.Firebase.Model will @@ -203,11 +248,19 @@ function SyncModel() { // Set up sync events - + this._initialSync = {}; // apply remote changes locally this.firebase.on('value', function(snap) { this._setLocal(snap); + this._initialSync.resolve = true; + this._initialSync.success = true; this.trigger('sync', this, null, null); + }, function(err) { + // indicate that the call has been received from the server + // and that an error has occurred + this._initialSync.resolve = true; + this._initialSync.err = err; + this.trigger('error', this, err, null); }, this); // apply local changes remotely @@ -218,18 +271,20 @@ } SyncModel.protoype = { - save: function() { - console.warn('Save called on a Firebase model with autoSync enabled, ignoring.'); - }, - fetch: function() { - console.warn('Save called on a Firebase model with autoSync enabled, ignoring.'); - }, - sync: function(method, model, options) { - if(method === 'delete') { - Backbone.Firebase.sync(method, model, options); - } else { - console.warn('Sync called on a Fireabse model with autoSync enabled, ignoring.'); - } + fetch: function(options) { + Backbone.Firebase._promiseEvent({ + syncPromise: this._initialSync, + context: this, + success: function() { + this.trigger('sync', this, null, options); + }, + error: function(err) { + this.trigger('err', this, err, options); + }, + complete: function() { + Backbone.Firebase._onCompleteCheck(this._initialSync.err, this, options); + } + }); } }; @@ -253,14 +308,6 @@ } - OnceModel.protoype = { - - sync: function(method, model, options) { - Backbone.Firebase.sync(method, model, options); - } - - }; - return OnceModel; }()); @@ -301,7 +348,10 @@ } }, - + + sync: function(method, model, options) { + Backbone.Firebase.sync(method, model, options); + }, /** * Siliently set the id of the model to the snapshot key @@ -390,7 +440,7 @@ create: function(model, options) { model.id = Backbone.Firebase._getKey(this.firebase.push()); options = _.extend({ autoSync: false }, options); - return Backbone.Collection.prototype.create.apply(this, [model, options]); + return Backbone.Collection.prototype.create.call(this, model, options); }, /** * Create an id from a Firebase push-id and call Backbone.add, which @@ -400,7 +450,7 @@ add: function(model, options) { model.id = Backbone.Firebase._getKey(this.firebase.push()); options = _.extend({ autoSync: false }, options); - return Backbone.Collection.prototype.add.apply(this, [model, options]); + return Backbone.Collection.prototype.add.call(this, model, options); }, /** * Proxy to Backbone.Firebase.sync @@ -442,7 +492,7 @@ var SyncCollection = (function() { function SyncCollection() { - + this._initialSync = {}; // Add handlers for remote events this.firebase.on('child_added', _.bind(this._childAdded, this)); this.firebase.on('child_moved', _.bind(this._childMoved, this)); @@ -450,8 +500,24 @@ this.firebase.on('child_removed', _.bind(this._childRemoved, this)); // Once handler to emit 'sync' event whenever data changes - this.firebase.on('value', _.bind(function() { - this.trigger('sync', this, null, null); + // Defer the listener incase the data is cached, because + // then the once call would be synchronous + _.defer(_.bind(function() { + + this.firebase.once('value', function() { + // indicate that the call has been received from the server + // and the data has successfully loaded + this._initialSync.resolve = true; + this._initialSync.success = true; + this.trigger('sync', this, null, null); + }, function(err) { + // indicate that the call has been received from the server + // and that an error has occurred + this._initialSync.resolve = true; + this._initialSync.err = err; + this.trigger('error', this, err, null); + }, this); + }, this)); // Handle changes in any local models. @@ -461,11 +527,8 @@ } SyncCollection.protoype = { - comparator: function(model) { - return model.id; - }, - add: function(models, options) { + // prepare models var parsed = this._parseModels(models); options = options ? _.clone(options) : {}; options.success = @@ -528,6 +591,27 @@ return ret; }, + // This function does not actually fetch data from the server. + // Rather, the "sync" event is fired when data has been loaded + // from the server. Since the _initialSync property will indicate + // whether the initial load has occurred, the "sync" event can + // be fired once _initialSync has been resolved. + fetch: function(options) { + Backbone.Firebase._promiseEvent({ + syncPromise: this._initialSync, + context: this, + success: function() { + this.trigger('sync', this, null, options); + }, + error: function(err) { + this.trigger('err', this, err, options); + }, + complete: function() { + Backbone.Firebase._onCompleteCheck(this._initialSync.err, this, options); + } + }); + }, + _log: function(msg) { if (console && console.log) { console.log(msg); @@ -549,8 +633,8 @@ } // call Backbone's prepareModel to apply options - model = Backbone.Collection.prototype._prepareModel.apply( - this, [model, options || {}] + model = Backbone.Collection.prototype._prepareModel.call( + this, model, options ); if (model.toJSON && typeof model.toJSON == 'function') { @@ -569,16 +653,17 @@ if (this._suppressEvent === true) { this._suppressEvent = false; - Backbone.Collection.prototype.add.apply(this, [model], {silent: true}); + Backbone.Collection.prototype.add.call(this, [model], {silent: true}); } else { - Backbone.Collection.prototype.add.apply(this, [model]); + Backbone.Collection.prototype.add.call(this, [model]); } this.get(model.id)._remoteAttributes = model; }, - _childMoved: function(snap) { - // TODO: Investigate: can this occur without the ID changing? - this._log('_childMoved called with ' + snap.val()); + // TODO: child_moved is emitted when the priority for a child is changed, so it + // should update the priority of the model and maybe trigger a sort + _childMoved: function() { + }, // when a model has changed remotely find differences between the @@ -620,13 +705,13 @@ if (this._suppressEvent === true) { this._suppressEvent = false; - Backbone.Collection.prototype.remove.apply( + Backbone.Collection.prototype.remove.call( this, [model], {silent: true} ); } else { // trigger sync because data has been received from the server this.trigger('sync', this); - Backbone.Collection.prototype.remove.apply(this, [model]); + Backbone.Collection.prototype.remove.call(this, [model]); } }, @@ -750,7 +835,7 @@ var newItem = new BaseModel(attrs, opts); newItem.autoSync = false; - newItem.firebase = self.firebase.child(newItem.id); + newItem.firebase = self.firebase.ref().child(newItem.id); newItem.sync = Backbone.Firebase.sync; newItem.on('change', function(model) { var updated = Backbone.Firebase.Model.prototype._updateModel(model); @@ -761,6 +846,10 @@ }; + }, + + comparator: function(model) { + return model.id; } }); diff --git a/test/specs/collection_test.js b/test/specs/collection_test.js index ec780d0..f5783aa 100644 --- a/test/specs/collection_test.js +++ b/test/specs/collection_test.js @@ -246,7 +246,7 @@ describe('Backbone.Firebase.Collection', function() { return expect(model.autoSync).to.be.ok; }); - it('should call sync when added', function() { + it('should call add when added', function() { var spy = sinon.spy(); var Models = Backbone.Firebase.Collection.extend({ url: 'Mock://', @@ -255,7 +255,7 @@ describe('Backbone.Firebase.Collection', function() { var models = new Models(); - models.on('sync', spy); + models.on('add', spy); models.add({ title: 'blah' }); models.firebase.flush(); @@ -321,22 +321,7 @@ describe('Backbone.Firebase.Collection', function() { }); describe('#_childMoved', function() { - - it('shoud call _log', function() { - sinon.spy(collection, '_log'); - var mockSnap = new MockSnap({ - name: '1', - val: { - name: 'David' - } - }); - collection._childMoved(mockSnap); - - expect(collection._log.calledOnce).to.be.ok; - - collection._log.restore(); - }); - + it('should set priority on model'); }); describe('#reset', function() { @@ -390,6 +375,31 @@ describe('Backbone.Firebase.Collection', function() { }); + describe('#fetch', function() { + + it('should call Backbone.Firebase._promiseEvent', function() { + sinon.spy(Backbone.Firebase, '_promiseEvent'); + + collection.fetch(); + collection.firebase.flush(); + + expect(Backbone.Firebase._promiseEvent.calledOnce).to.be.ok; + + Backbone.Firebase._promiseEvent.restore(); + }); + + it('should fire success', function() { + var successCalled = false; + collection.fetch(); + collection.on('sync', function() { + successCalled = true; + }); + collection.firebase.flush(); + expect(successCalled).to.be.ok; + }); + + }); + describe('#_log', function() { beforeEach(function() { @@ -774,6 +784,30 @@ describe('Backbone.Firebase.Collection', function() { }); + // TODO: Resolve issue with Mockfirebase + // describe('#fetch', function() { + // + // it('should call the success option if provided', function() { + // var options = { + // success: sinon.spy() + // }; + // collection.fetch(options); + // collection.firebase.flush(); + // expect(options.success.calledOnce).to.be.ok; + // }); + // + // it('should trigger the "sync" event', function() { + // var isSyncCalled = false; + // collection.fetch(); + // collection.on('sync', function() { + // isSyncCalled = true; + // }); + // collection.firebase.flush(); + // expect(isSyncCalled).to.be.ok; + // }); + // + // }); + describe('#add', function() { it('should call Backbone.Collection.prototype.add', function() { sinon.spy(Backbone.Collection.prototype, 'add'); @@ -803,4 +837,4 @@ describe('Backbone.Firebase.Collection', function() { }); -}); \ No newline at end of file +}); diff --git a/test/specs/model_test.js b/test/specs/model_test.js index a93ffc6..fa9f14e 100644 --- a/test/specs/model_test.js +++ b/test/specs/model_test.js @@ -18,6 +18,18 @@ describe('Backbone.Firebase.Model', function() { return expect(new Model()).to.be.ok; }); + it('should create a model with a urlRoot from its id', function() { + var mockUrl = 'https://mock-bf.firebaseio.com/users'; + var Model = Backbone.Firebase.Model.extend({ + urlRoot: mockUrl + }); + var model = new Model({ + id: 'david' + }); + + model.url().should.equal(mockUrl + '/' + model.get('id')); + }); + describe('#constructor', function() { it('should throw an error if an invalid url is provided', function() { @@ -69,15 +81,6 @@ describe('Backbone.Firebase.Model', function() { }); - it('should update model', function() { - // TODO: Test _updateModel - }); - - it('should set changed attributes to null', function() { - // TODO: Test _updateModel - - }); - describe('#_unsetAttributes', function() { it('should unset attributes that have been deleted on the server', function() { @@ -157,32 +160,46 @@ describe('Backbone.Firebase.Model', function() { }); }); - describe('ignored methods', function() { - - beforeEach(function() { - sinon.spy(console, 'warn'); - }); - - afterEach(function() { - console.warn.restore(); - }); + describe('#fetch', function() { - it('should do nothing when save is called', function() { + it('should trigger "sync" when fetch is called', function() { var model = new Model(); - model.save(); - return expect(console.warn.calledOnce).to.be.ok; + var syncIsCalled = false; + model.fetch(); + model.on('sync', function() { + syncIsCalled = true; + }); + model.firebase.flush(); + return expect(syncIsCalled).to.be.ok; }); - it('should do nothing when fetch is called', function() { + it('should call Backbone.Firebase._promiseEvent', function() { var model = new Model(); + sinon.spy(Backbone.Firebase, '_promiseEvent'); + model.fetch(); - return expect(console.warn.calledOnce).to.be.ok; + model.firebase.flush(); + + expect(Backbone.Firebase._promiseEvent.calledOnce).to.be.ok; + + Backbone.Firebase._promiseEvent.restore(); }); - it('should do nothing when sync is called', function() { - var model = new Model(); - model.sync(); - return expect(console.warn.calledOnce).to.be.ok; + }); + + describe('#sync', function() { + + // Backbone.Firebase.Model.sync should proxy to Backbone.Firebase.sync + // if it comes from a OnceModel + it('should call Backbone.Firebase.sync', function() { + sinon.spy(Backbone.Firebase, 'sync'); + + model = new Model(); + + model.sync('read', model, null); + + expect(Backbone.Firebase.sync.calledOnce).to.be.ok; + Backbone.Firebase.sync.restore(); }); }); @@ -386,4 +403,4 @@ describe('Backbone.Firebase.Model', function() { }); -}); \ No newline at end of file +}); diff --git a/test/specs/prototype_test.js b/test/specs/prototype_test.js index 4354a1b..afac069 100644 --- a/test/specs/prototype_test.js +++ b/test/specs/prototype_test.js @@ -4,6 +4,67 @@ describe('Backbone.Firebase', function() { return expect(Backbone.Firebase).to.be.ok; }); + describe('#_promiseEvent', function() { + var syncPromise; + var clock; + beforeEach(function() { + syncPromise = { + resolve: true + }; + clock = sinon.useFakeTimers(); + }); + + afterEach(function() { + clock.restore(); + }); + + it('should resolve with a success', function() { + var successCalled = false; + syncPromise.success = true; + Backbone.Firebase._promiseEvent({ + syncPromise: syncPromise, + success: function() { + successCalled = true; + }, + context: this + }); + clock.tick(100); + expect(successCalled).to.be.ok; + }); + + it('should resolve with an error', function() { + var errorCalled = false; + syncPromise.err = new Error('Error!'); + Backbone.Firebase._promiseEvent({ + syncPromise: syncPromise, + error: function() { + errorCalled = true; + }, + context: this + }); + clock.tick(1000); + expect(errorCalled).to.be.ok; + }); + + it('should resolve with a complete', function() { + var completeCalled = false; + syncPromise.success = true; + Backbone.Firebase._promiseEvent({ + syncPromise: syncPromise, + success: function() { + + }, + complete: function() { + completeCalled = true; + }, + context: this + }); + clock.tick(100); + expect(completeCalled).to.be.ok; + }); + + }); + describe("#_isPrimitive", function() { it('should return false for null', function() { @@ -333,4 +394,4 @@ describe('Backbone.Firebase', function() { }); -}); \ No newline at end of file +});