From c995f69e8c93ed3c65c79e48807aae422afc6919 Mon Sep 17 00:00:00 2001 From: Amir Jafarian Date: Tue, 19 Apr 2016 15:15:38 -0400 Subject: [PATCH 1/6] Implement patchById --- lib/dao.js | 13 +- lib/patch-by-id.js | 143 ++++++++++++ test/dao.suite/index.js | 17 ++ test/dao.suite/patch-by-id.suite.js | 140 ++++++++++++ test/manipulation.test.js | 116 +++++++++- .../dao-patch-by-id.suite.js | 211 ++++++++++++++++++ test/persistence-hooks.suite.js | 178 +++++++++++++++ 7 files changed, 811 insertions(+), 7 deletions(-) create mode 100644 lib/patch-by-id.js create mode 100644 test/dao.suite/index.js create mode 100644 test/dao.suite/patch-by-id.suite.js create mode 100644 test/operation-hooks.suite/dao-patch-by-id.suite.js diff --git a/lib/dao.js b/lib/dao.js index 73a85e774..1d39a1fcd 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)]; @@ -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..917dad8d5 --- /dev/null +++ b/lib/patch-by-id.js @@ -0,0 +1,143 @@ +// 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 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 = {}; + // 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 + if (data[idPropertyName] !== undefined && !idEquals(data[idPropertyName], id)) { + var err = new Error('id property (' + idPropertyName + ') ' + + 'cannot be updated from ' + id + ' to ' + data[idPropertyName]); + // return with 403 (forbidden) error status code + err.statusCode = 403; + process.nextTick(function() { + cb(err); + }); + return cb.promise; + } + // 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; + } + doUpdate(ctx.where, ctx.data); + }); + + 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; +}; 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..672ee1f45 --- /dev/null +++ b/test/dao.suite/patch-by-id.suite.js @@ -0,0 +1,140 @@ +// 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 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 updateAttributes', + function(done) { + Person.patchById(personId, { id: personId + 1, name: 'John' }, + function(err, info) { + should.exist(err); + done(); + }); + }); + + it.skip('should allow model instance on updateAttributes', 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..2f98de9f1 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(); @@ -727,6 +729,118 @@ describe('manipulation', function() { }); }); +// describe('patchById', function() { +// var personId; +// before(function(done) { +// Person.destroyAll(function() { +// Person.create({ name: 'Mary', age: 15 }, function(err, p) { +// if (err) return done(err); +// personId = p.id; +// person = p; +// 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, p) { +// 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 id value on patchById', function(done) { +// Person.patchById(personId, { id: personId, name: 'John' }, +// function(err, p) { +// 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, p) { +// 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 updateAttributes', +// function(done) { +// Person.patchById(personId, { id: personId + 1, name: 'John' }, +// function(err, p) { +// should.exist(err); +// done(); +// }); +// }); +// +// it.skip('should allow model instance on updateAttributes', 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, p) { +// 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(p) { +// return Person.findById(personId) +// .then(function(p) { +// p.name.should.equal('Jane'); +// p.age.should.equal(15); +// done(); +// }); +// }) +// .catch(done); +// }); +// }); + if (!getSchema().connector.replaceById) { describe.skip('replaceById - not implemented', function() {}); } else { 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..60f3c9066 --- /dev/null +++ b/test/operation-hooks.suite/dao-patch-by-id.suite.js @@ -0,0 +1,211 @@ +// 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 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('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); + }; + } + }); +}; diff --git a/test/persistence-hooks.suite.js b/test/persistence-hooks.suite.js index e8b0333fc..f2e798372 100644 --- a/test/persistence-hooks.suite.js +++ b/test/persistence-hooks.suite.js @@ -1178,6 +1178,184 @@ module.exports = function(dataSource, should, connectorCapabilities) { }); }); +// describe.skip('patchById', function() { +// var instanceId; +// +// beforeEach(function createTestData(done) { +// TestModel.create({ name: 'John', extra: 'extra info' }, function(err, instance) { +// if (err) return done(err); +// instanceId = instance.id; +// done(); +// }); +// }); +// +// it('triggers hooks in the correct order', function(done) { +// monitorHookExecution(); +// +// TestModel.patchById(instanceId, +// { name: 'changed' }, +// function(err, info) { +// if (err) return done(err); +// hookMonitor.names.should.eql([ +// 'before save', +// 'persist', +// 'loaded', +// '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('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()); +// +// existingInstance.name = 'changed'; +// TestModel.patchById(instanceId, { name: 'changed' }, function(err) { +// 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) { +// [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, instance) { +// if (err) return done(err); +// ctxRecorder.records.should.eql(aCtxForModel(TestModel, { +// isNewInstance: false, +// })); +// done(); +// }); +// }); +// +// }); + describe('PersistedModel.prototype.updateAttributes', function() { it('triggers hooks in the correct order', function(done) { monitorHookExecution(); From 0adbf16b024eba3d2e1ba8a8de00b81b7385fa80 Mon Sep 17 00:00:00 2001 From: Amir Jafarian Date: Mon, 25 Apr 2016 18:05:40 -0400 Subject: [PATCH 2/6] Add validation --- lib/patch-by-id.js | 54 +++++++++++++++---- test/dao.suite/patch-by-id.suite.js | 4 +- .../dao-patch-by-id.suite.js | 16 ++++++ 3 files changed, 61 insertions(+), 13 deletions(-) diff --git a/lib/patch-by-id.js b/lib/patch-by-id.js index 917dad8d5..471d977f6 100644 --- a/lib/patch-by-id.js +++ b/lib/patch-by-id.js @@ -3,6 +3,7 @@ // 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'); @@ -46,16 +47,8 @@ module.exports = function patchById(id, data, options, cb) { 'update() must be implemented by the connector'); // Make sure id cannot be changed - if (data[idPropertyName] !== undefined && !idEquals(data[idPropertyName], id)) { - var err = new Error('id property (' + idPropertyName + ') ' + - 'cannot be updated from ' + id + ' to ' + data[idPropertyName]); - // return with 403 (forbidden) error status code - err.statusCode = 403; - process.nextTick(function() { - cb(err); - }); - return cb.promise; - } + 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(); @@ -85,7 +78,29 @@ module.exports = function patchById(id, data, options, cb) { }); return cb.promise; } - doUpdate(ctx.where, ctx.data); + + 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) { @@ -141,3 +156,20 @@ module.exports = function patchById(id, data, options, cb) { } 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/patch-by-id.suite.js b/test/dao.suite/patch-by-id.suite.js index 672ee1f45..f8f9e4ed6 100644 --- a/test/dao.suite/patch-by-id.suite.js +++ b/test/dao.suite/patch-by-id.suite.js @@ -100,7 +100,7 @@ module.exports = function(dataSource, should, connectorCapabilities) { }); }); - it('should fail if an id value is to be changed on updateAttributes', + 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) { @@ -109,7 +109,7 @@ module.exports = function(dataSource, should, connectorCapabilities) { }); }); - it.skip('should allow model instance on updateAttributes', function(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 }), diff --git a/test/operation-hooks.suite/dao-patch-by-id.suite.js b/test/operation-hooks.suite/dao-patch-by-id.suite.js index 60f3c9066..52c52c8a4 100644 --- a/test/operation-hooks.suite/dao-patch-by-id.suite.js +++ b/test/operation-hooks.suite/dao-patch-by-id.suite.js @@ -3,6 +3,7 @@ // 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; @@ -98,6 +99,16 @@ module.exports = function(dataSource, should, connectorCapabilities) { }); }); + 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) { @@ -207,5 +218,10 @@ module.exports = function(dataSource, should, connectorCapabilities) { next(err); }; } + + function invalidateData(context, next) { + delete context.data.name; + next(); + } }); }; From 46ee720ee09bf8f7f10e9dd4d65615caf5835fb9 Mon Sep 17 00:00:00 2001 From: Amir Jafarian Date: Mon, 25 Apr 2016 19:29:26 -0400 Subject: [PATCH 3/6] Add strict check --- lib/dao.js | 4 +-- lib/patch-by-id.js | 49 +++++++++++++++++----------- test/dao.suite/patch-by-id.suite.js | 50 +++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 20 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 1d39a1fcd..a41688f2c 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -81,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; @@ -97,7 +97,7 @@ function applyStrictCheck(model, strict, data, inst, cb) { } } cb(null, result); -} +}; function setIdValue(m, data, value) { if (data) { diff --git a/lib/patch-by-id.js b/lib/patch-by-id.js index 471d977f6..af6e3b902 100644 --- a/lib/patch-by-id.js +++ b/lib/patch-by-id.js @@ -19,6 +19,8 @@ module.exports = function patchById(id, data, options, cb) { var Model = this; var where = {}; + // TODO: `this.__strict` is always undefined... + var strict = this.__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?) @@ -79,28 +81,39 @@ module.exports = function patchById(id, data, options, cb) { return cb.promise; } - if (options.validate === false) { - return doUpdate(ctx.where, ctx.data); + if (strict) { + var instInfo = { '__unknownProperties': [] }; + dao.applyStrictCheck(Model, strict, data, + instInfo, validateAndCallConnector); + } else { + validateAndCallConnector(); } - // 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); + function validateAndCallConnector(err) { + if (err) return cb(err); + if (options.validate === false) { + return doUpdate(ctx.where, ctx.data); + } - 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); + // 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) { diff --git a/test/dao.suite/patch-by-id.suite.js b/test/dao.suite/patch-by-id.suite.js index f8f9e4ed6..84d2e2ad9 100644 --- a/test/dao.suite/patch-by-id.suite.js +++ b/test/dao.suite/patch-by-id.suite.js @@ -68,6 +68,56 @@ module.exports = function(dataSource, should, connectorCapabilities) { }); }); + 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(); + }); + }); + }); + + // TODO: skiping for now; for some reason`var strict = this.__strict;` + // in patch-by-id.js is not working in `patch-by-id.js` + it.skip('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) { From 7e7275c56ce84822a88c18c5a37986db57ea93d8 Mon Sep 17 00:00:00 2001 From: Amir Jafarian Date: Tue, 26 Apr 2016 14:45:56 -0400 Subject: [PATCH 4/6] Remove commented codes --- test/manipulation.test.js | 112 -------------------- test/persistence-hooks.suite.js | 178 -------------------------------- 2 files changed, 290 deletions(-) diff --git a/test/manipulation.test.js b/test/manipulation.test.js index 2f98de9f1..33765d639 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -729,118 +729,6 @@ describe('manipulation', function() { }); }); -// describe('patchById', function() { -// var personId; -// before(function(done) { -// Person.destroyAll(function() { -// Person.create({ name: 'Mary', age: 15 }, function(err, p) { -// if (err) return done(err); -// personId = p.id; -// person = p; -// 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, p) { -// 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 id value on patchById', function(done) { -// Person.patchById(personId, { id: personId, name: 'John' }, -// function(err, p) { -// 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, p) { -// 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 updateAttributes', -// function(done) { -// Person.patchById(personId, { id: personId + 1, name: 'John' }, -// function(err, p) { -// should.exist(err); -// done(); -// }); -// }); -// -// it.skip('should allow model instance on updateAttributes', 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, p) { -// 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(p) { -// return Person.findById(personId) -// .then(function(p) { -// p.name.should.equal('Jane'); -// p.age.should.equal(15); -// done(); -// }); -// }) -// .catch(done); -// }); -// }); - if (!getSchema().connector.replaceById) { describe.skip('replaceById - not implemented', function() {}); } else { diff --git a/test/persistence-hooks.suite.js b/test/persistence-hooks.suite.js index f2e798372..e8b0333fc 100644 --- a/test/persistence-hooks.suite.js +++ b/test/persistence-hooks.suite.js @@ -1178,184 +1178,6 @@ module.exports = function(dataSource, should, connectorCapabilities) { }); }); -// describe.skip('patchById', function() { -// var instanceId; -// -// beforeEach(function createTestData(done) { -// TestModel.create({ name: 'John', extra: 'extra info' }, function(err, instance) { -// if (err) return done(err); -// instanceId = instance.id; -// done(); -// }); -// }); -// -// it('triggers hooks in the correct order', function(done) { -// monitorHookExecution(); -// -// TestModel.patchById(instanceId, -// { name: 'changed' }, -// function(err, info) { -// if (err) return done(err); -// hookMonitor.names.should.eql([ -// 'before save', -// 'persist', -// 'loaded', -// '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('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()); -// -// existingInstance.name = 'changed'; -// TestModel.patchById(instanceId, { name: 'changed' }, function(err) { -// 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) { -// [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, instance) { -// if (err) return done(err); -// ctxRecorder.records.should.eql(aCtxForModel(TestModel, { -// isNewInstance: false, -// })); -// done(); -// }); -// }); -// -// }); - describe('PersistedModel.prototype.updateAttributes', function() { it('triggers hooks in the correct order', function(done) { monitorHookExecution(); From bc398279dd78c3e40e7e2e7e4bd71528256d253f Mon Sep 17 00:00:00 2001 From: Amir Jafarian Date: Tue, 26 Apr 2016 15:37:25 -0400 Subject: [PATCH 5/6] Fix a bug --- lib/patch-by-id.js | 3 +-- test/dao.suite/patch-by-id.suite.js | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/patch-by-id.js b/lib/patch-by-id.js index af6e3b902..90879738c 100644 --- a/lib/patch-by-id.js +++ b/lib/patch-by-id.js @@ -19,8 +19,7 @@ module.exports = function patchById(id, data, options, cb) { var Model = this; var where = {}; - // TODO: `this.__strict` is always undefined... - var strict = this.__strict; + 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?) diff --git a/test/dao.suite/patch-by-id.suite.js b/test/dao.suite/patch-by-id.suite.js index 84d2e2ad9..e98dea624 100644 --- a/test/dao.suite/patch-by-id.suite.js +++ b/test/dao.suite/patch-by-id.suite.js @@ -83,7 +83,7 @@ module.exports = function(dataSource, should, connectorCapabilities) { // TODO: skiping for now; for some reason`var strict = this.__strict;` // in patch-by-id.js is not working in `patch-by-id.js` - it.skip('should throw error on unknown attributes when strict: throw', function(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' }, From baea1c5c1070c78c6185c46fcd4029908cca3fb4 Mon Sep 17 00:00:00 2001 From: Amir Jafarian Date: Tue, 26 Apr 2016 15:38:46 -0400 Subject: [PATCH 6/6] cleanup --- test/dao.suite/patch-by-id.suite.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/dao.suite/patch-by-id.suite.js b/test/dao.suite/patch-by-id.suite.js index e98dea624..4df0e92fe 100644 --- a/test/dao.suite/patch-by-id.suite.js +++ b/test/dao.suite/patch-by-id.suite.js @@ -81,8 +81,6 @@ module.exports = function(dataSource, should, connectorCapabilities) { }); }); - // TODO: skiping for now; for some reason`var strict = this.__strict;` - // in patch-by-id.js is not working in `patch-by-id.js` it('should throw error on unknown attributes when strict: throw', function(done) { Person.definition.settings.strict = 'throw'; Person.findById(personId, function(err, p) {