From 83c3a17f87ab65c61ac00758a6f5fe7416db2a62 Mon Sep 17 00:00:00 2001 From: Clark Wang Date: Mon, 19 Jan 2015 22:49:20 +0800 Subject: [PATCH 01/18] support embeds data for belongsTo relation Signed-off-by: Clark Wang --- lib/model.js | 11 +++++++++++ lib/relation-definition.js | 11 ++++++++++- test/relations.test.js | 39 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) diff --git a/lib/model.js b/lib/model.js index 543af043..773e2c59 100644 --- a/lib/model.js +++ b/lib/model.js @@ -13,6 +13,8 @@ var List = require('./list'); var Hookable = require('./hooks'); var validations = require('./validations'); var _extend = util._extend; +var utils = require('./utils'); +var fieldsToArray = utils.fieldsToArray; // Set up an object for quick lookup var BASE_TYPES = { @@ -170,7 +172,16 @@ ModelBaseClass.prototype._initProperties = function (data, options) { if (relationType === 'belongsTo' && propVal != null) { // If the related model is populated self.__data[ctor.relations[p].keyFrom] = propVal[ctor.relations[p].keyTo]; + + if (ctor.relations[p].options.embedsProperties) { + var fields = fieldsToArray(ctor.relations[p].properties, modelTo.definition.properties); + if (!~fields.indexOf(ctor.relations[p].keyTo)) { + fields.push(ctor.relations[p].keyTo); + } + self.__data[p] = new modelTo(propVal, { fields: fields, applySetters: false, persisted: options.persisted }); + } } + self.__cachedRelations[p] = propVal; } else { // Un-managed property diff --git a/lib/relation-definition.js b/lib/relation-definition.js index 504eb939..aae43be1 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -208,11 +208,20 @@ RelationDefinition.prototype.applyProperties = function(modelInstance, obj) { if (this.options.invertProperties) { source = obj, target = modelInstance; } + if (this.options.embedsProperties) { + target = target.__data[this.name] = {}; + target[this.keyTo] = source[this.keyTo]; + } if (typeof this.properties === 'function') { var data = this.properties.call(this, source); for(var k in data) { target[k] = data[k]; } + } else if (Array.isArray(this.properties)) { + for(var k = 0; k < this.properties.length; k++) { + var key = this.properties[k]; + target[key] = source[key]; + } } else if (typeof this.properties === 'object') { for(var k in this.properties) { var key = this.properties[k]; @@ -1309,7 +1318,7 @@ BelongsTo.prototype.related = function (refresh, params) { } var cb = params; - if (cachedValue === undefined) { + if (cachedValue === undefined || !(cachedValue instanceof ModelBaseClass)) { var query = {where: {}}; query.where[pk] = modelInstance[fk]; diff --git a/test/relations.test.js b/test/relations.test.js index 7d8bed84..8acc391a 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1718,6 +1718,45 @@ describe('relations', function () { }); + describe('belongsTo with embed', function () { + var Person, Passport; + + it('can be declared with embed and properties', function (done) { + Person = db.define('Person', {name: String, age: Number}); + Passport = db.define('Passport', {name: String, notes: String}); + Passport.belongsTo(Person, { + properties: ['name'], + options: { embedsProperties: true, invertProperties: true } + }); + db.automigrate(done); + }); + + it('should create record with embedded data', function (done) { + Person.create({name: 'Fred', age: 36 }, function(err, person) { + var p = new Passport({ name: 'Passport', notes: 'Some notes...' }); + p.person(person); + p.personId.should.equal(person.id); + var data = p.toObject(true); + data.person.id.should.equal(person.id); + data.person.name.should.equal('Fred'); + p.save(function (err) { + should.not.exists(err); + done(); + }); + }); + }); + + it('should find record with embedded data', function (done) { + Passport.findOne(function (err, p) { + should.not.exists(err); + var data = p.toObject(true); + data.person.id.should.equal(p.personId); + data.person.name.should.equal('Fred'); + done(); + }); + }); + }); + describe('hasOne', function () { var Supplier, Account; var supplierId, accountId; From d39f539413a760aa9bb5de58a9b45c2291ac75ad Mon Sep 17 00:00:00 2001 From: Christian Enevoldsen Date: Tue, 20 Jan 2015 20:58:52 +0100 Subject: [PATCH 02/18] adds support for protected properties. --- lib/list.js | 4 +-- lib/model.js | 46 ++++++++++++++++++++++++------- test/model-definition.test.js | 51 ++++++++++++++++++++++++++++++----- 3 files changed, 84 insertions(+), 17 deletions(-) diff --git a/lib/list.js b/lib/list.js index 27c2a534..93bcb755 100644 --- a/lib/list.js +++ b/lib/list.js @@ -72,11 +72,11 @@ List.prototype.push = function (obj) { return item; }; -List.prototype.toObject = function (onlySchema, removeHidden) { +List.prototype.toObject = function (onlySchema, removeHidden, removeProtected) { var items = []; this.forEach(function (item) { if (item && typeof item === 'object' && item.toObject) { - items.push(item.toObject(onlySchema, removeHidden)); + items.push(item.toObject(onlySchema, removeHidden, removeProtected)); } else { items.push(item); } diff --git a/lib/model.js b/lib/model.js index 773e2c59..cf44d678 100644 --- a/lib/model.js +++ b/lib/model.js @@ -302,7 +302,7 @@ ModelBaseClass.toString = function () { * * @param {Boolean} onlySchema Restrict properties to dataSource only. Default is false. If true, the function returns only properties defined in the schema; Otherwise it returns all enumerable properties. */ -ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) { +ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden, removeProtected) { if (onlySchema === undefined) { onlySchema = true; } @@ -334,11 +334,15 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) { continue; } + if (removeProtected && Model.isProtectedProperty(propertyName)) { + continue; + } + if (val instanceof List) { - data[propertyName] = val.toObject(!schemaLess, removeHidden); + data[propertyName] = val.toObject(!schemaLess, removeHidden, true); } else { if (val !== undefined && val !== null && val.toObject) { - data[propertyName] = val.toObject(!schemaLess, removeHidden); + data[propertyName] = val.toObject(!schemaLess, removeHidden, true); } else { data[propertyName] = val; } @@ -362,13 +366,16 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) { if (removeHidden && Model.isHiddenProperty(propertyName)) { continue; } + if (removeProtected && Model.isProtectedProperty(propertyName)) { + continue; + } val = self[propertyName]; if (val !== undefined && data[propertyName] === undefined) { if (typeof val === 'function') { continue; } if (val !== null && val.toObject) { - data[propertyName] = val.toObject(!schemaLess, removeHidden); + data[propertyName] = val.toObject(!schemaLess, removeHidden, true); } else { data[propertyName] = val; } @@ -386,16 +393,18 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) { if (removeHidden && Model.isHiddenProperty(propertyName)) { continue; } + if (removeHidden && Model.isProtectedProperty(propertyName)) { + continue; + } var ownVal = self[propertyName]; // The ownVal can be a relation function - val = (ownVal !== undefined && (typeof ownVal !== 'function')) - ? ownVal : self.__data[propertyName]; + val = (ownVal !== undefined && (typeof ownVal !== 'function')) ? ownVal : self.__data[propertyName]; if (typeof val === 'function') { continue; } if (val !== undefined && val !== null && val.toObject) { - data[propertyName] = val.toObject(!schemaLess, removeHidden); + data[propertyName] = val.toObject(!schemaLess, removeHidden, true); } else { data[propertyName] = val; } @@ -406,6 +415,25 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden) { return data; }; +ModelBaseClass.isProtectedProperty = function (propertyName) { + var model = this; + var settings = Model.definition && Model.definition.settings; + var protectedProperties = settings & (settings.protectedProperties || settings.protected); + + if (Array.isArray(protectedProperties)) { + settings.protectedProperties = {}; + for (var i = 0; i < protectedProperties.length; i++) { + settings.protectedProperties[protectedProperties[i]] = true; + } + protectedProperties = settings.protectedProperties; + } + if (protectedProperties) { + return protectedProperties[propertyName]; + } else { + return false; + } +}; + ModelBaseClass.isHiddenProperty = function (propertyName) { var Model = this; var settings = Model.definition && Model.definition.settings; @@ -423,10 +451,10 @@ ModelBaseClass.isHiddenProperty = function (propertyName) { } else { return false; } -} +}; ModelBaseClass.prototype.toJSON = function () { - return this.toObject(false, true); + return this.toObject(false, true, false); }; ModelBaseClass.prototype.fromObject = function (obj) { diff --git a/test/model-definition.test.js b/test/model-definition.test.js index 342dfde3..c81f518d 100644 --- a/test/model-definition.test.js +++ b/test/model-definition.test.js @@ -36,9 +36,9 @@ describe('ModelDefinition class', function () { assert.equal(json.properties.approved.type, "Boolean"); assert.equal(json.properties.joinedAt.type, "Date"); assert.equal(json.properties.age.type, "Number"); - + assert.deepEqual(User.toJSON(), json); - + done(); }); @@ -55,7 +55,7 @@ describe('ModelDefinition class', function () { }); User.build(); - + var json = User.toJSON(); User.defineProperty("id", {type: "number", id: true}); @@ -64,12 +64,12 @@ describe('ModelDefinition class', function () { assert.equal(User.properties.approved.type, Boolean); assert.equal(User.properties.joinedAt.type, Date); assert.equal(User.properties.age.type, Number); - + assert.equal(User.properties.id.type, Number); - + json = User.toJSON(); assert.deepEqual(json.properties.id, {type: 'Number', id: true}); - + done(); }); @@ -270,6 +270,45 @@ describe('ModelDefinition class', function () { assert(grandChild.prototype instanceof child); }); + it('should serialize protected properties into JSON', function() { + var memory = new DataSoruce({connector: Memory}); + var modelBuilder = memory.modelBuilder; + var ProtectedModel = memory.createModel('protected', {}, { + protected: ['protectedProperty'] + }); + var pm = new ProtectedModel({ + id: 1, foo: 'bar', protectedProperty: 'protected' + }); + var serialized = pm.toJSON(); + assert.deepEqual(serialized, { + id: 1, foo: 'bar', protectedProperty: 'protected' + }); + }); + + it('should not serialized protected properties of nested models into JSON', function(done){ + var memory = new DataSource({connector: Memory}); + var modelBuilder = memory.modelBuilder; + var Parent = memory.createModel('parent'); + var Child = memory.createModel('child', {}, {protected: ['protectedProperty']}); + Parent.hasMany(Child); + Parent.create({ + name: 'parent' + }, function(err, parent) { + parent.children.create({ + name: 'child', + protectedProperty: 'protectedValue' + }, function(err, child) { + Parent.find({include: 'children'}, function(err, parents) { + var serialized = parents[0].toJSON(); + var child = serialized.children[0]; + assert.equal(child.name, 'child'); + assert.notEqual(child.protectedProperty, 'protectedValue'); + done(); + }); + }); + }); + }); + it('should not serialize hidden properties into JSON', function () { var memory = new DataSource({connector: Memory}); var modelBuilder = memory.modelBuilder; From 7372fafc976a5b927a2ad3fe2d385a42e73b0902 Mon Sep 17 00:00:00 2001 From: Christian Enevoldsen Date: Tue, 20 Jan 2015 21:32:31 +0100 Subject: [PATCH 03/18] Fixed typos and logic for protected properties --- lib/model.js | 9 +++++---- test/model-definition.test.js | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/lib/model.js b/lib/model.js index cf44d678..3c39f2a6 100644 --- a/lib/model.js +++ b/lib/model.js @@ -321,6 +321,7 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden, removePr var props = Model.definition.properties; var keys = Object.keys(props); var propertyName, val; + for (var i = 0; i < keys.length; i++) { propertyName = keys[i]; val = self[propertyName]; @@ -393,7 +394,7 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden, removePr if (removeHidden && Model.isHiddenProperty(propertyName)) { continue; } - if (removeHidden && Model.isProtectedProperty(propertyName)) { + if (removeProtected && Model.isProtectedProperty(propertyName)) { continue; } var ownVal = self[propertyName]; @@ -416,11 +417,11 @@ ModelBaseClass.prototype.toObject = function (onlySchema, removeHidden, removePr }; ModelBaseClass.isProtectedProperty = function (propertyName) { - var model = this; + var Model = this; var settings = Model.definition && Model.definition.settings; - var protectedProperties = settings & (settings.protectedProperties || settings.protected); - + var protectedProperties = settings && (settings.protectedProperties || settings.protected); if (Array.isArray(protectedProperties)) { + // Cache the protected properties as an object for quick lookup settings.protectedProperties = {}; for (var i = 0; i < protectedProperties.length; i++) { settings.protectedProperties[protectedProperties[i]] = true; diff --git a/test/model-definition.test.js b/test/model-definition.test.js index c81f518d..9f394f05 100644 --- a/test/model-definition.test.js +++ b/test/model-definition.test.js @@ -271,7 +271,7 @@ describe('ModelDefinition class', function () { }); it('should serialize protected properties into JSON', function() { - var memory = new DataSoruce({connector: Memory}); + var memory = new DataSource({connector: Memory}); var modelBuilder = memory.modelBuilder; var ProtectedModel = memory.createModel('protected', {}, { protected: ['protectedProperty'] @@ -285,7 +285,7 @@ describe('ModelDefinition class', function () { }); }); - it('should not serialized protected properties of nested models into JSON', function(done){ + it('should not serialize protected properties of nested models into JSON', function(done){ var memory = new DataSource({connector: Memory}); var modelBuilder = memory.modelBuilder; var Parent = memory.createModel('parent'); From c37de7f00844199ac0868c7dae1b4e59319d856b Mon Sep 17 00:00:00 2001 From: Clark Wang Date: Wed, 21 Jan 2015 20:16:34 +0800 Subject: [PATCH 04/18] fix id properties should sort by its index Signed-off-by: Clark Wang --- lib/model-definition.js | 2 +- test/model-definition.test.js | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/model-definition.js b/lib/model-definition.js index 32bbcd1c..21094812 100644 --- a/lib/model-definition.js +++ b/lib/model-definition.js @@ -141,7 +141,7 @@ ModelDefinition.prototype.ids = function () { ids.push({name: key, id: id, property: props[key]}); } ids.sort(function (a, b) { - return a.key - b.key; + return a.id - b.id; }); this._ids = ids; return ids; diff --git a/test/model-definition.test.js b/test/model-definition.test.js index 342dfde3..b44fd971 100644 --- a/test/model-definition.test.js +++ b/test/model-definition.test.js @@ -215,6 +215,28 @@ describe('ModelDefinition class', function () { done(); }); + it('should sort id properties by its index', function () { + var modelBuilder = new ModelBuilder(); + + var User = new ModelDefinition(modelBuilder, 'User', { + userId: {type: String, id: 2}, + userType: {type: String, id: 1}, + name: "string", + bio: ModelBuilder.Text, + approved: Boolean, + joinedAt: Date, + age: "number" + }); + + var ids = User.ids(); + assert.ok(Array.isArray(ids)); + assert.equal(ids.length, 2); + assert.equal(ids[0].id, 1); + assert.equal(ids[0].name, 'userType'); + assert.equal(ids[1].id, 2); + assert.equal(ids[1].name, 'userId'); + }); + it('should report correct table/column names', function (done) { var modelBuilder = new ModelBuilder(); From 2e9e2dd192aba7c9f7bf6cebb80b16a2411e8a71 Mon Sep 17 00:00:00 2001 From: Clark Wang Date: Thu, 22 Jan 2015 10:39:47 +0800 Subject: [PATCH 05/18] fix id property for composite ids Signed-off-by: Clark Wang --- lib/model-builder.js | 3 ++- test/loopback-dl.test.js | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/model-builder.js b/lib/model-builder.js index 8af0a02d..eec0034a 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -266,7 +266,8 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett get: function () { var compositeId = {}; var idNames = ModelClass.definition.idNames(); - for (var p in idNames) { + for (var i = 0, p; i < idNames.length; i++) { + p = idNames[i]; compositeId[p] = this.__data[p]; } return compositeId; diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index 41d600f6..1d570d13 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -243,6 +243,19 @@ describe('ModelBuilder define model', function () { done(null, User); }); + it('should define an id property for composite ids', function () { + var modelBuilder = new ModelBuilder(); + var Follow = modelBuilder.define('Follow', { + followerId: { type: String, id: 1 }, + followeeId: { type: String, id: 2 }, + followAt: Date + }); + var follow = new Follow({ followerId: 1, followeeId: 2 }); + + follow.should.have.property('id'); + assert.deepEqual(follow.id, { followerId: 1, followeeId: 2 }); + }); + }); describe('DataSource ping', function() { From a08ef823bec4b911a6dd9e58c9d167a114575780 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 26 Jan 2015 19:09:29 +0100 Subject: [PATCH 06/18] Supply target to applyProperties function --- lib/relation-definition.js | 2 +- test/relations.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/relation-definition.js b/lib/relation-definition.js index aae43be1..cc314f1a 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -213,7 +213,7 @@ RelationDefinition.prototype.applyProperties = function(modelInstance, obj) { target[this.keyTo] = source[this.keyTo]; } if (typeof this.properties === 'function') { - var data = this.properties.call(this, source); + var data = this.properties.call(this, source, target); for(var k in data) { target[k] = data[k]; } diff --git a/test/relations.test.js b/test/relations.test.js index 8acc391a..8318e73b 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -727,7 +727,7 @@ describe('relations', function () { Job = db.define('Job', {name: String, type: String}); Category.hasMany(Job, { - properties: function(inst) { + properties: function(inst, target) { if (!inst.jobType) return; // skip return { type: inst.jobType }; }, From 28f3f5d9f8eaa99fce73f12debb897d5e45f5287 Mon Sep 17 00:00:00 2001 From: James Billingham Date: Tue, 27 Jan 2015 09:02:07 +0000 Subject: [PATCH 07/18] Fixed nullCheck in validations to correct behavior when dealing with undefined attributes --- lib/validations.js | 3 +-- test/validations.test.js | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/lib/validations.js b/lib/validations.js index fafcf5c2..b66c99c5 100644 --- a/lib/validations.js +++ b/lib/validations.js @@ -589,8 +589,7 @@ var defaultMessages = { }; function nullCheck(attr, conf, err) { - var isNull = this[attr] === null || !(attr in this); - if (isNull) { + if (this[attr] == null) { if (!conf.allowNull) { err('null'); } diff --git a/test/validations.test.js b/test/validations.test.js index 85c469b0..9df68d46 100644 --- a/test/validations.test.js +++ b/test/validations.test.js @@ -387,6 +387,30 @@ describe('validations', function () { describe('format', function () { it('should validate format'); it('should overwrite default blank message with custom format message'); + + it('should skip missing values when allowing null', function () { + User.validatesFormatOf('email', { with: /^\S+@\S+\.\S+$/, allowNull: true }); + var u = new User({}); + u.isValid().should.be.true; + }); + + it('should skip null values when allowing null', function () { + User.validatesFormatOf('email', { with: /^\S+@\S+\.\S+$/, allowNull: true }); + var u = new User({ email: null }); + u.isValid().should.be.true; + }); + + it('should not skip missing values', function () { + User.validatesFormatOf('email', { with: /^\S+@\S+\.\S+$/ }); + var u = new User({}); + u.isValid().should.be.false; + }); + + it('should not skip null values', function () { + User.validatesFormatOf('email', { with: /^\S+@\S+\.\S+$/ }); + var u = new User({ email: null }); + u.isValid().should.be.false; + }); }); describe('numericality', function () { From f9b0ac482c4833c7fbe10f970a03f3b77486e014 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 23 Jan 2015 14:23:58 +0100 Subject: [PATCH 08/18] Upgrade `should` to the latest 1.x version --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index aaf40176..ca1fbad4 100644 --- a/package.json +++ b/package.json @@ -23,8 +23,8 @@ "node >= 0.6" ], "devDependencies": { - "should": "~1.2.2", - "mocha": "~1.20.1" + "mocha": "~1.20.1", + "should": "^1.3.0" }, "dependencies": { "async": "~0.9.0", From b3d07ebbe82564a6c64aaa7bbb796ecd2a9843a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 19 Jan 2015 13:39:31 +0100 Subject: [PATCH 09/18] ModelBaseClass: implement async observe/notify Implement infrastructure for intent-based hooks. --- lib/model-builder.js | 21 ++++---- lib/model.js | 48 +++++++++++++++++ test/async-observer.test.js | 103 ++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 10 deletions(-) create mode 100644 test/async-observer.test.js diff --git a/lib/model-builder.js b/lib/model-builder.js index 8af0a02d..c3892759 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -29,7 +29,7 @@ var slice = Array.prototype.slice; /** * ModelBuilder - A builder to define data models. - * + * * @property {Object} definitions Definitions of the models. * @property {Object} models Model constructors * @class @@ -57,7 +57,7 @@ function isModelClass(cls) { /** * Get a model by name. - * + * * @param {String} name The model name * @param {Boolean} forceCreate Whether the create a stub for the given name if a model doesn't exist. * @returns {*} The model class @@ -101,7 +101,7 @@ ModelBuilder.prototype.getModelDefinition = function (name) { * }); * ``` * - * @param {String} className Name of class + * @param {String} className Name of class * @param {Object} properties Hash of class properties in format `{property: Type, property2: Type2, ...}` or `{property: {type: Type}, property2: {type: Type2}, ...}` * @param {Object} settings Other configuration of class * @return newly created class @@ -112,10 +112,10 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett var args = slice.call(arguments); var pluralName = (settings && settings.plural) || inflection.pluralize(className); - + var httpOptions = (settings && settings.http) || {}; var pathName = httpOptions.path || pluralName; - + if (!className) { throw new Error('Class name required'); } @@ -199,6 +199,7 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett hiddenProperty(ModelClass, 'relations', {}); hiddenProperty(ModelClass, 'http', { path: '/' + pathName }); hiddenProperty(ModelClass, 'base', ModelBaseClass); + hiddenProperty(ModelClass, '_observers', {}); // inherit ModelBaseClass static methods for (var i in ModelBaseClass) { @@ -304,12 +305,12 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett * ```js * var user = loopback.Model.extend('user', properties, options); * ``` - * + * * @param {String} className Name of the new model being defined. * @options {Object} properties Properties to define for the model, added to properties of model being extended. * @options {Object} settings Model settings, such as relations and acls. * - */ + */ ModelClass.extend = function (className, subclassProperties, subclassSettings) { var properties = ModelClass.definition.properties; var settings = ModelClass.definition.settings; @@ -461,7 +462,7 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett modelBuilder.mixins.applyMixin(ModelClass, name, mixin); } } - + ModelClass.emit('defined', ModelClass); return ModelClass; @@ -499,7 +500,7 @@ ModelBuilder.prototype.defineValueType = function(type, aliases) { * * Example: * Instead of extending a model with attributes like this (for example): - * + * * ```js * db.defineProperty('Content', 'competitionType', * { type: String }); @@ -518,7 +519,7 @@ ModelBuilder.prototype.defineValueType = function(type, aliases) { *``` * * @param {String} model Name of model - * @options {Object} properties JSON object specifying properties. Each property is a key whos value is + * @options {Object} properties JSON object specifying properties. Each property is a key whos value is * either the [type](http://docs.strongloop.com/display/LB/LoopBack+types) or `propertyName: {options}` * where the options are described below. * @property {String} type Datatype of property: Must be an [LDL type](http://docs.strongloop.com/display/LB/LoopBack+types). diff --git a/lib/model.js b/lib/model.js index 773e2c59..c01cc125 100644 --- a/lib/model.js +++ b/lib/model.js @@ -7,6 +7,7 @@ module.exports = ModelBaseClass; * Module dependencies */ +var async = require('async'); var util = require('util'); var jutil = require('./jutil'); var List = require('./list'); @@ -503,5 +504,52 @@ ModelBaseClass.prototype.setStrict = function (strict) { this.__strict = strict; }; +/** + * Register an asynchronous observer for the given operation (event). + * @param {String} operation The operation name. + * @callback {function} listener The listener function. It will be invoked with + * `this` set to the model constructor, e.g. `User`. + * @param {Object} context Operation-specific context. + * @param {function(Error=)} next The callback to call when the observer + * has finished. + * @end + */ +ModelBaseClass.observe = function(operation, listener) { + if (!this._observers[operation]) { + this._observers[operation] = []; + } + + this._observers[operation].push(listener); +}; + +/** + * Invoke all async observers for the given operation. + * @param {String} operation The operation name. + * @param {Object} context Operation-specific context. + * @param {function(Error=)} callback The callback to call when all observers + * has finished. + */ +ModelBaseClass.notifyObserversOf = function(operation, context, callback) { + var observers = this._observers && this._observers[operation]; + + this._notifyBaseObservers(operation, context, function doNotify(err) { + if (err) return callback(err, context); + if (!observers || !observers.length) return callback(null, context); + + async.eachSeries( + observers, + function(fn, next) { fn(context, next); }, + function(err) { callback(err, context) } + ); + }); +} + +ModelBaseClass._notifyBaseObservers = function(operation, context, callback) { + if (this.base && this.base.notifyObserversOf) + this.base.notifyObserversOf(operation, context, callback); + else + callback(); +} + jutil.mixin(ModelBaseClass, Hookable); jutil.mixin(ModelBaseClass, validations.Validatable); diff --git a/test/async-observer.test.js b/test/async-observer.test.js new file mode 100644 index 00000000..32040018 --- /dev/null +++ b/test/async-observer.test.js @@ -0,0 +1,103 @@ +var ModelBuilder = require('../').ModelBuilder; +var should = require('./init'); + +describe('async observer', function() { + var TestModel; + beforeEach(function defineTestModel() { + var modelBuilder = new ModelBuilder(); + TestModel = modelBuilder.define('TestModel', { name: String }); + }); + + it('calls registered async observers', function(done) { + var notifications = []; + TestModel.observe('before', pushAndNext(notifications, 'before')); + TestModel.observe('after', pushAndNext(notifications, 'after')); + + TestModel.notifyObserversOf('before', {}, function(err) { + if (err) return done(err); + notifications.push('call'); + TestModel.notifyObserversOf('after', {}, function(err) { + if (err) return done(err); + + notifications.should.eql(['before', 'call', 'after']); + done(); + }); + }); + }); + + it('allows multiple observers for the same operation', function(done) { + var notifications = []; + TestModel.observe('event', pushAndNext(notifications, 'one')); + TestModel.observe('event', pushAndNext(notifications, 'two')); + + TestModel.notifyObserversOf('event', {}, function(err) { + if (err) return done(err); + notifications.should.eql(['one', 'two']); + done(); + }); + }); + + it('inherits observers from base model', function(done) { + var notifications = []; + TestModel.observe('event', pushAndNext(notifications, 'base')); + + var Child = TestModel.extend('Child'); + Child.observe('event', pushAndNext(notifications, 'child')); + + Child.notifyObserversOf('event', {}, function(err) { + if (err) return done(err); + notifications.should.eql(['base', 'child']); + done(); + }); + }); + + it('does not modify observers in the base model', function(done) { + var notifications = []; + TestModel.observe('event', pushAndNext(notifications, 'base')); + + var Child = TestModel.extend('Child'); + Child.observe('event', pushAndNext(notifications, 'child')); + + TestModel.notifyObserversOf('event', {}, function(err) { + if (err) return done(err); + notifications.should.eql(['base']); + done(); + }); + }); + + it('always calls inherited observers', function(done) { + var notifications = []; + TestModel.observe('event', pushAndNext(notifications, 'base')); + + var Child = TestModel.extend('Child'); + // Important: there are no observers on the Child model + + Child.notifyObserversOf('event', {}, function(err) { + if (err) return done(err); + notifications.should.eql(['base']); + done(); + }); + }); + + it('handles no observers', function(done) { + TestModel.notifyObserversOf('no-observers', {}, function(err) { + // the test passes when no error was raised + done(err); + }); + }); + + it('passes context to final callback', function(done) { + var context = {}; + TestModel.notifyObserversOf('event', context, function(err, ctx) { + (ctx || "null").should.equal(context); + done(); + }); + }); +}); + +function pushAndNext(array, value) { + return function(ctx, next) { + array.push(value); + process.nextTick(next); + }; +} From 1fd6eff10fae7e5afd5d4499d97e177d5857fdc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 21 Jan 2015 17:57:47 +0100 Subject: [PATCH 10/18] Intent-based hooks for persistence This patch introduces a new API for "intent-based" hooks. These hooks are not tied to a particular method (e.g. "find" or "update"). Instead, they are triggered from all methods that execute a particular "intent". The consumer API is very simple, there is a new method Model.observe(name, observer), where the observer is function observer(context, callback). Observers are inherited by child models and it is possible to register multiple observers for the same hook. List of hooks: - query - before save - after save - after delete --- lib/connectors/memory.js | 3 + lib/dao.js | 598 +++++++++++----- test/hooks.test.js | 2 +- test/memory.test.js | 24 +- test/persistence-hooks.suite.js | 1181 +++++++++++++++++++++++++++++++ 5 files changed, 1610 insertions(+), 198 deletions(-) create mode 100644 test/persistence-hooks.suite.js diff --git a/lib/connectors/memory.js b/lib/connectors/memory.js index 7c80930c..efadb665 100644 --- a/lib/connectors/memory.js +++ b/lib/connectors/memory.js @@ -594,6 +594,9 @@ Memory.prototype.updateAttributes = function updateAttributes(model, id, data, c } } + // Do not modify the data object passed in arguments + data = Object.create(data); + this.setIdValue(model, data, id); var cachedModels = this.collection(model); diff --git a/lib/dao.js b/lib/dao.js index a4d12867..4e868279 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -7,6 +7,7 @@ module.exports = DataAccessObject; /*! * Module dependencies */ +var async = require('async'); var jutil = require('./jutil'); var ValidationError = require('./validations').ValidationError; var Relation = require('./relations.js'); @@ -62,6 +63,16 @@ function byIdQuery(m, id) { return query; } +function isWhereByGivenId(Model, where, idValue) { + var keys = Object.keys(where); + if (keys.length != 1) return false; + + var pk = idName(Model); + if (keys[0] !== pk) return false; + + return where[pk] === idValue; +} + DataAccessObject._forDB = function (data) { if (!(this.getDataSource().isRelational && this.getDataSource().isRelational())) { return data; @@ -133,7 +144,7 @@ DataAccessObject.create = function (data, callback) { var Model = this; var self = this; - + if (typeof data === 'function') { callback = data; data = {}; @@ -183,39 +194,42 @@ DataAccessObject.create = function (data, callback) { } } } - + var enforced = {}; var obj; var idValue = getIdValue(this, data); - + // if we come from save if (data instanceof Model && !idValue) { obj = data; } else { obj = new Model(data); } - + this.applyProperties(enforced, obj); obj.setAttributes(enforced); - + Model = this.lookupModel(data); // data-specific if (Model !== obj.constructor) obj = new Model(data); - - data = obj.toObject(true); - - // validation required - obj.isValid(function (valid) { - if (valid) { - create(); - } else { - callback(new ValidationError(obj), obj); - } - }, data); - + + Model.notifyObserversOf('before save', { Model: Model, instance: obj }, function(err) { + if (err) return callback(err); + + data = obj.toObject(true); + + // validation required + obj.isValid(function (valid) { + if (valid) { + create(); + } else { + callback(new ValidationError(obj), obj); + } + }, data); + }); + function create() { obj.trigger('create', function (createDone) { obj.trigger('save', function (saveDone) { - var _idName = idName(Model); var modelName = Model.modelName; this._adapter().create(modelName, this.constructor._forDB(obj.toObject(true)), function (err, id, rev) { @@ -232,8 +246,13 @@ DataAccessObject.create = function (data, callback) { obj.__persisted = true; saveDone.call(obj, function () { createDone.call(obj, function () { - callback(err, obj); - if(!err) Model.emit('changed', obj); + if (err) { + return callback(err, obj); + } + Model.notifyObserversOf('after save', { Model: Model, instance: obj }, function(err) { + callback(err, obj); + if(!err) Model.emit('changed', obj); + }); }); }); }, obj); @@ -267,45 +286,83 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data } var self = this; var Model = this; - if (!getIdValue(this, data)) { + var id = getIdValue(this, data); + if (!id) { return this.create(data, callback); } - if (this.getDataSource().connector.updateOrCreate) { - var update = data; - var inst = data; - if(!(data instanceof Model)) { - inst = new Model(data); + + Model.notifyObserversOf('query', { Model: Model, query: byIdQuery(Model, id) }, doUpdateOrCreate); + + function doUpdateOrCreate(err, ctx) { + if (err) return callback(err); + + var isOriginalQuery = isWhereByGivenId(Model, ctx.query.where, id) + if (Model.getDataSource().connector.updateOrCreate && isOriginalQuery) { + var context = { Model: Model, where: ctx.query.where, data: data }; + Model.notifyObserversOf('before save', context, function(err, ctx) { + if (err) return callback(err); + + data = ctx.data; + var update = data; + var inst = data; + if(!(data instanceof Model)) { + inst = new Model(data); + } + update = inst.toObject(false); + + Model.applyProperties(update, inst); + Model = Model.lookupModel(update); + + // FIXME(bajtos) validate the model! + // https://github.com/strongloop/loopback-datasource-juggler/issues/262 + + update = inst.toObject(true); + update = removeUndefined(update); + self.getDataSource().connector + .updateOrCreate(Model.modelName, update, done); + + function done(err, data) { + var obj; + if (data && !(data instanceof Model)) { + inst._initProperties(data); + obj = inst; + } else { + obj = data; + } + if (err) { + callback(err, obj); + if(!err) { + Model.emit('changed', inst); + } + } else { + Model.notifyObserversOf('after save', { Model: Model, instance: obj }, function(err) { + callback(err, obj); + if(!err) { + Model.emit('changed', inst); + } + }); + } + } + }); + } else { + Model.findOne({ where: ctx.query.where }, { notify: false }, function (err, inst) { + if (err) { + return callback(err); + } + if (!isOriginalQuery) { + // The custom query returned from a hook may hide the fact that + // there is already a model with `id` value `data[idName(Model)]` + delete data[idName(Model)]; + } + if (inst) { + inst.updateAttributes(data, callback); + } else { + Model = self.lookupModel(data); + var obj = new Model(data); + obj.save(data, callback); + } + }); } - update = inst.toObject(false); - this.applyProperties(update, inst); - update = removeUndefined(update); - Model = this.lookupModel(update); - this.getDataSource().connector.updateOrCreate(Model.modelName, update, function (err, data) { - var obj; - if (data && !(data instanceof Model)) { - inst._initProperties(data); - obj = inst; - } else { - obj = data; - } - callback(err, obj); - if(!err) { - Model.emit('changed', inst); - } - }); - } else { - this.findById(getIdValue(this, data), function (err, inst) { - if (err) { - return callback(err); - } - if (inst) { - inst.updateAttributes(data, callback); - } else { - Model = self.lookupModel(data); - var obj = new Model(data); - obj.save(data, callback); - } - }); } }; @@ -332,11 +389,11 @@ DataAccessObject.findOrCreate = function findOrCreate(query, data, callback) { }; } - var t = this; - this.findOne(query, function (err, record) { + var Model = this; + Model.findOne(query, function (err, record) { if (err) return callback(err); if (record) return callback(null, record, false); - t.create(data, function (err, record) { + Model.create(data, function (err, record) { callback(err, record, record != null); }); }); @@ -383,13 +440,13 @@ DataAccessObject.findByIds = function(ids, cond, cb) { cb = cond; cond = {}; } - + var pk = idName(this); if (ids.length === 0) { process.nextTick(function() { cb(null, []); }); return; } - + var filter = { where: {} }; filter.where[pk] = { inq: [].concat(ids) }; mergeQuery(filter, cond || {}); @@ -731,17 +788,26 @@ DataAccessObject._coerce = function (where) { * @param {Function} callback Required callback function. Call this function with two arguments: `err` (null or Error) and an array of instances. */ -DataAccessObject.find = function find(query, cb) { +DataAccessObject.find = function find(query, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; if (arguments.length === 1) { cb = query; query = null; + options = {}; } + + if (cb === undefined && typeof options === 'function') { + cb = options; + options = {}; + } + + if (!options) options = {}; + var self = this; query = query || {}; - + try { this._normalize(query); } catch (err) { @@ -751,7 +817,7 @@ DataAccessObject.find = function find(query, cb) { } this.applyScope(query); - + var near = query && geo.nearFilter(query.where); var supportsGeo = !!this.getDataSource().connector.buildNearFilter; @@ -763,42 +829,48 @@ DataAccessObject.find = function find(query, cb) { // do in memory query // using all documents // TODO [fabien] use default scope here? - this.getDataSource().connector.all(this.modelName, {}, function (err, data) { - var memory = new Memory(); - var modelName = self.modelName; - if (err) { - cb(err); - } else if (Array.isArray(data)) { - memory.define({ - properties: self.dataSource.definitions[self.modelName].properties, - settings: self.dataSource.definitions[self.modelName].settings, - model: self - }); + self.notifyObserversOf('query', { Model: self, query: query }, function(err, ctx) { + if (err) return cb(err); - data.forEach(function (obj) { - memory.create(modelName, obj, function () { - // noop + self.getDataSource().connector.all(self.modelName, {}, function (err, data) { + var memory = new Memory(); + var modelName = self.modelName; + + if (err) { + cb(err); + } else if (Array.isArray(data)) { + memory.define({ + properties: self.dataSource.definitions[self.modelName].properties, + settings: self.dataSource.definitions[self.modelName].settings, + model: self }); - }); - memory.all(modelName, query, cb); - } else { - cb(null, []); - } - }.bind(this)); + data.forEach(function (obj) { + memory.create(modelName, obj, function () { + // noop + }); + }); + + // FIXME: apply "includes" and other transforms - see allCb below + memory.all(modelName, ctx.query, cb); + } else { + cb(null, []); + } + }); + }); // already handled return; } } - this.getDataSource().connector.all(this.modelName, query, function (err, data) { + var allCb = function (err, data) { if (data && data.forEach) { data.forEach(function (d, i) { var Model = self.lookupModel(d); var obj = new Model(d, {fields: query.fields, applySetters: false, persisted: true}); - + if (query && query.include) { if (query.collect) { // The collect property indicates that the query is to return the @@ -815,7 +887,7 @@ DataAccessObject.find = function find(query, cb) { if (utils.isPlainObject(inc)) { relationName = Object.keys(inc)[0]; } - + // Promote the included model as a direct property var data = obj.__cachedRelations[relationName]; if(Array.isArray(data)) { @@ -840,7 +912,18 @@ DataAccessObject.find = function find(query, cb) { } else cb(err, []); - }); + } + + var self = this; + if (options.notify === false) { + self.getDataSource().connector.all(self.modelName, query, allCb); + } else { + this.notifyObserversOf('query', { Model: this, query: query }, function(err, ctx) { + if (err) return cb(err); + var query = ctx.query; + self.getDataSource().connector.all(self.modelName, query, allCb); + }); + } }; /** @@ -850,16 +933,22 @@ DataAccessObject.find = function find(query, cb) { * For example: `{where: {test: 'me'}}`. * @param {Function} cb Callback function called with (err, instance) */ -DataAccessObject.findOne = function findOne(query, cb) { +DataAccessObject.findOne = function findOne(query, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; if (typeof query === 'function') { cb = query; query = {}; } + + if (cb === undefined && typeof options === 'function') { + cb = options; + options = {}; + } + query = query || {}; query.limit = 1; - this.find(query, function (err, collection) { + this.find(query, options, function (err, collection) { if (err || !collection || !collection.length > 0) return cb(err, null); cb(err, collection[0]); }); @@ -878,41 +967,79 @@ DataAccessObject.findOne = function findOne(query, cb) { * @param {Object} [where] Optional object that defines the criteria. This is a "where" object. Do NOT pass a filter object. * @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, options, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; + var Model = this; - if (!cb && 'function' === typeof where) { + if (!cb && !options && 'function' === typeof where) { cb = where; where = undefined; } - + + if (!cb && typeof options === 'function') { + cb = options; + } + + if (!cb) cb = function(){}; + if (!options) options = {}; + var query = { where: where }; this.applyScope(query); where = query.where; - - if (!where || (typeof where === 'object' && Object.keys(where).length === 0)) { - this.getDataSource().connector.destroyAll(this.modelName, function (err, data) { - cb && cb(err, data); - if(!err) Model.emit('deletedAll'); - }.bind(this)); + + var context = { Model: Model, where: whereIsEmpty(where) ? {} : where }; + if (options.notify === false) { + doDelete(where); } else { - try { - // Support an optional where object - where = removeUndefined(where); - where = this._coerce(where); - } catch (err) { - return process.nextTick(function() { - cb && cb(err); + query = { where: whereIsEmpty(where) ? {} : where }; + Model.notifyObserversOf('query', + { Model: Model, query: query }, + function(err, ctx) { + if (err) return cb(err); + doDelete(ctx.query.where); + }); + } + + function doDelete(where) { + if (whereIsEmpty(where)) { + Model.getDataSource().connector.destroyAll(Model.modelName, done); + } else { + try { + // Support an optional where object + where = removeUndefined(where); + where = Model._coerce(where); + } catch (err) { + return process.nextTick(function() { + cb && cb(err); + }); + } + + Model.getDataSource().connector.destroyAll(Model.modelName, where, done); + + } + + function done(err, data) { + if (err) return cb(er); + + if (options.notify === false) { + return cb(err, data); + } + + Model.notifyObserversOf('after delete', { Model: Model, where: where }, function(err) { + cb(err, data); + if (!err) + Model.emit('deletedAll', whereIsEmpty(where) ? undefined : where); }); } - this.getDataSource().connector.destroyAll(this.modelName, where, function (err, data) { - cb && cb(err, data); - if(!err) Model.emit('deletedAll', where); - }.bind(this)); } }; +function whereIsEmpty(where) { + return !where || + (typeof where === 'object' && Object.keys(where).length === 0) +} + /** * Delete the record with the specified ID. * Aliases are `destroyById` and `deleteById`. @@ -926,7 +1053,7 @@ DataAccessObject.remove = DataAccessObject.deleteAll = DataAccessObject.destroyA DataAccessObject.removeById = DataAccessObject.destroyById = DataAccessObject.deleteById = function deleteById(id, cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; var Model = this; - + this.remove(byIdQuery(this, id).where, function(err) { if ('function' === typeof cb) { cb(err); @@ -955,11 +1082,11 @@ DataAccessObject.count = function (where, cb) { cb = where; where = null; } - + var query = { where: where }; this.applyScope(query); where = query.where; - + try { where = removeUndefined(where); where = this._coerce(where); @@ -968,8 +1095,13 @@ DataAccessObject.count = function (where, cb) { cb && cb(err); }); } - - this.getDataSource().connector.count(this.modelName, cb, where); + + var Model = this; + this.notifyObserversOf('query', { Model: Model, query: { where: where } }, function(err, ctx) { + if (err) return cb(err); + where = ctx.query.where; + Model.getDataSource().connector.count(Model.modelName, cb, where); + }); }; /** @@ -999,59 +1131,67 @@ DataAccessObject.prototype.save = function (options, callback) { if (!('throws' in options)) { options.throws = false; } - + var inst = this; var data = inst.toObject(true); var modelName = Model.modelName; - + Model.applyProperties(data, this); - + if (this.isNewRecord()) { return Model.create(this, callback); } else { inst.setAttributes(data); } - // validate first - if (!options.validate) { - return save(); - } + Model.notifyObserversOf('before save', { Model: Model, instance: inst }, function(err) { + if (err) return callback(err); + data = inst.toObject(true); - inst.isValid(function (valid) { - if (valid) { - save(); - } else { - var err = new ValidationError(inst); - // throws option is dangerous for async usage - if (options.throws) { - throw err; - } - callback(err, inst); + // validate first + if (!options.validate) { + return save(); } - }); - // then save - 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); - } - inst._initProperties(data, { persisted: true }); - updateDone.call(inst, function () { - saveDone.call(inst, function () { - callback(err, inst); - if(!err) { - Model.emit('changed', inst); - } + inst.isValid(function (valid) { + if (valid) { + save(); + } else { + var err = new ValidationError(inst); + // throws option is dangerous for async usage + if (options.throws) { + throw err; + } + callback(err, inst); + } + }); + + // then save + 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); + } + inst._initProperties(data, { persisted: true }); + Model.notifyObserversOf('after save', { Model: Model, instance: inst }, function(err) { + if (err) return callback(err, inst); + updateDone.call(inst, function () { + saveDone.call(inst, function () { + callback(err, inst); + if(!err) { + Model.emit('changed', inst); + } + }); + }); }); }); - }); + }, data, callback); }, data, callback); - }, data, callback); - } + } + }); }; /** @@ -1093,24 +1233,56 @@ DataAccessObject.updateAll = function (where, data, cb) { assert(typeof where === 'object', 'The where argument should be an object'); assert(typeof data === 'object', 'The data argument should be an object'); assert(cb === null || typeof cb === 'function', 'The cb argument should be a function'); - + var query = { where: where }; this.applyScope(query); this.applyProperties(data); - + where = query.where; - - try { - where = removeUndefined(where); - where = this._coerce(where); - } catch (err) { - return process.nextTick(function () { - cb && cb(err); + + var Model = this; + + Model.notifyObserversOf('query', { Model: Model, query: { where: where } }, function(err, ctx) { + if (err) return cb && cb(err); + Model.notifyObserversOf( + 'before save', + { + Model: Model, + where: ctx.query.where, + data: data + }, + function(err, ctx) { + if (err) return cb && cb(err); + doUpdate(ctx.where, ctx.data); + }); + }); + + + function doUpdate(where, data) { + try { + where = removeUndefined(where); + where = Model._coerce(where); + } catch (err) { + return process.nextTick(function () { + cb && cb(err); + }); + } + + var connector = Model.getDataSource().connector; + connector.update(Model.modelName, where, data, function(err, count) { + if (err) return cb && cb (err); + Model.notifyObserversOf( + 'after save', + { + Model: Model, + where: where, + data: data + }, + function(err, ctx) { + return cb && cb(err, count); + }); }); } - - var connector = this.getDataSource().connector; - connector.update(this.modelName, where, data, cb); }; DataAccessObject.prototype.isNewRecord = function () { @@ -1134,23 +1306,50 @@ DataAccessObject.prototype.remove = DataAccessObject.prototype.delete = DataAccessObject.prototype.destroy = function (cb) { if (stillConnecting(this.getDataSource(), this, arguments)) return; + var self = this; var Model = this.constructor; var id = getIdValue(this.constructor, this); - this.trigger('destroy', function (destroyed) { - this._adapter().destroy(this.constructor.modelName, id, function (err) { - if (err) { - return cb(err); - } + Model.notifyObserversOf( + 'query', + { Model: Model, query: byIdQuery(Model, id) }, + function(err, ctx) { + if (err) return cb(err); + doDeleteInstance(ctx.query.where); + }); - destroyed(function () { - if (cb) cb(); - Model.emit('deleted', id); + function doDeleteInstance(where) { + if (!isWhereByGivenId(Model, where, id)) { + // A hook modified the query, it is no longer + // a simple 'delete model with the given id'. + // We must switch to full query-based delete. + Model.deleteAll(where, { notify: false }, function(err) { + if (err) return cb && cb(err); + Model.notifyObserversOf('after delete', { Model: Model, where: where }, function(err) { + cb && cb(err); + if (!err) Model.emit('deleted', id); + }); }); - }.bind(this)); - }, null, cb); + return; + } + + self.trigger('destroy', function (destroyed) { + self._adapter().destroy(self.constructor.modelName, id, function (err) { + if (err) { + return cb(err); + } + + destroyed(function () { + Model.notifyObserversOf('after delete', { Model: Model, where: where }, function(err) { + cb && cb(err); + if (!err) Model.emit('deleted', id); + }); + }); + }); + }, null, cb); + } }; - + /** * Set a single attribute. * Equivalent to `setAttributes({name: value})` @@ -1160,7 +1359,7 @@ DataAccessObject.prototype.remove = */ DataAccessObject.prototype.setAttribute = function setAttribute(name, value) { this[name] = value; // TODO [fabien] - currently not protected by applyProperties -}; +}; /** * Update a single attribute. @@ -1184,17 +1383,17 @@ DataAccessObject.prototype.updateAttribute = function updateAttribute(name, valu */ DataAccessObject.prototype.setAttributes = function setAttributes(data) { if (typeof data !== 'object') return; - + this.constructor.applyProperties(data, this); - + var Model = this.constructor; var inst = this; - + // update instance's properties for (var key in data) { inst.setAttribute(key, data[key]); } - + Model.emit('set', inst); }; @@ -1231,15 +1430,29 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb data = {}; } - // update instance's properties - inst.setAttributes(data); + if (!cb) { + cb = function() {}; + } - inst.isValid(function (valid) { - if (!valid) { - if (cb) { + var context = { + Model: Model, + where: byIdQuery(Model, getIdValue(Model, inst)).where, + data: data + }; + + Model.notifyObserversOf('before save', context, function(err, ctx) { + if (err) return cb(err); + data = ctx.data; + + // update instance's properties + inst.setAttributes(data); + + inst.isValid(function (valid) { + if (!valid) { cb(new ValidationError(inst), inst); + return; } - } else { + inst.trigger('save', function (saveDone) { inst.trigger('update', function (done) { var typedData = {}; @@ -1248,7 +1461,7 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb // Convert the properties by type inst[key] = data[key]; typedData[key] = inst[key]; - if (typeof typedData[key] === 'object' + if (typeof typedData[key] === 'object' && typedData[key] !== null && typeof typedData[key].toObject === 'function') { typedData[key] = typedData[key].toObject(); @@ -1260,15 +1473,18 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb if (!err) inst.__persisted = true; done.call(inst, function () { saveDone.call(inst, function () { - if(cb) cb(err, inst); - if(!err) Model.emit('changed', inst); + if (err) return cb(err, inst); + Model.notifyObserversOf('after save', { Model: Model, instance: inst }, function(err) { + if(!err) Model.emit('changed', inst); + cb(err, inst); + }); }); }); }); }, data, cb); }, data, cb); - } - }, data); + }, data); + }); }; /** diff --git a/test/hooks.test.js b/test/hooks.test.js index b1502f99..319a2fec 100644 --- a/test/hooks.test.js +++ b/test/hooks.test.js @@ -445,7 +445,7 @@ function addHooks(name, done) { }; User['after' + name] = function (next) { (new Boolean(called)).should.equal(true); - this.email.should.equal(random); + this.should.have.property('email', random); done(); }; } diff --git a/test/memory.test.js b/test/memory.test.js index 6b1a608e..65e1ec9f 100644 --- a/test/memory.test.js +++ b/test/memory.test.js @@ -5,6 +5,7 @@ var fs = require('fs'); var assert = require('assert'); var async = require('async'); var should = require('./init.js'); +var Memory = require('../lib/connectors/memory').Memory; describe('Memory connector', function () { var file = path.join(__dirname, 'memory.json'); @@ -278,27 +279,27 @@ describe('Memory connector', function () { } }); - + it('should use collection setting', function (done) { var ds = new DataSource({ connector: 'memory' }); - + var Product = ds.createModel('Product', { name: String }); - + var Tool = ds.createModel('Tool', { name: String }, {memory: {collection: 'Product'}}); - + var Widget = ds.createModel('Widget', { name: String }, {memory: {collection: 'Product'}}); - + ds.connector.getCollection('Tool').should.equal('Product'); ds.connector.getCollection('Widget').should.equal('Product'); - + async.series([ function(next) { Tool.create({ name: 'Tool A' }, next); @@ -359,6 +360,17 @@ describe('Memory connector', function () { }); }); + require('./persistence-hooks.suite')( + new DataSource({ connector: Memory }), + should); +}); + +describe('Unoptimized connector', function() { + var ds = new DataSource({ connector: Memory }); + // disable optimized methods + ds.connector.updateOrCreate = false; + + require('./persistence-hooks.suite')(ds, should); }); diff --git a/test/persistence-hooks.suite.js b/test/persistence-hooks.suite.js new file mode 100644 index 00000000..6d4865e8 --- /dev/null +++ b/test/persistence-hooks.suite.js @@ -0,0 +1,1181 @@ +var ValidationError = require('../').ValidationError; +var traverse = require('traverse'); + +module.exports = function(dataSource, should) { + describe('Persistence hooks', function() { + var observedContexts, expectedError, observersCalled; + var TestModel, existingInstance; + var migrated = false, lastId; + + beforeEach(function setupDatabase(done) { + observedContexts = "hook not called"; + expectedError = new Error('test error'); + observersCalled = []; + + TestModel = dataSource.createModel('TestModel', { + id: { type: String, id: true, default: uid }, + name: { type: String, required: true }, + extra: { type: String, required: false } + }); + + lastId = 0; + + if (migrated) { + TestModel.deleteAll(done); + } else { + dataSource.automigrate(TestModel.modelName, function(err) { + migrated = true; + done(err); + }); + } + }); + + beforeEach(function createTestData(done) { + TestModel.create({ name: 'first' }, function(err, instance) { + if (err) return done(err); + existingInstance = instance; + + TestModel.create({ name: 'second' }, function(err) { + if (err) return done(err); + done(); + }); + }); + }); + + describe('PersistedModel.find', function() { + it('triggers `query` hook', function(done) { + TestModel.observe('query', pushContextAndNext()); + + TestModel.find({ where: { id: '1' } }, function(err, list) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + query: { where: { id: '1' } } + })); + done(); + }); + }); + + it('aborts when `query` hook fails', function(done) { + TestModel.observe('query', nextWithError(expectedError)); + + TestModel.find(function(err, list) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('applies updates from `query` hook', function(done) { + TestModel.observe('query', function(ctx, next) { + ctx.query = { where: { id: existingInstance.id } }; + next(); + }); + + TestModel.find(function(err, list) { + if (err) return done(err); + list.map(get('name')).should.eql([existingInstance.name]); + done(); + }); + }); + + it('triggers `query` hook for geo queries', function(done) { + TestModel.observe('query', pushContextAndNext()); + + TestModel.find({ where: { geo: { near: '10,20' }}}, function(err, list) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + query: { where: { geo: { near: '10,20' } } } + })); + done(); + }); + }); + + it('applies updates from `query` hook for geo queries', function(done) { + TestModel.observe('query', function(ctx, next) { + ctx.query = { where: { id: existingInstance.id } }; + next(); + }); + + TestModel.find({ where: { geo: { near: '10,20' } } }, function(err, list) { + if (err) return done(err); + list.map(get('name')).should.eql([existingInstance.name]); + done(); + }); + }); + }); + + describe('PersistedModel.create', function() { + it('triggers `before save` hook', function(done) { + TestModel.observe('before save', pushContextAndNext()); + + TestModel.create({ name: 'created' }, function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ instance: { + id: instance.id, + name: 'created', + extra: undefined + }})); + done(); + }); + }); + + it('aborts when `before save` hook fails', function(done) { + TestModel.observe('before save', nextWithError(expectedError)); + + TestModel.create({ name: 'created' }, function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('applies updates from `before save` hook', function(done) { + TestModel.observe('before save', function(ctx, next) { + ctx.instance.should.be.instanceOf(TestModel); + ctx.instance.extra = 'hook data'; + next(); + }); + + TestModel.create({ id: uid(), name: 'a-name' }, function(err, instance) { + if (err) return done(err); + instance.should.have.property('extra', 'hook data'); + done(); + }); + }); + + it('sends `before save` for each model in an array', function(done) { + TestModel.observe('before save', pushContextAndNext()); + + TestModel.create( + [{ name: 'one' }, { name: 'two' }], + function(err, list) { + if (err) return done(err); + observedContexts.should.eql([ + aTestModelCtx({ + instance: { id: list[0].id, name: 'one', extra: undefined } + }), + aTestModelCtx({ + instance: { id: list[1].id, name: 'two', extra: undefined } + }), + ]); + done(); + }); + }); + + it('validates model after `before save` hook', function(done) { + TestModel.observe('before save', invalidateTestModel()); + + TestModel.create({ name: 'created' }, function(err) { + (err || {}).should.be.instanceOf(ValidationError); + (err.details.codes || {}).should.eql({ name: ['presence'] }); + done(); + }); + }); + + it('triggers `after save` hook', function(done) { + TestModel.observe('after save', pushContextAndNext()); + + TestModel.create({ name: 'created' }, function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ instance: { + id: instance.id, + name: 'created', + extra: undefined + }})); + done(); + }); + }); + + it('aborts when `after save` hook fails', function(done) { + TestModel.observe('after save', nextWithError(expectedError)); + + TestModel.create({ name: 'created' }, function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('applies updates from `after save` hook', function(done) { + TestModel.observe('after save', function(ctx, next) { + ctx.instance.should.be.instanceOf(TestModel); + ctx.instance.extra = 'hook data'; + next(); + }); + + TestModel.create({ name: 'a-name' }, function(err, instance) { + if (err) return done(err); + instance.should.have.property('extra', 'hook data'); + done(); + }); + }); + + it('sends `after save` for each model in an array', function(done) { + TestModel.observe('after save', pushContextAndNext()); + + TestModel.create( + [{ name: 'one' }, { name: 'two' }], + function(err, list) { + if (err) return done(err); + observedContexts.should.eql([ + aTestModelCtx({ + instance: { id: list[0].id, name: 'one', extra: undefined } + }), + aTestModelCtx({ + instance: { id: list[1].id, name: 'two', extra: undefined } + }), + ]); + done(); + }); + }); + + it('emits `after save` when some models were not saved', function(done) { + TestModel.observe('before save', function(ctx, next) { + if (ctx.instance.name === 'fail') + next(expectedError); + else + next(); + }); + + TestModel.observe('after save', pushContextAndNext()); + + TestModel.create( + [{ name: 'ok' }, { name: 'fail' }], + function(err, list) { + (err || []).should.have.length(2); + err[1].should.eql(expectedError); + + // NOTE(bajtos) The current implementation of `Model.create(array)` + // passes all models in the second callback argument, including + // the models that were not created due to an error. + list.map(get('name')).should.eql(['ok', 'fail']); + + observedContexts.should.eql(aTestModelCtx({ + instance: { id: list[0].id, name: 'ok', extra: undefined } + })); + done(); + }); + }); + }); + + describe('PersistedModel.findOrCreate', function() { + it('triggers `query` hook', function(done) { + TestModel.observe('query', pushContextAndNext()); + + TestModel.findOrCreate( + { where: { name: 'new-record' } }, + { name: 'new-record' }, + function(err, record, created) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ query: { + where: { name: 'new-record' }, + limit: 1, + offset: 0, + skip: 0 + }})); + done(); + }); + }); + + // TODO(bajtos) Enable this test for all connectors that + // provide optimized implementation of findOrCreate. + // The unoptimized implementation does not trigger the hook + // when an existing model was found. + it.skip('triggers `before save` hook when found', function(done) { + TestModel.observe('before save', pushContextAndNext()); + + TestModel.findOrCreate( + { where: { name: existingInstance.name } }, + { name: existingInstance.name }, + function(err, record, created) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ instance: { + id: record.id, + name: existingInstance.name, + extra: undefined + }})); + done(); + }); + }); + + it('triggers `before save` hook when not found', function(done) { + TestModel.observe('before save', pushContextAndNext()); + + TestModel.findOrCreate( + { where: { name: 'new-record' } }, + { name: 'new-record' }, + function(err, record, created) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ instance: { + id: record.id, + name: 'new-record', + extra: undefined + }})); + done(); + }); + }); + + it('validates model after `before save` hook', function(done) { + TestModel.observe('before save', invalidateTestModel()); + + TestModel.findOrCreate( + { where: { name: 'new-record' } }, + { name: 'new-record' }, + function(err) { + (err || {}).should.be.instanceOf(ValidationError); + (err.details.codes || {}).should.eql({ name: ['presence'] }); + done(); + }); + }); + + it('triggers hooks in the correct order when not found', function(done) { + var triggered = []; + TestModel._notify = TestModel.notifyObserversOf; + TestModel.notifyObserversOf = function(operation, context, callback) { + triggered.push(operation); + this._notify.apply(this, arguments); + }; + + TestModel.findOrCreate( + { where: { name: 'new-record' } }, + { name: 'new-record' }, + function(err, record, created) { + if (err) return done(err); + triggered.should.eql([ + 'query', + 'before save', + 'after save' + ]); + done(); + }); + }); + + it('aborts when `query` hook fails', function(done) { + TestModel.observe('query', nextWithError(expectedError)); + + TestModel.findOrCreate( + { where: { id: 'does-not-exist' } }, + { name: 'does-not-exist' }, + function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('aborts when `before save` hook fails', function(done) { + TestModel.observe('before save', nextWithError(expectedError)); + + TestModel.findOrCreate( + { where: { id: 'does-not-exist' } }, + { name: 'does-not-exist' }, + function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('triggers `after save` hook when not found', function(done) { + TestModel.observe('after save', pushContextAndNext()); + + TestModel.findOrCreate( + { where: { name: 'new name' } }, + { name: 'new name' }, + function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ instance: { + id: instance.id, + name: 'new name', + extra: undefined + }})); + done(); + }); + }); + + it('does not trigger `after save` hook when found', function(done) { + TestModel.observe('after save', pushContextAndNext()); + + TestModel.findOrCreate( + { where: { id: existingInstance.id } }, + { name: existingInstance.name }, + function(err, instance) { + if (err) return done(err); + observedContexts.should.eql("hook not called"); + done(); + }); + }); + }); + + describe('PersistedModel.count', function(done) { + it('triggers `query` hook', function(done) { + TestModel.observe('query', pushContextAndNext()); + + TestModel.count({ id: existingInstance.id }, function(err, count) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ query: { + where: { id: existingInstance.id } + }})); + done(); + }); + }); + + it('applies updates from `query` hook', function(done) { + TestModel.observe('query', function(ctx, next) { + ctx.query.where = { id: existingInstance.id }; + next(); + }); + + TestModel.count(function(err, count) { + if (err) return done(err); + count.should.equal(1); + done(); + }); + }); + }); + + describe('PersistedModel.prototype.save', function() { + it('triggers `before save` hook', function(done) { + TestModel.observe('before save', pushContextAndNext()); + + existingInstance.name = 'changed'; + existingInstance.save(function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ instance: { + id: existingInstance.id, + name: 'changed', + extra: undefined + }})); + done(); + }); + }); + + it('aborts when `before save` hook fails', function(done) { + TestModel.observe('before save', nextWithError(expectedError)); + + existingInstance.save(function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('applies updates from `before save` hook', function(done) { + TestModel.observe('before save', function(ctx, next) { + ctx.instance.should.be.instanceOf(TestModel); + ctx.instance.extra = 'hook data'; + next(); + }); + + existingInstance.save(function(err, instance) { + if (err) return done(err); + instance.should.have.property('extra', 'hook data'); + done(); + }); + }); + + it('validates model after `before save` hook', function(done) { + TestModel.observe('before save', invalidateTestModel()); + + existingInstance.save(function(err) { + (err || {}).should.be.instanceOf(ValidationError); + (err.details.codes || {}).should.eql({ name: ['presence'] }); + done(); + }); + }); + + it('triggers `after save` hook', function(done) { + TestModel.observe('after save', pushContextAndNext()); + + existingInstance.name = 'changed'; + existingInstance.save(function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ instance: { + id: existingInstance.id, + name: 'changed', + extra: undefined + }})); + done(); + }); + }); + + it('aborts when `after save` hook fails', function(done) { + TestModel.observe('after save', nextWithError(expectedError)); + + existingInstance.save(function(err, instance) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('applies updates from `after save` hook', function(done) { + TestModel.observe('after save', function(ctx, next) { + ctx.instance.should.be.instanceOf(TestModel); + ctx.instance.extra = 'hook data'; + next(); + }); + + existingInstance.save(function(err, instance) { + if (err) return done(err); + instance.should.have.property('extra', 'hook data'); + done(); + }); + }); + }); + + describe('PersistedModel.prototype.updateAttributes', function() { + it('triggers `before save` hook', function(done) { + TestModel.observe('before save', pushContextAndNext()); + + existingInstance.name = 'changed'; + existingInstance.updateAttributes({ name: 'changed' }, function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + where: { id: existingInstance.id }, + data: { name: 'changed' } + })); + done(); + }); + }); + + it('aborts when `before save` hook fails', function(done) { + TestModel.observe('before save', nextWithError(expectedError)); + + existingInstance.updateAttributes({ name: 'updated' }, function(err) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('applies updates from `before save` hook', function(done) { + TestModel.observe('before save', function(ctx, next) { + ctx.data.extra = 'extra data'; + ctx.data.name = 'hooked name'; + next(); + }); + + existingInstance.updateAttributes({ name: 'updated' }, function(err) { + if (err) return done(err); + // We must query the database here because `updateAttributes` + // returns effectively `this`, not the data from the datasource + TestModel.findById(existingInstance.id, function(err, instance) { + if (err) return done(err); + instance.toObject(true).should.eql({ + id: existingInstance.id, + name: 'hooked name', + extra: 'extra data' + }); + done(); + }); + }); + }); + + it('validates model after `before save` hook', function(done) { + TestModel.observe('before save', invalidateTestModel()); + + existingInstance.updateAttributes({ name: 'updated' }, function(err) { + (err || {}).should.be.instanceOf(ValidationError); + (err.details.codes || {}).should.eql({ name: ['presence'] }); + done(); + }); + }); + + it('triggers `after save` hook', function(done) { + TestModel.observe('after save', pushContextAndNext()); + + existingInstance.name = 'changed'; + existingInstance.updateAttributes({ name: 'changed' }, function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ instance: { + id: existingInstance.id, + name: 'changed', + extra: undefined + }})); + done(); + }); + }); + + it('aborts when `after save` hook fails', function(done) { + TestModel.observe('after save', nextWithError(expectedError)); + + existingInstance.updateAttributes({ name: 'updated' }, function(err) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('applies updates from `after save` hook', function(done) { + TestModel.observe('after save', function(ctx, next) { + ctx.instance.should.be.instanceOf(TestModel); + ctx.instance.extra = 'hook data'; + next(); + }); + + existingInstance.updateAttributes({ name: 'updated' }, function(err, instance) { + if (err) return done(err); + instance.should.have.property('extra', 'hook data'); + done(); + }); + }); + }); + + describe('PersistedModel.updateOrCreate', function() { + it('triggers `query` hook on create', function(done) { + TestModel.observe('query', pushContextAndNext()); + + TestModel.updateOrCreate( + { id: 'not-found', name: 'not found' }, + function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ query: { + where: { id: 'not-found' } + }})); + done(); + }); + }); + + it('triggers `query` hook on update', function(done) { + TestModel.observe('query', pushContextAndNext()); + + TestModel.updateOrCreate( + { id: existingInstance.id, name: 'new name' }, + function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ query: { + where: { id: existingInstance.id } + }})); + done(); + }); + }); + + it('does not trigger `query` on missing id', function(done) { + TestModel.observe('query', pushContextAndNext()); + + TestModel.updateOrCreate( + { name: 'new name' }, + function(err, instance) { + if (err) return done(err); + observedContexts.should.equal('hook not called'); + done(); + }); + }); + + it('applies updates from `query` hook when found', function(done) { + TestModel.observe('query', function(ctx, next) { + ctx.query = { where: { id: { neq: existingInstance.id } } }; + next(); + }); + + TestModel.updateOrCreate( + { id: existingInstance.id, name: 'new name' }, + function(err, instance) { + if (err) return done(err); + findTestModels({ fields: ['id', 'name' ] }, function(err, list) { + if (err) return done(err); + (list||[]).map(toObject).should.eql([ + { id: existingInstance.id, name: existingInstance.name, extra: undefined }, + { id: instance.id, name: 'new name', extra: undefined } + ]); + done(); + }); + }); + }); + + it('applies updates from `query` hook when not found', function(done) { + TestModel.observe('query', function(ctx, next) { + ctx.query = { where: { id: 'not-found' } }; + next(); + }); + + TestModel.updateOrCreate( + { id: existingInstance.id, name: 'new name' }, + function(err, instance) { + if (err) return done(err); + findTestModels({ fields: ['id', 'name' ] }, function(err, list) { + if (err) return done(err); + (list||[]).map(toObject).should.eql([ + { id: existingInstance.id, name: existingInstance.name, extra: undefined }, + { id: list[1].id, name: 'second', extra: undefined }, + { id: instance.id, name: 'new name', extra: undefined } + ]); + done(); + }); + }); + }); + + it('triggers hooks only once', function(done) { + TestModel.observe('query', pushNameAndNext('query')); + TestModel.observe('before save', pushNameAndNext('before save')); + + TestModel.observe('query', function(ctx, next) { + ctx.query = { where: { id: { neq: existingInstance.id } } }; + next(); + }); + + TestModel.updateOrCreate( + { id: 'ignored', name: 'new name' }, + function(err, instance) { + if (err) return done(err); + observersCalled.should.eql(['query', 'before save']); + done(); + }); + }); + + it('triggers `before save` hook on update', function(done) { + TestModel.observe('before save', pushContextAndNext()); + + TestModel.updateOrCreate( + { id: existingInstance.id, name: 'updated name' }, + function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + where: { id: existingInstance.id }, + data: { id: existingInstance.id, name: 'updated name' } + })); + done(); + }); + }); + + it('triggers `before save` hook on create', function(done) { + TestModel.observe('before save', pushContextAndNext()); + + TestModel.updateOrCreate( + { id: 'new-id', name: 'a name' }, + function(err, instance) { + if (err) return done(err); + + if (dataSource.connector.updateOrCreate) { + // Atomic implementations of `updateOrCreate` cannot + // provide full instance as that depends on whether + // UPDATE or CREATE will be triggered + observedContexts.should.eql(aTestModelCtx({ + where: { id: 'new-id' }, + data: { id: 'new-id', name: 'a name' } + })); + } else { + // The default unoptimized implementation runs + // `instance.save` and thus a full instance is availalbe + observedContexts.should.eql(aTestModelCtx({ + instance: { id: 'new-id', name: 'a name', extra: undefined } + })); + } + + done(); + }); + }); + + it('applies updates from `before save` hook on update', function(done) { + TestModel.observe('before save', function(ctx, next) { + ctx.data.name = 'hooked'; + next(); + }); + + TestModel.updateOrCreate( + { id: existingInstance.id, name: 'updated name' }, + function(err, instance) { + if (err) return done(err); + instance.name.should.equal('hooked'); + done(); + }); + }); + + it('applies updates from `before save` hook on create', function(done) { + TestModel.observe('before save', function(ctx, next) { + if (ctx.instance) { + ctx.instance.name = 'hooked'; + } else { + ctx.data.name = 'hooked'; + } + next(); + }); + + TestModel.updateOrCreate( + { id: 'new-id', name: 'new name' }, + function(err, instance) { + if (err) return done(err); + instance.name.should.equal('hooked'); + done(); + }); + }); + + // FIXME(bajtos) this fails with connector-specific updateOrCreate + // implementations, see the comment inside lib/dao.js (updateOrCreate) + it.skip('validates model after `before save` hook on update', function(done) { + TestModel.observe('before save', invalidateTestModel()); + + TestModel.updateOrCreate( + { id: existingInstance.id, name: 'updated name' }, + function(err, instance) { + (err || {}).should.be.instanceOf(ValidationError); + (err.details.codes || {}).should.eql({ name: ['presence'] }); + done(); + }); + }); + + // FIXME(bajtos) this fails with connector-specific updateOrCreate + // implementations, see the comment inside lib/dao.js (updateOrCreate) + it.skip('validates model after `before save` hook on create', function(done) { + TestModel.observe('before save', invalidateTestModel()); + + TestModel.updateOrCreate( + { id: 'new-id', name: 'new name' }, + function(err, instance) { + (err || {}).should.be.instanceOf(ValidationError); + (err.details.codes || {}).should.eql({ name: ['presence'] }); + done(); + }); + }); + + + it('triggers `after save` hook on update', function(done) { + TestModel.observe('after save', pushContextAndNext()); + + TestModel.updateOrCreate( + { id: existingInstance.id, name: 'updated name' }, + function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ instance: { + id: existingInstance.id, + name: 'updated name', + extra: undefined + }})); + done(); + }); + }); + + it('triggers `after save` hook on create', function(done) { + TestModel.observe('after save', pushContextAndNext()); + + TestModel.updateOrCreate( + { id: 'new-id', name: 'a name' }, + function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ instance: { + id: instance.id, + name: 'a name', + extra: undefined + }})); + done(); + }); + }); + }); + + describe('PersistedModel.deleteAll', function() { + it('triggers `query` hook with query', function(done) { + TestModel.observe('query', pushContextAndNext()); + + TestModel.deleteAll({ name: existingInstance.name }, function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + query: { where: { name: existingInstance.name } } + })); + done(); + }); + }); + + it('triggers `query` hook without query', function(done) { + TestModel.observe('query', pushContextAndNext()); + + TestModel.deleteAll(function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ query: { where: {} } })); + done(); + }); + }); + + it('applies updates from `query` hook', function(done) { + TestModel.observe('query', function(ctx, next) { + ctx.query = { where: { id: { neq: existingInstance.id } } }; + next(); + }); + + TestModel.deleteAll(function(err) { + if (err) return done(err); + findTestModels(function(err, list) { + if (err) return done(err); + (list || []).map(get('id')).should.eql([existingInstance.id]); + done(); + }); + }); + }); + + it('triggers `after delete` hook without query', function(done) { + TestModel.observe('after delete', pushContextAndNext()); + + TestModel.deleteAll(function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ where: {} })); + done(); + }); + }); + + it('triggers `after delete` hook without query', function(done) { + TestModel.observe('after delete', pushContextAndNext()); + + TestModel.deleteAll({ name: existingInstance.name }, function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + where: { name: existingInstance.name } + })); + done(); + }); + }); + + it('aborts when `after delete` hook fails', function(done) { + TestModel.observe('after delete', nextWithError(expectedError)); + + TestModel.deleteAll(function(err) { + [err].should.eql([expectedError]); + done(); + }); + }); + }); + + describe('PersistedModel.prototype.delete', function() { + it('triggers `query` hook', function(done) { + TestModel.observe('query', pushContextAndNext()); + + existingInstance.delete(function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + query: { where: { id: existingInstance.id } } + })); + done(); + }); + }); + + it('applies updated from `query` hook', function(done) { + TestModel.observe('query', function(ctx, next) { + ctx.query = { where: { id: { neq: existingInstance.id } } }; + next(); + }); + + existingInstance.delete(function(err) { + if (err) return done(err); + findTestModels(function(err, list) { + if (err) return done(err); + (list || []).map(get('id')).should.eql([existingInstance.id]); + done(); + }); + }); + }); + + it('triggers `after delete` hook', function(done) { + TestModel.observe('after delete', pushContextAndNext()); + + existingInstance.delete(function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + where: { id: existingInstance.id } + })); + done(); + }); + }); + + it('triggers `after delete` hook without query', function(done) { + TestModel.observe('after delete', pushContextAndNext()); + + TestModel.deleteAll({ name: existingInstance.name }, function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + where: { name: existingInstance.name } + })); + done(); + }); + }); + + it('aborts when `after delete` hook fails', function(done) { + TestModel.observe('after delete', nextWithError(expectedError)); + + TestModel.deleteAll(function(err) { + [err].should.eql([expectedError]); + done(); + }); + }); + + it('triggers hooks only once', function(done) { + TestModel.observe('query', pushNameAndNext('query')); + TestModel.observe('after delete', pushNameAndNext('after delete')); + TestModel.observe('query', function(ctx, next) { + ctx.query = { where: { id: { neq: existingInstance.id } } }; + next(); + }); + + existingInstance.delete(function(err) { + if (err) return done(err); + observersCalled.should.eql(['query', 'after delete']); + done(); + }); + }); + }); + + describe('PersistedModel.updateAll', function() { + it('triggers `query` hook', function(done) { + TestModel.observe('query', pushContextAndNext()); + + TestModel.updateAll( + { name: 'searched' }, + { name: 'updated' }, + function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ query: { + where: { name: 'searched' } + }})); + done(); + }); + }); + + it('applies updates from `query` hook', function(done) { + TestModel.observe('query', function(ctx, next) { + ctx.query = { where: { id: { neq: existingInstance.id } } }; + next(); + }); + + TestModel.updateAll( + { id: existingInstance.id }, + { name: 'new name' }, + function(err) { + if (err) return done(err); + findTestModels({ fields: ['id', 'name' ] }, function(err, list) { + if (err) return done(err); + (list||[]).map(toObject).should.eql([ + { id: existingInstance.id, name: existingInstance.name, extra: undefined }, + { id: '2', name: 'new name', extra: undefined } + ]); + done(); + }); + }); + }); + + it('triggers `before save` hook', function(done) { + TestModel.observe('before save', pushContextAndNext()); + + TestModel.updateAll( + { name: 'searched' }, + { name: 'updated' }, + function(err, instance) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + where: { name: 'searched' }, + data: { name: 'updated' }, + })); + done(); + }); + }); + + it('applies updates from `before save` hook', function(done) { + TestModel.observe('before save', function(ctx, next) { + ctx.data = { name: 'hooked', extra: 'added' }; + next(); + }); + + TestModel.updateAll( + { id: existingInstance.id }, + { name: 'updated name' }, + function(err) { + if (err) return done(err); + loadTestModel(existingInstance.id, function(err, instance) { + if (err) return done(err); + instance.should.have.property('name', 'hooked'); + instance.should.have.property('extra', 'added'); + done(); + }); + }); + }); + + it('triggers `after save` hook', function(done) { + TestModel.observe('after save', pushContextAndNext()); + + TestModel.updateAll( + { id: existingInstance.id }, + { name: 'updated name' }, + function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + where: { id: existingInstance.id }, + data: { name: 'updated name' } + })); + done(); + }); + }); + }); + + function pushContextAndNext() { + return function(context, next) { + context = deepCloneToObject(context); + + if (typeof observedContexts === 'string') { + observedContexts = context; + return next(); + } + + if (!Array.isArray(observedContexts)) { + observedContexts = [observedContexts]; + } + + observedContexts.push(context); + next(); + }; + } + + function pushNameAndNext(name) { + return function(context, next) { + observersCalled.push(name); + next(); + }; + } + + function nextWithError(err) { + return function(context, next) { + next(err); + }; + } + + function invalidateTestModel() { + return function(context, next) { + if (context.instance) { + context.instance.name = ''; + } else { + context.data.name = ''; + } + next(); + }; + } + + function aTestModelCtx(ctx) { + ctx.Model = TestModel; + return deepCloneToObject(ctx); + } + + function findTestModels(query, cb) { + if (cb === undefined && typeof query === 'function') { + cb = query; + query = null; + } + + TestModel.find(query, { notify: false }, cb); + } + + function loadTestModel(id, cb) { + TestModel.findOne({ where: { id: id } }, { notify: false }, cb); + } + + function uid() { + lastId += 1; + return '' + lastId; + } + }); + + function deepCloneToObject(obj) { + return traverse(obj).map(function(x) { + if (x && x.toObject) + return x.toObject(true); + if (x && typeof x === 'function' && x.modelName) + return '[ModelCtor ' + x.modelName + ']'; + }); + } + + function get(propertyName) { + return function(obj) { + return obj[propertyName]; + }; + } + + function toObject(obj) { + return obj.toObject ? obj.toObject() : obj; + } +}; From 7d42202d404d165bb0593fc084ace53842568eb4 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 29 Jan 2015 11:52:39 -0800 Subject: [PATCH 11/18] Make sure batch create calls back with correct data See https://github.com/strongloop/loopback/issues/1031 --- lib/dao.js | 61 +++++++++++++++++++-------------------- test/manipulation.test.js | 23 +++++++++++++++ 2 files changed, 53 insertions(+), 31 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 4e868279..81c542f3 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -160,39 +160,38 @@ DataAccessObject.create = function (data, callback) { } if (Array.isArray(data)) { - var instances = []; - var errors = Array(data.length); - var gotError = false; - var wait = data.length; - if (wait === 0) { - callback(null, []); - } - - for (var i = 0; i < data.length; i += 1) { - (function (d, i) { - Model = self.lookupModel(d); // data-specific - instances.push(Model.create(d, function (err, inst) { - if (err) { - errors[i] = err; - gotError = true; - } - modelCreated(); - })); - })(data[i], i); - } - - return instances; - - function modelCreated() { - if (--wait === 0) { - callback(gotError ? errors : null, instances); - if(!gotError) { - instances.forEach(function(inst) { - inst.constructor.emit('changed'); - }); - } + // Undefined item will be skipped by async.map() which internally uses + // Array.prototype.map(). The following loop makes sure all items are + // iterated + for (var i = 0, n = data.length; i < n; i++) { + if (data[i] === undefined) { + data[i] = {}; } } + async.map(data, function(item, done) { + self.create(item, function(err, result) { + // Collect all errors and results + done(null, {err: err, result: result || item}); + }); + }, function(err, results) { + if (err) { + return callback && callback(err, results); + } + // Convert the results into two arrays + var errors = null; + var data = []; + for (var i = 0, n = results.length; i < n; i++) { + if (results[i].err) { + if (!errors) { + errors = []; + } + errors[i] = results[i].err; + } + data[i] = results[i].result; + } + callback && callback(errors, data); + }); + return data; } var enforced = {}; diff --git a/test/manipulation.test.js b/test/manipulation.test.js index 3f3dbed1..727e286a 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -153,6 +153,29 @@ describe('manipulation', function () { }).should.be.instanceOf(Array); }).should.have.lengthOf(3); }); + + it('should create batch of objects with beforeCreate', function(done) { + Person.beforeCreate = function(next, data) { + if (data && data.name === 'A') { + return next(null, {id: 'a', name: 'A'}); + } else { + return next(); + } + }; + var batch = [ + {name: 'A'}, + {name: 'B'}, + undefined + ]; + Person.create(batch, function(e, ps) { + should.not.exist(e); + should.exist(ps); + ps.should.be.instanceOf(Array); + ps.should.have.lengthOf(batch.length); + ps[0].should.be.eql({id: 'a', name: 'A'}); + done(); + }); + }); }); describe('save', function () { From e46bd0cdb506a72ab51c2e5ddb8aa9aeed749038 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 29 Jan 2015 23:26:11 -0800 Subject: [PATCH 12/18] Fix hasOne remoting --- lib/dao.js | 6 ++++++ lib/relation-definition.js | 24 ++++++++++++++++++++---- test/manipulation.test.js | 29 ++++++++++++++++++++++++++++- test/relations.test.js | 18 ++++++++++++++++++ 4 files changed, 72 insertions(+), 5 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 4e868279..db50cc25 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -1434,6 +1434,12 @@ DataAccessObject.prototype.updateAttributes = function updateAttributes(data, cb cb = function() {}; } + // Convert the data to be plain object so that update won't be confused + if (data instanceof Model) { + data = data.toObject(false); + } + data = removeUndefined(data); + var context = { Model: Model, where: byIdQuery(Model, getIdValue(Model, inst)).where, diff --git a/lib/relation-definition.js b/lib/relation-definition.js index cc314f1a..40584393 100644 --- a/lib/relation-definition.js +++ b/lib/relation-definition.js @@ -1490,12 +1490,26 @@ RelationDefinition.hasOne = function (modelFrom, modelTo, params) { // FIXME: [rfeng] Wrap the property into a function for remoting // so that it can be accessed as /api/// // For example, /api/orders/1/customer - var fn = function() { + modelFrom.prototype['__get__' + relationName] = function() { var f = this[relationName]; f.apply(this, arguments); }; - modelFrom.prototype['__get__' + relationName] = fn; - + + modelFrom.prototype['__create__' + relationName] = function() { + var f = this[relationName].create; + f.apply(this, arguments); + }; + + modelFrom.prototype['__update__' + relationName] = function() { + var f = this[relationName].update; + f.apply(this, arguments); + }; + + modelFrom.prototype['__destroy__' + relationName] = function() { + var f = this[relationName].destroy; + f.apply(this, arguments); + }; + return definition; }; @@ -1545,10 +1559,12 @@ HasOne.prototype.create = function (targetModelData, cb) { }); }; -HasOne.prototype.update = function (targetModelData, cb) { +HasOne.prototype.update = function(targetModelData, cb) { var definition = this.definition; + var fk = this.definition.keyTo; this.fetch(function(err, targetModel) { if (targetModel instanceof ModelBaseClass) { + delete targetModelData[fk]; targetModel.updateAttributes(targetModelData, cb); } else { cb(new Error('HasOne relation ' + definition.name diff --git a/test/manipulation.test.js b/test/manipulation.test.js index 3f3dbed1..25671cf6 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -221,7 +221,7 @@ describe('manipulation', function () { before(function (done) { Person.destroyAll(function () { - person = Person.create(done); + person = Person.create({name: 'Mary', age: 15}, done); }); }); @@ -236,6 +236,33 @@ describe('manipulation', function () { }); }); }); + + it('should ignore undefined values on updateAttributes', function(done) { + person.updateAttributes({'name': 'John', age: undefined}, + function(err, p) { + should.not.exist(err); + Person.findById(p.id, function(e, p) { + should.not.exist(err); + p.name.should.equal('John'); + p.age.should.equal(15); + done(); + }); + }); + }); + + it('should allows model instance on updateAttributes', function(done) { + person.updateAttributes(new Person({'name': 'John', age: undefined}), + function(err, p) { + should.not.exist(err); + Person.findById(p.id, function(e, p) { + should.not.exist(err); + p.name.should.equal('John'); + p.age.should.equal(15); + done(); + }); + }); + }); + }); describe('destroy', function () { diff --git a/test/relations.test.js b/test/relations.test.js index 8318e73b..cbc5225f 100644 --- a/test/relations.test.js +++ b/test/relations.test.js @@ -1809,6 +1809,24 @@ describe('relations', function () { }); }); }); + + it('should ignore the foreign key in the update', function(done) { + Supplier.create({name: 'Supplier 2'}, function (e, supplier) { + var sid = supplier.id; + Supplier.findById(supplierId, function(e, supplier) { + should.not.exist(e); + should.exist(supplier); + supplier.account.update({supplierName: 'Supplier A', + supplierId: sid}, + function(err, act) { + should.not.exist(e); + act.supplierName.should.equal('Supplier A'); + act.supplierId.should.equal(supplierId); + done(); + }); + }); + }); + }); it('should get the related item on scope', function(done) { Supplier.findById(supplierId, function(e, supplier) { From 5cfbfe3a196efb7b9e7e8c0d00cb55c15806a965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 30 Jan 2015 11:01:48 +0100 Subject: [PATCH 13/18] Fix regression in `.save()` from 1fd6eff1 The commit 1fd6eff1 (intent-based hooks) introduced a subtle regression in `.save()` method where dynamic property setters were invoked twice. This commit fixes the problem by moving pre-save data normalization into `before save` callback. --- lib/dao.js | 16 ++++----- test/manipulation.test.js | 69 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 4e868279..afb11518 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -1132,21 +1132,19 @@ DataAccessObject.prototype.save = function (options, callback) { options.throws = false; } - var inst = this; - var data = inst.toObject(true); - var modelName = Model.modelName; - - Model.applyProperties(data, this); - if (this.isNewRecord()) { return Model.create(this, callback); - } else { - inst.setAttributes(data); } + var inst = this; + var modelName = Model.modelName; + Model.notifyObserversOf('before save', { Model: Model, instance: inst }, function(err) { if (err) return callback(err); - data = inst.toObject(true); + + var data = inst.toObject(true); + Model.applyProperties(data, inst); + inst.setAttributes(data); // validate first if (!options.validate) { diff --git a/test/manipulation.test.js b/test/manipulation.test.js index 3f3dbed1..adc6a379 100644 --- a/test/manipulation.test.js +++ b/test/manipulation.test.js @@ -22,6 +22,22 @@ describe('manipulation', function () { }); + // A simplified implementation of LoopBack's User model + // to reproduce problems related to properties with dynamic setters + // For the purpose of the tests, we use a counter instead of a hash fn. + var StubUser, stubPasswordCounter; + before(function setupStubUserModel(done) { + StubUser = db.createModel('StubUser', { password: String }, { forceId: true }); + StubUser.setter.password = function(plain) { + this.$password = plain + '-' + (++stubPasswordCounter); + }; + db.automigrate('StubUser', done); + }); + + beforeEach(function resetStubPasswordCounter() { + stubPasswordCounter = 0; + }); + describe('create', function () { before(function (done) { @@ -40,7 +56,7 @@ describe('manipulation', function () { }); }); }); - + it('should instantiate an object', function (done) { var p = new Person({name: 'Anatoliy'}); p.name.should.equal('Anatoliy'); @@ -62,7 +78,7 @@ describe('manipulation', function () { person.should.be.an.instanceOf(Person); should.not.exist(person.id); }); - + it('should not allow user-defined value for the id of object - create', function (done) { Person.create({id: 123456}, function (err, p) { err.should.be.instanceof(ValidationError); @@ -74,7 +90,7 @@ describe('manipulation', function () { done(); }); }); - + it('should not allow user-defined value for the id of object - save', function (done) { var p = new Person({id: 123456}); p.isNewRecord().should.be.true; @@ -214,6 +230,23 @@ describe('manipulation', function () { }); }); + it('should preserve properties with dynamic setters', function(done) { + // This test reproduces a problem discovered by LoopBack unit-test + // "User.hasPassword() should match a password after it is changed" + StubUser.create({ password: 'foo' }, function(err, created) { + if (err) return done(err); + created.password = 'bar'; + created.save(function(err, saved) { + if (err) return done(err); + saved.password.should.equal('bar-2'); + StubUser.findById(created.id, function(err, found) { + if (err) return done(err); + found.password.should.equal('bar-2'); + done(); + }); + }); + }); + }); }); describe('updateAttributes', function () { @@ -238,6 +271,36 @@ describe('manipulation', function () { }); }); + describe('updateOrCreate', function() { + it('should preserve properties with dynamic setters on create', function(done) { + StubUser.updateOrCreate({ id: 'newid', password: 'foo' }, function(err, created) { + if (err) return done(err); + created.password.should.equal('foo-1'); + StubUser.findById(created.id, function(err, found) { + if (err) return done(err); + found.password.should.equal('foo-1'); + done(); + }); + }); + }); + + it('should preserve properties with dynamic setters on update', function(done) { + StubUser.create({ password: 'foo' }, function(err, created) { + if (err) return done(err); + var data = { id: created.id, password: 'bar' }; + StubUser.updateOrCreate(data, function(err, updated) { + if (err) return done(err); + updated.password.should.equal('bar-2'); + StubUser.findById(created.id, function(err, found) { + if (err) return done(err); + found.password.should.equal('bar-2'); + done(); + }); + }); + }); + }); + }); + describe('destroy', function () { it('should destroy record', function (done) { From 31b9da7d442be5b12b7a05bc317cb004b635d600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 30 Jan 2015 11:31:30 +0100 Subject: [PATCH 14/18] Remove redundant `.toObject()` call from `upsert` --- lib/dao.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/dao.js b/lib/dao.js index afb11518..6dccd9b7 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -316,7 +316,6 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data // FIXME(bajtos) validate the model! // https://github.com/strongloop/loopback-datasource-juggler/issues/262 - update = inst.toObject(true); update = removeUndefined(update); self.getDataSource().connector .updateOrCreate(Model.modelName, update, done); From 370966df999e0745b1de57115ec2fb939abb99e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 29 Jan 2015 10:09:01 +0100 Subject: [PATCH 15/18] Implement intent hook `before delete` Methods `DAO.deleteAll` and `DAO.prototype.delete` now invoke `before delete` hook too. The hook receives `ctx.where` describing models to be deleted. --- lib/dao.js | 14 ++++- test/persistence-hooks.suite.js | 92 +++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 2 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index 51e64417..e002bf2e 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -995,7 +995,11 @@ DataAccessObject.remove = DataAccessObject.deleteAll = DataAccessObject.destroyA { Model: Model, query: query }, function(err, ctx) { if (err) return cb(err); - doDelete(ctx.query.where); + var context = { Model: Model, where: ctx.query.where }; + Model.notifyObserversOf('before delete', context, function(err, ctx) { + if (err) return cb(err); + doDelete(ctx.where); + }); }); } @@ -1311,7 +1315,13 @@ DataAccessObject.prototype.remove = { Model: Model, query: byIdQuery(Model, id) }, function(err, ctx) { if (err) return cb(err); - doDeleteInstance(ctx.query.where); + Model.notifyObserversOf( + 'before delete', + { Model: Model, where: ctx.query.where }, + function(err, ctx) { + if (err) return cb(err); + doDeleteInstance(ctx.where); + }); }); function doDeleteInstance(where) { diff --git a/test/persistence-hooks.suite.js b/test/persistence-hooks.suite.js index 6d4865e8..9a523d6a 100644 --- a/test/persistence-hooks.suite.js +++ b/test/persistence-hooks.suite.js @@ -893,6 +893,57 @@ module.exports = function(dataSource, should) { }); }); + it('triggers `before delete` hook with query', function(done) { + TestModel.observe('before delete', pushContextAndNext()); + + TestModel.deleteAll({ name: existingInstance.name }, function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + where: { name: existingInstance.name } + })); + done(); + }); + }); + + it('triggers `before delete` hook without query', function(done) { + TestModel.observe('before delete', pushContextAndNext()); + + TestModel.deleteAll(function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ where: {} })); + done(); + }); + }); + + it('applies updates from `before delete` hook', function(done) { + TestModel.observe('before delete', function(ctx, next) { + ctx.where = { id: { neq: existingInstance.id } }; + next(); + }); + + TestModel.deleteAll(function(err) { + if (err) return done(err); + findTestModels(function(err, list) { + if (err) return done(err); + (list || []).map(get('id')).should.eql([existingInstance.id]); + done(); + }); + }); + }); + + it('aborts when `before delete` hook fails', function(done) { + TestModel.observe('before delete', nextWithError(expectedError)); + + TestModel.deleteAll(function(err, list) { + [err].should.eql([expectedError]); + TestModel.findById(existingInstance.id, function(err, inst) { + if (err) return done(err); + (inst ? inst.toObject() : 'null').should.eql(existingInstance.toObject()); + done(); + }); + }); + }); + it('triggers `after delete` hook without query', function(done) { TestModel.observe('after delete', pushContextAndNext()); @@ -954,6 +1005,47 @@ module.exports = function(dataSource, should) { }); }); + it('triggers `before delete` hook', function(done) { + TestModel.observe('before delete', pushContextAndNext()); + + existingInstance.delete(function(err) { + if (err) return done(err); + observedContexts.should.eql(aTestModelCtx({ + where: { id: existingInstance.id } + })); + done(); + }); + }); + + it('applies updated from `before delete` hook', function(done) { + TestModel.observe('before delete', function(ctx, next) { + ctx.where = { id: { neq: existingInstance.id } }; + next(); + }); + + existingInstance.delete(function(err) { + if (err) return done(err); + findTestModels(function(err, list) { + if (err) return done(err); + (list || []).map(get('id')).should.eql([existingInstance.id]); + done(); + }); + }); + }); + + it('aborts when `before delete` hook fails', function(done) { + TestModel.observe('before delete', nextWithError(expectedError)); + + existingInstance.delete(function(err, list) { + [err].should.eql([expectedError]); + TestModel.findById(existingInstance.id, function(err, inst) { + if (err) return done(err); + (inst ? inst.toObject() : 'null').should.eql(existingInstance.toObject()); + done(); + }); + }); + }); + it('triggers `after delete` hook', function(done) { TestModel.observe('after delete', pushContextAndNext()); From fcaf19a1d204ad27dfa1f6fbaa25e9e5dac9d126 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 2 Feb 2015 10:31:45 +0100 Subject: [PATCH 16/18] Rename hook "query" to "access" The name "query" creates incorrect assumption that hook handlers may return the result of a query to bypass database access. That is far from true, since this hook is called also by methods like `deleteAll` or `updateAll` that don't perform any SELECT query. --- lib/dao.js | 14 ++--- test/persistence-hooks.suite.js | 98 ++++++++++++++++----------------- 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/lib/dao.js b/lib/dao.js index e002bf2e..27688a57 100644 --- a/lib/dao.js +++ b/lib/dao.js @@ -290,7 +290,7 @@ DataAccessObject.updateOrCreate = DataAccessObject.upsert = function upsert(data return this.create(data, callback); } - Model.notifyObserversOf('query', { Model: Model, query: byIdQuery(Model, id) }, doUpdateOrCreate); + Model.notifyObserversOf('access', { Model: Model, query: byIdQuery(Model, id) }, doUpdateOrCreate); function doUpdateOrCreate(err, ctx) { if (err) return callback(err); @@ -828,7 +828,7 @@ DataAccessObject.find = function find(query, options, cb) { // using all documents // TODO [fabien] use default scope here? - self.notifyObserversOf('query', { Model: self, query: query }, function(err, ctx) { + self.notifyObserversOf('access', { Model: self, query: query }, function(err, ctx) { if (err) return cb(err); self.getDataSource().connector.all(self.modelName, {}, function (err, data) { @@ -916,7 +916,7 @@ DataAccessObject.find = function find(query, options, cb) { if (options.notify === false) { self.getDataSource().connector.all(self.modelName, query, allCb); } else { - this.notifyObserversOf('query', { Model: this, query: query }, function(err, ctx) { + this.notifyObserversOf('access', { Model: this, query: query }, function(err, ctx) { if (err) return cb(err); var query = ctx.query; self.getDataSource().connector.all(self.modelName, query, allCb); @@ -991,7 +991,7 @@ DataAccessObject.remove = DataAccessObject.deleteAll = DataAccessObject.destroyA doDelete(where); } else { query = { where: whereIsEmpty(where) ? {} : where }; - Model.notifyObserversOf('query', + Model.notifyObserversOf('access', { Model: Model, query: query }, function(err, ctx) { if (err) return cb(err); @@ -1099,7 +1099,7 @@ DataAccessObject.count = function (where, cb) { } var Model = this; - this.notifyObserversOf('query', { Model: Model, query: { where: where } }, function(err, ctx) { + this.notifyObserversOf('access', { Model: Model, query: { where: where } }, function(err, ctx) { if (err) return cb(err); where = ctx.query.where; Model.getDataSource().connector.count(Model.modelName, cb, where); @@ -1242,7 +1242,7 @@ DataAccessObject.updateAll = function (where, data, cb) { var Model = this; - Model.notifyObserversOf('query', { Model: Model, query: { where: where } }, function(err, ctx) { + Model.notifyObserversOf('access', { Model: Model, query: { where: where } }, function(err, ctx) { if (err) return cb && cb(err); Model.notifyObserversOf( 'before save', @@ -1311,7 +1311,7 @@ DataAccessObject.prototype.remove = var id = getIdValue(this.constructor, this); Model.notifyObserversOf( - 'query', + 'access', { Model: Model, query: byIdQuery(Model, id) }, function(err, ctx) { if (err) return cb(err); diff --git a/test/persistence-hooks.suite.js b/test/persistence-hooks.suite.js index 9a523d6a..1df8699d 100644 --- a/test/persistence-hooks.suite.js +++ b/test/persistence-hooks.suite.js @@ -43,8 +43,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.find', function() { - it('triggers `query` hook', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.find({ where: { id: '1' } }, function(err, list) { if (err) return done(err); @@ -55,8 +55,8 @@ module.exports = function(dataSource, should) { }); }); - it('aborts when `query` hook fails', function(done) { - TestModel.observe('query', nextWithError(expectedError)); + it('aborts when `access` hook fails', function(done) { + TestModel.observe('access', nextWithError(expectedError)); TestModel.find(function(err, list) { [err].should.eql([expectedError]); @@ -64,8 +64,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: existingInstance.id } }; next(); }); @@ -77,8 +77,8 @@ module.exports = function(dataSource, should) { }); }); - it('triggers `query` hook for geo queries', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook for geo queries', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.find({ where: { geo: { near: '10,20' }}}, function(err, list) { if (err) return done(err); @@ -89,8 +89,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook for geo queries', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook for geo queries', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: existingInstance.id } }; next(); }); @@ -256,8 +256,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.findOrCreate', function() { - it('triggers `query` hook', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.findOrCreate( { where: { name: 'new-record' } }, @@ -339,7 +339,7 @@ module.exports = function(dataSource, should) { function(err, record, created) { if (err) return done(err); triggered.should.eql([ - 'query', + 'access', 'before save', 'after save' ]); @@ -347,8 +347,8 @@ module.exports = function(dataSource, should) { }); }); - it('aborts when `query` hook fails', function(done) { - TestModel.observe('query', nextWithError(expectedError)); + it('aborts when `access` hook fails', function(done) { + TestModel.observe('access', nextWithError(expectedError)); TestModel.findOrCreate( { where: { id: 'does-not-exist' } }, @@ -403,8 +403,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.count', function(done) { - it('triggers `query` hook', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.count({ id: existingInstance.id }, function(err, count) { if (err) return done(err); @@ -415,8 +415,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query.where = { id: existingInstance.id }; next(); }); @@ -614,8 +614,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.updateOrCreate', function() { - it('triggers `query` hook on create', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook on create', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.updateOrCreate( { id: 'not-found', name: 'not found' }, @@ -628,8 +628,8 @@ module.exports = function(dataSource, should) { }); }); - it('triggers `query` hook on update', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook on update', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.updateOrCreate( { id: existingInstance.id, name: 'new name' }, @@ -642,8 +642,8 @@ module.exports = function(dataSource, should) { }); }); - it('does not trigger `query` on missing id', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('does not trigger `access` on missing id', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.updateOrCreate( { name: 'new name' }, @@ -654,8 +654,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook when found', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook when found', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); @@ -675,8 +675,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook when not found', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook when not found', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: 'not-found' } }; next(); }); @@ -698,10 +698,10 @@ module.exports = function(dataSource, should) { }); it('triggers hooks only once', function(done) { - TestModel.observe('query', pushNameAndNext('query')); + TestModel.observe('access', pushNameAndNext('access')); TestModel.observe('before save', pushNameAndNext('before save')); - TestModel.observe('query', function(ctx, next) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); @@ -710,7 +710,7 @@ module.exports = function(dataSource, should) { { id: 'ignored', name: 'new name' }, function(err, instance) { if (err) return done(err); - observersCalled.should.eql(['query', 'before save']); + observersCalled.should.eql(['access', 'before save']); done(); }); }); @@ -855,8 +855,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.deleteAll', function() { - it('triggers `query` hook with query', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook with query', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.deleteAll({ name: existingInstance.name }, function(err) { if (err) return done(err); @@ -867,8 +867,8 @@ module.exports = function(dataSource, should) { }); }); - it('triggers `query` hook without query', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook without query', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.deleteAll(function(err) { if (err) return done(err); @@ -877,8 +877,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); @@ -977,8 +977,8 @@ module.exports = function(dataSource, should) { }); describe('PersistedModel.prototype.delete', function() { - it('triggers `query` hook', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook', function(done) { + TestModel.observe('access', pushContextAndNext()); existingInstance.delete(function(err) { if (err) return done(err); @@ -989,8 +989,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updated from `query` hook', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updated from `access` hook', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); @@ -1080,24 +1080,24 @@ module.exports = function(dataSource, should) { }); it('triggers hooks only once', function(done) { - TestModel.observe('query', pushNameAndNext('query')); + TestModel.observe('access', pushNameAndNext('access')); TestModel.observe('after delete', pushNameAndNext('after delete')); - TestModel.observe('query', function(ctx, next) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); existingInstance.delete(function(err) { if (err) return done(err); - observersCalled.should.eql(['query', 'after delete']); + observersCalled.should.eql(['access', 'after delete']); done(); }); }); }); describe('PersistedModel.updateAll', function() { - it('triggers `query` hook', function(done) { - TestModel.observe('query', pushContextAndNext()); + it('triggers `access` hook', function(done) { + TestModel.observe('access', pushContextAndNext()); TestModel.updateAll( { name: 'searched' }, @@ -1111,8 +1111,8 @@ module.exports = function(dataSource, should) { }); }); - it('applies updates from `query` hook', function(done) { - TestModel.observe('query', function(ctx, next) { + it('applies updates from `access` hook', function(done) { + TestModel.observe('access', function(ctx, next) { ctx.query = { where: { id: { neq: existingInstance.id } } }; next(); }); From 4afb2385a9ea6758384e899b7fa2a0b299e12f36 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Mon, 2 Feb 2015 08:44:36 -0800 Subject: [PATCH 17/18] Fix id type issue for update https://github.com/strongloop/loopback/issues/1058 --- lib/connectors/memory.js | 3 + test/memory.test.js | 234 +++++++++++++++++++++++---------------- 2 files changed, 140 insertions(+), 97 deletions(-) diff --git a/lib/connectors/memory.js b/lib/connectors/memory.js index efadb665..91c00263 100644 --- a/lib/connectors/memory.js +++ b/lib/connectors/memory.js @@ -573,6 +573,9 @@ Memory.prototype.update = async.each(ids, function (id, done) { var inst = self.fromDb(model, cache[id]); if (!filter || filter(inst)) { + // The id value from the cache is string + // Get the real id from the inst + id = self.getIdValue(model, inst); self.updateAttributes(model, id, data, done); } else { process.nextTick(done); diff --git a/test/memory.test.js b/test/memory.test.js index 65e1ec9f..341913e7 100644 --- a/test/memory.test.js +++ b/test/memory.test.js @@ -7,11 +7,11 @@ var async = require('async'); var should = require('./init.js'); var Memory = require('../lib/connectors/memory').Memory; -describe('Memory connector', function () { +describe('Memory connector', function() { var file = path.join(__dirname, 'memory.json'); function readModels(done) { - fs.readFile(file, function (err, data) { + fs.readFile(file, function(err, data) { var json = JSON.parse(data.toString()); assert(json.models); assert(json.ids.User); @@ -19,82 +19,122 @@ describe('Memory connector', function () { }); } - before(function (done) { - fs.unlink(file, function (err) { + before(function(done) { + fs.unlink(file, function(err) { if (!err || err.code === 'ENOENT') { done(); } }); }); - it('should save to a json file', function (done) { - var ds = new DataSource({ - connector: 'memory', - file: file - }); - - var User = ds.createModel('User', { - name: String, - bio: String, - approved: Boolean, - joinedAt: Date, - age: Number - }); - - var count = 0; - var ids = []; - async.eachSeries(['John1', 'John2', 'John3'], function (item, cb) { - User.create({name: item}, function (err, result) { - ids.push(result.id); - count++; - readModels(function (err, json) { - assert.equal(Object.keys(json.models.User).length, count); - cb(err); - }); + describe('with file', function() { + function createUserModel() { + var ds = new DataSource({ + connector: 'memory', + file: file }); - }, function (err, results) { - // Now try to delete one - User.deleteById(ids[0], function (err) { - readModels(function (err, json) { - assert.equal(Object.keys(json.models.User).length, 2); - User.upsert({id: ids[1], name: 'John'}, function(err, result) { - readModels(function (err, json) { - assert.equal(Object.keys(json.models.User).length, 2); - var user = JSON.parse(json.models.User[ids[1]]); - assert.equal(user.name, 'John'); - done(); - }); + + var User = ds.createModel('User', { + id: { + type: Number, + id: true, + generated: true + }, + name: String, + bio: String, + approved: Boolean, + joinedAt: Date, + age: Number + }); + return User; + } + + var User; + var ids = []; + + before(function() { + User = createUserModel(); + }); + + it('should persist create', function(done) { + var count = 0; + async.eachSeries(['John1', 'John2', 'John3'], function(item, cb) { + User.create({name: item}, function(err, result) { + ids.push(result.id); + count++; + readModels(function(err, json) { + assert.equal(Object.keys(json.models.User).length, count); + cb(err); }); }); + }, done); + }); + + it('should persist delete', function(done) { + // Now try to delete one + User.deleteById(ids[0], function(err) { + if (err) { + return done(err); + } + readModels(function(err, json) { + if (err) { + return done(err); + } + assert.equal(Object.keys(json.models.User).length, 2); + done(); + }); }); }); + it('should persist upsert', function(done) { + User.upsert({id: ids[1], name: 'John'}, function(err, result) { + if (err) { + return done(err); + } + readModels(function(err, json) { + if (err) { + return done(err); + } + assert.equal(Object.keys(json.models.User).length, 2); + var user = JSON.parse(json.models.User[ids[1]]); + assert.equal(user.name, 'John'); + assert(user.id === ids[1]); + done(); + }); + }); + }); + + it('should persist update', function(done) { + User.update({id: ids[1]}, {name: 'John1'}, + function(err, result) { + if (err) { + return done(err); + } + readModels(function(err, json) { + if (err) { + return done(err); + } + assert.equal(Object.keys(json.models.User).length, 2); + var user = JSON.parse(json.models.User[ids[1]]); + assert.equal(user.name, 'John1'); + assert(user.id === ids[1]); + done(); + }); + }); + }); + + // The saved memory.json from previous test should be loaded + it('should load from the json file', function(done) { + User.find(function(err, users) { + // There should be 2 records + assert.equal(users.length, 2); + done(err); + }); + + }); }); - // The saved memory.json from previous test should be loaded - it('should load from the json file', function (done) { - var ds = new DataSource({ - connector: 'memory', - file: file - }); - - var User = ds.createModel('User', { - name: String, - bio: String, - approved: Boolean, - joinedAt: Date, - age: Number - }); - - User.find(function (err, users) { - // There should be 2 records - assert.equal(users.length, 2); - done(err); - }); - - }); - - describe('Query for memory connector', function () { + describe('Query for memory connector', function() { var ds = new DataSource({ connector: 'memory' }); @@ -110,82 +150,82 @@ describe('Memory connector', function () { }); before(seed); - it('should allow to find using like', function (done) { - User.find({where: {name: {like: '%St%'}}}, function (err, posts) { + it('should allow to find using like', function(done) { + User.find({where: {name: {like: '%St%'}}}, function(err, posts) { should.not.exist(err); posts.should.have.property('length', 2); done(); }); }); - it('should support like for no match', function (done) { - User.find({where: {name: {like: 'M%XY'}}}, function (err, posts) { + it('should support like for no match', function(done) { + User.find({where: {name: {like: 'M%XY'}}}, function(err, posts) { should.not.exist(err); posts.should.have.property('length', 0); done(); }); }); - it('should allow to find using nlike', function (done) { - User.find({where: {name: {nlike: '%St%'}}}, function (err, posts) { + it('should allow to find using nlike', function(done) { + User.find({where: {name: {nlike: '%St%'}}}, function(err, posts) { should.not.exist(err); posts.should.have.property('length', 4); done(); }); }); - it('should support nlike for no match', function (done) { - User.find({where: {name: {nlike: 'M%XY'}}}, function (err, posts) { + it('should support nlike for no match', function(done) { + User.find({where: {name: {nlike: 'M%XY'}}}, function(err, posts) { should.not.exist(err); posts.should.have.property('length', 6); done(); }); }); - it('should throw if the like value is not string or regexp', function (done) { - User.find({where: {name: {like: 123}}}, function (err, posts) { + it('should throw if the like value is not string or regexp', function(done) { + User.find({where: {name: {like: 123}}}, function(err, posts) { should.exist(err); done(); }); }); - it('should throw if the nlike value is not string or regexp', function (done) { - User.find({where: {name: {nlike: 123}}}, function (err, posts) { + it('should throw if the nlike value is not string or regexp', function(done) { + User.find({where: {name: {nlike: 123}}}, function(err, posts) { should.exist(err); done(); }); }); - it('should throw if the inq value is not an array', function (done) { - User.find({where: {name: {inq: '12'}}}, function (err, posts) { + it('should throw if the inq value is not an array', function(done) { + User.find({where: {name: {inq: '12'}}}, function(err, posts) { should.exist(err); done(); }); }); - it('should throw if the nin value is not an array', function (done) { - User.find({where: {name: {nin: '12'}}}, function (err, posts) { + it('should throw if the nin value is not an array', function(done) { + User.find({where: {name: {nin: '12'}}}, function(err, posts) { should.exist(err); done(); }); }); - it('should throw if the between value is not an array', function (done) { - User.find({where: {name: {between: '12'}}}, function (err, posts) { + it('should throw if the between value is not an array', function(done) { + User.find({where: {name: {between: '12'}}}, function(err, posts) { should.exist(err); done(); }); }); - it('should throw if the between value is not an array of length 2', function (done) { - User.find({where: {name: {between: ['12']}}}, function (err, posts) { + it('should throw if the between value is not an array of length 2', function(done) { + User.find({where: {name: {between: ['12']}}}, function(err, posts) { should.exist(err); done(); }); }); - it('should support order with multiple fields', function (done) { - User.find({order: 'vip ASC, seq DESC'}, function (err, posts) { + it('should support order with multiple fields', function(done) { + User.find({order: 'vip ASC, seq DESC'}, function(err, posts) { should.not.exist(err); posts[0].seq.should.be.eql(4); posts[1].seq.should.be.eql(3); @@ -193,8 +233,8 @@ describe('Memory connector', function () { }); }); - it('should sort undefined values to the end when ordered DESC', function (done) { - User.find({order: 'vip ASC, order DESC'}, function (err, posts) { + it('should sort undefined values to the end when ordered DESC', function(done) { + User.find({order: 'vip ASC, order DESC'}, function(err, posts) { should.not.exist(err); posts[4].seq.should.be.eql(1); @@ -203,15 +243,15 @@ describe('Memory connector', function () { }); }); - it('should throw if order has wrong direction', function (done) { - User.find({order: 'seq ABC'}, function (err, posts) { + it('should throw if order has wrong direction', function(done) { + User.find({order: 'seq ABC'}, function(err, posts) { should.exist(err); done(); }); }); - it('should support neq operator for number', function (done) { - User.find({where: {seq: {neq: 4}}}, function (err, users) { + it('should support neq operator for number', function(done) { + User.find({where: {seq: {neq: 4}}}, function(err, users) { should.not.exist(err); users.length.should.be.equal(5); for (var i = 0; i < users.length; i++) { @@ -221,8 +261,8 @@ describe('Memory connector', function () { }); }); - it('should support neq operator for string', function (done) { - User.find({where: {role: {neq: 'lead'}}}, function (err, users) { + it('should support neq operator for string', function(done) { + User.find({where: {role: {neq: 'lead'}}}, function(err, users) { should.not.exist(err); users.length.should.be.equal(4); for (var i = 0; i < users.length; i++) { @@ -234,8 +274,8 @@ describe('Memory connector', function () { }); }); - it('should support neq operator for null', function (done) { - User.find({where: {role: {neq: null}}}, function (err, users) { + it('should support neq operator for null', function(done) { + User.find({where: {role: {neq: null}}}, function(err, users) { should.not.exist(err); users.length.should.be.equal(2); for (var i = 0; i < users.length; i++) { @@ -280,7 +320,7 @@ describe('Memory connector', function () { }); - it('should use collection setting', function (done) { + it('should use collection setting', function(done) { var ds = new DataSource({ connector: 'memory' }); From b169650618df8bb63d052c9c3fd7f1b96ddd2861 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 2 Feb 2015 19:10:36 +0100 Subject: [PATCH 18/18] v2.15.0 --- CHANGES.md | 75 ++++++++++++++++++++++++++++++++++++++-------------- package.json | 2 +- 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index dddfeb2b..5769ac39 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,41 @@ +2015-02-02, Version 2.15.0 +========================== + + * Fix id type issue for update (Raymond Feng) + + * Rename hook "query" to "access" (Miroslav Bajtoš) + + * Implement intent hook `before delete` (Miroslav Bajtoš) + + * Remove redundant `.toObject()` call from `upsert` (Miroslav Bajtoš) + + * Fix regression in `.save()` from 1fd6eff1 (Miroslav Bajtoš) + + * Fix hasOne remoting (Raymond Feng) + + * Make sure batch create calls back with correct data (Raymond Feng) + + * Intent-based hooks for persistence (Miroslav Bajtoš) + + * ModelBaseClass: implement async observe/notify (Miroslav Bajtoš) + + * Upgrade `should` to the latest 1.x version (Miroslav Bajtoš) + + * Fixed nullCheck in validations to correct behavior when dealing with undefined attributes (James Billingham) + + * Supply target to applyProperties function (Fabien Franzen) + + * fix id property for composite ids (Clark Wang) + + * fix id properties should sort by its index (Clark Wang) + + * Fixed typos and logic for protected properties (Christian Enevoldsen) + + * adds support for protected properties. (Christian Enevoldsen) + + * support embeds data for belongsTo relation Signed-off-by: Clark Wang (Clark Wang) + + 2015-01-15, Version 2.14.1 ========================== @@ -395,6 +433,13 @@ * Properly handle LDL for polymorphic relations (Fabien Franzen) + * Check null (Raymond Feng) + + +2014-08-15, Version 2.4.0 +========================= + + 2014-08-15, Version 2.4.1 ========================= @@ -403,12 +448,6 @@ * Check null (Raymond Feng) - -2014-08-15, Version 2.4.0 -========================= - - * Bump version (Raymond Feng) - * Fix the test cases to avoid hard-coded ids (Raymond Feng) * Add strict flag to sortObjectsByIds (Fabien Franzen) @@ -455,19 +494,16 @@ * Cleanup mixin tests (Fabien Franzen) - * Fix a name conflict in scope metadata (Raymond Feng) - - -2014-08-08, Version 2.3.0 -========================= - - 2014-08-08, Version 2.3.1 ========================= * Fix a name conflict in scope metadata (Raymond Feng) + +2014-08-08, Version 2.3.0 +========================= + * Fix the test case so that it works with other DBs (Raymond Feng) * Bump version (Raymond Feng) @@ -580,8 +616,6 @@ * Implemented embedsMany relation (Fabien Franzen) - * Fix a regression where undefined id should not match any record (Raymond Feng) - * Minor tweaks; pass-through properties/scope for hasAndBelongsToMany (Fabien Franzen) * Implemented polymorphic hasMany through inverse (Fabien Franzen) @@ -597,11 +631,6 @@ * Implemented polymorphic hasMany (Fabien Franzen) -2014-07-27, Version 2.1.0 -========================= - - - 2014-07-27, Version 2.1.1 ========================= @@ -609,6 +638,12 @@ * Fix a regression where undefined id should not match any record (Raymond Feng) + +2014-07-27, Version 2.1.0 +========================= + + * Bump version (Raymond Feng) + * datasource: support connectors without `getTypes` (Miroslav Bajtoš) * relation: add `scope._target` for `hasOne` (Miroslav Bajtoš) diff --git a/package.json b/package.json index ca1fbad4..2074e809 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "2.14.1", + "version": "2.15.0", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop",