From 94b2a45a6c9672bbb3fa7c2b80bd41c5f7db6b2b Mon Sep 17 00:00:00 2001 From: Clark Wang Date: Thu, 1 Jan 2015 15:26:58 +0800 Subject: [PATCH 01/11] fix nestRemoting is nesting hooks from other relations Signed-off-by: Clark Wang --- lib/model.js | 12 ++++++------ test/relations.integration.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/lib/model.js b/lib/model.js index d93e4ab4..3ea73d9b 100644 --- a/lib/model.js +++ b/lib/model.js @@ -654,7 +654,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.rest.delegateTo = method; opts.http = []; var routes = [].concat(method.http || []); @@ -718,18 +718,18 @@ Model.nestRemoting = function(relationName, options, cb) { sharedClass.methods().forEach(function(method) { var delegateTo = method.rest && method.rest.delegateTo; - if (delegateTo) { + if (delegateTo && delegateTo.ctor == relation.modelTo) { 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 && before[delegateTo]) { + if (before && before[delegateTo.name]) { self.beforeRemote(m, function(ctx, result, next) { - before[delegateTo]._listeners.call(null, ctx, next); + before[delegateTo.name]._listeners.call(null, ctx, next); }); } - if (after && after[delegateTo]) { + if (after && after[delegateTo.name]) { self.afterRemote(m, function(ctx, result, next) { - after[delegateTo]._listeners.call(null, ctx, next); + after[delegateTo.name]._listeners.call(null, ctx, next); }); } } diff --git a/test/relations.integration.js b/test/relations.integration.js index f1aa72da..59e77df2 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -1164,8 +1164,15 @@ describe('relations - integration', function() { { properties: { text: 'string' }, dataSource: 'db', plural: 'notes' } ); + var Chapter = app.model( + 'Chapter', + { properties: { name: 'string' }, dataSource: 'db', + plural: 'chapters' } + ); Book.hasMany(Page); + Book.hasMany(Chapter); Page.hasMany(Note); + Chapter.hasMany(Note); Image.belongsTo(Book); // fake a remote method that match the filter in Model.nestRemoting() @@ -1176,6 +1183,7 @@ describe('relations - integration', function() { Page.remoteMethod('__throw__errors', { isStatic: false, http: { path: '/throws', verb: 'get' } }); Book.nestRemoting('pages'); + Book.nestRemoting('chapters'); Image.nestRemoting('book'); expect(Book.prototype['__findById__pages__notes']).to.be.a.function; @@ -1212,6 +1220,19 @@ describe('relations - integration', function() { }); }); + before(function createChapters(done) { + var test = this, book = test.book; + book.chapters.create({ name: 'Chapter 1' }, + function(err, chapter) { + if (err) return done(err); + test.chapter = chapter; + chapter.notes.create({ text: 'Chapter Note 1' }, function(err, note) { + test.cnote = note; + done(); + }); + }); + }); + before(function createCover(done) { var test = this; app.models.Image.create({ name: 'Cover 1', book: test.book }, @@ -1300,6 +1321,16 @@ describe('relations - integration', function() { }); }); + it('should nest remote hooks of ModelTo - hasMany findById', function(done) { + var test = this; + this.get('/api/books/' + test.book.id + '/chapters/' + test.chapter.id + '/notes/' + test.cnote.id) + .expect(200, function(err, res) { + expect(res.headers['x-before']).to.empty(); + expect(res.headers['x-after']).to.empty(); + done(); + }); + }); + it('should have proper http.path for remoting', function() { [app.models.Book, app.models.Image].forEach(function(Model) { Model.sharedClass.methods().forEach(function(method) { From 58f67e92d157ba4fb23ee1ada3541e376858e859 Mon Sep 17 00:00:00 2001 From: Clark Wang Date: Sun, 4 Jan 2015 18:24:29 +0800 Subject: [PATCH 02/11] fix jscs warning Signed-off-by: Clark Wang --- test/relations.integration.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/relations.integration.js b/test/relations.integration.js index 59e77df2..edd90064 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -1221,8 +1221,8 @@ describe('relations - integration', function() { }); before(function createChapters(done) { - var test = this, book = test.book; - book.chapters.create({ name: 'Chapter 1' }, + var test = this; + test.book.chapters.create({ name: 'Chapter 1' }, function(err, chapter) { if (err) return done(err); test.chapter = chapter; From 3b4cadf7a3c87978f6c4cb23fcb786ee77169e86 Mon Sep 17 00:00:00 2001 From: Ron Edgecomb Date: Mon, 22 Dec 2014 15:39:47 -0500 Subject: [PATCH 03/11] Update to demonstrate unit test is actually failing due to incorrect values of invalidCredentials - strongloop/loopback#944 --- test/user.test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/user.test.js b/test/user.test.js index 60ef9182..a8852c83 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -223,6 +223,9 @@ describe('User', function() { .expect(401) .send(invalidCredentials) .end(function(err, res) { + if (err) { + return done(err); + } done(); }); }); From 6de1da5d223146cf9fc1152febf1ae0fa538ff29 Mon Sep 17 00:00:00 2001 From: Ron Edgecomb Date: Mon, 22 Dec 2014 15:41:49 -0500 Subject: [PATCH 04/11] Correct invalidCredentials so that it differs from validCredentialsEmailVerified, unit test now passes as desired. - strongloop/loopback#944 --- test/user.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/user.test.js b/test/user.test.js index a8852c83..76c6b4c9 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -11,7 +11,7 @@ describe('User', function() { var validCredentialsEmailVerified = {email: 'foo1@bar.com', password: 'bar1', emailVerified: true}; var validCredentialsEmailVerifiedOverREST = {email: 'foo2@bar.com', password: 'bar2', emailVerified: true}; var validCredentialsWithTTL = {email: 'foo@bar.com', password: 'bar', ttl: 3600}; - var invalidCredentials = {email: 'foo1@bar.com', password: 'bar1'}; + var invalidCredentials = {email: 'foo1@bar.com', password: 'invalid'}; var incompleteCredentials = {password: 'bar1'}; beforeEach(function() { From 572a8bb4237fa9d953a257a8f047bdc2b79e9a85 Mon Sep 17 00:00:00 2001 From: Ron Edgecomb Date: Mon, 22 Dec 2014 15:48:12 -0500 Subject: [PATCH 05/11] Ensure error checking logic is in place for all REST calls, expand formatting for consistency with existing instances. - strongloop/loopback#944 --- test/user.test.js | 44 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) diff --git a/test/user.test.js b/test/user.test.js index 76c6b4c9..daacc031 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -142,6 +142,9 @@ describe('User', function() { .expect(200) .send(validCredentialsEmailVerifiedOverREST) .end(function(err, res) { + if (err) { + return done(err); + } assert(!res.body.emailVerified); done(); }); @@ -204,7 +207,9 @@ describe('User', function() { .expect(200) .send(validCredentials) .end(function(err, res) { - if (err) return done(err); + if (err) { + return done(err); + } var accessToken = res.body; assert(accessToken.userId); @@ -237,6 +242,9 @@ describe('User', function() { .expect(400) .send(incompleteCredentials) .end(function(err, res) { + if (err) { + return done(err); + } done(); }); }); @@ -249,6 +257,9 @@ describe('User', function() { .expect(400) .send(validCredentials) .end(function(err, res) { + if (err) { + return done(err); + } done(); }); }); @@ -260,7 +271,9 @@ describe('User', function() { .expect(200) .expect('Content-Type', /json/) .end(function(err, res) { - if (err) return done(err); + if (err) { + return done(err); + } var token = res.body; expect(token.user, 'body.user').to.not.equal(undefined); expect(token.user, 'body.user') @@ -276,7 +289,9 @@ describe('User', function() { .expect(200) .expect('Content-Type', /json/) .end(function(err, res) { - if (err) return done(err); + if (err) { + return done(err); + } var token = res.body; expect(token.user, 'body.user').to.not.equal(undefined); expect(token.user, 'body.user') @@ -332,7 +347,9 @@ describe('User', function() { .expect(200) .send(validCredentialsEmailVerified) .end(function(err, res) { - if (err) return done(err); + if (err) { + return done(err); + } var accessToken = res.body; assertGoodToken(accessToken); @@ -349,6 +366,9 @@ describe('User', function() { .expect(401) .send(validCredentials) .end(function(err, res) { + if (err) { + return done(err); + } done(); }); }); @@ -538,7 +558,9 @@ describe('User', function() { .expect(200) .send({email: 'foo@bar.com', password: 'bar'}) .end(function(err, res) { - if (err) return done(err); + if (err) { + return done(err); + } var accessToken = res.body; assert(accessToken.userId); @@ -650,7 +672,9 @@ describe('User', function() { .expect(200) .send({email: 'bar@bat.com', password: 'bar'}) .end(function(err, res) { - if (err) return done(err); + if (err) { + return done(err); + } }); }); @@ -681,7 +705,9 @@ describe('User', function() { .expect(200) .send({email: 'bar@bat.com', password: 'bar'}) .end(function(err, res) { - if (err) return done(err); + if (err) { + return done(err); + } }); }); @@ -764,7 +790,9 @@ describe('User', function() { + '&redirect=' + encodeURIComponent(options.redirect)) .expect(400) .end(function(err, res) { - if (err) return done(err); + if (err) { + return done(err); + } assert(res.body.error); done(); }); From e4a1baa4a33b9d127d2c9847d0aaea1e4409e647 Mon Sep 17 00:00:00 2001 From: Ron Edgecomb Date: Mon, 22 Dec 2014 16:49:12 -0500 Subject: [PATCH 06/11] Force request to send body as string, this ensures headers aren't automatically set to application/json - strongloop/loopback#944 --- test/user.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/user.test.js b/test/user.test.js index daacc031..d88f0f33 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -255,7 +255,7 @@ describe('User', function() { .set('Content-Type', null) .expect('Content-Type', /json/) .expect(400) - .send(validCredentials) + .send(JSON.stringify(validCredentials)) .end(function(err, res) { if (err) { return done(err); From 36112d2b509eded44f107630d63621cd78d63a4d Mon Sep 17 00:00:00 2001 From: Ron Edgecomb Date: Mon, 22 Dec 2014 17:20:15 -0500 Subject: [PATCH 07/11] Simplify the API test for invalidCredentials (removed create), move above REST calls for better grouping of tests - strongloop/loopback#944 --- test/user.test.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/test/user.test.js b/test/user.test.js index d88f0f33..cf96924a 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -200,6 +200,14 @@ describe('User', function() { }); }); + it('Login should only allow correct credentials', function(done) { + User.login(invalidCredentials, function(err, accessToken) { + assert(err); + assert(!accessToken); + done(); + }); + }); + it('Login a user over REST by providing credentials', function(done) { request(app) .post('/users/login') @@ -300,15 +308,6 @@ describe('User', function() { }); }); - it('Login should only allow correct credentials', function(done) { - User.create({email: 'foo22@bar.com', password: 'bar'}, function(user, err) { - User.login({email: 'foo44@bar.com', password: 'bar'}, function(err, accessToken) { - assert(err); - assert(!accessToken); - done(); - }); - }); - }); }); function assertGoodToken(accessToken) { From 9ac620c11340364733d6aa8ea6730fe67d9c38b5 Mon Sep 17 00:00:00 2001 From: Ron Edgecomb Date: Mon, 22 Dec 2014 17:24:30 -0500 Subject: [PATCH 08/11] Small formatting update to have consistency with identical logic in other areas. - strongloop/loopback#944 --- test/user.test.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/user.test.js b/test/user.test.js index cf96924a..260e9c99 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -582,7 +582,9 @@ describe('User', function() { assert(token); return function(err) { - if (err) return done(err); + if (err) { + return done(err); + } AccessToken.findById(token, function(err, accessToken) { assert(!accessToken, 'accessToken should not exist after logging out'); From 62bb63b4f252a9ddac6c1296bc38ff90a0549bc5 Mon Sep 17 00:00:00 2001 From: Ron Edgecomb Date: Mon, 22 Dec 2014 22:12:50 -0500 Subject: [PATCH 09/11] Additional password reset unit tests for API and REST - strongloop/loopback#944 --- test/user.test.js | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/test/user.test.js b/test/user.test.js index 260e9c99..a3b43c93 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -804,9 +804,17 @@ describe('User', function() { describe('Password Reset', function() { describe('User.resetPassword(options, cb)', function() { + var email = 'foo@bar.com'; + + it('Requires email address to reset password', function(done) { + User.resetPassword({ }, function(err) { + assert(err); + done(); + }); + }); + it('Creates a temp accessToken to allow a user to change password', function(done) { var calledBack = false; - var email = 'foo@bar.com'; User.resetPassword({ email: email @@ -826,6 +834,35 @@ describe('User', function() { }); }); }); + + it('Password reset over REST rejected without email address', function(done) { + request(app) + .post('/users/reset') + .expect('Content-Type', /json/) + .expect(400) + .send({ }) + .end(function(err, res) { + if (err) { + return done(err); + } + done(); + }); + }); + + it('Password reset over REST requires email address', function(done) { + request(app) + .post('/users/reset') + .expect('Content-Type', /json/) + .expect(204) + .send({ email: email }) + .end(function(err, res) { + if (err) { + return done(err); + } + assert.deepEqual(res.body, { }); + done(); + }); + }); }); }); From ca0208ddd936a8573f1af6eb18edaff329fd4571 Mon Sep 17 00:00:00 2001 From: Pham Anh Tuan Date: Mon, 1 Dec 2014 17:36:34 +0700 Subject: [PATCH 10/11] Fix context middleware to preserve domains When executing a request using a pooled connection, connectors like MongoDB and/or MySQL rebind callbacks to the domain which issued the request, as opposed to the domain which opened the pooled connection. This commit fixes the context middleware to play nicely with that mechanism and preserve domain rebinds. --- server/middleware/context.js | 26 ++++++++++--- test/loopback.test.js | 71 ++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 6 deletions(-) diff --git a/server/middleware/context.js b/server/middleware/context.js index c86903a0..23912db4 100644 --- a/server/middleware/context.js +++ b/server/middleware/context.js @@ -2,6 +2,7 @@ var loopback = require('../../lib/loopback'); var juggler = require('loopback-datasource-juggler'); var remoting = require('strong-remoting'); var cls = require('continuation-local-storage'); +var domain = require('domain'); module.exports = context; @@ -44,6 +45,13 @@ function context(options) { var scope = options.name || name; var enableHttpContext = options.enableHttpContext || false; var ns = createContext(scope); + + var currentDomain = process.domain = domain.create(); + currentDomain.oldBind = currentDomain.bind; + currentDomain.bind = function(callback, context) { + return currentDomain.oldBind(ns.bind(callback, context), context); + }; + // Return the middleware return function contextHandler(req, res, next) { if (req.loopbackContext) { @@ -53,13 +61,19 @@ function context(options) { // Bind req/res event emitters to the given namespace ns.bindEmitter(req); ns.bindEmitter(res); + + currentDomain.add(req); + currentDomain.add(res); + // Create namespace for the request context - ns.run(function processRequestInContext(context) { - // Run the code in the context of the namespace - if (enableHttpContext) { - ns.set('http', {req: req, res: res}); // Set up the transport context - } - next(); + currentDomain.run(function() { + ns.run(function processRequestInContext(context) { + // Run the code in the context of the namespace + if (enableHttpContext) { + ns.set('http', {req: req, res: res}); // Set up the transport context + } + next(); + }); }); }; } diff --git a/test/loopback.test.js b/test/loopback.test.js index 8f217f6a..26c4f22f 100644 --- a/test/loopback.test.js +++ b/test/loopback.test.js @@ -1,4 +1,7 @@ var it = require('./util/it'); +var describe = require('./util/describe'); +var Domain = require('domain'); +var EventEmitter = require('events').EventEmitter; describe('loopback', function() { var nameCounter = 0; @@ -388,4 +391,72 @@ describe('loopback', function() { }); }); }); + + describe.onServer('loopback.getCurrentContext', function() { + var runInOtherDomain; + var runnerInterval; + + before(function setupRunInOtherDomain() { + var emitterInOtherDomain = new EventEmitter(); + Domain.create().add(emitterInOtherDomain); + + runInOtherDomain = function(fn) { + emitterInOtherDomain.once('run', fn); + }; + + runnerInterval = setInterval(function() { + emitterInOtherDomain.emit('run'); + }, 10); + }); + + after(function tearDownRunInOtherDomain() { + clearInterval(runnerInterval); + }); + + // See the following two items for more details: + // https://github.com/strongloop/loopback/issues/809 + // https://github.com/strongloop/loopback/pull/337#issuecomment-61680577 + it('preserves callback domain', function(done) { + var app = loopback(); + app.use(loopback.rest()); + app.dataSource('db', { connector: 'memory' }); + + var TestModel = loopback.createModel({ name: 'TestModel' }); + app.model(TestModel, { dataSource: 'db', public: true }); + + // function for remote method + TestModel.test = function(inst, cb) { + var tmpCtx = loopback.getCurrentContext(); + if (tmpCtx) tmpCtx.set('data', 'a value stored in context'); + if (process.domain) cb = process.domain.bind(cb); // IMPORTANT + runInOtherDomain(cb); + }; + + // remote method + TestModel.remoteMethod('test', { + accepts: { arg: 'inst', type: uniqueModelName }, + returns: { root: true }, + http: { path: '/test', verb: 'get' } + }); + + // after remote hook + TestModel.afterRemote('**', function(ctxx, inst, next) { + var tmpCtx = loopback.getCurrentContext(); + if (tmpCtx) { + ctxx.result.data = tmpCtx.get('data'); + }else { + ctxx.result.data = 'context not available'; + } + next(); + }); + + request(app) + .get('/TestModels/test') + .end(function(err, res) { + if (err) return done(err); + expect(res.body.data).to.equal('a value stored in context'); + done(); + }); + }); + }); }); From fc6022256960e650f05587b23c3756ea2e50c47c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Wed, 7 Jan 2015 12:08:07 +0100 Subject: [PATCH 11/11] v2.8.8 --- CHANGES.md | 54 +++++++++++++++++++++++++++++++++------------------- package.json | 4 ++-- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 5e4a8979..26f35006 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,27 @@ +2015-01-07, Version 2.8.8 +========================= + + * Fix context middleware to preserve domains (Pham Anh Tuan) + + * Additional password reset unit tests for API and REST - strongloop/loopback#944 (Ron Edgecomb) + + * Small formatting update to have consistency with identical logic in other areas. - strongloop/loopback#944 (Ron Edgecomb) + + * Simplify the API test for invalidCredentials (removed create), move above REST calls for better grouping of tests - strongloop/loopback#944 (Ron Edgecomb) + + * Force request to send body as string, this ensures headers aren't automatically set to application/json - strongloop/loopback#944 (Ron Edgecomb) + + * Ensure error checking logic is in place for all REST calls, expand formatting for consistency with existing instances. - strongloop/loopback#944 (Ron Edgecomb) + + * Correct invalidCredentials so that it differs from validCredentialsEmailVerified, unit test now passes as desired. - strongloop/loopback#944 (Ron Edgecomb) + + * Update to demonstrate unit test is actually failing due to incorrect values of invalidCredentials - strongloop/loopback#944 (Ron Edgecomb) + + * fix jscs warning (Clark Wang) + + * fix nestRemoting is nesting hooks from other relations (Clark Wang) + + 2015-01-06, Version 2.8.7 ========================= @@ -671,6 +695,10 @@ * Enhance the error message (Raymond Feng) + +2014-07-16, Version 2.0.0-beta7 +=============================== + * Bump version (Raymond Feng) * 2.0.0-beta6 (Miroslav Bajtoš) @@ -811,13 +839,6 @@ 2014-07-16, Version 1.10.0 ========================== - - -2014-07-16, Version 2.0.0-beta7 -=============================== - - * Bump version (Raymond Feng) - * Remove unused dep (Raymond Feng) * Bump version and update deps (Raymond Feng) @@ -1264,14 +1285,6 @@ * 2.0.0-beta1 (Ritchie Martori) - * Bump version (Raymond Feng) - - * Add postgresql to the keywords (Raymond Feng) - - * updated package.json with SOAP and framework keywords (altsang) - - * updated package.json with keywords and updated description (Raymond Feng) - * Make app.datasources unique per app instance (Miroslav Bajtoš) * Add RC version (Ritchie Martori) @@ -1337,11 +1350,6 @@ * Add Change model (Ritchie Martori) -2014-05-27, Version 1.8.4 -========================= - - - 2014-05-27, Version 1.8.5 ========================= @@ -1353,8 +1361,14 @@ * updated package.json with keywords and updated description (Raymond Feng) + +2014-05-27, Version 1.8.4 +========================= + * Add more keywords (Raymond Feng) + * Bump version (Raymond Feng) + * app: flatten model config (Miroslav Bajtoš) * Fix the test for mocha 1.19.0 (Raymond Feng) diff --git a/package.json b/package.json index 12680342..36a94553 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback", - "version": "2.8.7", + "version": "2.8.8", "description": "LoopBack: Open Source Framework for Node.js", "homepage": "http://loopback.io", "keywords": [ @@ -102,6 +102,6 @@ "url": "https://github.com/strongloop/loopback/blob/master/LICENSE" }, "optionalDependencies": { - "sl-blip": "http://blip.strongloop.com/loopback@2.8.7" + "sl-blip": "http://blip.strongloop.com/loopback@2.8.8" } }