Merge pull request #1304 from strongloop/upsert-forceid

Fix forceId bug for updateOrCreate

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);
   }
 };
 

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 {

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,72 @@ describe('manipulation', function() {
     });
   });
 
+  bdd.describeIf(connectorCapabilities.supportForceId !== false,
+    '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('fails when id does not exist in db & validate is false when using updateAttributes',
+    function(done) {
+      var unknownId = uid.fromConnector(db) || 123;
+      var post = new Post({id: unknownId});
+      post.updateAttributes({title: 'updated title', content: 'AAA'}, {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 {