From 89e4555bc022a63a30e3697266a04093e4a88afb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 18 Dec 2015 15:42:36 +0100 Subject: [PATCH 1/4] describe-operation-hooks: add "loaded" hook --- support/describe-operation-hooks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/support/describe-operation-hooks.js b/support/describe-operation-hooks.js index d9a30391..26f2a5d7 100644 --- a/support/describe-operation-hooks.js +++ b/support/describe-operation-hooks.js @@ -11,7 +11,7 @@ var Memory = require('../lib/connectors/memory').Memory; var HOOK_NAMES = [ 'access', - 'before save', 'persist', 'after save', + 'before save', 'persist', 'loaded', 'after save', 'before delete', 'after delete' ]; From 3028329126702456d59b42d6a88da17effe89919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 18 Dec 2015 15:43:20 +0100 Subject: [PATCH 2/4] "loaded" hook in DAO.find: ctx.data, not instance Fix the implementation od DAO.find to provide "ctx.data" to the "loaded" hook. --- lib/dao.js | 96 ++++++++++++++++++--------------- test/persistence-hooks.suite.js | 9 ++-- 2 files changed, 57 insertions(+), 48 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index f4929317..1e9f6952 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -266,7 +266,7 @@ DataAccessObject.create = function (data, options, cb) { if (options.validate === undefined && Model.settings.automaticValidation === false) { return create(); } - + // validation required obj.isValid(function (valid) { if (valid) { @@ -1455,56 +1455,64 @@ DataAccessObject.find = function find(query, options, cb) { async.each(data, function(item, callback) { var d = item;//data[i]; var Model = self.lookupModel(d); - var obj = new Model(d, {fields: query.fields, applySetters: false, persisted: true}); - if (query && query.include) { - if (query.collect) { - // The collect property indicates that the query is to return the - // standalone items for a related model, not as child of the parent object - // For example, article.tags - obj = obj.__cachedRelations[query.collect]; - if (obj === null) { - obj = undefined; - } - } else { - // This handles the case to return parent items including the related - // models. For example, Article.find({include: 'tags'}, ...); - // Try to normalize the include - var includes = Inclusion.normalizeInclude(query.include || []); - includes.forEach(function(inc) { - var relationName = inc; - if (utils.isPlainObject(inc)) { - relationName = Object.keys(inc)[0]; - } + context = { + Model: Model, + data: d, + isNewInstance: false, + hookState: hookState, + options: options + }; - // Promote the included model as a direct property - var included = obj.__cachedRelations[relationName]; - if (Array.isArray(included)) { - included = new List(included, null, obj); - } - if (included) obj.__data[relationName] = included; - }); - delete obj.__data.__cachedRelations; - } - } - if (obj !== undefined) { - context = { - Model: Model, - instance: obj, - isNewInstance: false, - hookState: hookState, - options: options + Model.notifyObserversOf('loaded', context, function(err) { + if (err) return callback(err); + d = context.data; + + var ctorOpts = { + fields: query.fields, + applySetters: false, + persisted: true }; + var obj = new Model(d, ctorOpts); - Model.notifyObserversOf('loaded', context, function(err) { - if (err) return callback(err); + if (query && query.include) { + if (query.collect) { + // The collect property indicates that the query is to return the + // standalone items for a related model, not as child of the parent object + // For example, article.tags + obj = obj.__cachedRelations[query.collect]; + if (obj === null) { + obj = undefined; + } + } else { + // This handles the case to return parent items including the related + // models. For example, Article.find({include: 'tags'}, ...); + // Try to normalize the include + var includes = Inclusion.normalizeInclude(query.include || []); + includes.forEach(function(inc) { + var relationName = inc; + if (utils.isPlainObject(inc)) { + relationName = Object.keys(inc)[0]; + } + // Promote the included model as a direct property + var included = obj.__cachedRelations[relationName]; + if (Array.isArray(included)) { + included = new List(included, null, obj); + } + if (included) obj.__data[relationName] = included; + }); + delete obj.__data.__cachedRelations; + } + } + + if (obj !== undefined) { results.push(obj); callback(); - }); - } else { - callback(); - } + } else { + callback(); + } + }); }, function(err) { if (err) return cb(err); diff --git a/test/persistence-hooks.suite.js b/test/persistence-hooks.suite.js index 5c7dce5f..a2881c04 100644 --- a/test/persistence-hooks.suite.js +++ b/test/persistence-hooks.suite.js @@ -129,7 +129,7 @@ module.exports = function(dataSource, should) { it('applies updates from `loaded` hook', function(done) { TestModel.observe('loaded', pushContextAndNext(function(ctx) { - ctx.instance.extra = 'hook data'; + ctx.data.extra = 'hook data'; })); TestModel.find( @@ -138,7 +138,7 @@ module.exports = function(dataSource, should) { if (err) return done(err); observedContexts.should.eql(aTestModelCtx({ - instance: { + data: { id: "1", name: "first", extra: "hook data" @@ -147,6 +147,8 @@ module.exports = function(dataSource, should) { hookState: { test: true }, options: {} })); + + list[0].should.have.property('extra', 'hook data'); done(); }); }) @@ -1747,10 +1749,9 @@ module.exports = function(dataSource, should) { // returns an array and NOT a single instance. observedContexts.should.eql([ aTestModelCtx({ - instance: { + data: { id: existingInstance.id, name: 'first', - extra: null }, isNewInstance: false, options: { notify: false } From fd9bef4aa78dbf4f3a6d7ccfb4b6badd64e47076 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 18 Dec 2015 15:59:19 +0100 Subject: [PATCH 3/4] Enhance "persisted" hook in DAO.updateAttributes Add `isNewInstance:false` to the context reported by DAO.updateAttributes() --- lib/dao.js | 1 + test/persistence-hooks.suite.js | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 1e9f6952..827cdd3e 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -2556,6 +2556,7 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, op where: byIdQuery(Model, getIdValue(Model, inst)).where, data: context.data, currentInstance: inst, + isNewInstance: false, hookState: hookState, options: options }; diff --git a/test/persistence-hooks.suite.js b/test/persistence-hooks.suite.js index a2881c04..897933d0 100644 --- a/test/persistence-hooks.suite.js +++ b/test/persistence-hooks.suite.js @@ -1212,7 +1212,8 @@ module.exports = function(dataSource, should) { id: existingInstance.id, name: 'changed', extra: null - } + }, + isNewInstance: false })); done(); @@ -1687,7 +1688,7 @@ module.exports = function(dataSource, should) { function(err, instance) { if (err) return done(err); - observedContexts.should.eql(aTestModelCtx({ + var expectedContext = aTestModelCtx({ where: { id: existingInstance.id }, data: { id: existingInstance.id, @@ -1698,7 +1699,15 @@ module.exports = function(dataSource, should) { name: 'updated name', extra: undefined } - })); + }); + + if (!dataSource.connector.updateOrCreate) { + // When the connector does not provide updateOrCreate, + // DAO falls back to updateAttributes which sets this flag + expectedContext.isNewInstance = false; + } + + observedContexts.should.eql(expectedContext); done(); }); }); From e9899a93cf3bbb95f99298da238d78d30a9cf992 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 18 Dec 2015 16:06:53 +0100 Subject: [PATCH 4/4] Enhance "persist" hook in DAO.updateOrCreate Report `ctx.isNewInstance` when the connector provides this info. --- lib/dao.js | 1 + test/persistence-hooks.suite.js | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 827cdd3e..70355dee 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -521,6 +521,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data var context = { Model: Model, data: data, + isNewInstance: info && info.isNewInstance, hookState: ctx.hookState, options: options }; diff --git a/test/persistence-hooks.suite.js b/test/persistence-hooks.suite.js index 897933d0..050a7fa2 100644 --- a/test/persistence-hooks.suite.js +++ b/test/persistence-hooks.suite.js @@ -1722,7 +1722,8 @@ module.exports = function(dataSource, should) { if (dataSource.connector.updateOrCreate) { observedContexts.should.eql(aTestModelCtx({ - data: { id: 'new-id', name: 'a name' } + data: { id: 'new-id', name: 'a name' }, + isNewInstance: true, })); } else { observedContexts.should.eql(aTestModelCtx({ @@ -1750,7 +1751,8 @@ module.exports = function(dataSource, should) { data: { id: existingInstance.id, name: 'updated name' - } + }, + isNewInstance: false })); } else { // For Unoptimized connector, the callback function `pushContextAndNext`