From f1d10b47cec29b5a4e56d417b5992e999d57011d Mon Sep 17 00:00:00 2001 From: Loay Date: Mon, 3 Apr 2017 16:00:31 -0400 Subject: [PATCH] Fix forceId bug for updateOrCreate --- lib/connectors/memory.js | 5 ++- lib/dao.js | 49 ++++++++++++++++++++-------- test/manipulation.test.js | 68 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 107 insertions(+), 15 deletions(-) diff --git a/lib/connectors/memory.js b/lib/connectors/memory.js index 01f012c2..d8f77561 100644 --- a/lib/connectors/memory.js +++ b/lib/connectors/memory.js @@ -815,7 +815,10 @@ Memory.prototype.updateAttributes = function updateAttributes(model, id, data, o if (modelData) { this.save(model, data, options, cb); } else { - cb(new Error(g.f('Could not update attributes. {{Object}} with {{id}} %s does not exist!', id))); + var msg = g.f('Could not update attributes. {{Object}} with {{id}} %s does not exist!', id); + var error = new Error(msg); + error.statusCode = error.status = 404; + cb(error); } }; diff --git a/lib/dao.js b/lib/dao.js index f542a46d..86752951 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -123,6 +123,13 @@ function isWhereByGivenId(Model, where, idValue) { return where[pk] === idValue; } +function errorModelNotFound(idValue) { + var msg = g.f('Could not update attributes. {{Object}} with {{id}} %s does not exist!', idValue); + var error = new Error(msg); + error.statusCode = error.status = 404; + return error; +} + DataAccessObject._forDB = function(data) { if (!(this.getDataSource().isRelational && this.getDataSource().isRelational())) { return data; @@ -509,6 +516,35 @@ DataAccessObject.upsert = function(data, options, cb) { if (id === undefined || id === null) { return this.create(data, options, cb); } + var doValidate = undefined; + if (options.validate === undefined) { + if (Model.settings.validateUpsert === undefined) { + if (Model.settings.automaticValidation !== undefined) { + doValidate = Model.settings.automaticValidation; + } + } else { + doValidate = Model.settings.validateUpsert; + } + } else { + doValidate = options.validate; + } + + var forceId = this.settings.forceId; + if (forceId) { + options = Object.create(options); + options.validate = !!doValidate; + if (doValidate) { + Model.findById(id, options, function(err, model) { + if (err) return cb(err); + if (!model) return cb(errorModelNotFound(id)); + model.updateAttributes(data, options, cb); + }); + } else { + const model = new Model({id: id}, {persisted: true}); + model.updateAttributes(data, options, cb); + } + return; + } var context = { Model: Model, @@ -546,19 +582,6 @@ DataAccessObject.upsert = function(data, options, cb) { var connector = self.getConnector(); - var doValidate = undefined; - if (options.validate === undefined) { - if (Model.settings.validateUpsert === undefined) { - if (Model.settings.automaticValidation !== undefined) { - doValidate = Model.settings.automaticValidation; - } - } else { - doValidate = Model.settings.validateUpsert; - } - } else { - doValidate = options.validate; - } - if (doValidate === false) { callConnector(); } else { diff --git a/test/manipulation.test.js b/test/manipulation.test.js index cc7e5d38..bee6cba7 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -752,7 +752,18 @@ describe('manipulation', function() { Todo = db.define('Todo', { content: String, }); - db.automigrate(['Post', 'Todo'], done); + // Here `Person` model overrides the one outside 'updataOrCreate' + // with forceId: false. Related test cleanup see issue: + // https://github.com/strongloop/loopback-datasource-juggler/issues/1317 + Person = db.define('Person', { + name: String, + gender: String, + married: Boolean, + age: {type: Number, index: true}, + dob: Date, + createdAt: {type: Date, default: Date}, + }, {forceId: false}); + db.automigrate(['Post', 'Todo', 'Person'], done); }); beforeEach(function deleteModelsInstances(done) { @@ -902,6 +913,61 @@ describe('manipulation', function() { }); }); + describe('updateOrCreate when forceId is true', function() { + var Post; + before(function definePostModel(done) { + var ds = getSchema(); + Post = ds.define('Post', { + title: {type: String, length: 255}, + content: {type: String}, + }, {forceId: true}); + ds.automigrate('Post', done); + }); + + it('fails when id does not exist in db & validate is true', function(done) { + var unknownId = uid.fromConnector(db) || 123; + var post = {id: unknownId, title: 'a', content: 'AAA'}; + Post.updateOrCreate(post, {validate: true}, (err) => { + err.statusCode.should.equal(404); + done(); + }); + }); + + it('fails when id does not exist in db & validate is false', function(done) { + var unknownId = uid.fromConnector(db) || 123; + var post = {id: unknownId, title: 'a', content: 'AAA'}; + Post.updateOrCreate(post, {validate: false}, (err) => { + err.statusCode.should.equal(404); + done(); + }); + }); + + it('works on create if the request does not include an id', function(done) { + var post = {title: 'a', content: 'AAA'}; + Post.updateOrCreate(post, (err, p) => { + if (err) return done(err); + p.title.should.equal(post.title); + p.content.should.equal(post.content); + done(); + }); + }); + + it('works on update if the request includes an existing id in db', function(done) { + Post.create({title: 'a', content: 'AAA'}, (err, post) => { + if (err) return done(err); + post = post.toObject(); + delete post.content; + post.title = 'b'; + Post.updateOrCreate(post, function(err, p) { + if (err) return done(err); + p.id.should.equal(post.id); + p.title.should.equal('b'); + done(); + }); + }); + }); + }); + if (!getSchema().connector.replaceById) { describe.skip('replaceById - not implemented', function() {}); } else {