Implement operation hooks for EmbedsMany methods

create() triggers
 - before save
 - after save

updateById() triggers
 - before save
 - after save

destroy() triggers
 - before delete
 - after delete

The implementation here is intentionally left with less features
than the regular DAO methods provide, the goal is to get a partial
(but still useful!) version released soon.

Limitations:

 - `before save` & `after save` hooks don't provide `ctx.isNewInstance`
 - async validations are not supported yet
 - `persist` and `loaded` hooks are not triggered at all

 - `before delete` hook does not provide `ctx.where` property and
    it's not possible to change the outcome of `destroy()` using this
    hook. Note that regular DAO does support this.

 - updating embedded instances triggers update of the parent (owning)
   model, which is correct and expected. However, the context provided
   by `before save` and `after save` hooks on the parent model is sort of
   arbitrary and may include wrong/extra data. The same probably applies
   to the scenario when deleting embedded instances triggers update of
   the parent model.

[back-port of #911]

lib/relation-definition.js

@@ -2582,6 +2582,7 @@ EmbedsMany.prototype.updateById = function(fkId, data, options, cb) {
     cb = data;
     data = {};
   }
+  options = options || {};
 
   var modelTo = this.definition.modelTo;
   var propertyName = this.definition.keyFrom;
@@ -2592,22 +2593,48 @@ EmbedsMany.prototype.updateById = function(fkId, data, options, cb) {
   var inst = this.findById(fkId);
 
   if (inst instanceof modelTo) {
-    if (typeof data === 'object') {
+    var hookState = {};
-      inst.setAttributes(data);
+    var context = {
-    }
+      Model: modelTo,
-    var err = inst.isValid() ? null : new ValidationError(inst);
+      currentInstance: inst,
-    if (err && typeof cb === 'function') {
+      data: data,
-      return process.nextTick(function() {
+      options: options,
-        cb(err, inst);
+      hookState: hookState,
-      });
+    };
-    }
+    modelTo.notifyObserversOf('before save', context, function(err) {
+      if (err) return cb && cb(err);
 
-    if (typeof cb === 'function') {
+      inst.setAttributes(data);
-      modelInstance.updateAttribute(propertyName,
+
-        embeddedList, options, function(err) {
+      var err = inst.isValid() ? null : new ValidationError(inst);
+      if (err && typeof cb === 'function') {
+        return process.nextTick(function() {
           cb(err, inst);
         });
-    }
+      }
+
+      context = {
+        Model: modelTo,
+        instance: inst,
+        options: options,
+        hookState: hookState,
+      };
+
+      if (typeof cb === 'function') {
+        modelInstance.updateAttribute(propertyName, embeddedList, options,
+        function(err) {
+          if (err) return cb(err, inst);
+          modelTo.notifyObserversOf('after save', context, function(err) {
+            cb(err, inst);
+          });
+        });
+      } else {
+        modelTo.notifyObserversOf('after save', context, function(err) {
+          if (!err) return;
+          debug('Unhandled error in "after save" hooks: %s', err.stack || err);
+        });
+      }
+    });
   } else if (typeof cb === 'function') {
     process.nextTick(function() {
       cb(null, null); // not found
@@ -2631,15 +2658,27 @@ EmbedsMany.prototype.destroyById = function(fkId, options, cb) {
   var inst = (fkId instanceof modelTo) ? fkId : this.findById(fkId);
 
   if (inst instanceof modelTo) {
-    var index = embeddedList.indexOf(inst);
+    var context = {
-    if (index > -1) embeddedList.splice(index, 1);
+      Model: modelTo,
-    if (typeof cb === 'function') {
+      instance: inst,
+      options: options || {},
+      hookState: {},
+    };
+    modelTo.notifyObserversOf('before delete', context, function(err) {
+      if (err) return cb(err);
+
+      var index = embeddedList.indexOf(inst);
+      if (index > -1) embeddedList.splice(index, 1);
+      if (typeof cb !== 'function') return;
       modelInstance.updateAttribute(propertyName,
         embeddedList, function(err) {
-          cb(err);
+          if (err) return cb(err);
-          modelTo.emit('deleted', inst.id, inst.toJSON());
+          modelTo.notifyObserversOf('after delete', context, function(err) {
+            cb(err);
+            modelTo.emit('deleted', inst.id, inst.toJSON());
+          });
         });
-    }
+    });
   } else if (typeof cb === 'function') {
     process.nextTick(cb); // not found
   }
@@ -2725,16 +2764,16 @@ EmbedsMany.prototype.create = function(targetModelData, options, cb) {
 
   var inst = this.callScopeMethod('build', targetModelData);
 
-  var updateEmbedded = function() {
+  var updateEmbedded = function(callback) {
     if (modelInstance.isNewRecord()) {
       modelInstance.setAttribute(propertyName, embeddedList);
       modelInstance.save(options, function(err) {
-        cb(err, err ? null : inst);
+        callback(err, err ? null : inst);
       });
     } else {
       modelInstance.updateAttribute(propertyName,
         embeddedList, options, function(err) {
-          cb(err, err ? null : inst);
+          callback(err, err ? null : inst);
         });
     }
   };
@@ -2742,7 +2781,7 @@ EmbedsMany.prototype.create = function(targetModelData, options, cb) {
   if (this.definition.options.persistent) {
     inst.save(function(err) { // will validate
       if (err) return cb(err, inst);
-      updateEmbedded();
+      updateEmbedded(cb);
     });
   } else {
     var err = inst.isValid() ? null : new ValidationError(inst);
@@ -2751,7 +2790,21 @@ EmbedsMany.prototype.create = function(targetModelData, options, cb) {
         cb(err);
       });
     } else {
-      updateEmbedded();
+      var context = {
+        Model: modelTo,
+        instance: inst,
+        options: options || {},
+        hookState: {},
+      };
+      modelTo.notifyObserversOf('before save', context, function(err) {
+        if (err) return cb(err);
+        updateEmbedded(function(err, inst) {
+          if (err) return cb(err, null);
+          modelTo.notifyObserversOf('after save', context, function(err) {
+            cb(err, err ? null : inst);
+          });
+        });
+      });
     }
   }
   return cb.promise;

test/operation-hooks.suite/embeds-many-create.suite.js

@@ -0,0 +1,191 @@
+// Copyright IBM Corp. 2015,2016. All Rights Reserved.
+// Node module: loopback-datasource-juggler
+// This file is licensed under the MIT License.
+// License text available at https://opensource.org/licenses/MIT
+
+var ValidationError = require('../..').ValidationError;
+
+var contextTestHelpers = require('../helpers/context-test-helpers');
+var ContextRecorder = contextTestHelpers.ContextRecorder;
+var aCtxForModel = contextTestHelpers.aCtxForModel;
+
+var uid = require('../helpers/uid-generator');
+var HookMonitor = require('../helpers/hook-monitor');
+
+module.exports = function(dataSource, should, connectorCapabilities) {
+  describe('EmbedsMany - create', function() {
+    var ctxRecorder, hookMonitor, expectedError;
+
+    beforeEach(function setupHelpers() {
+      ctxRecorder = new ContextRecorder('hook not called');
+      hookMonitor = new HookMonitor({ includeModelName: true });
+      expectedError = new Error('test error');
+    });
+
+    var Owner, Embedded, ownerInstance;
+    var migrated = false;
+
+    beforeEach(function setupDatabase() {
+      Embedded = dataSource.createModel('Embedded', {
+        // Set id.generated to false to honor client side values
+        id: { type: String, id: true, generated: false, default: uid.next },
+        name: { type: String, required: true },
+        extra: { type: String, required: false },
+      });
+
+      Owner = dataSource.createModel('Owner', {});
+      Owner.embedsMany(Embedded);
+
+      hookMonitor.install(Embedded);
+      hookMonitor.install(Owner);
+
+      if (migrated) {
+        return Owner.deleteAll();
+      } else {
+        return dataSource.automigrate(Owner.modelName)
+          .then(function() { migrated = true; });
+      }
+    });
+
+    beforeEach(function setupData() {
+      return Owner.create({}).then(function(inst) {
+        ownerInstance = inst;
+        hookMonitor.resetNames();
+      });
+    });
+
+    function callCreate() {
+      var item = new Embedded({ name: 'created' });
+      return ownerInstance.embeddedList.create(item);
+    }
+
+    it('triggers hooks in the correct order', function() {
+      return callCreate().then(function(result) {
+        hookMonitor.names.should.eql([
+          'Embedded:before save',
+          //TODO 'Embedded:persist',
+          'Owner:before save',
+          'Owner:persist',
+          'Owner:loaded',
+          'Owner:after save',
+          //TODO 'Embedded:loaded',
+          'Embedded:after save',
+        ]);
+      });
+    });
+
+    it('trigers `before save` hook on embedded model', function() {
+      Embedded.observe('before save', ctxRecorder.recordAndNext());
+      return callCreate().then(function(instance) {
+        ctxRecorder.records.should.eql(aCtxForModel(Embedded, {
+          instance: {
+            id: instance.id,
+            name: 'created',
+            extra: undefined,
+          },
+          // TODO isNewInstance: true,
+        }));
+      });
+    });
+
+    // TODO
+    it('trigers `before save` hook on owner model');
+
+    it('applies updates from `before save` hook', function() {
+      Embedded.observe('before save', function(ctx, next) {
+        ctx.instance.should.be.instanceOf(Embedded);
+        ctx.instance.extra = 'hook data';
+        next();
+      });
+      return callCreate().then(function(instance) {
+        instance.should.have.property('extra', 'hook data');
+      });
+    });
+
+    it('validates model after `before save` hook', function() {
+      Embedded.observe('before save', invalidateEmbeddedModel);
+      return callCreate().then(throwShouldHaveFailed, function(err) {
+        err.should.be.instanceOf(ValidationError);
+        // NOTE Apparently `create` is deferring validation to the owner
+        // model, which in turn validates all embedded instances
+        // and produces a single "invalid" error only
+        // Compare this to `embedsOne.create`, which correctly reports
+        // codes: { name: ['presence'] }
+        (err.details.codes || {}).should.eql({ embeddeds: ['invalid'] });
+      });
+    });
+
+    it('aborts when `before save` hook fails', function() {
+      Embedded.observe('before save', nextWithError(expectedError));
+      return callCreate().then(throwShouldHaveFailed, function(err) {
+        err.should.eql(expectedError);
+      });
+    });
+
+    // TODO
+    it('triggers `persist` hook on embedded model');
+    it('triggers `persist` hook on owner model');
+    it('applies updates from `persist` hook');
+    it('aborts when `persist` hook fails');
+
+    // TODO
+    it('triggers `loaded` hook on embedded model');
+    it('triggers `loaded` hook on owner model');
+    it('applies updates from `loaded` hook');
+    it('aborts when `loaded` hook fails');
+
+    it('triggers `after save` hook on embedded model', function() {
+      Embedded.observe('after save', ctxRecorder.recordAndNext());
+      return callCreate().then(function(instance) {
+        ctxRecorder.records.should.eql(aCtxForModel(Embedded, {
+          instance: {
+            id: instance.id,
+            name: 'created',
+            extra: undefined,
+          },
+          // TODO isNewInstance: true,
+        }));
+      });
+    });
+
+    // TODO
+    it('triggers `after save` hook on owner model');
+
+    it('applies updates from `after save` hook', function() {
+      Embedded.observe('after save', function(ctx, next) {
+        ctx.instance.should.be.instanceOf(Embedded);
+        ctx.instance.extra = 'hook data';
+        next();
+      });
+      return callCreate().then(function(instance) {
+        instance.should.have.property('extra', 'hook data');
+      });
+    });
+
+    it('aborts when `after save` hook fails', function() {
+      Embedded.observe('after save', nextWithError(expectedError));
+      return callCreate().then(throwShouldHaveFailed, function(err) {
+        err.should.eql(expectedError);
+      });
+    });
+
+    function invalidateEmbeddedModel(context, next) {
+      if (context.instance) {
+        context.instance.name = '';
+      } else {
+        context.data.name = '';
+      }
+      next();
+    }
+
+    function nextWithError(err) {
+      return function(context, next) {
+        next(err);
+      };
+    }
+
+    function throwShouldHaveFailed() {
+      throw new Error('operation should have failed');
+    }
+  });
+};

test/operation-hooks.suite/embeds-many-destroy.suite.js

@@ -0,0 +1,149 @@
+// Copyright IBM Corp. 2015,2016. All Rights Reserved.
+// Node module: loopback-datasource-juggler
+// This file is licensed under the MIT License.
+// License text available at https://opensource.org/licenses/MIT
+
+var Promise = require('bluebird');
+var ValidationError = require('../..').ValidationError;
+
+var contextTestHelpers = require('../helpers/context-test-helpers');
+var ContextRecorder = contextTestHelpers.ContextRecorder;
+var aCtxForModel = contextTestHelpers.aCtxForModel;
+
+var uid = require('../helpers/uid-generator');
+var HookMonitor = require('../helpers/hook-monitor');
+
+module.exports = function(dataSource, should, connectorCapabilities) {
+  describe('EmbedsMany - destroy', function() {
+    var ctxRecorder, hookMonitor, expectedError;
+    beforeEach(function sharedSetup() {
+      ctxRecorder = new ContextRecorder('hook not called');
+      hookMonitor = new HookMonitor({ includeModelName: true });
+      expectedError = new Error('test error');
+    });
+
+    var Owner, Embedded;
+    var migrated = false;
+    beforeEach(function setupDatabase() {
+      Embedded = dataSource.createModel('Embedded', {
+        // Set id.generated to false to honor client side values
+        id: { type: String, id: true, generated: false, default: uid.next },
+        name: { type: String, required: true },
+        extra: { type: String, required: false },
+      });
+
+      Owner = dataSource.createModel('Owner', {});
+      Owner.embedsMany(Embedded);
+
+      hookMonitor.install(Embedded);
+      hookMonitor.install(Owner);
+
+      if (migrated) {
+        return Owner.deleteAll();
+      } else {
+        return dataSource.automigrate(Owner.modelName)
+          .then(function() { migrated = true; });
+      }
+    });
+
+    var ownerInstance, existingInstance;
+    beforeEach(function setupData() {
+      return Owner.create({})
+        .then(function(inst) {
+          ownerInstance = inst;
+        })
+        .then(function() {
+          var item = new Embedded({ name: 'created' });
+          return ownerInstance.embeddedList.create(item).then(function(it) {
+            existingItem = it;
+          });
+        })
+        .then(function() {
+          hookMonitor.resetNames();
+        });
+    });
+
+    function callDestroy() {
+      // Unfortunately, updateById was not promisified yet
+      return new Promise(function(resolve, reject) {
+        return ownerInstance.embeddedList.destroy(
+          existingItem.id,
+          function(err, result) {
+            if (err) reject(err);
+            else resolve(result);
+          });
+      });
+    }
+
+    it('triggers hooks in the correct order', function() {
+      return callDestroy().then(function(result) {
+        hookMonitor.names.should.eql([
+          'Embedded:before delete',
+          'Owner:before save',
+          'Owner:persist',
+          'Owner:loaded',
+          'Owner:after save',
+          'Embedded:after delete',
+        ]);
+      });
+    });
+
+    it('trigers `before delete` hook', function() {
+      Embedded.observe('before delete', ctxRecorder.recordAndNext());
+      return callDestroy().then(function() {
+        ctxRecorder.records.should.eql(aCtxForModel(Embedded, {
+          instance: {
+            id: existingItem.id,
+            name: 'created',
+            extra: undefined,
+          },
+        }));
+      });
+    });
+
+    // TODO
+    // In order to allow "before delete" hook to make changes,
+    // we need to enhance the context to include information
+    // about the model instance being deleted.
+    // "ctx.where: { id: embedded.id }" may not be enough,
+    // as it does not identify the parent (owner) model
+    it('applies updates from `before delete` hook');
+
+    it('aborts when `before delete` hook fails', function() {
+      Embedded.observe('before delete', nextWithError(expectedError));
+      return callDestroy().then(throwShouldHaveFailed, function(err) {
+        err.should.eql(expectedError);
+      });
+    });
+
+    it('trigers `after delete` hook', function() {
+      Embedded.observe('after delete', ctxRecorder.recordAndNext());
+      return callDestroy().then(function() {
+        ctxRecorder.records.should.eql(aCtxForModel(Embedded, {
+          instance: {
+            id: existingItem.id,
+            name: 'created',
+            extra: undefined,
+          },
+        }));
+      });
+    });
+
+    it('aborts when `after delete` hook fails', function() {
+      Embedded.observe('after delete', nextWithError(expectedError));
+      return callDestroy().then(throwShouldHaveFailed, function(err) {
+        err.should.eql(expectedError);
+      });
+    });
+
+    function nextWithError(err) {
+      return function(context, next) {
+        next(err);
+      };
+    }
+
+    function throwShouldHaveFailed() {
+      throw new Error('operation should have failed');
+    }
+  });
+};

test/operation-hooks.suite/embeds-many-update-by-id.suite.js

@@ -0,0 +1,205 @@
+// Copyright IBM Corp. 2015,2016. All Rights Reserved.
+// Node module: loopback-datasource-juggler
+// This file is licensed under the MIT License.
+// License text available at https://opensource.org/licenses/MIT
+
+var Promise = require('bluebird');
+var ValidationError = require('../..').ValidationError;
+
+var contextTestHelpers = require('../helpers/context-test-helpers');
+var ContextRecorder = contextTestHelpers.ContextRecorder;
+var aCtxForModel = contextTestHelpers.aCtxForModel;
+
+var uid = require('../helpers/uid-generator');
+var HookMonitor = require('../helpers/hook-monitor');
+
+module.exports = function(dataSource, should, connectorCapabilities) {
+  describe('EmbedsMany - update', function() {
+    var ctxRecorder, hookMonitor, expectedError;
+    beforeEach(function setupHelpers() {
+      ctxRecorder = new ContextRecorder('hook not called');
+      hookMonitor = new HookMonitor({ includeModelName: true });
+      expectedError = new Error('test error');
+    });
+
+    var Owner, Embedded;
+    var migrated = false;
+    beforeEach(function setupDatabase() {
+      Embedded = dataSource.createModel('Embedded', {
+        // Set id.generated to false to honor client side values
+        id: { type: String, id: true, generated: false, default: uid.next },
+        name: { type: String, required: true },
+        extra: { type: String, required: false },
+      });
+
+      Owner = dataSource.createModel('Owner', {});
+      Owner.embedsMany(Embedded);
+
+      hookMonitor.install(Embedded);
+      hookMonitor.install(Owner);
+
+      if (migrated) {
+        return Owner.deleteAll();
+      } else {
+        return dataSource.automigrate(Owner.modelName)
+          .then(function() { migrated = true; });
+      }
+    });
+
+    var ownerInstance, existingItem;
+    beforeEach(function setupData() {
+      return Owner.create({})
+        .then(function(inst) {
+          ownerInstance = inst;
+        })
+        .then(function() {
+          var item = new Embedded({ name: 'created' });
+          return ownerInstance.embeddedList.create(item).then(function(it) {
+            existingItem = it;
+          });
+        })
+        .then(function() {
+          hookMonitor.resetNames();
+        });
+    });
+
+    function callUpdate() {
+      // Unfortunately, updateById was not promisified yet
+      return new Promise(function(resolve, reject) {
+        ownerInstance.embeddedList.updateById(
+          existingItem.id,
+          { name: 'updated' },
+          function(err, result) {
+            if (err) reject(err);
+            else resolve(result);
+          });
+      });
+    }
+
+    it('triggers hooks in the correct order', function() {
+      return callUpdate().then(function(result) {
+        hookMonitor.names.should.eql([
+          'Embedded:before save',
+          //TODO 'Embedded:persist',
+          'Owner:before save',
+          'Owner:persist',
+          'Owner:loaded',
+          'Owner:after save',
+          //TODO 'Embedded:loaded',
+          'Embedded:after save',
+        ]);
+      });
+    });
+
+    it('trigers `before save` hook on embedded model', function() {
+      Embedded.observe('before save', ctxRecorder.recordAndNext());
+      return callUpdate().then(function(instance) {
+        ctxRecorder.records.should.eql(aCtxForModel(Embedded, {
+          currentInstance: {
+            id: instance.id,
+            name: 'created',
+            extra: undefined,
+          },
+          data: {
+            name: 'updated',
+          },
+          // TODO isNewInstance: true,
+        }));
+      });
+    });
+
+    // TODO
+    it('trigers `before save` hook on owner model');
+
+    it('applies updates from `before save` hook', function() {
+      Embedded.observe('before save', function(ctx, next) {
+        ctx.data.extra = 'hook data';
+        next();
+      });
+      return callUpdate().then(function(instance) {
+        instance.should.have.property('extra', 'hook data');
+      });
+    });
+
+    it('validates model after `before save` hook', function() {
+      Embedded.observe('before save', invalidateEmbeddedModel);
+      return callUpdate().then(throwShouldHaveFailed, function(err) {
+        err.should.be.instanceOf(ValidationError);
+        (err.details.codes || {}).should.eql({ name: ['presence'] });
+      });
+    });
+
+    it('aborts when `before save` hook fails', function() {
+      Embedded.observe('before save', nextWithError(expectedError));
+      return callUpdate().then(throwShouldHaveFailed, function(err) {
+        err.should.eql(expectedError);
+      });
+    });
+
+    // TODO
+    it('triggers `persist` hook on embedded model');
+    it('triggers `persist` hook on owner model');
+    it('applies updates from `persist` hook');
+    it('aborts when `persist` hook fails');
+
+    // TODO
+    it('triggers `loaded` hook on embedded model');
+    it('triggers `loaded` hook on owner model');
+    it('applies updates from `loaded` hook');
+    it('aborts when `loaded` hook fails');
+
+    it('triggers `after save` hook on embedded model', function() {
+      Embedded.observe('after save', ctxRecorder.recordAndNext());
+      return callUpdate().then(function(instance) {
+        ctxRecorder.records.should.eql(aCtxForModel(Embedded, {
+          instance: {
+            id: instance.id,
+            name: 'updated',
+            extra: undefined,
+          },
+          // TODO isNewInstance: true,
+        }));
+      });
+    });
+
+    // TODO
+    it('triggers `after save` hook on owner model');
+
+    it('applies updates from `after save` hook', function() {
+      Embedded.observe('after save', function(ctx, next) {
+        ctx.instance.should.be.instanceOf(Embedded);
+        ctx.instance.extra = 'hook data';
+        next();
+      });
+      return callUpdate().then(function(instance) {
+        instance.should.have.property('extra', 'hook data');
+      });
+    });
+
+    it('aborts when `after save` hook fails', function() {
+      Embedded.observe('after save', nextWithError(expectedError));
+      return callUpdate().then(throwShouldHaveFailed, function(err) {
+        err.should.eql(expectedError);
+      });
+    });
+
+    function invalidateEmbeddedModel(context, next) {
+      if (context.instance) {
+        context.instance.name = '';
+      } else {
+        context.data.name = '';
+      }
+      next();
+    }
+
+    function nextWithError(err) {
+      return function(context, next) {
+        next(err);
+      };
+    }
+
+    function throwShouldHaveFailed() {
+      throw new Error('operation should have failed');
+    }
+  });
+};