diff --git a/lib/dao.js b/lib/dao.js index 73a85e774..a41688f2c 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -29,6 +29,7 @@ var util = require('util'); var assert = require('assert'); var BaseModel = require('./model'); var debug = require('debug')('loopback:dao'); +DataAccessObject.patchById = require('./patch-by-id'); /** * Base class for all persistent objects. @@ -48,9 +49,9 @@ function DataAccessObject() { } } -function idName(m) { +var idName = module.exports.idName = function(m) { return m.definition.idName() || 'id'; -} +}; function getIdValue(m, data) { return data && data[idName(m)]; @@ -80,7 +81,7 @@ function convertSubsetOfPropertiesByType(inst, data) { * 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 applyStrictCheck = module.exports.applyStrictCheck = function(model, strict, data, inst, cb) { var props = model.definition.properties; var keys = Object.keys(data); var result = {}, key; @@ -96,7 +97,7 @@ function applyStrictCheck(model, strict, data, inst, cb) { } } cb(null, result); -} +}; function setIdValue(m, data, value) { if (data) { @@ -111,7 +112,7 @@ function byIdQuery(m, id) { return query; } -function isWhereByGivenId(Model, where, idValue) { +var isWhereByGivenId = module.exports.isWhereByGivenId = function(Model, where, idValue) { var keys = Object.keys(where); if (keys.length != 1) return false; @@ -119,7 +120,7 @@ function isWhereByGivenId(Model, where, idValue) { if (keys[0] !== pk) return false; return where[pk] === idValue; -} +}; DataAccessObject._forDB = function(data) { if (!(this.getDataSource().isRelational && this.getDataSource().isRelational())) { @@ -408,7 +409,7 @@ DataAccessObject.create = function(data, options, cb) { return cb.promise || obj; }; -function stillConnecting(dataSource, obj, args) { +var stillConnecting = module.exports.stillConnecting = function(dataSource, obj, args) { if (typeof args[args.length - 1] === 'function') { return dataSource.ready(obj, args); } @@ -423,7 +424,7 @@ function stillConnecting(dataSource, obj, args) { } else { return false; } -} +}; /** * Update or insert a model instance: update exiting record if one is found, such that parameter `data.id` matches `id` of model instance; diff --git a/lib/patch-by-id.js b/lib/patch-by-id.js new file mode 100644 index 000000000..90879738c --- /dev/null +++ b/lib/patch-by-id.js @@ -0,0 +1,187 @@ +// 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('./validations').ValidationError; +var assert = require('assert'); +var dao = require('./dao'); +var utils = require('./utils'); +var idEquals = utils.idEquals; +var removeUndefined = utils.removeUndefined; + +module.exports = function patchById(id, data, options, cb) { + var connectionPromise = dao.stillConnecting(this.getDataSource(), this, arguments); + if (connectionPromise) { + return connectionPromise; + } + assert(arguments.length >= 2, 'At least two arguments are required'); + + var Model = this; + var where = {}; + var strict = this.settings.strict; + // TODO: the methods which we have imported from dao (idName(), + // stillConnecting(), isWhereByGivenId(),...), may need to be + // moved to a more appropriate file (utils.js?) + var idPropertyName = dao.idName(Model); + + where[idPropertyName] = id; + if (cb === undefined) { + if (typeof options === 'function') { + // patchById(id, data, cb) + cb = options; + options = {}; + } + } + + data = data || {}; + cb = cb || utils.createPromiseCallback(); + 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 connector = Model.getDataSource().connector; + assert(typeof connector.update === 'function', + 'update() must be implemented by the connector'); + + // Make sure id cannot be changed + checkId(data[idPropertyName], id, idPropertyName, cb); + + // Question[1]: + // Please see QUESTION-test[1] in`test\dao.suit\patch-by-id.suits.js` for failing test + // data = data.toObject(); + // data = removeUndefined(data); + + var hookState = {}; + this.applyProperties(data); + + var context = { + Model: Model, + where: where, + data: data, + hookState: hookState, + options: options, + }; + + Model.notifyObserversOf('before save', context, function(err, ctx) { + if (err) return cb(err); + var isOriginalQuery = dao.isWhereByGivenId(Model, ctx.where, id); + + if (!isOriginalQuery) { + var err = new Error('ctx.where:' + JSON.stringify(ctx.where) + ' is not valid; the `' + + idPropertyName + '` original value is ' + id); + err.statusCode = 403; + process.nextTick(function() { + cb(err); + }); + return cb.promise; + } + + if (strict) { + var instInfo = { '__unknownProperties': [] }; + dao.applyStrictCheck(Model, strict, data, + instInfo, validateAndCallConnector); + } else { + validateAndCallConnector(); + } + + function validateAndCallConnector(err) { + if (err) return cb(err); + if (options.validate === false) { + return doUpdate(ctx.where, ctx.data); + } + + // only when options.validate is not set, take model-setting into consideration + if (options.validate === undefined && Model.settings.automaticValidation === false) { + return doUpdate(ctx.where, ctx.data); + } + + // Make sure id is not changed in the context of `before save` + checkId(ctx.data[idPropertyName], id, idPropertyName, cb); + + dataObj = new Model(ctx.data); + dataObj.unsetAttribute(idPropertyName); + // validation required + dataObj.isValid(function(valid) { + if (valid) { + doUpdate(ctx.where, ctx.data); + } else { + cb(new ValidationError(dataObj), dataObj); + } + }); + } + }); + + function doUpdate(where, data) { + try { + data = removeUndefined(data); + data = Model._coerce(data); + } catch (err) { + return process.nextTick(function() { + cb(err); + }); + } + + var ctx = { + Model: Model, + where: where, + data: data, + isNewInstance: false, + hookState: hookState, + options: options, + }; + Model.notifyObserversOf('persist', ctx, function(err) { + if (err) return cb (err); + if (connector.update.length === 5) { + connector.updateAttributes(Model.modelName, where[idPropertyName], ctx.data, options, patchCallback); + } else { + connector.updateAttributes(Model.modelName, where[idPropertyName], ctx.data, patchCallback); + } + }); + + function patchCallback(err, info) { + if (err) return cb(err); + var ctx = { + Model: Model, + data: data, + hookState: hookState, + options: options, + }; + Model.notifyObserversOf('loaded', ctx, function(err) { + if (err) return cb(err); + var context = { + Model: Model, + isNewInstance: false, + hookState: hookState, + options: options, + }; + Model.notifyObserversOf('after save', context, function(err) { + if (!err) Model.emit('changed', info); + // Just for consistency we return { count: 1} for all connectors + cb(err, { count: 1 }); + }); + }); + } + } + return cb.promise; +}; + +var idValidationFailure; +function checkId(idData, id, idPropertyName, cb) { + if (!idValidationFailure && idData !== undefined && !idEquals(idData, id)) { + idValidationFailure = true; + var err = new Error('id property (' + idPropertyName + ') ' + + 'cannot be updated from ' + id + ' to ' + idData); + // return with 403 (forbidden) error status code + err.statusCode = 403; + process.nextTick(function() { + cb(err); + }); + return cb.promise; + } else { + return false; + } +}; diff --git a/test/dao.suite/index.js b/test/dao.suite/index.js new file mode 100644 index 000000000..1b9fcab74 --- /dev/null +++ b/test/dao.suite/index.js @@ -0,0 +1,17 @@ +var debug = require('debug')('test'); +var fs = require('fs'); +var path = require('path'); + +module.exports = function(dataSource, should, connectorCapabilities) { + var operations = fs.readdirSync(__dirname); + operations = operations.filter(function(it) { + return it !== path.basename(__filename) && + !!require.extensions[path.extname(it).toLowerCase()]; + }); + for (var ix in operations) { + var name = operations[ix]; + var fullPath = require.resolve('./' + name); + debug('Loading test suite %s (%s)', name, fullPath); + require(fullPath).apply(this, arguments); + } +}; diff --git a/test/dao.suite/patch-by-id.suite.js b/test/dao.suite/patch-by-id.suite.js new file mode 100644 index 000000000..4df0e92fe --- /dev/null +++ b/test/dao.suite/patch-by-id.suite.js @@ -0,0 +1,188 @@ +// 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 + +module.exports = function(dataSource, should, connectorCapabilities) { + describe('patchById', function() { + var personId, Person, StubUser; + before(function setupDatabase(done) { + Person = dataSource.define('Person', { + name: String, + age: { type: Number, index: true }, + }, { forceId: true, strict: true }); + // A simplified implementation of LoopBack's User model + StubUser = dataSource.createModel('StubUser', { password: String }, { forceId: true }); + StubUser.setter.password = function(plain) { + var hashed = false; + if (!plain) return; + var pos = plain.indexOf('-'); + if (pos !== -1) { + var head = plain.substr(0, pos); + var tail = plain.substr(pos + 1, plain.length); + hashed = head.toUpperCase() === tail; + } + if (hashed) return; + this.$password = plain + '-' + plain.toUpperCase(); + }; + + dataSource.automigrate(['Person', 'StubUser'], done); + }); + + beforeEach(function setupData(done) { + Person.destroyAll(function() { + Person.create({ name: 'Mary', age: 15 }, function(err, p) { + if (err) return done(err); + personId = p.id; + done(); + }); + }); + }); + + // TODO: Please see loopback-datasource-juggler/issues#912 + it.skip('should have updated password hashed with patchById', + function(done) { + StubUser.create({ password: 'foo' }, function(err, created) { + if (err) return done(err); + StubUser.patchById(created.id, { 'password': 'test' }, function(err, info) { + if (err) return done(err); + StubUser.findById(created.id, function(err, found) { + if (err) return done(err); + found.password.should.equal('test-TEST'); + done(); + }); + }); + }); + }); + + it('should ignore undefined values on patchById', function(done) { + Person.patchById(personId, { 'name': 'John', age: undefined }, + function(err, info) { + if (err) return done(err); + Person.findById(personId, function(e, p) { + if (e) return done(e); + p.name.should.equal('John'); + p.age.should.equal(15); + done(); + }); + }); + }); + + it('should ignore unknown attributes when strict: true', function(done) { + Person.patchById(personId, { name: 'John', foo: 'bar' }, + function(err, p) { + if (err) return done(err); + should.not.exist(p.foo); + Person.findById(personId, function(e, p) { + if (e) return done(e); + should.not.exist(p.foo); + done(); + }); + }); + }); + + it('should throw error on unknown attributes when strict: throw', function(done) { + Person.definition.settings.strict = 'throw'; + Person.findById(personId, function(err, p) { + Person.patchById(personId, { foo: 'bar' }, + function(err, info) { + should.exist(err); + err.name.should.equal('Error'); + err.message.should.equal('Unknown property: foo'); + should.not.exist(info); + Person.findById(personId, function(e, p) { + if (e) return done(e); + should.not.exist(p.foo); + done(); + }); + }); + }); + }); + + it('should throw error on unknown attributes when strict: validate', function(done) { + Person.definition.settings.strict = 'validate'; + Person.findById(personId, function(err, p) { + Person.patchById(personId, { foo: 'bar' }, + function(err, info) { + should.exist(err); + err.name.should.equal('ValidationError'); + err.message.should.containEql('`foo` is not defined in the model'); + Person.findById(personId, function(e, p) { + if (e) return done(e); + should.not.exist(p.foo); + done(); + }); + }); + }); + }); + + it('should allow same id value on patchById', function(done) { + Person.patchById(personId, { id: personId, name: 'John' }, + function(err, info) { + if (err) return done(err); + Person.findById(personId, function(e, p) { + if (e) return done(e); + p.name.should.equal('John'); + p.age.should.equal(15); + done(); + }); + }); + }); + + it('should allow same stringified id value on patchById', + function(done) { + var pid = personId; + if (typeof pid === 'object' || typeof pid === 'number') { + // For example MongoDB ObjectId + pid = pid.toString(); + } + Person.patchById(pid, { id: pid, name: 'John' }, + function(err, info) { + if (err) return done(err); + Person.findById(personId, function(e, p) { + if (e) return done(e); + p.name.should.equal('John'); + p.age.should.equal(15); + done(); + }); + }); + }); + + it('should fail if an id value is to be changed on patchById', + function(done) { + Person.patchById(personId, { id: personId + 1, name: 'John' }, + function(err, info) { + should.exist(err); + done(); + }); + }); + + it.skip('should allow model instance on patchById', function(done) { + // QUESTION-test[1]: Can we pass undefined here and should we remove undefined in the method? + // Please see the other comment in patch-by-id.js + Person.patchById(personId, new Person({ 'name': 'John', age: undefined }), + function(err, info) { + if (err) return done(err); + Person.findById(personId, function(e, p) { + if (e) return done(e); + p.name.should.equal('John'); + p.age.should.equal(15); + done(); + }); + }); + }); + + it('should allow model instance on patchById (promise variant)', function(done) { + Person.patchById(personId, { 'name': 'Jane' }) + .then(function(info) { + return Person.findById(personId) + .then(function(p) { + p.name.should.equal('Jane'); + p.age.should.equal(15); + done(); + }); + }) + .catch(done); + }); + }); +}; diff --git a/test/manipulation.test.js b/test/manipulation.test.js index f2d21f6ea..33765d639 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -5,6 +5,7 @@ // This test written in mocha+should.js var async = require('async'); +var dao = require('./dao.suite/index.js'); var should = require('./init.js'); var db, Person; @@ -13,7 +14,8 @@ var ValidationError = require('..').ValidationError; var UUID_REGEXP = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; describe('manipulation', function() { - + db = getSchema(); + dao(db, should); before(function(done) { db = getSchema(); diff --git a/test/operation-hooks.suite/dao-patch-by-id.suite.js b/test/operation-hooks.suite/dao-patch-by-id.suite.js new file mode 100644 index 000000000..52c52c8a4 --- /dev/null +++ b/test/operation-hooks.suite/dao-patch-by-id.suite.js @@ -0,0 +1,227 @@ +// 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('patchById', function() { + var instanceId, TestModel, hookMonitor, ctxRecorder, expectedError; + beforeEach(function setupHelpers() { + ctxRecorder = new ContextRecorder('hook not called'); + hookMonitor = new HookMonitor({ includeModelName: true }); + expectedError = new Error('test error'); + }); + + beforeEach(function setupDatabase(done) { + TestModel = dataSource.createModel('TestModel', { + // 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 }, + }); + done(); + }); + + beforeEach(function setupData(done) { + TestModel.create({ name: 'John', extra: 'extra info' }, function(err, instance) { + if (err) return done(err); + instanceId = instance.id; + hookMonitor.install(TestModel); + done(); + }); + }); + + it('triggers hooks in the correct order', function(done) { + TestModel.patchById(instanceId, + { name: 'changed' }, + function(err, info) { + if (err) return done(err); + hookMonitor.names.should.eql([ + 'TestModel:before save', + 'TestModel:persist', + 'TestModel:loaded', + 'TestModel:after save', + ]); + done(); + }); + }); + + it('triggers `before save` hook', function(done) { + TestModel.observe('before save', ctxRecorder.recordAndNext()); + + TestModel.patchById(instanceId, { name: 'changed' }, function(err, info) { + if (err) return done(err); + ctxRecorder.records.should.eql(aCtxForModel(TestModel, { + where: { id: instanceId }, + data: { name: 'changed' }, + })); + done(); + }); + }); + + it('aborts when `before save` hook fails', function(done) { + TestModel.observe('before save', nextWithError(expectedError)); + + TestModel.patchById(instanceId, { name: 'changed' }, function(err, info) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('applies updates from `before save` hook', function(done) { + TestModel.observe('before save', function(ctx, next) { + ctx.data.extra = 'extra data'; + ctx.data.name = 'hooked name'; + next(); + }); + + TestModel.patchById(instanceId, { name: 'changed' }, function(err, info) { + if (err) return done(err); + TestModel.findById(instanceId, function(err, instance) { + if (err) return done(err); + should.exists(instance); + + instance.toObject(true).should.eql({ + id: instanceId, + name: 'hooked name', + extra: 'extra data', + }); + + done(); + }); + }); + }); + + it('validates model after `before save` hook', function(done) { + TestModel.observe('before save', invalidateData); + + TestModel.patchById(instanceId, { name: 'updated' }, function(err) { + err.should.be.instanceOf(ValidationError); + (err.details.codes || {}).should.eql({ name: ['presence'] }); + done(); + }); + }); + + it('triggers `persist` hook', function(done) { + TestModel.observe('persist', ctxRecorder.recordAndNext()); + TestModel.patchById(instanceId, { name: 'changed' }, function(err, info) { + if (err) return done(err); + + ctxRecorder.records.should.eql(aCtxForModel(TestModel, { + where: { id: instanceId }, + data: { name: 'changed' }, + isNewInstance: false, + })); + + done(); + }); + }); + + it('applies updates from `persist` hook', function(done) { + TestModel.observe('persist', ctxRecorder.recordAndNext(function(ctx) { + ctx.data.extra = 'hook data'; + })); + + TestModel.settings.updateOnLoad = true; + TestModel.patchById(instanceId, { name: 'changed' }, function(err, info) { + if (err) return done(err); + TestModel.findById(instanceId, function(err, instance) { + if (err) return done(err); + instance.should.have.property('extra', 'hook data'); + done(); + }); + }); + }); + + it('triggers `loaded` hook', function(done) { + TestModel.observe('loaded', ctxRecorder.recordAndNext()); + TestModel.patchById(instanceId, { name: 'changed' }, function(err, info) { + if (err) return done(err); + + ctxRecorder.records.should.eql(aCtxForModel(TestModel, { + data: { name: 'changed' }, + })); + + done(); + }); + }); + + it('emits error when `loaded` hook fails', function(done) { + TestModel.observe('loaded', nextWithError(expectedError)); + TestModel.patchById( + instanceId, + { name: 'changed' }, + function(err, info) { + [err].should.eql([expectedError]); + done(); + } + ); + }); + + it('applies updates from `loaded` hook', function(done) { + TestModel.observe('loaded', ctxRecorder.recordAndNext(function(ctx) { + ctx.data.extra = 'hook data'; + })); + + TestModel.patchById(instanceId, { name: 'changed' }, function(err, info) { + if (err) return done(err); + TestModel.findById(instanceId, function(err, instance) { + if (err) return done(err); + instance.should.have.property('extra', 'hook data'); + done(); + }); + }); + }); + + it('triggers `after save` hook', function(done) { + TestModel.observe('after save', ctxRecorder.recordAndNext()); + + TestModel.patchById(instanceId, { name: 'changed' }, function(err, info) { + if (err) return done(err); + ctxRecorder.records.should.eql(aCtxForModel(TestModel, { + isNewInstance: false, + })); + done(); + }); + }); + + it('aborts when `after save` hook fails', function(done) { + TestModel.observe('after save', nextWithError(expectedError)); + + TestModel.patchById(instanceId, { name: 'updated' }, function(err, info) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('applies updates from `after save` hook', function(done) { + TestModel.observe('after save', ctxRecorder.recordAndNext()); + + TestModel.patchById(instanceId, { name: 'updated' }, function(err, info) { + if (err) return done(err); + ctxRecorder.records.should.eql(aCtxForModel(TestModel, { + isNewInstance: false, + })); + done(); + }); + }); + + function nextWithError(err) { + return function(context, next) { + next(err); + }; + } + + function invalidateData(context, next) { + delete context.data.name; + next(); + } + }); +};