From 9be8d11431c02e8e7b6ab7b20aa34bf060add6c8 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 4 Aug 2014 18:27:50 +0200 Subject: [PATCH 1/5] Implement Model.nestRemoting Explicitly enable another level of nesting/accessing relations; limited to a depth of 2 levels. --- lib/models/model.js | 105 ++++++++++++++++++++++++++- test/relations.integration.js | 129 ++++++++++++++++++++++++++++++++++ 2 files changed, 232 insertions(+), 2 deletions(-) diff --git a/lib/models/model.js b/lib/models/model.js index 6f61d781..8e018a34 100644 --- a/lib/models/model.js +++ b/lib/models/model.js @@ -4,6 +4,8 @@ var registry = require('../registry'); var assert = require('assert'); var SharedClass = require('strong-remoting').SharedClass; +var extend = require('util')._extend; +var stringUtils = require('underscore.string'); /** * The base class for **all models**. @@ -203,7 +205,7 @@ Model.setup = function () { } } }); - + return ModelCtor; }; @@ -469,7 +471,106 @@ Model.scopeRemoting = function(relationName, relation, define) { http: {verb: 'delete', path: '/' + relationName}, description: 'Deletes all ' + relationName + ' of this model.' }); -} +}; + +Model.nestRemoting = function(relationName, options) { + var regExp = /^__([^_]+)__([^_]+)$/; + options = options || {}; + var relation = this.relations[relationName]; + if (relation && relation.modelTo && relation.modelTo.sharedClass) { + var self = this; + var sharedClass = this.sharedClass; + var sharedToClass = relation.modelTo.sharedClass; + var toModelName = relation.modelTo.modelName; + + var pathName = options.pathName || relation.options.path || relationName; + var paramName = options.paramName || 'nk'; + + var http = [].concat(sharedToClass.http || [])[0]; + + if (relation.multiple) { + var httpPath = pathName + '/:' + paramName; + var acceptArgs = [ + { + arg: paramName, type: 'any', http: { source: 'path' }, + description: 'Foreign key for ' + relation.name, + required: true + } + ]; + } else { + var httpPath = pathName; + var acceptArgs = []; + } + + sharedToClass.methods().forEach(function(method) { + var matches; + if (!method.isStatic && (matches = method.name.match(regExp))) { + var prefix = relation.multiple ? '__findById__' : '__get__'; + var getterName = options.getterName || (prefix + relationName); + + var getterFn = relation.modelFrom.prototype[getterName]; + if (typeof getterFn !== 'function') { + throw new Error('Invalid remote method: `' + getterName + '`'); + } + + var nestedFn = relation.modelTo.prototype[method.name]; + if (typeof nestedFn !== 'function') { + throw new Error('Invalid remote method: `' + method.name + '`'); + } + + var opts = {}; + var methodName = '__' + matches[1] + '__' + relationName + '__' + matches[2]; + + opts.accepts = acceptArgs.concat(method.accepts || []); + opts.returns = [].concat(method.returns || []); + opts.description = method.description; + opts.rest = extend({}, method.rest || {}); + + opts.http = []; + var routes = [].concat(method.http || []); + routes.forEach(function(route) { + if (route.path) { + var copy = extend({}, route); + copy.path = httpPath + route.path; + opts.http.push(copy); + } + }); + + if (relation.multiple) { + sharedClass.defineMethod(methodName, opts, function(fkId) { + var args = Array.prototype.slice.call(arguments, 1); + var last = args[args.length - 1]; + var cb = typeof last === 'function' ? last : null; + this[getterName](fkId, function(err, inst) { + if (err && cb) return cb(err); + if (inst instanceof relation.modelTo) { + nestedFn.apply(inst, args); + } else if (cb) { + cb(err, null); + } + }); + }, method.isStatic); + } else { + sharedClass.defineMethod(methodName, opts, function() { + var args = Array.prototype.slice.call(arguments); + var last = args[args.length - 1]; + var cb = typeof last === 'function' ? last : null; + this[getterName](function(err, inst) { + if (err && cb) return cb(err); + if (inst instanceof relation.modelTo) { + nestedFn.apply(inst, args); + } else if (cb) { + cb(err, null); + } + }); + }, method.isStatic); + } + } + }); + } else { + throw new Error('Relation `' + relationName + '` does not exist for model `' + this.modelName + '`'); + } +}; // setup the initial model Model.setup(); diff --git a/test/relations.integration.js b/test/relations.integration.js index 2c27cef9..6d5302b5 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -926,4 +926,133 @@ describe('relations - integration', function () { }); + describe('nested relations', function() { + + before(function defineProductAndCategoryModels() { + var Book = app.model( + 'Book', + { properties: { name: 'string' }, dataSource: 'db', + plural: 'books' } + ); + var Page = app.model( + 'Page', + { properties: { name: 'string' }, dataSource: 'db', + plural: 'pages' } + ); + var Image = app.model( + 'Image', + { properties: { name: 'string' }, dataSource: 'db', + plural: 'images' } + ); + var Note = app.model( + 'Note', + { properties: { text: 'string' }, dataSource: 'db', + plural: 'notes' } + ); + Book.hasMany(Page); + Page.hasMany(Note); + Image.belongsTo(Book); + + Book.nestRemoting('pages'); + Image.nestRemoting('book'); + + expect(Book.prototype['__findById__pages__notes']).to.be.a.function; + expect(Image.prototype['__findById__book__pages']).to.be.a.function; + }); + + before(function createBook(done) { + var test = this; + app.models.Book.create({ name: 'Book 1' }, + function(err, book) { + if (err) return done(err); + test.book = book; + book.pages.create({ name: 'Page 1' }, + function(err, page) { + if (err) return done(err); + test.page = page; + page.notes.create({ text: 'Page Note 1' }, + function(err, note) { + test.note = note; + done(); + }); + }); + }); + }); + + before(function createCover(done) { + var test = this; + app.models.Image.create({ name: 'Cover 1', book: test.book }, + function(err, image) { + if (err) return done(err); + test.image = image; + done(); + }); + }); + + it('has regular relationship routes', function(done) { + var test = this; + this.get('/api/books/' + test.book.id + '/pages') + .expect(200, function(err, res) { + expect(res.body).to.be.an.array; + expect(res.body).to.have.length(1); + expect(res.body[0].name).to.equal('Page 1'); + done(); + }); + }); + + it('has a basic error handler', function(done) { + var test = this; + this.get('/api/books/unknown/pages') + .expect(404, function(err, res) { + expect(res.body.error).to.be.an.object; + var expected = 'could not find a model with id unknown'; + expect(res.body.error.message).to.equal(expected); + done(); + }); + }); + + it('enables nested relationship routes - belongsTo find', function(done) { + var test = this; + this.get('/api/images/' + test.image.id + '/book/pages') + .end(function(err, res) { + expect(res.body).to.be.an.array; + expect(res.body).to.have.length(1); + expect(res.body[0].name).to.equal('Page 1'); + done(); + }); + }); + + it('enables nested relationship routes - belongsTo findById', function(done) { + var test = this; + this.get('/api/images/' + test.image.id + '/book/pages/' + test.page.id) + .end(function(err, res) { + expect(res.body).to.be.an.object; + expect(res.body.name).to.equal('Page 1'); + done(); + }); + }); + + it('enables nested relationship routes - hasMany find', function(done) { + var test = this; + this.get('/api/books/' + test.book.id + '/pages/' + test.page.id + '/notes') + .expect(200, function(err, res) { + expect(res.body).to.be.an.array; + expect(res.body).to.have.length(1); + expect(res.body[0].text).to.equal('Page Note 1'); + done(); + }); + }); + + it('enables nested relationship routes - hasMany findById', function(done) { + var test = this; + this.get('/api/books/' + test.book.id + '/pages/' + test.page.id + '/notes/' + test.note.id) + .expect(200, function(err, res) { + expect(res.body).to.be.an.object; + expect(res.body.text).to.equal('Page Note 1'); + done(); + }); + }); + + }); + }); From 42f938ed729ada0fc08060e14c8f3b984036010f Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 4 Aug 2014 18:47:15 +0200 Subject: [PATCH 2/5] Fix test to be more specific --- test/relations.integration.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/relations.integration.js b/test/relations.integration.js index 6d5302b5..5a856d9a 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -1002,7 +1002,7 @@ describe('relations - integration', function () { it('has a basic error handler', function(done) { var test = this; - this.get('/api/books/unknown/pages') + this.get('/api/books/unknown/pages/' + test.page.id + '/notes') .expect(404, function(err, res) { expect(res.body.error).to.be.an.object; var expected = 'could not find a model with id unknown'; From 097daf1debce1fe2cb2d72d2dd1859e4397e079d Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 4 Aug 2014 19:02:30 +0200 Subject: [PATCH 3/5] filterMethod option (fn) to filter nested remote methods --- lib/models/model.js | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/models/model.js b/lib/models/model.js index 8e018a34..3c9a21c9 100644 --- a/lib/models/model.js +++ b/lib/models/model.js @@ -502,9 +502,18 @@ Model.nestRemoting = function(relationName, options) { var acceptArgs = []; } + // A method should return the method name to use, if it is to be + // included as a nested method - a falsy return value will skip. + var filter = options.filterMethod || function(method, relation) { + var matches = method.name.match(regExp); + if (matches) { + return '__' + matches[1] + '__' + relation.name + '__' + matches[2]; + } + }; + sharedToClass.methods().forEach(function(method) { - var matches; - if (!method.isStatic && (matches = method.name.match(regExp))) { + var methodName; + if (!method.isStatic && (methodName = filter(method, relation))) { var prefix = relation.multiple ? '__findById__' : '__get__'; var getterName = options.getterName || (prefix + relationName); @@ -519,7 +528,6 @@ Model.nestRemoting = function(relationName, options) { } var opts = {}; - var methodName = '__' + matches[1] + '__' + relationName + '__' + matches[2]; opts.accepts = acceptArgs.concat(method.accepts || []); opts.returns = [].concat(method.returns || []); From cc0d376cc31a5cc777299af98fb55906abeaebe9 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Mon, 4 Aug 2014 19:08:43 +0200 Subject: [PATCH 4/5] filterMethod can also be a direct callback --- lib/models/model.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/models/model.js b/lib/models/model.js index 3c9a21c9..73ab8737 100644 --- a/lib/models/model.js +++ b/lib/models/model.js @@ -473,9 +473,14 @@ Model.scopeRemoting = function(relationName, relation, define) { }); }; -Model.nestRemoting = function(relationName, options) { - var regExp = /^__([^_]+)__([^_]+)$/; +Model.nestRemoting = function(relationName, options, cb) { + if (typeof options === 'function' && !cb) { + cb = options; + options = {}; + } options = options || {}; + + var regExp = /^__([^_]+)__([^_]+)$/; var relation = this.relations[relationName]; if (relation && relation.modelTo && relation.modelTo.sharedClass) { var self = this; @@ -504,7 +509,7 @@ Model.nestRemoting = function(relationName, options) { // A method should return the method name to use, if it is to be // included as a nested method - a falsy return value will skip. - var filter = options.filterMethod || function(method, relation) { + var filter = cb || options.filterMethod || function(method, relation) { var matches = method.name.match(regExp); if (matches) { return '__' + matches[1] + '__' + relation.name + '__' + matches[2]; From 539702ab3da73baac15402e44eba05a9a5a43411 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Tue, 5 Aug 2014 12:14:39 +0200 Subject: [PATCH 5/5] Inherit hooks when nesting Note that for now, the hook ctx.instance will be the root/entry object. Added `mounted` event - should be useful in other ways too. --- lib/application.js | 6 +++ lib/models/model.js | 76 +++++++++++++++++++++++++---------- test/relations.integration.js | 27 ++++++++++++- 3 files changed, 86 insertions(+), 23 deletions(-) diff --git a/lib/application.js b/lib/application.js index 9943f0a9..cee87927 100644 --- a/lib/application.js +++ b/lib/application.js @@ -269,6 +269,7 @@ app.remoteObjects = function () { /*! * Get a handler of the specified type from the handler cache. + * @triggers `mounted` events on shared class constructors (models) */ app.handler = function (type) { @@ -279,6 +280,11 @@ app.handler = function (type) { var remotes = this.remotes(); var handler = this._handlers[type] = remotes.handler(type); + + remotes.classes().forEach(function(sharedClass) { + sharedClass.ctor.emit('mounted', app, sharedClass, remotes); + }); + return handler; } diff --git a/lib/models/model.js b/lib/models/model.js index 73ab8737..4d73b130 100644 --- a/lib/models/model.js +++ b/lib/models/model.js @@ -538,6 +538,7 @@ Model.nestRemoting = function(relationName, options, cb) { opts.returns = [].concat(method.returns || []); opts.description = method.description; opts.rest = extend({}, method.rest || {}); + opts.rest.delegateTo = method.name; opts.http = []; var routes = [].concat(method.http || []); @@ -551,35 +552,66 @@ Model.nestRemoting = function(relationName, options, cb) { if (relation.multiple) { sharedClass.defineMethod(methodName, opts, function(fkId) { - var args = Array.prototype.slice.call(arguments, 1); - var last = args[args.length - 1]; - var cb = typeof last === 'function' ? last : null; - this[getterName](fkId, function(err, inst) { - if (err && cb) return cb(err); - if (inst instanceof relation.modelTo) { - nestedFn.apply(inst, args); - } else if (cb) { - cb(err, null); - } - }); + var args = Array.prototype.slice.call(arguments, 1); + var last = args[args.length - 1]; + var cb = typeof last === 'function' ? last : null; + this[getterName](fkId, function(err, inst) { + if (err && cb) return cb(err); + if (inst instanceof relation.modelTo) { + nestedFn.apply(inst, args); + } else if (cb) { + cb(err, null); + } + }); }, method.isStatic); } else { sharedClass.defineMethod(methodName, opts, function() { - var args = Array.prototype.slice.call(arguments); - var last = args[args.length - 1]; - var cb = typeof last === 'function' ? last : null; - this[getterName](function(err, inst) { - if (err && cb) return cb(err); - if (inst instanceof relation.modelTo) { - nestedFn.apply(inst, args); - } else if (cb) { - cb(err, null); - } - }); + var args = Array.prototype.slice.call(arguments); + var last = args[args.length - 1]; + var cb = typeof last === 'function' ? last : null; + this[getterName](function(err, inst) { + if (err && cb) return cb(err); + if (inst instanceof relation.modelTo) { + nestedFn.apply(inst, args); + } else if (cb) { + cb(err, null); + } + }); }, method.isStatic); } } }); + + if (options.hooks === false) return; // don't inherit before/after hooks + + self.once('mounted', function(app, sc, remotes) { + var listenerTree = extend({}, remotes.listenerTree || {}); + listenerTree.before = listenerTree.before || {}; + listenerTree.after = listenerTree.after || {}; + + var beforeListeners = remotes.listenerTree.before[toModelName] || {}; + var afterListeners = remotes.listenerTree.after[toModelName] || {}; + + sharedClass.methods().forEach(function(method) { + var delegateTo = method.rest && method.rest.delegateTo; + if (delegateTo) { + var before = method.isStatic ? beforeListeners : beforeListeners['prototype']; + var after = method.isStatic ? afterListeners : afterListeners['prototype']; + var m = method.isStatic ? method.name : 'prototype.' + method.name; + if (before[delegateTo]) { + self.beforeRemote(m, function(ctx, result, next) { + before[delegateTo]._listeners.call(null, ctx, next); + }); + } + if (after[delegateTo]) { + self.afterRemote(m, function(ctx, result, next) { + after[delegateTo]._listeners.call(null, ctx, next); + }); + } + } + }); + }); + } else { throw new Error('Relation `' + relationName + '` does not exist for model `' + this.modelName + '`'); } diff --git a/test/relations.integration.js b/test/relations.integration.js index 5a856d9a..58166cff 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -958,6 +958,17 @@ describe('relations - integration', function () { expect(Book.prototype['__findById__pages__notes']).to.be.a.function; expect(Image.prototype['__findById__book__pages']).to.be.a.function; + + Page.beforeRemote('prototype.__findById__notes', function(ctx, result, next) { + ctx.res.set('x-before', 'before'); + next(); + }); + + Page.afterRemote('prototype.__findById__notes', function(ctx, result, next) { + ctx.res.set('x-after', 'after'); + next(); + }); + }); before(function createBook(done) { @@ -989,7 +1000,7 @@ describe('relations - integration', function () { }); }); - it('has regular relationship routes', function(done) { + it('has regular relationship routes - pages', function(done) { var test = this; this.get('/api/books/' + test.book.id + '/pages') .expect(200, function(err, res) { @@ -1000,6 +1011,18 @@ describe('relations - integration', function () { }); }); + it('has regular relationship routes - notes', function(done) { + var test = this; + this.get('/api/pages/' + test.page.id + '/notes/' + test.note.id) + .expect(200, function(err, res) { + expect(res.headers['x-before']).to.equal('before'); + expect(res.headers['x-after']).to.equal('after'); + expect(res.body).to.be.an.object; + expect(res.body.text).to.equal('Page Note 1'); + done(); + }); + }); + it('has a basic error handler', function(done) { var test = this; this.get('/api/books/unknown/pages/' + test.page.id + '/notes') @@ -1047,6 +1070,8 @@ describe('relations - integration', function () { var test = this; this.get('/api/books/' + test.book.id + '/pages/' + test.page.id + '/notes/' + test.note.id) .expect(200, function(err, res) { + expect(res.headers['x-before']).to.equal('before'); + expect(res.headers['x-after']).to.equal('after'); expect(res.body).to.be.an.object; expect(res.body.text).to.equal('Page Note 1'); done();