Merge pull request #108 from strongloop/feature/fix-mongo-upsert

Feature/fix mongo upsert

lib/connectors/memory.js

@@ -179,7 +179,14 @@ Memory.prototype.updateOrCreate = function (model, data, callback) {
 };
 
 Memory.prototype.save = function save(model, data, callback) {
-  this.cache[model][this.getIdValue(model, data)] = serialize(data);
+  var id = this.getIdValue(model, data);
+  var cachedModels = this.cache[model];
+  var modelData = cachedModels && this.cache[model][id];
+  modelData = modelData && deserialize(modelData);
+  if (modelData) {
+    data = merge(modelData, data);
+  }
+  this.cache[model][id] = serialize(data);
   this.saveToFile(data, callback);
 };
 
@@ -190,7 +197,6 @@ Memory.prototype.exists = function exists(model, id, callback) {
 };
 
 Memory.prototype.find = function find(model, id, callback) {
-  var self = this;
   process.nextTick(function () {
     callback(null, id in this.cache[model] && this.fromDb(model, this.cache[model][id]));
   }.bind(this));
@@ -393,10 +399,9 @@ Memory.prototype.updateAttributes = function updateAttributes(model, id, data, c
 
   var cachedModels = this.cache[model];
   var modelData = cachedModels && this.cache[model][id];
-  modelData = modelData && deserialize(modelData);
 
   if (modelData) {
-    this.save(model, merge(modelData, data), cb);
+    this.save(model, data, cb);
   } else {
     cb(new Error('Could not update attributes. Object with id ' + id + ' does not exist!'));
   }
@@ -416,9 +421,17 @@ Memory.prototype.buildNearFilter = function (filter) {
 }
 
 function merge(base, update) {
-  if (!base) return update;
+  if (!base) {
-  Object.keys(update).forEach(function (key) {
+    return update;
-    base[key] = update[key];
+  }
-  });
+  // We cannot use Object.keys(update) if the update is an instance of the model
+  // class as the properties are defined at the ModelClass.prototype level
+  for(var key in update) {
+    var val = update[key];
+    if(typeof val === 'function') {
+      continue; // Skip methods
+    }
+    base[key] = val;
+  }
   return base;
 }

lib/dao.js

@@ -105,9 +105,10 @@ DataAccessObject.create = function (data, callback) {
     var errors = Array(data.length);
     var gotError = false;
     var wait = data.length;
-    if (wait === 0) callback(null, []);
+    if (wait === 0) {
+      callback(null, []);
+    }
 
-    var instances = [];
     for (var i = 0; i < data.length; i += 1) {
       (function (d, i) {
         instances.push(Model.create(d, function (err, inst) {
@@ -214,25 +215,37 @@ function stillConnecting(dataSource, obj, args) {
  * @param {Function} callback The callback function (optional).
  */
 DataAccessObject.upsert = DataAccessObject.updateOrCreate = function upsert(data, callback) {
-  if (stillConnecting(this.getDataSource(), this, arguments)) return;
+  if (stillConnecting(this.getDataSource(), this, arguments)) {
+    return;
+  }
 
   var Model = this;
-  if (!getIdValue(this, data)) return this.create(data, callback);
+  if (!getIdValue(this, data)) {
+    return this.create(data, callback);
+  }
   if (this.getDataSource().connector.updateOrCreate) {
-    var inst = new Model(data);
+    var update = data;
-    this.getDataSource().connector.updateOrCreate(Model.modelName, inst.toObject(true), function (err, data) {
+    var inst = data;
+    if(!(data instanceof Model)) {
+      inst = new Model(data);
+    }
+    update = inst.toObject(false);
+    update = removeUndefined(update);
+    this.getDataSource().connector.updateOrCreate(Model.modelName, update, function (err, data) {
       var obj;
-      if (data) {
+      if (data && !(data instanceof Model)) {
         inst._initProperties(data);
         obj = inst;
       } else {
-        obj = null;
+        obj = data;
       }
       callback(err, obj);
     });
   } else {
     this.findById(getIdValue(this, data), function (err, inst) {
-      if (err) return callback(err);
+      if (err) {
+        return callback(err);
+      }
       if (inst) {
         inst.updateAttributes(data, callback);
       } else {
@@ -763,6 +776,7 @@ DataAccessObject.prototype.save = function (options, callback) {
   function save() {
     inst.trigger('save', function (saveDone) {
       inst.trigger('update', function (updateDone) {
+        data = removeUndefined(data);
         inst._adapter().save(modelName, inst.constructor._forDB(data), function (err) {
           if (err) {
             return callback(err, inst);

lib/model.js

@@ -14,7 +14,7 @@ var List = require('./list');
 var Hookable = require('./hooks');
 var validations = require('./validations.js');
 
-var BASE_TYPES = ['String', 'Boolean', 'Number', 'Date', 'Text'];
+var BASE_TYPES = ['String', 'Boolean', 'Number', 'Date', 'Text', 'ObjectID'];
 
 /**
  * Model class: base class for all persistent objects.
@@ -57,6 +57,10 @@ ModelBaseClass.prototype._initProperties = function (data, options) {
   var self = this;
   var ctor = this.constructor;
 
+  if(data instanceof ctor) {
+    // Convert the data to be plain object to avoid polutions
+    data = data.toObject(false);
+  }
   var properties = ctor.definition.build();
   data = data || {};
 
@@ -246,7 +250,12 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) {
   var schemaLess = (strict === false) || !onlySchema;
 
   this.constructor.forEachProperty(function (propertyName) {
-    if(removeHidden && Model.isHiddenProperty(propertyName)) return;
+    if (removeHidden && Model.isHiddenProperty(propertyName)) {
+      return;
+    }
+    if (typeof self[propertyName] === 'function') {
+      return;
+    }
     if (self[propertyName] instanceof List) {
       data[propertyName] = self[propertyName].toObject(!schemaLess, removeHidden);
     } else if (self.__data.hasOwnProperty(propertyName)) {
@@ -266,9 +275,14 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) {
     // If the property is not declared in the model definition, no setter will be
     // triggered to add it to __data
     for (var propertyName in self) {
-      if(removeHidden && Model.isHiddenProperty(propertyName)) continue;
+      if(removeHidden && Model.isHiddenProperty(propertyName)) {
+        continue;
+      }
       if(self.hasOwnProperty(propertyName) && (!data.hasOwnProperty(propertyName))) {
         val = self[propertyName];
+        if (typeof val === 'function') {
+          continue;
+        }
         if (val !== undefined && val !== null && val.toObject) {
           data[propertyName] = val.toObject(!schemaLess, removeHidden);
         } else {
@@ -279,8 +293,13 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) {
     // Now continue to check __data
     for (propertyName in self.__data) {
       if (!data.hasOwnProperty(propertyName)) {
-        if(removeHidden && Model.isHiddenProperty(propertyName)) continue;
+        if(removeHidden && Model.isHiddenProperty(propertyName)) {
+          continue;
+        }
         val = self.hasOwnProperty(propertyName) ? self[propertyName] : self.__data[propertyName];
+        if (typeof val === 'function') {
+          continue;
+        }
         if (val !== undefined && val !== null && val.toObject) {
           data[propertyName] = val.toObject(!schemaLess, removeHidden);
         } else {

test/loopback-dl.test.js

@@ -576,6 +576,152 @@ describe('Load models with base', function () {
   });
 });
 
+describe('Models attached to a dataSource', function() {
+  var Post;
+  before(function() {
+    var ds = new DataSource('memory');// define models
+    Post = ds.define('Post', {
+      title: { type: String, length: 255, index: true },
+      content: { type: String }
+    });
+  });
+
+  beforeEach(function(done) {
+    Post.destroyAll(done);
+  });
+
+  it('updateOrCreate should update the instance', function (done) {
+    Post.create({title: 'a', content: 'AAA'}, function (err, post) {
+      post.title = 'b';
+      Post.updateOrCreate(post, function (err, p) {
+        should.not.exist(err);
+        p.id.should.be.equal(post.id);
+        p.content.should.be.equal(post.content);
+        should.not.exist(p._id);
+
+        Post.findById(post.id, function (err, p) {
+          p.id.should.be.equal(post.id);
+          should.not.exist(p._id);
+          p.content.should.be.equal(post.content);
+          p.title.should.be.equal('b');
+
+          done();
+        });
+      });
+
+    });
+  });
+
+  it('updateOrCreate should update the instance without removing existing properties', function (done) {
+    Post.create({title: 'a', content: 'AAA'}, function (err, post) {
+      post = post.toObject();
+      delete post.title;
+      Post.updateOrCreate(post, function (err, p) {
+        should.not.exist(err);
+        p.id.should.be.equal(post.id);
+        p.content.should.be.equal(post.content);
+        should.not.exist(p._id);
+
+        Post.findById(post.id, function (err, p) {
+          p.id.should.be.equal(post.id);
+          should.not.exist(p._id);
+          p.content.should.be.equal(post.content);
+          p.title.should.be.equal('a');
+
+          done();
+        });
+      });
+
+    });
+  });
+
+  it('updateOrCreate should create a new instance if it does not exist', function (done) {
+    var post = {id: 123, title: 'a', content: 'AAA'};
+    Post.updateOrCreate(post, function (err, p) {
+      should.not.exist(err);
+      p.title.should.be.equal(post.title);
+      p.content.should.be.equal(post.content);
+      p.id.should.be.equal(post.id);
+
+      Post.findById(p.id, function (err, p) {
+        p.id.should.be.equal(post.id);
+        should.not.exist(p._id);
+        p.content.should.be.equal(post.content);
+        p.title.should.be.equal(post.title);
+        p.id.should.be.equal(post.id);
+
+        done();
+      });
+    });
+
+  });
+
+  it('save should update the instance with the same id', function (done) {
+    Post.create({title: 'a', content: 'AAA'}, function (err, post) {
+      post.title = 'b';
+      post.save(function (err, p) {
+        should.not.exist(err);
+        p.id.should.be.equal(post.id);
+        p.content.should.be.equal(post.content);
+        should.not.exist(p._id);
+
+        Post.findById(post.id, function (err, p) {
+          p.id.should.be.equal(post.id);
+          should.not.exist(p._id);
+          p.content.should.be.equal(post.content);
+          p.title.should.be.equal('b');
+
+          done();
+        });
+      });
+
+    });
+  });
+
+  it('save should update the instance without removing existing properties', function (done) {
+    Post.create({title: 'a', content: 'AAA'}, function (err, post) {
+      delete post.title;
+      post.save(function (err, p) {
+        should.not.exist(err);
+        p.id.should.be.equal(post.id);
+        p.content.should.be.equal(post.content);
+        should.not.exist(p._id);
+
+        Post.findById(post.id, function (err, p) {
+          p.id.should.be.equal(post.id);
+          should.not.exist(p._id);
+          p.content.should.be.equal(post.content);
+          p.title.should.be.equal('a');
+
+          done();
+        });
+      });
+
+    });
+  });
+
+  it('save should create a new instance if it does not exist', function (done) {
+    var post = new Post({id: '123', title: 'a', content: 'AAA'});
+    post.save(post, function (err, p) {
+      should.not.exist(err);
+      p.title.should.be.equal(post.title);
+      p.content.should.be.equal(post.content);
+      p.id.should.be.equal(post.id);
+
+      Post.findById(p.id, function (err, p) {
+        p.id.should.be.equal(post.id);
+        should.not.exist(p._id);
+        p.content.should.be.equal(post.content);
+        p.title.should.be.equal(post.title);
+        p.id.should.be.equal(post.id);
+
+        done();
+      });
+    });
+
+  });
+});
+
 describe('DataSource connector types', function() {
   it('should return an array of types', function() {
     var ds = new DataSource('memory');