Fix bulkUpdate to not trigger rectifyAll

Fix `getIdFromWhereByModelId()` to correctly detect the situation when "bulkUpdate" performs a write operation using a where filter containing both id attribute but also all other model attributes. This should significantly improve the performance of change replication, because the cost of running rectifyAll is very high.
mikeblakeney · Dec 22, 2015 · 4aac277 · 4aac277
1 parent 69940ad
commit 4aac277
Show file tree

Hide file tree

Showing 2 changed files with 110 additions and 4 deletions.
diff --git a/lib/persisted-model.js b/lib/persisted-model.js
@@ -1488,11 +1488,8 @@ module.exports = function(registry) {
   }
 
   function getIdFromWhereByModelId(Model, where) {
-    var whereKeys = Object.keys(where);
-    if (whereKeys.length != 1) return undefined;
-
     var idName = Model.getIdName();
-    if (whereKeys[0] !== idName) return undefined;
+    if (!(idName in where)) return undefined;
 
     var id = where[idName];
     // TODO(bajtos) support object values that are not LB conditions

diff --git a/test/replication.test.js b/test/replication.test.js
@@ -52,6 +52,115 @@ describe('Replication / Change APIs', function() {
     };
   });
 
+  describe('optimization check rectifyChange Vs rectifyAllChanges', function() {
+    beforeEach(function initialData(done) {
+      var data = [{name: 'John', surname: 'Doe'}, {name: 'Jane', surname: 'Roe'}];
+      async.waterfall([
+        function(callback) {
+          SourceModel.create(data, callback);
+        },
+        function(data, callback) {
+          SourceModel.replicate(TargetModel, callback);
+        }], function(err, result) {
+        done(err);
+      });
+    });
+
+    it('should call rectifyAllChanges if no id is passed for rectifyOnDelete', function(done) {
+      SourceModel.rectifyChange = function() {
+        return done(new Error('Should not call rectifyChange'));
+      };
+      SourceModel.rectifyAllChanges = function() {
+        return done();
+      };
+      SourceModel.destroyAll({name: 'John'}, function(err, data) {
+        if (err)
+          return done(err);
+      });
+    });
+
+    it('should call rectifyAllChanges if no id is passed for rectifyOnSave', function(done) {
+      SourceModel.rectifyChange = function() {
+        return done(new Error('Should not call rectifyChange'));
+      };
+      SourceModel.rectifyAllChanges = function() {
+        return done();
+      };
+      var newData = {'name': 'Janie'};
+      SourceModel.update({name: 'Jane'}, newData, function(err, data) {
+        if (err)
+          return done(err);
+      });
+    });
+
+    it('rectifyOnDelete for Delete should call rectifyChange instead of rectifyAllChanges', function(done) {
+      TargetModel.rectifyChange = function() {
+        return done();
+      };
+      TargetModel.rectifyAllChanges = function() {
+        return done(new Error('Should not call rectifyAllChanges'));
+      };
+
+      async.waterfall([
+        function(callback) {
+          SourceModel.destroyAll({name: 'John'}, callback);
+        },
+        function(data, callback) {
+          SourceModel.replicate(TargetModel, callback);
+          // replicate should call `rectifyOnSave` and then `rectifyChange` not `rectifyAllChanges` through `after save` operation
+        }
+      ], function(err, results) {
+        if (err)
+          return done(err);
+      });
+    });
+
+    it('rectifyOnSave for Update should call rectifyChange instead of rectifyAllChanges', function(done) {
+      TargetModel.rectifyChange = function() {
+        return done();
+      };
+      TargetModel.rectifyAllChanges = function() {
+        return done(new Error('Should not call rectifyAllChanges'));
+      };
+
+      var newData = {'name': 'Janie'};
+      async.waterfall([
+        function(callback) {
+          SourceModel.update({name: 'Jane'}, newData, callback);
+        },
+        function(data, callback) {
+          SourceModel.replicate(TargetModel, callback);
+          // replicate should call `rectifyOnSave` and then `rectifyChange` not `rectifyAllChanges` through `after save` operation
+        }], function(err, result) {
+        if (err)
+          return done(err);
+      });
+    });
+
+    it('rectifyOnSave for Create should call rectifyChange instead of rectifyAllChanges', function(done) {
+      TargetModel.rectifyChange = function() {
+        return done();
+      };
+      TargetModel.rectifyAllChanges = function() {
+        return done(new Error('Should not call rectifyAllChanges'));
+      };
+
+      var newData = [{name: 'Janie', surname: 'Doe'}];
+      async.waterfall([
+        function(callback) {
+          SourceModel.create(newData, callback);
+        },
+        function(data, callback) {
+          SourceModel.replicate(TargetModel, callback);
+          // replicate should call `rectifyOnSave` and then `rectifyChange` not `rectifyAllChanges` through `after save` operation
+        }
+      ], function(err, result) {
+        if (err)
+          return done(err);
+      });
+    });
+  });
+
   describe('Model.changes(since, filter, callback)', function() {
     it('Get changes since the given checkpoint', function(done) {
       var test = this;