From bec9142100cd2bfa1e3e028d9b547b9484434482 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 20 Apr 2016 13:52:06 +0200 Subject: [PATCH] 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 | 101 +++++++-- .../embeds-many-create.suite.js | 191 ++++++++++++++++ .../embeds-many-destroy.suite.js | 149 +++++++++++++ .../embeds-many-update-by-id.suite.js | 205 ++++++++++++++++++ 4 files changed, 622 insertions(+), 24 deletions(-) create mode 100644 test/operation-hooks.suite/embeds-many-create.suite.js create mode 100644 test/operation-hooks.suite/embeds-many-destroy.suite.js create mode 100644 test/operation-hooks.suite/embeds-many-update-by-id.suite.js diff --git a/lib/relation-definition.js b/lib/relation-definition.js index d52d3f4b..8982c2f4 100644 --- a/lib/relation-definition.js +++ b/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') { - inst.setAttributes(data); - } - var err = inst.isValid() ? null : new ValidationError(inst); - if (err && typeof cb === 'function') { - return process.nextTick(function() { - cb(err, inst); - }); - } + var hookState = {}; + var context = { + Model: modelTo, + currentInstance: inst, + data: data, + options: options, + hookState: hookState, + }; + modelTo.notifyObserversOf('before save', context, function(err) { + if (err) return cb && cb(err); - if (typeof cb === 'function') { - modelInstance.updateAttribute(propertyName, - embeddedList, options, function(err) { + inst.setAttributes(data); + + 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); - if (index > -1) embeddedList.splice(index, 1); - if (typeof cb === 'function') { + var context = { + Model: modelTo, + 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); - modelTo.emit('deleted', inst.id, inst.toJSON()); + if (err) return cb(err); + 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; diff --git a/test/operation-hooks.suite/embeds-many-create.suite.js b/test/operation-hooks.suite/embeds-many-create.suite.js new file mode 100644 index 00000000..53973fe3 --- /dev/null +++ b/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'); + } + }); +}; diff --git a/test/operation-hooks.suite/embeds-many-destroy.suite.js b/test/operation-hooks.suite/embeds-many-destroy.suite.js new file mode 100644 index 00000000..e4d08dd9 --- /dev/null +++ b/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'); + } + }); +}; diff --git a/test/operation-hooks.suite/embeds-many-update-by-id.suite.js b/test/operation-hooks.suite/embeds-many-update-by-id.suite.js new file mode 100644 index 00000000..5efae691 --- /dev/null +++ b/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'); + } + }); +};