Merge pull request #1999 from strongloop/promisify-change

Promisify change

common/models/change.js

@@ -4,6 +4,7 @@
 
 var PersistedModel = require('../../lib/loopback').PersistedModel;
 var loopback = require('../../lib/loopback');
+var utils = require('../../lib/utils');
 var crypto = require('crypto');
 var CJSON = {stringify: require('canonical-json')};
 var async = require('async');
@@ -77,6 +78,8 @@ module.exports = function(Change) {
     var Change = this;
     var errors = [];
 
+    callback = callback || utils.createPromiseCallback();
+
     var tasks = modelIds.map(function(id) {
       return function(cb) {
         Change.findOrCreateChange(modelName, id, function(err, change) {
@@ -111,6 +114,7 @@ module.exports = function(Change) {
       }
       callback();
     });
+    return callback.promise;
   };
 
   /**
@@ -138,6 +142,7 @@ module.exports = function(Change) {
 
   Change.findOrCreateChange = function(modelName, modelId, callback) {
     assert(loopback.findModel(modelName), modelName + ' does not exist');
+    callback = callback || utils.createPromiseCallback();
     var id = this.idForModel(modelName, modelId);
     var Change = this;
 
@@ -155,6 +160,7 @@ module.exports = function(Change) {
         Change.updateOrCreate(ch, callback);
       }
     });
+    return callback.promise;
   };
 
   /**
@@ -171,9 +177,7 @@ module.exports = function(Change) {
 
     change.debug('rectify change');
 
-    cb = cb || function(err) {
-      if (err) throw new Error(err);
-    };
+    cb = cb || utils.createPromiseCallback();
 
     change.currentRevision(function(err, rev) {
       if (err) return cb(err);
@@ -194,6 +198,7 @@ module.exports = function(Change) {
         }
       );
     });
+    return cb.promise;
 
     function doRectify(checkpoint, rev) {
       if (rev) {
@@ -248,6 +253,7 @@ module.exports = function(Change) {
    */
 
   Change.prototype.currentRevision = function(cb) {
+    cb = cb || utils.createPromiseCallback();
     var model = this.getModelCtor();
     var id = this.getModelId();
     model.findById(id, function(err, inst) {
@@ -258,6 +264,7 @@ module.exports = function(Change) {
         cb(null, null);
       }
     });
+    return cb.promise;
   };
 
   /**
@@ -390,8 +397,11 @@ module.exports = function(Change) {
    */
 
   Change.diff = function(modelName, since, remoteChanges, callback) {
+    callback = callback || utils.createPromiseCallback();
+
     if (!Array.isArray(remoteChanges) || remoteChanges.length === 0) {
-      return callback(null, {deltas: [], conflicts: []});
+      callback(null, {deltas: [], conflicts: []});
+      return callback.promise;
     }
     var remoteChangeIndex = {};
     var modelIds = [];
@@ -455,6 +465,7 @@ module.exports = function(Change) {
         conflicts: conflicts
       });
     });
+    return callback.promise;
   };
 
   /**

test/change.test.js

@@ -82,6 +82,38 @@ describe('Change', function() {
     });
   });
 
+  describe('Change.rectifyModelChanges - promise variant', function() {
+    describe('using an existing untracked model', function() {
+      beforeEach(function(done) {
+        var test = this;
+        Change.rectifyModelChanges(this.modelName, [this.modelId])
+          .then(function(trackedChanges) {
+            done();
+          })
+          .catch(done);
+      });
+
+      it('should create an entry', function(done) {
+        var test = this;
+        Change.find()
+          .then(function(trackedChanges) {
+            assert.equal(trackedChanges[0].modelId, test.modelId.toString());
+            done();
+          })
+          .catch(done);
+      });
+
+      it('should only create one change', function(done) {
+        Change.count()
+          .then(function(count) {
+            assert.equal(count, 1);
+            done();
+          })
+          .catch(done);
+      });
+    });
+  });
+
   describe('Change.findOrCreateChange(modelName, modelId, callback)', function() {
 
     describe('when a change doesnt exist', function() {
@@ -104,6 +136,27 @@ describe('Change', function() {
       });
     });
 
+    describe('when a change doesnt exist - promise variant', function() {
+      beforeEach(function(done) {
+        var test = this;
+        Change.findOrCreateChange(this.modelName, this.modelId)
+        .then(function(result) {
+          test.result = result;
+          done();
+        })
+        .catch(done);
+      });
+
+      it('should create an entry', function(done) {
+        var test = this;
+        Change.findById(this.result.id, function(err, change) {
+          if (err) return done(err);
+          assert.equal(change.id, test.result.id);
+          done();
+        });
+      });
+    });
+
     describe('when a change does exist', function() {
       beforeEach(function(done) {
         var test = this;
@@ -219,6 +272,28 @@ describe('Change', function() {
     });
   });
 
+  describe('change.rectify - promise variant', function() {
+    var change;
+    beforeEach(function(done) {
+      Change.findOrCreateChange(this.modelName, this.modelId)
+        .then(function(ch) {
+          change = ch;
+          done();
+        })
+        .catch(done);
+    });
+
+    it('should create a new change with the correct revision', function(done) {
+      var test = this;
+      change.rectify()
+        .then(function(ch) {
+          assert.equal(ch.rev, test.revisionForModel);
+          done();
+        })
+        .catch(done);
+    });
+  });
+
   describe('change.currentRevision(callback)', function() {
     it('should get the correct revision', function(done) {
       var test = this;
@@ -234,6 +309,23 @@ describe('Change', function() {
     });
   });
 
+  describe('change.currentRevision - promise variant', function() {
+    it('should get the correct revision', function(done) {
+      var test = this;
+      var change = new Change({
+        modelName: this.modelName,
+        modelId: this.modelId
+      });
+
+      change.currentRevision()
+      .then(function(rev) {
+        assert.equal(rev, test.revisionForModel);
+        done();
+      })
+      .catch(done);
+    });
+  });
+
   describe('Change.hash(str)', function() {
     // todo(ritch) test other hashing algorithms
     it('should hash the given string', function() {
@@ -374,6 +466,25 @@ describe('Change', function() {
       });
     });
 
+    it('should return delta and conflict lists - promise variant', function(done) {
+      var remoteChanges = [
+        // an update => should result in a delta
+        {rev: 'foo2', prev: 'foo', modelName: this.modelName, modelId: 9, checkpoint: 1},
+        // no change => should not result in a delta / conflict
+        {rev: 'bar', prev: 'bar', modelName: this.modelName, modelId: 10, checkpoint: 1},
+        // a conflict => should result in a conflict
+        {rev: 'bat2', prev: 'bat0', modelName: this.modelName, modelId: 11, checkpoint: 1},
+      ];
+
+      Change.diff(this.modelName, 0, remoteChanges)
+        .then(function(diff) {
+          assert.equal(diff.deltas.length, 1);
+          assert.equal(diff.conflicts.length, 1);
+          done();
+        })
+        .catch(done);
+    });
+
     it('should set "prev" to local revision in non-conflicting delta', function(done) {
       var updateRecord = {
         rev: 'foo-new',