From aabe5fb1c41033f6b4c8804be2470a9a3ee82e3e Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Mon, 16 Dec 2013 17:14:56 -0800 Subject: [PATCH 1/7] Fix a bug in merging ACLs --- lib/utils.js | 12 +++----- test/util.test.js | 73 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 8 deletions(-) diff --git a/lib/utils.js b/lib/utils.js index 2dd6da47..806f9c2d 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -147,13 +147,9 @@ function mergeSettings(target, src) { if (array) { target = target || []; dst = dst.concat(target); - src.forEach(function (e, i) { - if (typeof target[i] === 'undefined') { - dst[i] = e; - } else { - if (target.indexOf(e) === -1) { - dst.push(e); - } + src.forEach(function (e) { + if (dst.indexOf(e) === -1) { + dst.push(e); } }); } else { @@ -170,7 +166,7 @@ function mergeSettings(target, src) { if (!target[key]) { dst[key] = src[key] } else { - dst[key] = mergeSettings(target[key], src[key]) + dst[key] = mergeSettings(target[key], src[key]); } } }); diff --git a/test/util.test.js b/test/util.test.js index c8234f16..815a01e6 100644 --- a/test/util.test.js +++ b/test/util.test.js @@ -2,6 +2,7 @@ var should = require('./init.js'); var utils = require('../lib/utils'); var fieldsToArray = utils.fieldsToArray; var removeUndefined = utils.removeUndefined; +var mergeSettings = utils.mergeSettings; describe('util.fieldsToArray', function(){ @@ -114,4 +115,76 @@ describe('util.parseSettings', function(){ }); +}); + +describe('mergeSettings', function () { + it('should merge settings correctly', function () { + var src = { base: 'User', + relations: { accessTokens: { model: 'accessToken', type: 'hasMany', + foreignKey: 'userId' }, + account: { model: 'account', type: 'belongsTo' } }, + acls: [ + { accessType: '*', + permission: 'DENY', + principalType: 'ROLE', + principalId: '$everyone' }, + { accessType: '*', + permission: 'ALLOW', + principalType: 'ROLE', + property: 'login', + principalId: '$everyone' }, + { permission: 'ALLOW', + property: 'findById', + principalType: 'ROLE', + principalId: '$owner' } + ] }; + var tgt = { strict: false, + acls: [ + { principalType: 'ROLE', + principalId: '$everyone', + permission: 'ALLOW', + property: 'create' }, + { principalType: 'ROLE', + principalId: '$owner', + permission: 'ALLOW', + property: 'removeById' } + ], + maxTTL: 31556926, + ttl: 1209600 }; + + var dst = mergeSettings(tgt, src); + + var expected = { strict: false, + acls: [ + { principalType: 'ROLE', + principalId: '$everyone', + permission: 'ALLOW', + property: 'create' }, + { principalType: 'ROLE', + principalId: '$owner', + permission: 'ALLOW', + property: 'removeById' }, + { accessType: '*', + permission: 'DENY', + principalType: 'ROLE', + principalId: '$everyone' }, + { accessType: '*', + permission: 'ALLOW', + principalType: 'ROLE', + property: 'login', + principalId: '$everyone' }, + { permission: 'ALLOW', + property: 'findById', + principalType: 'ROLE', + principalId: '$owner' } + ], + maxTTL: 31556926, + ttl: 1209600, + base: 'User', + relations: { accessTokens: { model: 'accessToken', type: 'hasMany', + foreignKey: 'userId' }, + account: { model: 'account', type: 'belongsTo' } } }; + + should.deepEqual(dst.acls, expected.acls, 'Merged settings should match the expectation'); + }); }); \ No newline at end of file From e1ec152c78f3ee6cd0f0e46d1b9171c4036bea8e Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Wed, 18 Dec 2013 16:13:41 -0800 Subject: [PATCH 2/7] Add models to LDL options 1. Use 'models' to specify the dependencies to other models 2. The 'models' property is an object, such as: { Model1: 'Model1', Model2: Model2 } 3. The model classes will be injected into the newly defined class as static properties using the keys from the models option. --- lib/datasource.js | 16 ++++------------ lib/model-builder.js | 23 +++++++++++++++++++++-- test/loopback-dl.test.js | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/lib/datasource.js b/lib/datasource.js index dc03afd9..a667e4c3 100644 --- a/lib/datasource.js +++ b/lib/datasource.js @@ -363,18 +363,10 @@ DataSource.prototype.defineRelations = function(modelClass, relations) { for (var rn in relations) { var r = relations[rn]; assert(DataSource.relationTypes.indexOf(r.type) !== -1, "Invalid relation type: " + r.type); - var targetModel = isModelClass(r.model) ? r.model : this.getModel(r.model); - if (!targetModel) { - // The target model doesn't exist, let create a place holder for it - targetModel = this.define(r.model, {}, {unresolved: true}); - } + var targetModel = isModelClass(r.model) ? r.model : this.getModel(r.model, true); var throughModel = null; if (r.through) { - throughModel = isModelClass(r.through) ? r.through : this.getModel(r.through); - if (!throughModel) { - // The through model doesn't exist, let create a place holder for it - throughModel = this.define(r.through, {}, {unresolved: true}); - } + throughModel = isModelClass(r.through) ? r.through : this.getModel(r.through, true); } if (!isModelDataSourceAttached(targetModel) || (throughModel && !isModelDataSourceAttached(throughModel))) { // Create a listener to defer the relation set up @@ -533,8 +525,8 @@ DataSource.prototype.mixin = function (ModelCtor) { }); }; -DataSource.prototype.getModel = function(name) { - return this.modelBuilder.getModel(name); +DataSource.prototype.getModel = function(name, forceCreate) { + return this.modelBuilder.getModel(name, forceCreate); }; DataSource.prototype.getModelDefinition = function(name) { diff --git a/lib/model-builder.js b/lib/model-builder.js index dcbb3a99..59a62a17 100644 --- a/lib/model-builder.js +++ b/lib/model-builder.js @@ -56,8 +56,19 @@ function isModelClass(cls) { return cls.prototype instanceof DefaultModelBaseClass; } -ModelBuilder.prototype.getModel = function(name) { - return this.models[name]; +/** + * Get a model by name + * @param {String} name The model name + * @param {Boolean} forceCreate Indicate if a stub should be created for the + * given name if a model doesn't exist + * @returns {*} The model class + */ +ModelBuilder.prototype.getModel = function(name, forceCreate) { + var model = this.models[name]; + if(!model && forceCreate) { + model = this.define(name, {}, {unresolved: true}); + } + return model; }; ModelBuilder.prototype.getModelDefinition = function(name) { @@ -185,6 +196,14 @@ ModelBuilder.prototype.define = function defineClass(className, properties, sett ModelClass[i] = ModelBaseClass[i]; } } + + // Load and inject the model classes + if(settings.models) { + Object.keys(settings.models).forEach(function(m) { + var model = settings.models[m]; + ModelClass[m] = typeof model === 'string' ? modelBuilder.getModel(model, true) : model; + }); + } ModelClass.getter = {}; ModelClass.setter = {}; diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index bb3c6457..a6225b59 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -906,3 +906,39 @@ describe('Injected methods from connectors', function(){ }); }); + + +describe('ModelBuilder options.models', function(){ + it('should inject model classes from models', function() { + var builder = new ModelBuilder(); + var M1 = builder.define('M1'); + var M2 = builder.define('M2', {}, {models: { + 'M1': M1 + }}); + + assert.equal(M2.M1, M1, 'M1 should be injected to M2'); + }); + + it('should inject model classes by name in the models', function() { + var builder = new ModelBuilder(); + var M1 = builder.define('M1'); + var M2 = builder.define('M2', {}, {models: { + 'M1': 'M1' + }}); + + assert.equal(M2.M1, M1, 'M1 should be injected to M2'); + }); + + it('should inject model classes by name in the models before the class is defined', + function() { + var builder = new ModelBuilder(); + var M2 = builder.define('M2', {}, {models: { + 'M1': 'M1' + }}); + assert(M2.M1, 'M1 should be injected to M2'); + assert(M2.M1.settings.unresolved, 'M1 is still a proxy'); + var M1 = builder.define('M1'); + assert.equal(M2.M1, M1, 'M1 should be injected to M2'); + }); + +}); \ No newline at end of file From c9e133d7bc9ba9c6a412a9bd425beb50431a96f3 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Wed, 18 Dec 2013 17:14:54 -0800 Subject: [PATCH 3/7] Add a EOL --- test/loopback-dl.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/loopback-dl.test.js b/test/loopback-dl.test.js index a6225b59..f4214df5 100644 --- a/test/loopback-dl.test.js +++ b/test/loopback-dl.test.js @@ -941,4 +941,5 @@ describe('ModelBuilder options.models', function(){ assert.equal(M2.M1, M1, 'M1 should be injected to M2'); }); -}); \ No newline at end of file +}); + From 1f965bfedb4afd56da4880a4f830be438c4ea7c8 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 20 Dec 2013 14:47:41 -0800 Subject: [PATCH 4/7] Fix the remoting method with the current receiver (this) --- lib/scope.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/scope.js b/lib/scope.js index e4672d5d..44513ddc 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -83,8 +83,8 @@ function defineScope(cls, targetClass, name, params, methods) { // Wrap the property into a function for remoting var fn = function() { - var f = this[name]; - f.apply(this, arguments); + var f = cls[name]; + f.apply(cls, arguments); }; fn.shared = true; @@ -96,8 +96,8 @@ function defineScope(cls, targetClass, name, params, methods) { cls['__get__' + name] = fn; var fn_create = function() { - var f = this[name].create; - f.apply(this, arguments); + var f = cls[name].create; + f.apply(cls[name], arguments); }; fn_create.shared = true; @@ -109,8 +109,8 @@ function defineScope(cls, targetClass, name, params, methods) { cls['__create__' + name] = fn_create; var fn_delete = function() { - var f = this[name].destroyAll; - f.apply(this, arguments); + var f = cls[name].destroyAll; + f.apply(cls[name], arguments); }; fn_delete.shared = true; fn_delete.http = {verb: 'delete', path: '/' + name}; From f1773857bb031fe95de818ab4384c094989c08fd Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 20 Dec 2013 17:28:21 -0800 Subject: [PATCH 5/7] Fix the remote delegation --- lib/relations.js | 4 +--- lib/scope.js | 15 ++++++++------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/lib/relations.js b/lib/relations.js index a5b043f4..65e98824 100644 --- a/lib/relations.js +++ b/lib/relations.js @@ -26,7 +26,7 @@ Relation.relationNameFor = function relationNameFor(foreignKey) { * @example `User.hasMany(Post, {as: 'posts', foreignKey: 'authorId'});` */ Relation.hasMany = function hasMany(anotherClass, params) { - var thisClass = this, thisClassName = this.modelName; + var thisClassName = this.modelName; params = params || {}; if (typeof anotherClass === 'string') { params.as = anotherClass; @@ -71,7 +71,6 @@ Relation.hasMany = function hasMany(anotherClass, params) { done = function() {}; } var self = this; - var id = this[idName]; anotherClass.create(data, function(err, ac) { if (err) return done(err, ac); var d = {}; @@ -98,7 +97,6 @@ Relation.hasMany = function hasMany(anotherClass, params) { params.through.findOrCreate({where: query}, data, done); }; scopeMethods.remove = function(acInst, done) { - var self = this; var q = {}; q[fk2] = acInst[idName] || acInst; params.through.findOne({where: q}, function(err, d) { diff --git a/lib/scope.js b/lib/scope.js index 44513ddc..3094cc97 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -60,6 +60,7 @@ function defineScope(cls, targetClass, name, params, methods) { } }; f._scope = typeof params === 'function' ? params.call(this) : params; + f.build = build; f.create = create; f.destroyAll = destroyAll; @@ -83,8 +84,8 @@ function defineScope(cls, targetClass, name, params, methods) { // Wrap the property into a function for remoting var fn = function() { - var f = cls[name]; - f.apply(cls, arguments); + var f = this[name]; + f.apply(this[name], arguments); }; fn.shared = true; @@ -96,21 +97,21 @@ function defineScope(cls, targetClass, name, params, methods) { cls['__get__' + name] = fn; var fn_create = function() { - var f = cls[name].create; - f.apply(cls[name], arguments); + var f = this[name].create; + f.apply(this[name], arguments); }; fn_create.shared = true; fn_create.http = {verb: 'post', path: '/' + name}; - fn_create.accepts = {arg: 'data', type: 'object', source: 'body'}; + fn_create.accepts = {arg: 'data', type: 'object', http: {source: 'body'}}; fn_create.description = 'Creates ' + name; fn_create.returns = {arg: 'data', type: 'object', root: true}; cls['__create__' + name] = fn_create; var fn_delete = function() { - var f = cls[name].destroyAll; - f.apply(cls[name], arguments); + var f = this[name].destroyAll; + f.apply(this[name], arguments); }; fn_delete.shared = true; fn_delete.http = {verb: 'delete', path: '/' + name}; From d9d9d82141d239d955f5414910b13f4fd98aed55 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 20 Dec 2013 17:49:14 -0800 Subject: [PATCH 6/7] Add more comments --- lib/scope.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/scope.js b/lib/scope.js index 3094cc97..c1e6a51d 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -24,6 +24,17 @@ function defineScope(cls, targetClass, name, params, methods) { Object.defineProperty(cls, name, { enumerable: false, configurable: true, + /** + * This defines a property for the scope. For example, user.accounts or + * User.vips. Please note the cls can be the model class or prototype of + * the model class. + * + * The property value is function. It can be used to query the scope, + * such as user.accounts(condOrRefresh, cb) or User.vips(cb). The value + * can also have child properties for create/build/delete. For example, + * user.accounts.create(act, cb). + * + */ get: function () { var f = function caller(condOrRefresh, cb) { var actualCond = {}; @@ -84,7 +95,9 @@ function defineScope(cls, targetClass, name, params, methods) { // Wrap the property into a function for remoting var fn = function() { + // primaryObject.scopeName, such as user.accounts var f = this[name]; + // set receiver to be the scope property whose value is a function f.apply(this[name], arguments); }; From 96d9da290d7dae5f2ca1c05967c02aa8ef9539ac Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Fri, 20 Dec 2013 18:25:56 -0800 Subject: [PATCH 7/7] Bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6d0d29c6..c4dca516 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback-datasource-juggler", - "version": "1.2.10", + "version": "1.2.11", "description": "LoopBack DataSoure Juggler", "keywords": [ "StrongLoop",