From a765ea31ab8c3c24be6be8fd22b1149a5b7c5f93 Mon Sep 17 00:00:00 2001 From: Rand McKinney Date: Tue, 6 May 2014 14:18:10 -0700 Subject: [PATCH 1/7] Correct JSDoc for discoverModelDefinitions --- lib/datasource.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index 283f4f6e..e20d481e 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -718,7 +718,7 @@ DataSource.prototype.defineProperty = function (model, prop, params) { * Drop each model table and re-create. * This method applies only to SQL connectors. * - * @param {String} Models to be migrated, if not present, apply to all models. This can also be an array of Strings. + * @param {String} model Model to migrate. If not present, apply to all models. Can also be an array of Strings. * @param {Function} cb Callback function. Optional. * * WARNING: Calling this function will cause all data to be lost! Use autoupdate if you need to preserve data. @@ -740,7 +740,7 @@ DataSource.prototype.automigrate = function (models, cb) { * Update existing database tables. * This method make sense only for sql connectors. * - * @param {String} Models to be migrated, if not present, apply to all models. This can also be an array of Strings. + * @param {String} model Model to migrate. If not present, apply to all models. Can also be an array of Strings. * @param {Function} [cb] The callback function */ DataSource.prototype.autoupdate = function (models, cb) { @@ -760,15 +760,16 @@ DataSource.prototype.autoupdate = function (models, cb) { * Discover existing database tables. * This method returns an array of model objects, including {type, name, onwer} * - * Kyes in options object: - * - * - all: true - Discovering all models, false - Discovering the models owned by the current user - * - views: true - Including views, false - only tables - * - limit: The page size - * - offset: The starting index - * * @param {Object} options The options * @param {Function} Callback function. Optional. + * @options {Object} options Discovery options. + * + * Keys in options object: + * + * @property all {Boolean} If true, discover all models; if false, discover only models owned by the current user. + * @property views {Boolean} If true, nclude views; if false, only tables. + * @property limit {Number} Page size + * @property offset {Number} Starting index * */ DataSource.prototype.discoverModelDefinitions = function (options, cb) { From cf75f55f73193f6373252e1d4728fee3435f3c73 Mon Sep 17 00:00:00 2001 From: crandmck Date: Wed, 7 May 2014 11:24:49 -0700 Subject: [PATCH 2/7] Fix JSDoc - remove newlines from function alias declarations, etc. --- lib/dao.js | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 3a4ebfed..267d609e 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -209,6 +209,7 @@ function stillConnecting(dataSource, obj, args) { /** * Update or insert a model instance. + * `updateOrCreate` is an alias * @param {Object} data The model instance data * @param {Function} callback The callback function (optional). */ @@ -470,7 +471,6 @@ DataAccessObject._coerce = function (where) { * - limit: Number * - skip: Number * - * @param {Object} params (optional) * @param {Function} callback (required) called with two arguments: err (null or Error), array of instances */ @@ -628,9 +628,7 @@ setRemoting(DataAccessObject.findOne, { * @param {Object} [where] An object that defines the criteria * @param {Function} [cb] Callback called with (err) */ -DataAccessObject.remove = - DataAccessObject.deleteAll = - DataAccessObject.destroyAll = function destroyAll(where, cb) { +DataAccessObject.remove = DataAccessObject.deleteAll = DataAccessObject.destroyAll = function destroyAll(where, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; var Model = this; @@ -655,13 +653,13 @@ DataAccessObject.remove = }; /** - * Destroy a record by id + * Delete the record with the specified ID. + * Aliases are `destroyById` and `deleteById`. * @param {*} id The id value * @param {Function} cb Callback called with (err) */ -DataAccessObject.removeById = - DataAccessObject.deleteById = - DataAccessObject.destroyById = function deleteById(id, cb) { + +DataAccessObject.removeById = DataAccessObject.deleteById = DataAccessObject.destroyById = function deleteById(id, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; var Model = this; @@ -708,10 +706,12 @@ setRemoting(DataAccessObject.count, { }); /** - * Save instance. When instance haven't id, create method called instead. - * Triggers: validate, save, update | create - * @param options {validate: true, throws: false} [optional] - * @param callback(err, obj) + * Save instance. If the instance does not have an ID, call `create` instead. + * Triggers: validate, save, update or create. + * @options {Object} options Optional options to use. + * @property {Boolean} validate Default is true. + * @property {Boolean} throws Default is false. + * @param {Function} callback Callback function with err and object arguments */ DataAccessObject.prototype.save = function (options, callback) { if (stillConnecting(this.getDataSource(), this, arguments)) return; @@ -821,9 +821,8 @@ DataAccessObject.prototype.remove = }; /** - * Update single attribute - * - * equals to `updateAttributes({name: value}, cb) + * Update a single attribute. + * Equivalent to `updateAttributes({name: value}, cb)` * * @param {String} name Name of property * @param {Mixed} value Value of property @@ -836,9 +835,8 @@ DataAccessObject.prototype.updateAttribute = function updateAttribute(name, valu }; /** - * Update set of attributes - * - * this method performs validation before updating + * Update saet of attributes. + * Performs validation before updating. * * @trigger `validation`, `save` and `update` hooks * @param {Object} data Data to update From 8b128b566d2e89d130481a07a7a5217a02cb857d Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 8 May 2014 15:46:14 -0700 Subject: [PATCH 3/7] 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 4/7] 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 5/7] 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 6/7] 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'); From 4b93adfbe9410b0228a1cb6b2eaf2e285d2b4431 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 13 May 2014 08:34:44 -0700 Subject: [PATCH 7/7] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ce13aaf0..4f456189 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "1.3.12", + "version": "1.3.13", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop",