From 760ac97902e553393f48fc86a74a6b17570ee7d9 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 10 Feb 2015 23:57:05 -0800 Subject: [PATCH] Add an optional `options` argument to all CRUD methods --- lib/dao.js | 535 +++++++++++++++++++++++---------- test/crud-with-options.test.js | 441 +++++++++++++++++++++++++++ 2 files changed, 816 insertions(+), 160 deletions(-) create mode 100644 test/crud-with-options.test.js diff --git a/lib/dao.js b/lib/dao.js index f690a87b..9b1afbbb 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -23,6 +23,7 @@ var mergeQuery = utils.mergeQuery; var util = require('util'); var assert = require('assert'); var BaseModel = require('./model'); +var debug = require('debug')('loopback:dao'); /** * Base class for all persistent objects. @@ -123,6 +124,12 @@ DataAccessObject.lookupModel = function(data) { return this; }; +// Empty callback function +function noCallback(err, result) { + // NOOP + debug('callback is ignored: err=%j, result=%j', err, result); +} + /** * Create an instance of Model with given data and save to the attached data source. Callback is optional. * Example: @@ -134,30 +141,39 @@ DataAccessObject.lookupModel = function(data) { * Note: You must include a callback and use the created model provided in the callback if your code depends on your model being * saved or having an ID. * - * @param {Object} data Optional data object - * @param {Function} callback Callback function called with these arguments: + * @param {Object} [data] Optional data object + * @param {Object} [options] Options for create + * @param {Function} [cb] Callback function called with these arguments: * - err (null or Error) * - instance (null or Model) */ -DataAccessObject.create = function (data, callback) { +DataAccessObject.create = function (data, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; var Model = this; var self = this; - if (typeof data === 'function') { - callback = data; - data = {}; + if (options === undefined && cb === undefined) { + if (typeof data === 'function') { + // create(cb) + cb = data; + data = {}; + } + } else if (cb === undefined) { + if (typeof options === 'function') { + // create(data, cb); + cb = options; + options = {}; + } } - if (typeof callback !== 'function') { - callback = function () { - }; - } + cb = cb || noCallback; + data = data || {}; + options = options || {}; - if (!data) { - data = {}; - } + assert(typeof data === 'object', 'The data argument must be an object or array'); + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); if (Array.isArray(data)) { // Undefined item will be skipped by async.map() which internally uses @@ -169,13 +185,13 @@ DataAccessObject.create = function (data, callback) { } } async.map(data, function(item, done) { - self.create(item, function(err, result) { + self.create(item, options, function(err, result) { // Collect all errors and results done(null, {err: err, result: result || item}); }); }, function(err, results) { if (err) { - return callback && callback(err, results); + return cb(err, results); } // Convert the results into two arrays var errors = null; @@ -189,7 +205,7 @@ DataAccessObject.create = function (data, callback) { } data[i] = results[i].result; } - callback && callback(errors, data); + cb(errors, data); }); return data; } @@ -212,7 +228,7 @@ DataAccessObject.create = function (data, callback) { if (Model !== obj.constructor) obj = new Model(data); Model.notifyObserversOf('before save', { Model: Model, instance: obj }, function(err) { - if (err) return callback(err); + if (err) return cb(err); data = obj.toObject(true); @@ -221,7 +237,7 @@ DataAccessObject.create = function (data, callback) { if (valid) { create(); } else { - callback(new ValidationError(obj), obj); + cb(new ValidationError(obj), obj); } }, data); }); @@ -241,23 +257,23 @@ DataAccessObject.create = function (data, callback) { obj._rev = rev; } if (err) { - return callback(err, obj); + return cb(err, obj); } obj.__persisted = true; saveDone.call(obj, function () { createDone.call(obj, function () { if (err) { - return callback(err, obj); + return cb(err, obj); } Model.notifyObserversOf('after save', { Model: Model, instance: obj }, function(err) { - callback(err, obj); + cb(err, obj); if(!err) Model.emit('changed', obj); }); }); }); }, obj); - }, obj, callback); - }, obj, callback); + }, obj, cb); + }, obj, cb); } // for chaining @@ -275,32 +291,57 @@ function stillConnecting(dataSource, obj, args) { * NOTE: No setters, validations, or hooks are applied when using upsert. * `updateOrCreate` is an alias * @param {Object} data The model instance data - * @param {Function} callback The callback function (optional). + * @param {Object} [options] Options for upsert + * @param {Function} cb The callback function (optional). */ // [FIXME] rfeng: This is a hack to set up 'upsert' first so that // 'upsert' will be used as the name for strong-remoting to keep it backward // compatible for angular SDK -DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data, callback) { +DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) { return; } + + if (options === undefined && cb === undefined) { + if (typeof data === 'function') { + // upsert(cb) + cb = data; + data = {}; + } + } else if (cb === undefined) { + if (typeof options === 'function') { + // upsert(data, cb) + cb = options; + options = {}; + } + } + + cb = cb || noCallback; + data = data || {}; + options = options || {}; + + assert(typeof data === 'object', 'The data argument must be an object'); + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); + var self = this; var Model = this; + var id = getIdValue(this, data); if (!id) { - return this.create(data, callback); + return this.create(data, options, cb); } Model.notifyObserversOf('access', { Model: Model, query: byIdQuery(Model, id) }, doUpdateOrCreate); function doUpdateOrCreate(err, ctx) { - if (err) return callback(err); + if (err) return cb(err); var isOriginalQuery = isWhereByGivenId(Model, ctx.query.where, id) if (Model.getDataSource().connector.updateOrCreate && isOriginalQuery) { var context = { Model: Model, where: ctx.query.where, data: data }; Model.notifyObserversOf('before save', context, function(err, ctx) { - if (err) return callback(err); + if (err) return cb(err); data = ctx.data; var update = data; @@ -329,13 +370,13 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data obj = data; } if (err) { - callback(err, obj); + cb(err, obj); if(!err) { Model.emit('changed', inst); } } else { Model.notifyObserversOf('after save', { Model: Model, instance: obj }, function(err) { - callback(err, obj); + cb(err, obj); if(!err) { Model.emit('changed', inst); } @@ -346,7 +387,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data } else { Model.findOne({ where: ctx.query.where }, { notify: false }, function (err, inst) { if (err) { - return callback(err); + return cb(err); } if (!isOriginalQuery) { // The custom query returned from a hook may hide the fact that @@ -354,11 +395,11 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data delete data[idName(Model)]; } if (inst) { - inst.updateAttributes(data, callback); + inst.updateAttributes(data, options, cb); } else { Model = self.lookupModel(data); var obj = new Model(data); - obj.save(data, callback); + obj.save(options, cb); } }); } @@ -373,27 +414,44 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data * @param {Object} query Search conditions. See [find](#dataaccessobjectfindquery-callback) for query format. * For example: `{where: {test: 'me'}}`. * @param {Object} data Object to create. + * @param {Object} [options] Option for findOrCreate * @param {Function} cb Callback called with (err, instance, created) */ -DataAccessObject.findOrCreate = function findOrCreate(query, data, callback) { - if (query === undefined) { - query = {where: {}}; - } - if (typeof data === 'function' || typeof data === 'undefined') { - callback = data; - data = query && query.where; - } - if (typeof callback === 'undefined') { - callback = function () { - }; +DataAccessObject.findOrCreate = function findOrCreate(query, data, options, cb) { + + assert(arguments.length >= 2, 'At least two arguments are required'); + if (options === undefined && cb === undefined) { + if (typeof data === 'function') { + // findOrCreate(data, cb); + // query will be built from data + cb = data; + data = query; + query = {where: data}; + } + } else if (cb === undefined) { + if (typeof options === 'function') { + // findOrCreate(query, data, cb) + cb = options; + options = {}; + } } + cb = cb || noCallback; + query = query || {where: {}}; + data = data || {}; + options = options || {}; + + assert(typeof query === 'object', 'The query argument must be an object'); + assert(typeof data === 'object', 'The data argument must be an object'); + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); + var Model = this; Model.findOne(query, function (err, record) { - if (err) return callback(err); - if (record) return callback(null, record, false); - Model.create(data, function (err, record) { - callback(err, record, record != null); + if (err) return cb(err); + if (record) return cb(null, record, false); + Model.create(data, options, function (err, record) { + cb(err, record, record != null); }); }); }; @@ -402,17 +460,35 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, callback) { * Check whether a model instance exists in database * * @param {id} id Identifier of object (primary key value) + * @param {Object} [options] Options * @param {Function} cb Callback function called with (err, exists: Bool) */ -DataAccessObject.exists = function exists(id, cb) { +DataAccessObject.exists = function exists(id, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; + assert(arguments.length >= 1, 'The id argument is required'); + if (cb === undefined) { + if (typeof options === 'function') { + // exists(id, cb) + cb = options; + options = {}; + } + } + + cb = cb || noCallback; + options = options || {}; + + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); + if (id !== undefined && id !== null && id !== '') { - this.count(byIdQuery(this, id).where, function(err, count) { + this.count(byIdQuery(this, id).where, options, function(err, count) { cb(err, err ? false : count === 1); }); } else { - cb(new Error('Model::exists requires the id argument')); + return process.nextTick(function() { + cb(new Error('Model::exists requires the id argument')); + }); } }; @@ -427,19 +503,65 @@ DataAccessObject.exists = function exists(id, cb) { * ``` * * @param {*} id Primary key value + * @param {Object} [options] Options * @param {Function} cb Callback called with (err, instance) */ -DataAccessObject.findById = function find(id, cb) { +DataAccessObject.findById = function find(id, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; - this.findOne(byIdQuery(this, id), cb); + + assert(arguments.length >= 1, 'The id argument is required'); + if (cb === undefined) { + if (typeof options === 'function') { + // findById(id, cb) + cb = options; + options = {}; + } + } + cb = cb || noCallback; + options = options || {}; + + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); + + if (id == null || id === '') { + return process.nextTick(function() { + cb(new Error('Model::findById requires the id argument')); + }); + } + this.findOne(byIdQuery(this, id), options, cb); }; -DataAccessObject.findByIds = function(ids, cond, cb) { - if (typeof cond === 'function') { - cb = cond; - cond = {}; +/** + * Find model instances by ids + * @param {Array} ids An array of ids + * @param {Object} query Query filter + * @param {Object} [options] Options + * @param {Function} cb Callback called with (err, instance) + */ +DataAccessObject.findByIds = function(ids, query, options, cb) { + if (options === undefined && cb === undefined) { + if (typeof query === 'function') { + // findByIds(ids, cb) + cb = query; + query = {}; + } + } else if (cb === undefined) { + if (typeof options === 'function') { + // findByIds(ids, query, cb) + cb = options; + options = {}; + } } + cb = cb || noCallback; + options = options || {}; + query = query || {}; + + assert(Array.isArray(ids), 'The ids argument must be an array'); + assert(typeof query === 'object', 'The query argument must be an object'); + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); + var pk = idName(this); if (ids.length === 0) { process.nextTick(function() { cb(null, []); }); @@ -448,10 +570,10 @@ DataAccessObject.findByIds = function(ids, cond, cb) { var filter = { where: {} }; filter.where[pk] = { inq: [].concat(ids) }; - mergeQuery(filter, cond || {}); - this.find(filter, function(err, results) { + mergeQuery(filter, query || {}); + this.find(filter, options, function(err, results) { cb(err, err ? results : utils.sortObjectsByIds(pk, ids, results)); - }.bind(this)); + }); }; function convertNullToNotFoundError(ctx, cb) { @@ -784,34 +906,41 @@ DataAccessObject._coerce = function (where) { * - `{foo: true}` - include only foo * - `{bat: false}` - include all properties, exclude bat * - * @param {Function} callback Required callback function. Call this function with two arguments: `err` (null or Error) and an array of instances. + * @param {Function} cb Required callback function. Call this function with two arguments: `err` (null or Error) and an array of instances. */ DataAccessObject.find = function find(query, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; - if (arguments.length === 1) { - cb = query; - query = null; - options = {}; + if (options === undefined && cb === undefined) { + if (typeof query === 'function') { + // find(cb); + cb = query; + query = {}; + } + } else if (cb === undefined) { + if (typeof options === 'function') { + // find(query, cb); + cb = options; + options = {}; + } } - if (cb === undefined && typeof options === 'function') { - cb = options; - options = {}; - } + cb = cb || noCallback; + query = query || {}; + options = options || {}; - if (!options) options = {}; + assert(typeof query === 'object', 'The query argument must be an object'); + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); var self = this; - query = query || {}; - try { this._normalize(query); } catch (err) { return process.nextTick(function () { - cb && cb(err); + cb(err); }); } @@ -913,7 +1042,6 @@ DataAccessObject.find = function find(query, options, cb) { cb(err, []); } - var self = this; if (options.notify === false) { self.getDataSource().connector.all(self.modelName, query, allCb); } else { @@ -928,24 +1056,34 @@ DataAccessObject.find = function find(query, options, cb) { /** * Find one record, same as `find`, but limited to one result. This function returns an object, not a collection. * - * @param {Object} query Sarch conditions. See [find](#dataaccessobjectfindquery-callback) for query format. + * @param {Object} query Search conditions. See [find](#dataaccessobjectfindquery-callback) for query format. * For example: `{where: {test: 'me'}}`. + * @param {Object} [options] Options * @param {Function} cb Callback function called with (err, instance) */ DataAccessObject.findOne = function findOne(query, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; - if (typeof query === 'function') { - cb = query; - query = {}; - } - - if (cb === undefined && typeof options === 'function') { - cb = options; - options = {}; + if (options === undefined && cb === undefined) { + if (typeof query === 'function') { + cb = query; + query = {}; + } + } else if (cb === undefined) { + if (typeof options === 'function') { + cb = options; + options = {}; + } } + cb = cb || noCallback; query = query || {}; + options = options || {}; + + assert(typeof query === 'object', 'The query argument must be an object'); + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); + query.limit = 1; this.find(query, options, function (err, collection) { if (err || !collection || !collection.length > 0) return cb(err, null); @@ -964,6 +1102,7 @@ DataAccessObject.findOne = function findOne(query, options, cb) { * ```` * * @param {Object} [where] Optional object that defines the criteria. This is a "where" object. Do NOT pass a filter object. + * @param {Object) [options] Options * @param {Function} [cb] Callback called with (err) */ DataAccessObject.remove = DataAccessObject.deleteAll = DataAccessObject.destroyAll = function destroyAll(where, options, cb) { @@ -971,17 +1110,25 @@ DataAccessObject.remove = DataAccessObject.deleteAll = DataAccessObject.destroyA var Model = this; - if (!cb && !options && 'function' === typeof where) { - cb = where; - where = undefined; + if (options === undefined && cb === undefined) { + if (typeof where === 'function') { + cb = where; + where = {}; + } + } else if (cb === undefined) { + if (typeof options === 'function') { + cb = options; + options = {}; + } } - if (!cb && typeof options === 'function') { - cb = options; - } + cb = cb || noCallback; + where = where || {}; + options = options || {}; - if (!cb) cb = function(){}; - if (!options) options = {}; + assert(typeof where === 'object', 'The where argument must be an object'); + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); var query = { where: where }; this.applyScope(query); @@ -1014,7 +1161,7 @@ DataAccessObject.remove = DataAccessObject.deleteAll = DataAccessObject.destroyA where = Model._coerce(where); } catch (err) { return process.nextTick(function() { - cb && cb(err); + cb(err); }); } @@ -1053,11 +1200,29 @@ function whereIsEmpty(where) { // [FIXME] rfeng: This is a hack to set up 'deleteById' first so that // 'deleteById' will be used as the name for strong-remoting to keep it backward // compatible for angular SDK -DataAccessObject.removeById = DataAccessObject.destroyById = DataAccessObject.deleteById = function deleteById(id, cb) { +DataAccessObject.removeById = DataAccessObject.destroyById = DataAccessObject.deleteById = function deleteById(id, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; + + assert(arguments.length >= 1, 'The id argument is required'); + if (cb === undefined) { + if (typeof options === 'function') { + // destroyById(id, cb) + cb = options; + options = {}; + } + } + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); + + if (id == null || id === '') { + return process.nextTick(function() { + cb(new Error('Model::deleteById requires the id argument')); + }); + } + var Model = this; - this.remove(byIdQuery(this, id).where, function(err) { + this.remove(byIdQuery(this, id).where, options, function(err) { if ('function' === typeof cb) { cb(err); } @@ -1076,16 +1241,34 @@ DataAccessObject.removeById = DataAccessObject.destroyById = DataAccessObject.de * ``` * * @param {Object} [where] Search conditions (optional) + * @param {Object} [options] Options * @param {Function} cb Callback, called with (err, count) */ -DataAccessObject.count = function (where, cb) { +DataAccessObject.count = function (where, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; - if (typeof where === 'function') { - cb = where; - where = null; + if (options === undefined && cb === undefined) { + if (typeof where === 'function') { + // count(cb) + cb = where; + where = {}; + } + } else if (cb === undefined) { + if (typeof options === 'function') { + // count(where, cb) + cb = options; + options = {}; + } } + cb = cb || noCallback; + where = where || {}; + options = options || {}; + + assert(typeof where === 'object', 'The where argument must be an object'); + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); + var query = { where: where }; this.applyScope(query); where = query.where; @@ -1095,7 +1278,7 @@ DataAccessObject.count = function (where, cb) { where = this._coerce(where); } catch (err) { return process.nextTick(function () { - cb && cb(err); + cb(err); }); } @@ -1113,37 +1296,39 @@ DataAccessObject.count = function (where, cb) { * @options {Object} options Optional options to use. * @property {Boolean} validate Default is true. * @property {Boolean} throws Default is false. - * @param {Function} callback Callback function with err and object arguments + * @param {Function} cb Callback function with err and object arguments */ -DataAccessObject.prototype.save = function (options, callback) { +DataAccessObject.prototype.save = function (options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; var Model = this.constructor; - if (typeof options == 'function') { - callback = options; + if (typeof options === 'function') { + cb = options; options = {}; } - callback = callback || function () { - }; + cb = cb || noCallback; options = options || {}; - if (!('validate' in options)) { + assert(typeof options === 'object', 'The options argument should be an object'); + assert(typeof cb === 'function', 'The cb argument should be a function'); + + if (options.validate === undefined) { options.validate = true; } - if (!('throws' in options)) { + if (options.throws === undefined) { options.throws = false; } if (this.isNewRecord()) { - return Model.create(this, callback); + return Model.create(this, cb); } var inst = this; var modelName = Model.modelName; Model.notifyObserversOf('before save', { Model: Model, instance: inst }, function(err) { - if (err) return callback(err); + if (err) return cb(err); var data = inst.toObject(true); Model.applyProperties(data, inst); @@ -1163,7 +1348,7 @@ DataAccessObject.prototype.save = function (options, callback) { if (options.throws) { throw err; } - callback(err, inst); + cb(err, inst); } }); @@ -1174,14 +1359,14 @@ DataAccessObject.prototype.save = function (options, callback) { data = removeUndefined(data); inst._adapter().save(modelName, inst.constructor._forDB(data), function (err) { if (err) { - return callback(err, inst); + return cb(err, inst); } inst._initProperties(data, { persisted: true }); Model.notifyObserversOf('after save', { Model: Model, instance: inst }, function(err) { - if (err) return callback(err, inst); + if (err) return cb(err, inst); updateDone.call(inst, function () { saveDone.call(inst, function () { - callback(err, inst); + cb(err, inst); if(!err) { Model.emit('changed', inst); } @@ -1189,8 +1374,8 @@ DataAccessObject.prototype.save = function (options, callback) { }); }); }); - }, data, callback); - }, data, callback); + }, data, cb); + }, data, cb); } }); }; @@ -1208,32 +1393,37 @@ DataAccessObject.prototype.save = function (options, callback) { * * @param {Object} [where] Search conditions (optional) * @param {Object} data Changes to be made + * @param {Object} [options] Options for update * @param {Function} cb Callback, called with (err, count) */ DataAccessObject.update = -DataAccessObject.updateAll = function (where, data, cb) { +DataAccessObject.updateAll = function (where, data, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; - if (arguments.length === 1) { - // update(data) is being called - data = where; - where = null; - cb = null; - } else if (arguments.length === 2) { + assert(arguments.length >= 2, 'At least two arguments are required'); + if (options === undefined && cb === undefined) { if (typeof data === 'function') { - // update(data, cb) is being called + // updateAll(data, cb); cb = data; data = where; - where = null; - } else { - // update(where, data) is being called - cb = null; + where = {}; + } + } else if (cb === undefined) { + if (typeof options === 'function') { + // updateAll(query, data, cb) + cb = options; + options = {}; } } - assert(typeof where === 'object', 'The where argument should be an object'); - assert(typeof data === 'object', 'The data argument should be an object'); - assert(cb === null || typeof cb === 'function', 'The cb argument should be a function'); + cb = cb || noCallback; + data = data || {}; + options = options || {}; + + assert(typeof where === 'object', 'The where argument must be an object'); + assert(typeof data === 'object', 'The data argument must be an object'); + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); var query = { where: where }; this.applyScope(query); @@ -1244,7 +1434,7 @@ DataAccessObject.updateAll = function (where, data, cb) { var Model = this; Model.notifyObserversOf('access', { Model: Model, query: { where: where } }, function(err, ctx) { - if (err) return cb && cb(err); + if (err) return cb(err); Model.notifyObserversOf( 'before save', { @@ -1253,7 +1443,7 @@ DataAccessObject.updateAll = function (where, data, cb) { data: data }, function(err, ctx) { - if (err) return cb && cb(err); + if (err) return cb(err); doUpdate(ctx.where, ctx.data); }); }); @@ -1265,13 +1455,13 @@ DataAccessObject.updateAll = function (where, data, cb) { where = Model._coerce(where); } catch (err) { return process.nextTick(function () { - cb && cb(err); + cb(err); }); } var connector = Model.getDataSource().connector; connector.update(Model.modelName, where, data, function(err, count) { - if (err) return cb && cb (err); + if (err) return cb (err); Model.notifyObserversOf( 'after save', { @@ -1280,7 +1470,7 @@ DataAccessObject.updateAll = function (where, data, cb) { data: data }, function(err, ctx) { - return cb && cb(err, count); + return cb(err, count); }); }); } @@ -1302,11 +1492,26 @@ DataAccessObject.prototype._adapter = function () { * Delete object from persistence * * Triggers `destroy` hook (async) before and after destroying object + * + * @param {Object} [options] Options for delete + * @param {Function} cb Callback */ DataAccessObject.prototype.remove = DataAccessObject.prototype.delete = - DataAccessObject.prototype.destroy = function (cb) { + DataAccessObject.prototype.destroy = function (options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; + + if (cb === undefined && typeof options === 'function') { + cb = options; + options = {}; + } + + cb = cb || noCallback; + options = options || {}; + + assert(typeof options === 'object', 'The options argument should be an object'); + assert(typeof cb === 'function', 'The cb argument should be a function'); + var self = this; var Model = this.constructor; var id = getIdValue(this.constructor, this); @@ -1331,9 +1536,9 @@ DataAccessObject.prototype.remove = // a simple 'delete model with the given id'. // We must switch to full query-based delete. Model.deleteAll(where, { notify: false }, function(err) { - if (err) return cb && cb(err); + if (err) return cb(err); Model.notifyObserversOf('after delete', { Model: Model, where: where }, function(err) { - cb && cb(err); + cb(err); if (!err) Model.emit('deleted', id); }); }); @@ -1348,7 +1553,7 @@ DataAccessObject.prototype.remove = destroyed(function () { Model.notifyObserversOf('after delete', { Model: Model, where: where }, function(err) { - cb && cb(err); + cb(err); if (!err) Model.emit('deleted', id); }); }); @@ -1374,12 +1579,12 @@ DataAccessObject.prototype.setAttribute = function setAttribute(name, value) { * * @param {String} name Name of property * @param {Mixed} value Value of property - * @param {Function} callback Callback function called with (err, instance) + * @param {Function} cb Callback function called with (err, instance) */ -DataAccessObject.prototype.updateAttribute = function updateAttribute(name, value, callback) { +DataAccessObject.prototype.updateAttribute = function updateAttribute(name, value, cb) { var data = {}; data[name] = value; - this.updateAttributes(data, callback); + this.updateAttributes(data, cb); }; /** @@ -1419,28 +1624,38 @@ DataAccessObject.prototype.unsetAttribute = function unsetAttribute(name, nullif * * @trigger `validation`, `save` and `update` hooks * @param {Object} data Data to update - * @param {Function} callback Callback function called with (err, instance) + * @param {Object} [options] Options for updateAttributes + * @param {Function} cb Callback function called with (err, instance) */ -DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb) { +DataAccessObject.prototype.updateAttributes = function updateAttributes(data, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; + if (options === undefined && cb === undefined) { + if (typeof data === 'function') { + // updateAttributes(cb) + cb = data; + data = undefined; + } + } else if (cb === undefined) { + if (typeof options === 'function') { + // updateAttributes(data, cb) + cb = options; + options = {}; + } + } + + cb = cb || noCallback; + options = options || {}; + + assert((typeof data === 'object') && (data !== null), + 'The data argument must be an object'); + assert(typeof options === 'object', 'The options argument must be an object'); + assert(typeof cb === 'function', 'The cb argument must be a function'); + var inst = this; var Model = this.constructor; var model = Model.modelName; - if (typeof data === 'function') { - cb = data; - data = null; - } - - if (!data) { - data = {}; - } - - if (!cb) { - cb = function() {}; - } - // Convert the data to be plain object so that update won't be confused if (data instanceof Model) { data = data.toObject(false); @@ -1503,15 +1718,15 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb /** * Reload object from persistence * Requires `id` member of `object` to be able to call `find` - * @param {Function} callback Called with (err, instance) arguments + * @param {Function} cb Called with (err, instance) arguments * @private */ -DataAccessObject.prototype.reload = function reload(callback) { +DataAccessObject.prototype.reload = function reload(cb) { if (stillConnecting(this.getDataSource(), this, arguments)) { return; } - this.constructor.findById(getIdValue(this.constructor, this), callback); + this.constructor.findById(getIdValue(this.constructor, this), cb); }; diff --git a/test/crud-with-options.test.js b/test/crud-with-options.test.js new file mode 100644 index 00000000..1314732d --- /dev/null +++ b/test/crud-with-options.test.js @@ -0,0 +1,441 @@ +// This test written in mocha+should.js +var should = require('./init.js'); +var async = require('async'); +var db, User, options; + +describe('crud-with-options', function () { + + before(function (done) { + db = getSchema(); + User = db.define('User', { + seq: {type: Number, index: true}, + name: {type: String, index: true, sort: true}, + email: {type: String, index: true}, + birthday: {type: Date, index: true}, + role: {type: String, index: true}, + order: {type: Number, index: true, sort: true}, + vip: {type: Boolean} + }); + options = {}; + + db.automigrate(done); + + }); + + describe('findById', function () { + + before(function (done) { + User.destroyAll(done); + }); + + it('should allow findById(id, options, cb)', function (done) { + User.findById(1, options, function (err, u) { + should.not.exist(u); + should.not.exist(err); + done(); + }); + }); + + it('should allow findById(id)', function () { + User.findById(1); + }); + + it('should allow findById(id, options)', function () { + User.findById(1, options); + }); + + it('should throw when invalid options are provided for findById', + function(done) { + (function() { + User.findById(1, '123', function(err, u) { + }); + }).should.throw('The options argument must be an object'); + done(); + }); + + it('should report an invalid id via callback for findById', + function(done) { + User.findById(undefined, {}, function(err, u) { + err.should.be.eql( + new Error('Model::findById requires the id argument')); + done(); + }); + }); + + it('should allow findById(id, options, cb) for a matching id', + function(done) { + User.create(function(err, u) { + should.not.exist(err); + should.exist(u.id); + User.findById(u.id, options, function(err, u) { + should.exist(u); + should.not.exist(err); + u.should.be.an.instanceOf(User); + done(); + }); + }); + }); + + }); + + describe('findByIds', function () { + + before(function(done) { + var people = [ + { id: 1, name: 'a', vip: true }, + { id: 2, name: 'b' }, + { id: 3, name: 'c' }, + { id: 4, name: 'd', vip: true }, + { id: 5, name: 'e' }, + { id: 6, name: 'f' } + ]; + // Use automigrate so that serial keys are 1-6 + db.automigrate(['User'], function(err) { + User.create(people, options, function(err, users) { + done(); + }); + }); + }); + + it('should allow findByIds(ids, cb)', function (done) { + User.findByIds([3, 2, 1], function (err, users) { + should.exist(users); + should.not.exist(err); + var names = users.map(function(u) { return u.name; }); + names.should.eql(['c', 'b', 'a']); + done(); + }); + }); + + it('should allow findByIds(ids, filter, options, cb)', + function(done) { + User.findByIds([4, 3, 2, 1], + { where: { vip: true } }, options, function(err, users) { + should.exist(users); + should.not.exist(err); + var names = users.map(function(u) { + return u.name; + }); + names.should.eql(['d', 'a']); + done(); + }); + }); + + }); + + describe('find', function () { + + before(seed); + + it('should allow find(cb)', function(done) { + User.find(function(err, users) { + should.exists(users); + should.not.exists(err); + users.should.have.lengthOf(6); + done(); + }); + }); + + it('should allow find(filter, cb)', function(done) { + User.find({limit: 3}, function(err, users) { + should.exists(users); + should.not.exists(err); + users.should.have.lengthOf(3); + done(); + }); + }); + + it('should allow find(filter, options, cb)', function(done) { + User.find({}, options, function(err, users) { + should.exists(users); + should.not.exists(err); + users.should.have.lengthOf(6); + done(); + }); + }); + + it('should allow find(filter, options)', function() { + User.find({limit: 3}, options); + }); + + it('should allow find(filter)', function() { + User.find({limit: 3}); + }); + + it('should skip trailing undefined args', function(done) { + User.find({limit: 3}, function(err, users) { + should.exists(users); + should.not.exists(err); + users.should.have.lengthOf(3); + done(); + }, undefined, undefined); + }); + + it('should throw on an invalid query arg', function() { + (function() { + User.find('invalid query', function(err, users) { + // noop + }); + }).should.throw('The query argument must be an object'); + }); + + it('should throw on an invalid options arg', function() { + (function() { + User.find({limit: 3}, 'invalid option', function(err, users) { + // noop + }); + }).should.throw('The options argument must be an object'); + }); + + it('should throw on an invalid cb arg', function() { + (function() { + User.find({limit: 3}, {}, 'invalid cb'); + }).should.throw('The cb argument must be a function'); + }); + + }); + + describe('count', function () { + + before(seed); + + it('should allow count(cb)', function (done) { + User.count(function (err, n) { + should.not.exist(err); + should.exist(n); + n.should.equal(6); + done(); + }); + }); + + it('should allow count(where, cb)', function (done) { + User.count({role: 'lead'}, function (err, n) { + should.not.exist(err); + should.exist(n); + n.should.equal(2); + done(); + }); + }); + + it('should allow count(where, options, cb)', function (done) { + User.count({role: 'lead'}, options, function (err, n) { + should.not.exist(err); + should.exist(n); + n.should.equal(2); + done(); + }); + }); + + }); + + describe('findOne', function () { + + before(seed); + + it('should allow findOne(cb)', function (done) { + User.find({order: 'id'}, function (err, users) { + User.findOne(function (e, u) { + should.not.exist(e); + should.exist(u); + u.id.toString().should.equal(users[0].id.toString()); + done(); + }); + }); + }); + + it('should allow findOne(filter, options, cb)', function (done) { + User.findOne({order: 'order'}, options, function (e, u) { + should.not.exist(e); + should.exist(u); + u.order.should.equal(1); + u.name.should.equal('Paul McCartney'); + done(); + }); + }); + + it('should allow findOne(filter, cb)', function (done) { + User.findOne({order: 'order'}, function (e, u) { + should.not.exist(e); + should.exist(u); + u.order.should.equal(1); + u.name.should.equal('Paul McCartney'); + done(); + }); + }); + + it('should allow trailing undefined args', function (done) { + User.findOne({order: 'order'}, function (e, u) { + should.not.exist(e); + should.exist(u); + u.order.should.equal(1); + u.name.should.equal('Paul McCartney'); + done(); + }, undefined); + }); + + }); + + describe('exists', function () { + + before(seed); + + it('should allow exists(id, cb)', function (done) { + User.findOne(function (e, u) { + User.exists(u.id, function (err, exists) { + should.not.exist(err); + should.exist(exists); + exists.should.be.ok; + done(); + }); + }); + }); + + it('should allow exists(id, options, cb)', function (done) { + User.destroyAll(function () { + User.exists(42, options, function (err, exists) { + should.not.exist(err); + exists.should.not.be.ok; + done(); + }); + }); + }); + + }); + + describe('destroyAll with options', function () { + + beforeEach(seed); + + it('should allow destroyAll(where, options, cb)', function (done) { + User.destroyAll({name: 'John Lennon'}, options, function (err) { + should.not.exist(err); + User.find({where: {name: 'John Lennon'}}, function (err, data) { + should.not.exist(err); + data.length.should.equal(0); + User.find({where: {name: 'Paul McCartney'}}, function (err, data) { + should.not.exist(err); + data.length.should.equal(1); + done(); + }); + }); + }); + }); + + it('should allow destroyAll(where, cb)', function (done) { + User.destroyAll({name: 'John Lennon'}, function (err) { + should.not.exist(err); + User.find({where: {name: 'John Lennon'}}, function (err, data) { + should.not.exist(err); + data.length.should.equal(0); + User.find({where: {name: 'Paul McCartney'}}, function (err, data) { + should.not.exist(err); + data.length.should.equal(1); + done(); + }); + }); + }); + }); + + it('should allow destroyAll(cb)', function (done) { + User.destroyAll(function (err) { + should.not.exist(err); + User.find({where: {name: 'John Lennon'}}, function (err, data) { + should.not.exist(err); + data.length.should.equal(0); + User.find({where: {name: 'Paul McCartney'}}, function (err, data) { + should.not.exist(err); + data.length.should.equal(0); + done(); + }); + }); + }); + }); + + }); + + describe('updateAll ', function () { + + beforeEach(seed); + + it('should allow updateAll(where, data, cb)', function (done) { + User.update({name: 'John Lennon'}, {name: 'John Smith'}, function (err) { + should.not.exist(err); + User.find({where: {name: 'John Lennon'}}, function (err, data) { + should.not.exist(err); + data.length.should.equal(0); + User.find({where: {name: 'John Smith'}}, function (err, data) { + should.not.exist(err); + data.length.should.equal(1); + done(); + }); + }); + }); + }); + + it('should allow updateAll(where, data, options, cb)', function(done) { + User.update({name: 'John Lennon'}, {name: 'John Smith'}, options, + function(err) { + should.not.exist(err); + User.find({where: {name: 'John Lennon'}}, function(err, data) { + should.not.exist(err); + data.length.should.equal(0); + User.find({where: {name: 'John Smith'}}, function(err, data) { + should.not.exist(err); + data.length.should.equal(1); + done(); + }); + }); + }); + }); + + it('should allow updateAll(data, cb)', function (done) { + User.update({name: 'John Smith'}, function () { + User.find({where: {name: 'John Lennon'}}, function (err, data) { + should.not.exist(err); + data.length.should.equal(0); + User.find({where: {name: 'John Smith'}}, function (err, data) { + should.not.exist(err); + data.length.should.equal(6); + done(); + }); + }); + }); + }); + + }); + +}); + +function seed(done) { + var beatles = [ + { + seq: 0, + name: 'John Lennon', + email: 'john@b3atl3s.co.uk', + role: 'lead', + birthday: new Date('1980-12-08'), + order: 2, + vip: true + }, + { + seq: 1, + name: 'Paul McCartney', + email: 'paul@b3atl3s.co.uk', + role: 'lead', + birthday: new Date('1942-06-18'), + order: 1, + vip: true + }, + {seq: 2, name: 'George Harrison', order: 5, vip: false}, + {seq: 3, name: 'Ringo Starr', order: 6, vip: false}, + {seq: 4, name: 'Pete Best', order: 4}, + {seq: 5, name: 'Stuart Sutcliffe', order: 3, vip: true} + ]; + + async.series([ + User.destroyAll.bind(User), + function(cb) { + async.each(beatles, User.create.bind(User), cb); + } + ], done); +}