From 8b128b566d2e89d130481a07a7a5217a02cb857d Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 8 May 2014 15:46:14 -0700 Subject: [PATCH 1/4] Make sure ObjectID type is not parsed as object --- lib/model.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/model.js b/lib/model.js index 3a3b3999..7d2face2 100644 --- a/lib/model.js +++ b/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. From 4a907b0a18a02ef45cc597730caaf83e7edd4618 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 8 May 2014 15:46:39 -0700 Subject: [PATCH 2/4] Remove the undefined property to avoid mongodb upsert overwrite --- lib/dao.js | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 267d609e..4cf85929 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -214,25 +214,33 @@ 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); - this.getDataSource().connector.updateOrCreate(Model.modelName, inst.toObject(true), function (err, data) { + var update = inst.toObject(true); + 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 { From 0bcbe6ceaed09bb7784b2407227e62c99b354412 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 9 May 2014 09:59:34 -0700 Subject: [PATCH 3/4] Remove undefined for the data to be saved --- lib/dao.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/dao.js b/lib/dao.js index 4cf85929..6f07907b 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -771,6 +771,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); From 3f410cae21d0e9500ec05d54791438e48efa303e Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 9 May 2014 15:27:45 -0700 Subject: [PATCH 4/4] Add test cases for updateOrCreate/save and fix related issues --- lib/connectors/memory.js | 29 +++++--- lib/dao.js | 13 ++-- lib/model.js | 25 ++++++- test/loopback-dl.test.js | 146 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 198 insertions(+), 15 deletions(-) diff --git a/lib/connectors/memory.js b/lib/connectors/memory.js index 0db0dc32..b4e07ae0 100644 --- a/lib/connectors/memory.js +++ b/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; - Object.keys(update).forEach(function (key) { - base[key] = update[key]; - }); + if (!base) { + return update; + } + // 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; } \ No newline at end of file diff --git a/lib/dao.js b/lib/dao.js index 6f07907b..185d1fa5 100644 --- a/lib/dao.js +++ b/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) { @@ -223,8 +224,12 @@ DataAccessObject.upsert = DataAccessObject.updateOrCreate = function upsert(data return this.create(data, callback); } if (this.getDataSource().connector.updateOrCreate) { - var inst = new Model(data); - var update = inst.toObject(true); + var update = 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; diff --git a/lib/model.js b/lib/model.js index 7d2face2..0e879517 100644 --- a/lib/model.js +++ b/lib/model.js @@ -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 { diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index 01cc0114..6a1e013d 100644 --- a/test/loopback-dl.test.js +++ b/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');