From 550cfa2a86ef7813ad393c3b6f92eaefda4d4c6c Mon Sep 17 00:00:00 2001 From: Amir Jafarian Date: Wed, 20 Jan 2016 18:24:05 -0500 Subject: [PATCH] Refactor `updateAttributes` --- lib/dao.js | 228 +++++++++++++++++++++++++++++------------------------ 1 file changed, 126 insertions(+), 102 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index ffa47265..f17642b0 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -52,6 +52,48 @@ function getIdValue(m, data) { return data && data[idName(m)]; } +function copyData(from, to) { + for (var key in from) { + to[key] = from[key]; + } +} + +function convertDataPropertiesByType(data) { + var typedData = {}; + for (var key in data) { + // Convert the properties by type + typedData[key] = data[key]; + if (typeof typedData[key] === 'object' + && typedData[key] !== null + && typeof typedData[key].toObject === 'function') { + typedData[key] = typedData[key].toObject(); + } + } + return typedData; +} + +/** + * Apply strict check for model's data. + * Notice: Please note this method modifies `inst` when `strict` is `validate`. + */ +function applyStrictCheck(model, strict, data, inst, cb) { + var props = model.definition.properties; + var keys = Object.keys(data); + var result = {}, key; + for (var i = 0; i < keys.length; i++) { + key = keys[i]; + if (props[key]) { + result[key] = data[key]; + } else if (strict === 'throw') { + cb(new Error('Unknown property: ' + key)); + return; + } else if (strict === 'validate') { + inst.__unknownProperties.push(key); + } + } + cb(null, result); +} + function setIdValue(m, data, value) { if (data) { data[idName(m)] = value; @@ -2374,6 +2416,7 @@ DataAccessObject.prototype.unsetAttribute = function unsetAttribute(name, nullif * @param {Function} cb Callback function called with (err, instance) */ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, options, cb) { + var self = this; var connectionPromise = stillConnecting(this.getDataSource(), this, arguments); if (connectionPromise) { return connectionPromise; @@ -2452,126 +2495,107 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, op data = ctx.data; if (strict && !allowExtendedOperators) { - var props = Model.definition.properties; - var keys = Object.keys(data); - var result = {}, key; - for (var i = 0; i < keys.length; i++) { - key = keys[i]; - if (props[key]) { - result[key] = data[key]; - } else if (strict === 'throw') { - cb(new Error('Unknown property: ' + key)); - return; - } else if (strict === 'validate') { - inst.__unknownProperties.push(key); - } - } - data = removeUndefined(result); - } - - var doValidate = true; - if (options.validate === undefined) { - if (Model.settings.automaticValidation !== undefined) { - doValidate = Model.settings.automaticValidation; - } + applyStrictCheck(self.constructor, strict, data, inst, validateAndSave); } else { - doValidate = options.validate; + validateAndSave(null, data); } - // update instance's properties - inst.setAttributes(data); - - if (doValidate){ - inst.isValid(function (valid) { - if (!valid) { - cb(new ValidationError(inst), inst); - return; + function validateAndSave(err, data) { + if (err) return cb(err); + data = removeUndefined(data); + var doValidate = true; + if (options.validate === undefined) { + if (Model.settings.automaticValidation !== undefined) { + doValidate = Model.settings.automaticValidation; } + } else { + doValidate = options.validate; + } - triggerSave(); - }, data); - } else { - triggerSave(); - } + // update instance's properties + inst.setAttributes(data); - function triggerSave(){ - inst.trigger('save', function (saveDone) { - inst.trigger('update', function (done) { - var typedData = {}; - - for (var key in data) { - // Convert the properties by type - inst[key] = data[key]; - typedData[key] = inst[key]; - if (typeof typedData[key] === 'object' - && typedData[key] !== null - && typeof typedData[key].toObject === 'function') { - typedData[key] = typedData[key].toObject(); - } + if (doValidate){ + inst.isValid(function (valid) { + if (!valid) { + cb(new ValidationError(inst), inst); + return; } - context.data = typedData; + triggerSave(); + }, data); + } else { + triggerSave(); + } - function updateAttributesCallback(err) { - if (err) return cb(err); - var ctx = { - Model: Model, - data: context.data, - hookState: hookState, - options: options - }; - Model.notifyObserversOf('loaded', ctx, function(err) { + function triggerSave(){ + inst.trigger('save', function (saveDone) { + inst.trigger('update', function (done) { + copyData(data, inst); + var typedData = convertDataPropertiesByType(data); + context.data = typedData; + + function updateAttributesCallback(err) { if (err) return cb(err); + var ctx = { + Model: Model, + data: context.data, + hookState: hookState, + options: options + }; + Model.notifyObserversOf('loaded', ctx, function(err) { + if (err) return cb(err); - inst.__persisted = true; + inst.__persisted = true; - // By default, the instance passed to updateAttributes callback is NOT updated - // with the changes made through persist/loaded hooks. To preserve - // backwards compatibility, we introduced a new setting updateOnLoad, - // which if set, will apply these changes to the model instance too. - if(Model.settings.updateOnLoad) { - inst.setAttributes(ctx.data); - } - done.call(inst, function () { - saveDone.call(inst, function () { - if (err) return cb(err, inst); + // By default, the instance passed to updateAttributes callback is NOT updated + // with the changes made through persist/loaded hooks. To preserve + // backwards compatibility, we introduced a new setting updateOnLoad, + // which if set, will apply these changes to the model instance too. + if(Model.settings.updateOnLoad) { + inst.setAttributes(ctx.data); + } + done.call(inst, function () { + saveDone.call(inst, function () { + if (err) return cb(err, inst); - var context = { - Model: Model, - instance: inst, - isNewInstance: false, - hookState: hookState, - options: options - }; - Model.notifyObserversOf('after save', context, function(err) { - if(!err) Model.emit('changed', inst); - cb(err, inst); + var context = { + Model: Model, + instance: inst, + isNewInstance: false, + hookState: hookState, + options: options + }; + Model.notifyObserversOf('after save', context, function(err) { + if(!err) Model.emit('changed', inst); + cb(err, inst); + }); }); }); }); - }); - } - - var ctx = { - Model: Model, - where: byIdQuery(Model, getIdValue(Model, inst)).where, - data: context.data, - currentInstance: inst, - isNewInstance: false, - hookState: hookState, - options: options - }; - Model.notifyObserversOf('persist', ctx, function(err) { - if (connector.updateAttributes.length === 5) { - connector.updateAttributes(model, getIdValue(inst.constructor, inst), - inst.constructor._forDB(context.data), options, updateAttributesCallback); - } else { - connector.updateAttributes(model, getIdValue(inst.constructor, inst), - inst.constructor._forDB(context.data), updateAttributesCallback); } - }); + + var ctx = { + Model: Model, + where: byIdQuery(Model, getIdValue(Model, inst)).where, + data: context.data, + currentInstance: inst, + isNewInstance: false, + hookState: hookState, + options: options + }; + Model.notifyObserversOf('persist', ctx, function(err) { + if (connector.updateAttributes.length === 5) { + connector.updateAttributes(model, getIdValue(inst.constructor, inst), + inst.constructor._forDB(context.data), options, updateAttributesCallback); + } else { + connector.updateAttributes(model, getIdValue(inst.constructor, inst), + inst.constructor._forDB(context.data), updateAttributesCallback); + } + }); + }, data, cb); }, data, cb); - }, data, cb); + } } }); return cb.promise;