From 5974c6afdf73ecc4604ffcd686332c3f3d3d0838 Mon Sep 17 00:00:00 2001 From: crandmck Date: Fri, 8 Jan 2016 17:10:44 -0800 Subject: [PATCH 01/27] Pull in API doc fix from PR into master #1910 --- lib/persisted-model.js | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/persisted-model.js b/lib/persisted-model.js index b3bb62281..ab17ef16c 100644 --- a/lib/persisted-model.js +++ b/lib/persisted-model.js @@ -111,13 +111,29 @@ module.exports = function(registry) { }; /** - * Find one record matching the optional `where` filter. The same as `find`, but limited to one object. - * Returns an object, not collection. - * If not found, create the object using data provided as second argument. + * Finds one record matching the optional filter object. If not found, creates + * the object using the data provided as second argument. In this sense it is + * the same as `find`, but limited to one object. Returns an object, not + * collection. If you don't provide the filter object argument, it tries to + * locate an existing object that matches the `data` argument. * - * @param {Object} where Where clause, such as `{test: 'me'}` - *
see - * [Where filter](https://docs.strongloop.com/display/LB/Where+filter#Wherefilter-Whereclauseforothermethods). + * @options {Object} [filter] Optional Filter object; see below. + * @property {String|Object|Array} fields Identify fields to include in return result. + *
See [Fields filter](http://docs.strongloop.com/display/LB/Fields+filter). + * @property {String|Object|Array} include See PersistedModel.include documentation. + *
See [Include filter](http://docs.strongloop.com/display/LB/Include+filter). + * @property {Number} limit Maximum number of instances to return. + *
See [Limit filter](http://docs.strongloop.com/display/LB/Limit+filter). + * @property {String} order Sort order: either "ASC" for ascending or "DESC" for descending. + *
See [Order filter](http://docs.strongloop.com/display/LB/Order+filter). + * @property {Number} skip Number of results to skip. + *
See [Skip filter](http://docs.strongloop.com/display/LB/Skip+filter). + * @property {Object} where Where clause, like + * ``` + * {where: {key: val, key2: {gt: val2}, ...}} + * ``` + *
See + * [Where filter](https://docs.strongloop.com/display/LB/Where+filter#Wherefilter-Whereclauseforqueries). * @param {Object} data Data to insert if object matching the `where` filter is not found. * @callback {Function} callback Callback function called with `cb(err, instance)` arguments. Required. * @param {Error} err Error object; see [Error object](http://docs.strongloop.com/display/LB/Error+object). From 8deec2e89aeea65dfbd8a9f43e2dc15a11faef15 Mon Sep 17 00:00:00 2001 From: Amir Jafarian Date: Thu, 17 Dec 2015 12:02:35 -0500 Subject: [PATCH 02/27] Checkpoint speedup --- common/models/checkpoint.js | 64 ++++++++++++++++---------------- lib/persisted-model.js | 7 +--- test/checkpoint.test.js | 74 ++++++++++++++++++++++++++++++++----- 3 files changed, 99 insertions(+), 46 deletions(-) diff --git a/common/models/checkpoint.js b/common/models/checkpoint.js index 304f83148..59cc33de0 100644 --- a/common/models/checkpoint.js +++ b/common/models/checkpoint.js @@ -27,43 +27,45 @@ module.exports = function(Checkpoint) { * Get the current checkpoint id * @callback {Function} callback * @param {Error} err - * @param {Number} checkpointId The current checkpoint id + * @param {Number} checkpoint The current checkpoint seq */ - Checkpoint.current = function(cb) { var Checkpoint = this; - this.find({ - limit: 1, - order: 'seq DESC' - }, function(err, checkpoints) { - if (err) return cb(err); - var checkpoint = checkpoints[0]; - if (checkpoint) { - cb(null, checkpoint.seq); - } else { - Checkpoint.create({ seq: 1 }, function(err, checkpoint) { - if (err) return cb(err); - cb(null, checkpoint.seq); - }); - } + Checkpoint._getSingleton(function(err, cp) { + cb(err, cp.seq); }); }; - Checkpoint.observe('before save', function(ctx, next) { - if (!ctx.instance) { - // Example: Checkpoint.updateAll() and Checkpoint.updateOrCreate() - return next(new Error('Checkpoint does not support partial updates.')); - } + Checkpoint._getSingleton = function(cb) { + var query = {limit: 1}; // match all instances, return only one + var initialData = {seq: 1}; + this.findOrCreate(query, initialData, cb); + }; - var model = ctx.instance; - if (!model.getId() && model.seq === undefined) { - model.constructor.current(function(err, seq) { - if (err) return next(err); - model.seq = seq + 1; - next(); + /** + * Increase the current checkpoint if it already exists otherwise initialize it + * @callback {Function} callback + * @param {Error} err + * @param {Object} checkpoint The current checkpoint + */ + Checkpoint.bumpLastSeq = function(cb) { + var Checkpoint = this; + Checkpoint._getSingleton(function(err, cp) { + if (err) return cb(err); + var originalSeq = cp.seq; + cp.seq++; + // Update the checkpoint but only if it was not changed under our hands + Checkpoint.updateAll({id: cp.id, seq: originalSeq}, {seq: cp.seq}, function(err, info) { + if (err) return cb(err); + // possible outcomes + // 1) seq was updated to seq+1 - exactly what we wanted! + // 2) somebody else already updated seq to seq+1 and our call was a no-op. + // That should be ok, checkpoints are time based, so we reuse the one created just now + // 3) seq was bumped more than once, so we will be using a value that is behind the latest seq. + // @bajtos is not entirely sure if this is ok, but since it wasn't handled by the current implementation either, + // he thinks we can keep it this way. + cb(null, cp); }); - } else { - next(); - } - }); + }); + }; }; diff --git a/lib/persisted-model.js b/lib/persisted-model.js index ab17ef16c..bcac8e18d 100644 --- a/lib/persisted-model.js +++ b/lib/persisted-model.js @@ -898,12 +898,7 @@ module.exports = function(registry) { PersistedModel.checkpoint = function(cb) { var Checkpoint = this.getChangeModel().getCheckpointModel(); - this.getSourceId(function(err, sourceId) { - if (err) return cb(err); - Checkpoint.create({ - sourceId: sourceId - }, cb); - }); + Checkpoint.bumpLastSeq(cb); }; /** diff --git a/test/checkpoint.test.js b/test/checkpoint.test.js index 98e248efb..c824d02ea 100644 --- a/test/checkpoint.test.js +++ b/test/checkpoint.test.js @@ -1,20 +1,22 @@ var async = require('async'); var loopback = require('../'); +var expect = require('chai').expect; -// create a unique Checkpoint model var Checkpoint = loopback.Checkpoint.extend('TestCheckpoint'); -var memory = loopback.createDataSource({ - connector: loopback.Memory -}); -Checkpoint.attachTo(memory); - describe('Checkpoint', function() { - describe('current()', function() { + describe('bumpLastSeq() and current()', function() { + beforeEach(function() { + var memory = loopback.createDataSource({ + connector: loopback.Memory + }); + Checkpoint.attachTo(memory); + }); + it('returns the highest `seq` value', function(done) { async.series([ - Checkpoint.create.bind(Checkpoint), - Checkpoint.create.bind(Checkpoint), + Checkpoint.bumpLastSeq.bind(Checkpoint), + Checkpoint.bumpLastSeq.bind(Checkpoint), function(next) { Checkpoint.current(function(err, seq) { if (err) next(err); @@ -24,5 +26,59 @@ describe('Checkpoint', function() { } ], done); }); + + it('Should be no race condition for current() when calling in parallel', function(done) { + async.parallel([ + function(next) { Checkpoint.current(next); }, + function(next) { Checkpoint.current(next); } + ], function(err, list) { + if (err) return done(err); + Checkpoint.find(function(err, data) { + if (err) return done(err); + expect(data).to.have.length(1); + done(); + }); + }); + }); + + it('Should be no race condition for bumpLastSeq() when calling in parallel', function(done) { + async.parallel([ + function(next) { Checkpoint.bumpLastSeq(next); }, + function(next) { Checkpoint.bumpLastSeq(next); } + ], function(err, list) { + if (err) return done(err); + Checkpoint.find(function(err, data) { + if (err) return done(err); + // The invariant "we have at most 1 checkpoint instance" is preserved + // even when multiple calls are made in parallel + expect(data).to.have.length(1); + // There is a race condition here, we could end up with both 2 or 3 as the "seq". + // The current implementation of the memory connector always yields 2 though. + expect(data[0].seq).to.equal(2); + // In this particular case, since the new last seq is always 2, both results + // should be 2. + expect(list.map(function(it) {return it.seq;})) + .to.eql([2, 2]); + done(); + }); + }); + }); + + it('Checkpoint.current() for non existing checkpoint should initialize checkpoint', function(done) { + Checkpoint.current(function(err, seq) { + expect(seq).to.equal(1); + done(err); + }); + }); + + it('bumpLastSeq() works when singleton instance does not exists yet', function(done) { + Checkpoint.bumpLastSeq(function(err, cp) { + // We expect `seq` to be 2 since `checkpoint` does not exist and + // `bumpLastSeq` for the first time not only initializes it to one, + // but also increments the initialized value by one. + expect(cp.seq).to.equal(2); + done(err); + }); + }); }); }); From 17bd1016910780cafbab463c669ea70fbd49d8ef Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Thu, 31 Dec 2015 16:52:31 -0800 Subject: [PATCH 03/27] ensure app is booted before integration tests --- test/access-control.integration.js | 6 ++++++ test/relations.integration.js | 6 ++++++ test/remoting.integration.js | 6 ++++++ test/user.integration.js | 6 ++++++ 4 files changed, 24 insertions(+) diff --git a/test/access-control.integration.js b/test/access-control.integration.js index 3125a6535..7e417bfe3 100644 --- a/test/access-control.integration.js +++ b/test/access-control.integration.js @@ -11,6 +11,12 @@ var CURRENT_USER = {email: 'current@test.test', password: 'test'}; var debug = require('debug')('loopback:test:access-control.integration'); describe('access control - integration', function() { + before(function(done) { + if (app.booting) { + return app.once('booted', done); + } + done(); + }); lt.beforeEach.withApp(app); diff --git a/test/relations.integration.js b/test/relations.integration.js index 70f1ae122..5257338b4 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -11,6 +11,12 @@ var debug = require('debug')('loopback:test:relations.integration'); var async = require('async'); describe('relations - integration', function() { + before(function(done) { + if (app.booting) { + return app.once('booted', done); + } + done(); + }); lt.beforeEach.withApp(app); diff --git a/test/remoting.integration.js b/test/remoting.integration.js index c3288d799..b2b92cd81 100644 --- a/test/remoting.integration.js +++ b/test/remoting.integration.js @@ -6,6 +6,12 @@ var app = require(path.join(SIMPLE_APP, 'server/server.js')); var assert = require('assert'); describe('remoting - integration', function() { + before(function(done) { + if (app.booting) { + return app.once('booted', done); + } + done(); + }); lt.beforeEach.withApp(app); lt.beforeEach.givenModel('store'); diff --git a/test/user.integration.js b/test/user.integration.js index b2a92537b..fd4a198bc 100644 --- a/test/user.integration.js +++ b/test/user.integration.js @@ -7,6 +7,12 @@ var app = require(path.join(SIMPLE_APP, 'server/server.js')); var expect = require('chai').expect; describe('users - integration', function() { + before(function(done) { + if (app.booting) { + return app.once('booted', done); + } + done(); + }); lt.beforeEach.withApp(app); From aff49ff63fe37203658df7bf10e7942128ca6406 Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Thu, 31 Dec 2015 16:53:40 -0800 Subject: [PATCH 04/27] test: fail on error instead of crash If the supertest request fails its basic assertions, there may not even be a body to perform checks against, so bail early when possible. --- test/relations.integration.js | 52 ++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 7 deletions(-) diff --git a/test/relations.integration.js b/test/relations.integration.js index 5257338b4..dbb2e2556 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -97,6 +97,7 @@ describe('relations - integration', function() { this.get(url) .query({'filter': {'include' : 'pictures'}}) .expect(200, function(err, res) { + if (err) return done(err); // console.log(res.body); expect(res.body.name).to.be.equal('Reader 1'); expect(res.body.pictures).to.be.eql([ @@ -112,6 +113,7 @@ describe('relations - integration', function() { this.get(url) .query({'filter': {'include' : 'imageable'}}) .expect(200, function(err, res) { + if (err) return done(err); // console.log(res.body); expect(res.body[0].name).to.be.equal('Picture 1'); expect(res.body[1].name).to.be.equal('Picture 2'); @@ -125,6 +127,7 @@ describe('relations - integration', function() { this.get(url) .query({'filter': {'include' : {'relation': 'imageable', 'scope': { 'include' : 'team'}}}}) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body[0].name).to.be.equal('Picture 1'); expect(res.body[1].name).to.be.equal('Picture 2'); expect(res.body[0].imageable.name).to.be.eql('Reader 1'); @@ -139,6 +142,7 @@ describe('relations - integration', function() { it('should invoke scoped methods remotely', function(done) { this.get('/api/stores/superStores') .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.array; done(); }); @@ -375,7 +379,7 @@ describe('relations - integration', function() { self.url = root.relUrl; self.patient = root.patient; self.physician = root.physician; - done(); + done(err); }); }); @@ -414,7 +418,7 @@ describe('relations - integration', function() { self.url = root.relUrl; self.patient = root.patient; self.physician = root.physician; - done(); + done(err); }); }); @@ -459,7 +463,7 @@ describe('relations - integration', function() { self.url = root.relUrl; self.patient = root.patient; self.physician = root.physician; - done(); + done(err); }); }); @@ -479,7 +483,7 @@ describe('relations - integration', function() { '/patients/rel/' + '999'; self.patient = root.patient; self.physician = root.physician; - done(); + done(err); }); }); @@ -498,7 +502,7 @@ describe('relations - integration', function() { self.url = root.relUrl; self.patient = root.patient; self.physician = root.physician; - done(); + done(err); }); }); @@ -553,7 +557,7 @@ describe('relations - integration', function() { '/patients/' + root.patient.id; self.patient = root.patient; self.physician = root.physician; - done(); + done(err); }); }); @@ -574,7 +578,7 @@ describe('relations - integration', function() { '/patients/' + root.patient.id; self.patient = root.patient; self.physician = root.physician; - done(); + done(err); }); }); @@ -690,6 +694,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.have.property('products'); expect(res.body.products).to.eql([ { @@ -709,6 +714,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.have.property('products'); expect(res.body.products).to.eql([ { @@ -770,6 +776,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body.name).to.be.equal('Group 1'); expect(res.body.poster).to.be.eql( { url: 'http://image.url' } @@ -783,6 +790,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql( { url: 'http://image.url' } ); @@ -806,6 +814,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql( { url: 'http://changed.url' } ); @@ -863,6 +872,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body.name).to.be.equal('List A'); expect(res.body.todoItems).to.be.eql([ { content: 'Todo 1', id: 1 }, @@ -877,6 +887,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { content: 'Todo 1', id: 1 }, { content: 'Todo 2', id: 2 } @@ -891,6 +902,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { content: 'Todo 2', id: 2 } ]); @@ -916,6 +928,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { content: 'Todo 1', id: 1 }, { content: 'Todo 2', id: 2 }, @@ -930,6 +943,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql( { content: 'Todo 3', id: 3 } ); @@ -952,6 +966,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { content: 'Todo 1', id: 1 }, { content: 'Todo 3', id: 3 } @@ -963,6 +978,7 @@ describe('relations - integration', function() { it('returns a 404 response when embedded model is not found', function(done) { var url = '/api/todo-lists/' + this.todoList.id + '/items/2'; this.get(url).expect(404, function(err, res) { + if (err) return done(err); expect(res.body.error.status).to.be.equal(404); expect(res.body.error.message).to.be.equal('Unknown "todoItem" id "2".'); expect(res.body.error.code).to.be.equal('MODEL_NOT_FOUND'); @@ -1050,6 +1066,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body.ingredientIds).to.eql([test.ingredient1]); expect(res.body).to.not.have.property('ingredients'); done(); @@ -1075,6 +1092,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { name: 'Chocolate', id: test.ingredient1 }, { name: 'Sugar', id: test.ingredient2 }, @@ -1090,6 +1108,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { name: 'Chocolate', id: test.ingredient1 }, { name: 'Butter', id: test.ingredient3 } @@ -1105,6 +1124,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { name: 'Butter', id: test.ingredient3 } ]); @@ -1119,6 +1139,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body.ingredientIds).to.eql([ test.ingredient1, test.ingredient3 ]); @@ -1137,6 +1158,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql( { name: 'Butter', id: test.ingredient3 } ); @@ -1152,6 +1174,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body.ingredientIds).to.eql(expected); expect(res.body).to.not.have.property('ingredients'); done(); @@ -1175,6 +1198,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { name: 'Chocolate', id: test.ingredient1 }, { name: 'Sugar', id: test.ingredient2 } @@ -1189,6 +1213,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { name: 'Chocolate', id: test.ingredient1 } ]); @@ -1216,6 +1241,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { name: 'Chocolate', id: test.ingredient1 }, { name: 'Sugar', id: test.ingredient2 } @@ -1241,6 +1267,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { name: 'Sugar', id: test.ingredient2 } ]); @@ -1254,6 +1281,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.eql([ { name: 'Chocolate', id: test.ingredient1 }, { name: 'Sugar', id: test.ingredient2 } @@ -1267,6 +1295,7 @@ describe('relations - integration', function() { this.get(url) .expect(200, function(err, res) { + if (err) return done(err); expect(err).to.not.exist; expect(res.body.name).to.equal('Photo 1'); done(); @@ -1401,6 +1430,7 @@ describe('relations - integration', function() { var test = this; this.get('/api/books/' + test.book.id + '/pages') .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.an.array; expect(res.body).to.have.length(1); expect(res.body[0].name).to.equal('Page 1'); @@ -1412,6 +1442,7 @@ describe('relations - integration', function() { var test = this; this.get('/api/pages/' + test.page.id + '/notes/' + test.note.id) .expect(200, function(err, res) { + if (err) return done(err); expect(res.headers['x-before']).to.equal('before'); expect(res.headers['x-after']).to.equal('after'); expect(res.body).to.be.an.object; @@ -1424,6 +1455,7 @@ describe('relations - integration', function() { var test = this; this.get('/api/books/unknown/pages/' + test.page.id + '/notes') .expect(404, function(err, res) { + if (err) return done(err); 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); @@ -1436,6 +1468,7 @@ describe('relations - integration', function() { var test = this; this.get('/api/images/' + test.image.id + '/book/pages') .end(function(err, res) { + if (err) return done(err); expect(res.body).to.be.an.array; expect(res.body).to.have.length(1); expect(res.body[0].name).to.equal('Page 1'); @@ -1447,6 +1480,7 @@ describe('relations - integration', function() { var test = this; this.get('/api/images/' + test.image.id + '/book/pages/' + test.page.id) .end(function(err, res) { + if (err) return done(err); expect(res.body).to.be.an.object; expect(res.body.name).to.equal('Page 1'); done(); @@ -1457,6 +1491,7 @@ describe('relations - integration', function() { var test = this; this.get('/api/books/' + test.book.id + '/pages/' + test.page.id + '/notes') .expect(200, function(err, res) { + if (err) return done(err); expect(res.body).to.be.an.array; expect(res.body).to.have.length(1); expect(res.body[0].text).to.equal('Page Note 1'); @@ -1468,6 +1503,7 @@ 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) { + if (err) return done(err); expect(res.headers['x-before']).to.equal('before'); expect(res.headers['x-after']).to.equal('after'); expect(res.body).to.be.an.object; @@ -1480,6 +1516,7 @@ describe('relations - integration', function() { var test = this; this.get('/api/books/' + test.book.id + '/chapters/' + test.chapter.id + '/notes/' + test.cnote.id) .expect(200, function(err, res) { + if (err) return done(err); expect(res.headers['x-before']).to.empty; expect(res.headers['x-after']).to.empty; done(); @@ -1503,6 +1540,7 @@ describe('relations - integration', function() { var test = this; this.get('/api/books/' + test.book.id + '/pages/' + this.page.id + '/throws') .end(function(err, res) { + if (err) return done(err); expect(res.body).to.be.an('object'); expect(res.body.error).to.be.an('object'); expect(res.body.error.name).to.equal('Error'); From db0678baa68bf8ae9168e65fb3ab958d9f156bf1 Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Thu, 7 Jan 2016 13:29:27 -0800 Subject: [PATCH 05/27] test: use ephemeral port for e2e server --- Gruntfile.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Gruntfile.js b/Gruntfile.js index a7a755c81..3df919962 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -217,7 +217,10 @@ module.exports = function(grunt) { grunt.registerTask('e2e-server', function() { var done = this.async(); var app = require('./test/fixtures/e2e/app'); - app.listen(3000, done); + app.listen(0, function() { + process.env.PORT = this.address().port; + done(); + }); }); grunt.registerTask('e2e', ['e2e-server', 'karma:e2e']); From a0a1083564c84aaeb6ce7aa56808e6378e418e5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 26 Jan 2016 13:51:37 +0100 Subject: [PATCH 06/27] Hide verificationToken We should never be showing this publically. Adds unit test for hiding verification token. This is a back-port of pull request #1851 from gausie/patch-4 --- common/models/user.json | 2 +- test/user.test.js | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/common/models/user.json b/common/models/user.json index d70a89d3f..16545ab4b 100644 --- a/common/models/user.json +++ b/common/models/user.json @@ -32,7 +32,7 @@ "options": { "caseSensitiveEmail": true }, - "hidden": ["password"], + "hidden": ["password", "verificationToken"], "acls": [ { "principalType": "ROLE", diff --git a/test/user.test.js b/test/user.test.js index 8e2b14948..79f6547ee 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -1320,6 +1320,12 @@ describe('User', function() { }); }); + it('should hide verification tokens from user JSON', function(done) { + var user = new User({email: 'bar@bat.com', password: 'bar', verificationToken: 'a-token' }); + var data = user.toJSON(); + assert(!('verificationToken' in data)); + done(); + }); }); describe('User.confirm(options, fn)', function() { From 015e9cb80e9e7a537891219d8b50f9b94f9e0112 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 26 Jan 2016 13:52:57 +0100 Subject: [PATCH 07/27] Correct JSDoc findOrCreate() callback in PersistedModel Update PersistedModel.findOrCreate() JSDoc to reflect the callback accepts an additional created boolean parameter. This is a back-port of pull request #1983 from noderat/patch-1 --- lib/persisted-model.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/persisted-model.js b/lib/persisted-model.js index bcac8e18d..5dceeec04 100644 --- a/lib/persisted-model.js +++ b/lib/persisted-model.js @@ -135,9 +135,10 @@ module.exports = function(registry) { *
See * [Where filter](https://docs.strongloop.com/display/LB/Where+filter#Wherefilter-Whereclauseforqueries). * @param {Object} data Data to insert if object matching the `where` filter is not found. - * @callback {Function} callback Callback function called with `cb(err, instance)` arguments. Required. + * @callback {Function} callback Callback function called with `cb(err, instance, created)` arguments. Required. * @param {Error} err Error object; see [Error object](http://docs.strongloop.com/display/LB/Error+object). * @param {Object} instance Model instance matching the `where` filter, if found. + * @param {Boolean} created True if the instance matching the `where` filter was created. */ PersistedModel.findOrCreate = function findOrCreate(query, data, callback) { From 4753373f4fb03f820c0a66f96258fcfadb900cc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 4 Feb 2016 16:28:01 +0100 Subject: [PATCH 08/27] Travis: drop iojs, add v4.x and v5.x --- .travis.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 168369451..e918e73fb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,5 +3,6 @@ language: node_js node_js: - "0.10" - "0.12" - - "iojs" + - "4" + - "5" From 76ec49c96bd529715683ec2b6992f8a8f0429607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Thu, 4 Feb 2016 16:52:50 +0100 Subject: [PATCH 09/27] Fix race condition in error handler test --- test/error-handler.test.js | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/test/error-handler.test.js b/test/error-handler.test.js index d19abf47e..0494f2ef3 100644 --- a/test/error-handler.test.js +++ b/test/error-handler.test.js @@ -41,15 +41,23 @@ describe('loopback.errorHandler(options)', function() { //arrange var app = loopback(); app.use(loopback.urlNotFound()); - app.use(loopback.errorHandler({ includeStack: false, log: customLogger })); - //act - request(app).get('/url-does-not-exist').end(); + var errorLogged; + app.use(loopback.errorHandler({ + includeStack: false, + log: function customLogger(err, str, req) { + errorLogged = err; + } + })); - //assert - function customLogger(err, str, req) { - assert.ok(err.message === 'Cannot GET /url-does-not-exist'); + //act + request(app).get('/url-does-not-exist').end(function(err) { + if (err) return done(err); + //assert + expect(errorLogged) + .to.have.property('message', 'Cannot GET /url-does-not-exist'); done(); - } + }); + }); }); From 7a54da58708791a1bf92630a4b21ce7a270283cf Mon Sep 17 00:00:00 2001 From: Jue Hou Date: Wed, 27 Jan 2016 14:42:47 -0500 Subject: [PATCH 10/27] Promisify Model Change * Change.diff * Change.findOrCreateChange * Change.rectifyModelChanges * Change.prototype.currentRevision * Change.prototype.rectify --- common/models/change.js | 19 +++++-- test/change.test.js | 111 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 4 deletions(-) diff --git a/common/models/change.js b/common/models/change.js index be353a039..3b347ec5c 100644 --- a/common/models/change.js +++ b/common/models/change.js @@ -4,6 +4,7 @@ var PersistedModel = require('../../lib/loopback').PersistedModel; var loopback = require('../../lib/loopback'); +var utils = require('../../lib/utils'); var crypto = require('crypto'); var CJSON = {stringify: require('canonical-json')}; var async = require('async'); @@ -77,6 +78,8 @@ module.exports = function(Change) { var Change = this; var errors = []; + callback = callback || utils.createPromiseCallback(); + var tasks = modelIds.map(function(id) { return function(cb) { Change.findOrCreateChange(modelName, id, function(err, change) { @@ -111,6 +114,7 @@ module.exports = function(Change) { } callback(); }); + return callback.promise; }; /** @@ -138,6 +142,7 @@ module.exports = function(Change) { Change.findOrCreateChange = function(modelName, modelId, callback) { assert(loopback.findModel(modelName), modelName + ' does not exist'); + callback = callback || utils.createPromiseCallback(); var id = this.idForModel(modelName, modelId); var Change = this; @@ -155,6 +160,7 @@ module.exports = function(Change) { Change.updateOrCreate(ch, callback); } }); + return callback.promise; }; /** @@ -171,9 +177,7 @@ module.exports = function(Change) { change.debug('rectify change'); - cb = cb || function(err) { - if (err) throw new Error(err); - }; + cb = cb || utils.createPromiseCallback(); change.currentRevision(function(err, rev) { if (err) return cb(err); @@ -194,6 +198,7 @@ module.exports = function(Change) { } ); }); + return cb.promise; function doRectify(checkpoint, rev) { if (rev) { @@ -248,6 +253,7 @@ module.exports = function(Change) { */ Change.prototype.currentRevision = function(cb) { + cb = cb || utils.createPromiseCallback(); var model = this.getModelCtor(); var id = this.getModelId(); model.findById(id, function(err, inst) { @@ -258,6 +264,7 @@ module.exports = function(Change) { cb(null, null); } }); + return cb.promise; }; /** @@ -390,8 +397,11 @@ module.exports = function(Change) { */ Change.diff = function(modelName, since, remoteChanges, callback) { + callback = callback || utils.createPromiseCallback(); + if (!Array.isArray(remoteChanges) || remoteChanges.length === 0) { - return callback(null, {deltas: [], conflicts: []}); + callback(null, {deltas: [], conflicts: []}); + return callback.promise; } var remoteChangeIndex = {}; var modelIds = []; @@ -455,6 +465,7 @@ module.exports = function(Change) { conflicts: conflicts }); }); + return callback.promise; }; /** diff --git a/test/change.test.js b/test/change.test.js index 55d551c72..2f43c037f 100644 --- a/test/change.test.js +++ b/test/change.test.js @@ -82,6 +82,38 @@ describe('Change', function() { }); }); + describe('Change.rectifyModelChanges - promise variant', function() { + describe('using an existing untracked model', function() { + beforeEach(function(done) { + var test = this; + Change.rectifyModelChanges(this.modelName, [this.modelId]) + .then(function(trackedChanges) { + done(); + }) + .catch(done); + }); + + it('should create an entry', function(done) { + var test = this; + Change.find() + .then(function(trackedChanges) { + assert.equal(trackedChanges[0].modelId, test.modelId.toString()); + done(); + }) + .catch(done); + }); + + it('should only create one change', function(done) { + Change.count() + .then(function(count) { + assert.equal(count, 1); + done(); + }) + .catch(done); + }); + }); + }); + describe('Change.findOrCreateChange(modelName, modelId, callback)', function() { describe('when a change doesnt exist', function() { @@ -104,6 +136,27 @@ describe('Change', function() { }); }); + describe('when a change doesnt exist - promise variant', function() { + beforeEach(function(done) { + var test = this; + Change.findOrCreateChange(this.modelName, this.modelId) + .then(function(result) { + test.result = result; + done(); + }) + .catch(done); + }); + + it('should create an entry', function(done) { + var test = this; + Change.findById(this.result.id, function(err, change) { + if (err) return done(err); + assert.equal(change.id, test.result.id); + done(); + }); + }); + }); + describe('when a change does exist', function() { beforeEach(function(done) { var test = this; @@ -219,6 +272,28 @@ describe('Change', function() { }); }); + describe('change.rectify - promise variant', function() { + var change; + beforeEach(function(done) { + Change.findOrCreateChange(this.modelName, this.modelId) + .then(function(ch) { + change = ch; + done(); + }) + .catch(done); + }); + + it('should create a new change with the correct revision', function(done) { + var test = this; + change.rectify() + .then(function(ch) { + assert.equal(ch.rev, test.revisionForModel); + done(); + }) + .catch(done); + }); + }); + describe('change.currentRevision(callback)', function() { it('should get the correct revision', function(done) { var test = this; @@ -234,6 +309,23 @@ describe('Change', function() { }); }); + describe('change.currentRevision - promise variant', function() { + it('should get the correct revision', function(done) { + var test = this; + var change = new Change({ + modelName: this.modelName, + modelId: this.modelId + }); + + change.currentRevision() + .then(function(rev) { + assert.equal(rev, test.revisionForModel); + done(); + }) + .catch(done); + }); + }); + describe('Change.hash(str)', function() { // todo(ritch) test other hashing algorithms it('should hash the given string', function() { @@ -374,6 +466,25 @@ describe('Change', function() { }); }); + it('should return delta and conflict lists - promise variant', function(done) { + var remoteChanges = [ + // an update => should result in a delta + {rev: 'foo2', prev: 'foo', modelName: this.modelName, modelId: 9, checkpoint: 1}, + // no change => should not result in a delta / conflict + {rev: 'bar', prev: 'bar', modelName: this.modelName, modelId: 10, checkpoint: 1}, + // a conflict => should result in a conflict + {rev: 'bat2', prev: 'bat0', modelName: this.modelName, modelId: 11, checkpoint: 1}, + ]; + + Change.diff(this.modelName, 0, remoteChanges) + .then(function(diff) { + assert.equal(diff.deltas.length, 1); + assert.equal(diff.conflicts.length, 1); + done(); + }) + .catch(done); + }); + it('should set "prev" to local revision in non-conflicting delta', function(done) { var updateRecord = { rev: 'foo-new', From a0806eab89c2f62cba93e6579cd9b9c9a78ff4b2 Mon Sep 17 00:00:00 2001 From: Ryan Graham Date: Thu, 4 Feb 2016 08:21:15 -0800 Subject: [PATCH 11/27] test: remove errant console.log from test Using console.log like this can result in invalid xml when the xunit reporter is used. [Backport of pull request #2035] --- test/user.test.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/user.test.js b/test/user.test.js index 79f6547ee..f306332cb 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -1042,7 +1042,6 @@ describe('User', function() { user.verify(options) .then(function(result) { - console.log('here in then function'); assert(result.email); assert(result.email.response); assert(result.token); From e98ed99fe76621c82b5b4b3d690c51c726554caa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 5 Feb 2016 09:21:25 +0100 Subject: [PATCH 12/27] Fix race condition in replication tests --- test/replication.test.js | 98 ++++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 44 deletions(-) diff --git a/test/replication.test.js b/test/replication.test.js index 2606aaf53..ecdffbf8d 100644 --- a/test/replication.test.js +++ b/test/replication.test.js @@ -67,40 +67,26 @@ describe('Replication / Change APIs', function() { }); it('should call rectifyAllChanges if no id is passed for rectifyOnDelete', function(done) { - SourceModel.rectifyChange = function() { - return done(new Error('Should not call rectifyChange')); - }; - SourceModel.rectifyAllChanges = function() { - return done(); - }; + var calls = mockSourceModelRectify(); SourceModel.destroyAll({name: 'John'}, function(err, data) { - if (err) - return done(err); + if (err) return done(err); + expect(calls).to.eql(['rectifyAllChanges']); + done(); }); }); it('should call rectifyAllChanges if no id is passed for rectifyOnSave', function(done) { - SourceModel.rectifyChange = function() { - return done(new Error('Should not call rectifyChange')); - }; - SourceModel.rectifyAllChanges = function() { - return done(); - }; + var calls = mockSourceModelRectify(); var newData = {'name': 'Janie'}; SourceModel.update({name: 'Jane'}, newData, function(err, data) { - if (err) - return done(err); + if (err) return done(err); + expect(calls).to.eql(['rectifyAllChanges']); + done(); }); }); it('rectifyOnDelete for Delete should call rectifyChange instead of rectifyAllChanges', function(done) { - TargetModel.rectifyChange = function() { - return done(); - }; - TargetModel.rectifyAllChanges = function() { - return done(new Error('Should not call rectifyAllChanges')); - }; - + var calls = mockTargetModelRectify(); async.waterfall([ function(callback) { SourceModel.destroyAll({name: 'John'}, callback); @@ -110,19 +96,14 @@ describe('Replication / Change APIs', function() { // replicate should call `rectifyOnSave` and then `rectifyChange` not `rectifyAllChanges` through `after save` operation } ], function(err, results) { - if (err) - return done(err); + if (err) return done(err); + expect(calls).to.eql(['rectifyChange']); + done(); }); }); it('rectifyOnSave for Update should call rectifyChange instead of rectifyAllChanges', function(done) { - TargetModel.rectifyChange = function() { - return done(); - }; - TargetModel.rectifyAllChanges = function() { - return done(new Error('Should not call rectifyAllChanges')); - }; - + var calls = mockTargetModelRectify(); var newData = {'name': 'Janie'}; async.waterfall([ function(callback) { @@ -131,20 +112,16 @@ describe('Replication / Change APIs', function() { function(data, callback) { SourceModel.replicate(TargetModel, callback); // replicate should call `rectifyOnSave` and then `rectifyChange` not `rectifyAllChanges` through `after save` operation - }], function(err, result) { - if (err) - return done(err); + } + ], function(err, result) { + if (err) return done(err); + expect(calls).to.eql(['rectifyChange']); + done(); }); }); it('rectifyOnSave for Create should call rectifyChange instead of rectifyAllChanges', function(done) { - TargetModel.rectifyChange = function() { - return done(); - }; - TargetModel.rectifyAllChanges = function() { - return done(new Error('Should not call rectifyAllChanges')); - }; - + var calls = mockTargetModelRectify(); var newData = [{name: 'Janie', surname: 'Doe'}]; async.waterfall([ function(callback) { @@ -155,10 +132,43 @@ describe('Replication / Change APIs', function() { // replicate should call `rectifyOnSave` and then `rectifyChange` not `rectifyAllChanges` through `after save` operation } ], function(err, result) { - if (err) - return done(err); + if (err) return done(err); + expect(calls).to.eql(['rectifyChange']); + done(); }); }); + + function mockSourceModelRectify() { + var calls = []; + + SourceModel.rectifyChange = function(id, cb) { + calls.push('rectifyChange'); + process.nextTick(cb); + }; + + SourceModel.rectifyAllChanges = function(cb) { + calls.push('rectifyAllChanges'); + process.nextTick(cb); + }; + + return calls; + } + + function mockTargetModelRectify() { + var calls = []; + + TargetModel.rectifyChange = function(id, cb) { + calls.push('rectifyChange'); + process.nextTick(cb); + }; + + TargetModel.rectifyAllChanges = function(cb) { + calls.push('rectifyAllChanges'); + process.nextTick(cb); + }; + + return calls; + } }); describe('Model.changes(since, filter, callback)', function() { From 70aec01e844f610aca6b029d62a1d2c3684ce111 Mon Sep 17 00:00:00 2001 From: Candy Date: Thu, 18 Feb 2016 10:38:54 -0500 Subject: [PATCH 13/27] Remove sl-blip from dependency --- package.json | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/package.json b/package.json index bccf39157..e5a08074d 100644 --- a/package.json +++ b/package.json @@ -102,8 +102,5 @@ "depd": "loopback-datasource-juggler/lib/browser.depd.js", "bcrypt": false }, - "license": "MIT", - "optionalDependencies": { - "sl-blip": "http://blip.strongloop.com/loopback@2.26.2" - } + "license": "MIT" } From 97f376c23934930a421ff8c81e620aa04eb7aeb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 19 Feb 2016 10:29:24 +0100 Subject: [PATCH 14/27] 2.27.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove sl-blip from dependency (Candy) * Fix race condition in replication tests (Miroslav Bajtoš) * test: remove errant console.log from test (Ryan Graham) * Promisify Model Change (Jue Hou) * Fix race condition in error handler test (Miroslav Bajtoš) * Travis: drop iojs, add v4.x and v5.x (Miroslav Bajtoš) * Correct JSDoc findOrCreate() callback in PersistedModel (Miroslav Bajtoš) * Hide verificationToken (Miroslav Bajtoš) * test: use ephemeral port for e2e server (Ryan Graham) * test: fail on error instead of crash (Ryan Graham) * ensure app is booted before integration tests (Ryan Graham) * Checkpoint speedup (Amir Jafarian) * Pull in API doc fix from PR into master #1910 (crandmck) --- CHANGES.md | 38 ++++++++++++++++++++++++++++++-------- package.json | 2 +- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index ea87f5de4..914da0ae3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,33 @@ +2016-02-19, Version 2.27.0 +========================== + + * Remove sl-blip from dependency (Candy) + + * Fix race condition in replication tests (Miroslav Bajtoš) + + * test: remove errant console.log from test (Ryan Graham) + + * Promisify Model Change (Jue Hou) + + * Fix race condition in error handler test (Miroslav Bajtoš) + + * Travis: drop iojs, add v4.x and v5.x (Miroslav Bajtoš) + + * Correct JSDoc findOrCreate() callback in PersistedModel (Miroslav Bajtoš) + + * Hide verificationToken (Miroslav Bajtoš) + + * test: use ephemeral port for e2e server (Ryan Graham) + + * test: fail on error instead of crash (Ryan Graham) + + * ensure app is booted before integration tests (Ryan Graham) + + * Checkpoint speedup (Amir Jafarian) + + * Pull in API doc fix from PR into master #1910 (crandmck) + + 2015-12-22, Version 2.26.2 ========================== @@ -989,8 +1019,6 @@ 2014-07-15, Version 2.0.0-beta6 =============================== - * 2.0.0-beta6 (Miroslav Bajtoš) - * lib/application: publish Change models to REST API (Miroslav Bajtoš) * models/change: fix typo (Miroslav Bajtoš) @@ -1001,8 +1029,6 @@ 2014-07-03, Version 2.0.0-beta5 =============================== - * 2.0.0-beta5 (Miroslav Bajtoš) - * app: update `url` on `listening` event (Miroslav Bajtoš) * Fix "ReferenceError: loopback is not defined" in registry.memory(). (Guilherme Cirne) @@ -1021,8 +1047,6 @@ 2014-06-26, Version 2.0.0-beta4 =============================== - * 2.0.0-beta4 (Miroslav Bajtoš) - * package: upgrade juggler to 2.0.0-beta2 (Miroslav Bajtoš) * Fix loopback in PhantomJS, fix karma tests (Miroslav Bajtoš) @@ -1149,8 +1173,6 @@ 2014-05-28, Version 2.0.0-beta3 =============================== - * 2.0.0-beta3 (Miroslav Bajtoš) - * package.json: fix malformed json (Miroslav Bajtoš) * 2.0.0-beta2 (Ritchie Martori) diff --git a/package.json b/package.json index e5a08074d..b8f87ee89 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback", - "version": "2.26.2", + "version": "2.27.0", "description": "LoopBack: Open Source Framework for Node.js", "homepage": "http://loopback.io", "keywords": [ From a4c643e8904afaf2b08fd8d2f9acd50efae2e93d Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Thu, 18 Feb 2016 20:36:07 -0800 Subject: [PATCH 15/27] application: correct spelling of "cannont" [back-port of pull request #2088] --- lib/application.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/application.js b/lib/application.js index bd3375539..c1932c20f 100644 --- a/lib/application.js +++ b/lib/application.js @@ -397,7 +397,7 @@ function dataSourcesFromConfig(name, config, connectorRegistry, registry) { var connectorPath; assert(typeof config === 'object', - 'cannont create data source without config object'); + 'can not create data source without config object'); if (typeof config.connector === 'string') { name = config.connector; From 50e3578992a81c939a974e313165d78718560593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 26 Feb 2016 14:20:07 +0100 Subject: [PATCH 16/27] Improve error message on connector init error [back-port of pull request #2105] --- lib/application.js | 20 +++++++++++++++----- test/app.test.js | 10 ++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/application.js b/lib/application.js index c1932c20f..dd5fdb730 100644 --- a/lib/application.js +++ b/lib/application.js @@ -219,11 +219,19 @@ app.models = function() { * @param {Object} config The data source config */ app.dataSource = function(name, config) { - var ds = dataSourcesFromConfig(name, config, this.connectors, this.registry); - this.dataSources[name] = - this.dataSources[classify(name)] = - this.dataSources[camelize(name)] = ds; - return ds; + try { + var ds = dataSourcesFromConfig(name, config, this.connectors, this.registry); + this.dataSources[name] = + this.dataSources[classify(name)] = + this.dataSources[camelize(name)] = ds; + return ds; + } catch (err) { + if (err.message) { + err.message = 'Cannot create data source ' + JSON.stringify(name) + + ': ' + err.message; + } + throw err; + } }; /** @@ -410,6 +418,8 @@ function dataSourcesFromConfig(name, config, connectorRegistry, registry) { config.connector = require(connectorPath); } } + if (!config.connector.name) + config.connector.name = name; } return registry.createDataSource(config); diff --git a/test/app.test.js b/test/app.test.js index 2701ef9d2..1144db2eb 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -732,6 +732,16 @@ describe('app', function() { app.dataSource('custom', { connector: 'custom' }); expect(app.dataSources.custom.name).to.equal(loopback.Memory.name); }); + + it('adds data source name to error messages', function() { + app.connector('throwing', { + initialize: function() { throw new Error('expected test error'); }, + }); + + expect(function() { + app.dataSource('bad-ds', { connector: 'throwing' }); + }).to.throw(/bad-ds.*throwing/); + }); }); describe.onServer('listen()', function() { From e4b275243fe11b2d4d756e9f8995fefa66fa8873 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Kr=C3=B6ger?= Date: Mon, 29 Feb 2016 16:49:41 +0100 Subject: [PATCH 17/27] Allow built-in token middleware to run repeatedly Add two new options: - When `enableDoublecheck` is true, the middleware will run even if a previous middleware has already set `req.accessToken` (possibly to `null` for anonymous requests) - When `overwriteExistingToken` is true (and `enableDoublecheck` too), the middleware will overwrite `req.accessToken` set by a previous middleware instances. --- server/middleware/token.js | 20 ++++++++- test/access-token.test.js | 86 +++++++++++++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/server/middleware/token.js b/server/middleware/token.js index e80eb560b..146c75d17 100644 --- a/server/middleware/token.js +++ b/server/middleware/token.js @@ -62,6 +62,8 @@ function escapeRegExp(str) { * @property {Array} [headers] Array of header names. * @property {Array} [params] Array of param names. * @property {Boolean} [searchDefaultTokenKeys] Use the default search locations for Token in request + * @property {Boolean} [enableDoublecheck] Execute middleware although an instance mounted earlier in the chain didn't find a token + * @property {Boolean} [overwriteExistingToken] only has effect in combination with `enableDoublecheck`. If truthy, will allow to overwrite an existing accessToken. * @property {Function|String} [model] AccessToken model name or class to use. * @property {String} [currentUserLiteral] String literal for the current user. * @header loopback.token([options]) @@ -80,6 +82,9 @@ function token(options) { currentUserLiteral = escapeRegExp(currentUserLiteral); } + var enableDoublecheck = !!options.enableDoublecheck; + var overwriteExistingToken = !!options.overwriteExistingToken; + return function(req, res, next) { var app = req.app; var registry = app.registry; @@ -97,8 +102,19 @@ function token(options) { 'loopback.token() middleware requires a AccessToken model'); if (req.accessToken !== undefined) { - rewriteUserLiteral(req, currentUserLiteral); - return next(); + if (!enableDoublecheck) { + // req.accessToken is defined already (might also be "null" or "false") and enableDoublecheck + // has not been set --> skip searching for credentials + rewriteUserLiteral(req, currentUserLiteral); + return next(); + } + if (req.accessToken && req.accessToken.id && !overwriteExistingToken) { + // req.accessToken.id is defined, which means that some other middleware has identified a valid user. + // when overwriteExistingToken is not set to a truthy value, skip searching for credentials. + rewriteUserLiteral(req, currentUserLiteral); + return next(); + } + // continue normal operation (as if req.accessToken was undefined) } TokenModel.findForRequest(req, options, function(err, token) { req.accessToken = token || null; diff --git a/test/access-token.test.js b/test/access-token.test.js index 9e57cba2a..a2ddbdb09 100644 --- a/test/access-token.test.js +++ b/test/access-token.test.js @@ -65,7 +65,7 @@ describe('loopback.token(options)', function() { .end(done); }); - describe('populating req.toen from HTTP Basic Auth formatted authorization header', function() { + describe('populating req.token from HTTP Basic Auth formatted authorization header', function() { it('parses "standalone-token"', function(done) { var token = this.token.id; token = 'Basic ' + new Buffer(token).toString('base64'); @@ -199,6 +199,90 @@ describe('loopback.token(options)', function() { done(); }); }); + + describe('loading multiple instances of token middleware', function() { + it('should skip when req.token is already present and no further options are set', + function(done) { + var tokenStub = { id: 'stub id' }; + app.use(function(req, res, next) { + req.accessToken = tokenStub; + next(); + }); + app.use(loopback.token({ model: Token })); + app.get('/', function(req, res, next) { + res.send(req.accessToken); + }); + + request(app).get('/') + .set('Authorization', this.token.id) + .expect(200) + .end(function(err, res) { + if (err) return done(err); + expect(res.body).to.eql(tokenStub); + done(); + }); + }); + + it('should not overwrite valid existing token (has "id" property) ' + + ' when overwriteExistingToken is falsy', + function(done) { + var tokenStub = { id: 'stub id' }; + app.use(function(req, res, next) { + req.accessToken = tokenStub; + next(); + }); + app.use(loopback.token({ + model: Token, + enableDoublecheck: true, + })); + app.get('/', function(req, res, next) { + res.send(req.accessToken); + }); + + request(app).get('/') + .set('Authorization', this.token.id) + .expect(200) + .end(function(err, res) { + if (err) return done(err); + expect(res.body).to.eql(tokenStub); + done(); + }); + }); + + it('should overwrite existing token when enableDoublecheck ' + + 'and overwriteExistingToken options are truthy', + function(done) { + var token = this.token; + var tokenStub = { id: 'stub id' }; + + app.use(function(req, res, next) { + req.accessToken = tokenStub; + next(); + }); + app.use(loopback.token({ + model: Token, + enableDoublecheck: true, + overwriteExistingToken: true, + })); + app.get('/', function(req, res, next) { + res.send(req.accessToken); + }); + + request(app).get('/') + .set('Authorization', token.id) + .expect(200) + .end(function(err, res) { + if (err) return done(err); + expect(res.body).to.eql({ + id: token.id, + ttl: token.ttl, + userId: token.userId, + created: token.created.toJSON(), + }); + done(); + }); + }); + }); }); describe('AccessToken', function() { From 1ea1cd612a3a51cfeb73336c7b80d15097a66bb8 Mon Sep 17 00:00:00 2001 From: Tim Needham Date: Sun, 17 Jan 2016 14:42:38 +0000 Subject: [PATCH 18/27] Fix typo in Model.nestRemoting Prevent apps from crashing when using `Model.nestRemoting` without `{ hooks: false }` option. Note that it is not possible to reproduce this bug using our current Mocha test suite, because other tests modify the global state in such way that the bug no longer occurs. [back-port of #2245] --- lib/model.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/model.js b/lib/model.js index f3ec47b25..d6564bf49 100644 --- a/lib/model.js +++ b/lib/model.js @@ -806,8 +806,8 @@ module.exports = function(registry) { listenerTree.before = listenerTree.before || {}; listenerTree.after = listenerTree.after || {}; - var beforeListeners = remotes.listenerTree.before[toModelName] || {}; - var afterListeners = remotes.listenerTree.after[toModelName] || {}; + var beforeListeners = listenerTree.before[toModelName] || {}; + var afterListeners = listenerTree.after[toModelName] || {}; sharedClass.methods().forEach(function(method) { var delegateTo = method.rest && method.rest.delegateTo; From 2498c02f31469eb80f7edce60347b35f0b7a92e0 Mon Sep 17 00:00:00 2001 From: Supasate Choochaisri Date: Wed, 27 Apr 2016 18:15:24 +0700 Subject: [PATCH 19/27] Add new feature to emit a `remoteMethodDisabled` event when disabling a remote method. Signed-off-by: Supasate Choochaisri --- lib/application.js | 5 +++++ lib/model.js | 1 + package.json | 1 + test/app.test.js | 16 ++++++++++++++++ test/model.test.js | 18 ++++++++++++++++++ 5 files changed, 41 insertions(+) diff --git a/lib/application.js b/lib/application.js index dd5fdb730..c5485fdb1 100644 --- a/lib/application.js +++ b/lib/application.js @@ -153,6 +153,11 @@ app.model = function(Model, config) { this.emit('modelRemoted', Model.sharedClass); } + var self = this; + Model.on('remoteMethodDisabled', function(model, methodName) { + self.emit('remoteMethodDisabled', model, methodName); + }); + Model.shared = isPublic; Model.app = this; Model.emit('attached', this); diff --git a/lib/model.js b/lib/model.js index d6564bf49..685e84045 100644 --- a/lib/model.js +++ b/lib/model.js @@ -431,6 +431,7 @@ module.exports = function(registry) { Model.disableRemoteMethod = function(name, isStatic) { this.sharedClass.disableMethod(name, isStatic || false); + this.emit('remoteMethodDisabled', this.sharedClass, name); }; Model.belongsToRemoting = function(relationName, relation, define) { diff --git a/package.json b/package.json index b8f87ee89..01f16ec48 100644 --- a/package.json +++ b/package.json @@ -85,6 +85,7 @@ "loopback-testing": "~1.1.0", "mocha": "^2.1.0", "sinon": "^1.13.0", + "sinon-chai": "^2.8.0", "strong-task-emitter": "^0.0.6", "supertest": "^0.15.0" }, diff --git a/test/app.test.js b/test/app.test.js index 1144db2eb..347d738b7 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -606,6 +606,22 @@ describe('app', function() { expect(remotedClass).to.eql(Color.sharedClass); }); + it('emits a `remoteMethodDisabled` event', function() { + var Color = PersistedModel.extend('color', { name: String }); + Color.shared = true; + var remoteMethodDisabledClass, disabledRemoteMethod; + app.on('remoteMethodDisabled', function(sharedClass, methodName) { + remoteMethodDisabledClass = sharedClass; + disabledRemoteMethod = methodName; + }); + app.model(Color); + app.models.Color.disableRemoteMethod('findOne'); + expect(remoteMethodDisabledClass).to.exist; + expect(remoteMethodDisabledClass).to.eql(Color.sharedClass); + expect(disabledRemoteMethod).to.exist; + expect(disabledRemoteMethod).to.eql('findOne'); + }); + it.onServer('updates REST API when a new model is added', function(done) { app.use(loopback.rest()); request(app).get('/colors').expect(404, function(err, res) { diff --git a/test/model.test.js b/test/model.test.js index b9a16f36d..37147f40a 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -1,9 +1,13 @@ var async = require('async'); +var chai = require('chai'); +var expect = chai.expect; var loopback = require('../'); var ACL = loopback.ACL; var Change = loopback.Change; var defineModelTestsWithDataSource = require('./util/model-tests'); var PersistedModel = loopback.PersistedModel; +var sinonChai = require('sinon-chai'); +chai.use(sinonChai); var describe = require('./util/describe'); @@ -618,6 +622,20 @@ describe.onServer('Remote Methods', function() { 'createChangeStream' ]); }); + + it('emits a `remoteMethodDisabled` event', function() { + var app = loopback(); + var model = PersistedModel.extend('TestModelForDisablingRemoteMethod'); + app.dataSource('db', { connector: 'memory' }); + app.model(model, { dataSource: 'db' }); + + var callbackSpy = require('sinon').spy(); + var TestModel = app.models.TestModelForDisablingRemoteMethod; + TestModel.on('remoteMethodDisabled', callbackSpy); + TestModel.disableRemoteMethod('findOne'); + + expect(callbackSpy).to.have.been.calledWith(TestModel.sharedClass, 'findOne'); + }); }); describe('Model.getApp(cb)', function() { From 6d738690c82b59476d992d5696a093cca1894aff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 2 May 2016 13:00:09 +0200 Subject: [PATCH 20/27] 2.28.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add new feature to emit a `remoteMethodDisabled` event when disabling a remote method. (Supasate Choochaisri) * Fix typo in Model.nestRemoting (Tim Needham) * Allow built-in token middleware to run repeatedly (Benjamin Kröger) * Improve error message on connector init error (Miroslav Bajtoš) * application: correct spelling of "cannont" (Sam Roberts) --- CHANGES.md | 14 ++++++++++++++ package.json | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 914da0ae3..aa42725cf 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,17 @@ +2016-05-02, Version 2.28.0 +========================== + + * Add new feature to emit a `remoteMethodDisabled` event when disabling a remote method. (Supasate Choochaisri) + + * Fix typo in Model.nestRemoting (Tim Needham) + + * Allow built-in token middleware to run repeatedly (Benjamin Kröger) + + * Improve error message on connector init error (Miroslav Bajtoš) + + * application: correct spelling of "cannont" (Sam Roberts) + + 2016-02-19, Version 2.27.0 ========================== diff --git a/package.json b/package.json index 01f16ec48..0ac007a69 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "loopback", - "version": "2.27.0", + "version": "2.28.0", "description": "LoopBack: Open Source Framework for Node.js", "homepage": "http://loopback.io", "keywords": [ From 845c59ecedbf34e0375bd21a7854a916bc56c145 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 2 May 2016 13:48:12 +0200 Subject: [PATCH 21/27] test/user: use local registry Rework User tests to not depend on `app.autoAttach()` and global shared registry of Models. Instead, each tests creates a fresh app instance with a new in-memory datasource and a new set of Models. --- test/user.test.js | 98 +++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 46 deletions(-) diff --git a/test/user.test.js b/test/user.test.js index f306332cb..23fca2ad5 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -1,12 +1,6 @@ require('./support'); var loopback = require('../'); -var User; -var AccessToken; -var MailConnector = require('../lib/connectors/mail'); - -var userMemory = loopback.createDataSource({ - connector: 'memory' -}); +var User, AccessToken; describe('User', function() { var validCredentialsEmail = 'foo@bar.com'; @@ -19,45 +13,59 @@ describe('User', function() { var invalidCredentials = {email: 'foo1@bar.com', password: 'invalid'}; var incompleteCredentials = {password: 'bar1'}; - var defaultApp; + // Create a local app variable to prevent clashes with the global + // variable shared by all tests. While this should not be necessary if + // the tests were written correctly, it turns out that's not the case :( + var app; + + beforeEach(function setupAppAndModels(done) { + // override the global app object provided by test/support.js + // and create a local one that does not share state with other tests + app = loopback({ localRegistry: true, loadBuiltinModels: true }); + app.dataSource('db', { connector: 'memory' }); + + // setup Email model, it's needed by User tests + app.dataSource('email', { + connector: loopback.Mail, + transports: [{ type: 'STUB' }], + }); + var Email = app.registry.getModel('Email'); + app.model(Email, { dataSource: 'email' }); + + // attach User and related models + User = app.registry.createModel('TestUser', {}, { + base: 'User', + http: { path: 'test-users' }, + }); + app.model(User, { dataSource: 'db' }); - beforeEach(function() { - // FIXME: [rfeng] Remove loopback.User.app so that remote hooks don't go - // to the wrong app instance - defaultApp = loopback.User.app; - loopback.User.app = null; - User = loopback.User.extend('TestUser', {}, {http: {path: 'test-users'}}); - AccessToken = loopback.AccessToken.extend('TestAccessToken'); - User.email = loopback.Email.extend('email'); - loopback.autoAttach(); + AccessToken = app.registry.getModel('AccessToken'); + app.model(AccessToken, { dataSource: 'db' }); + + User.email = Email; // Update the AccessToken relation to use the subclass of User AccessToken.belongsTo(User, {as: 'user', foreignKey: 'userId'}); User.hasMany(AccessToken, {as: 'accessTokens', foreignKey: 'userId'}); + // Speed up the password hashing algorithm + // for tests using the built-in User model + User.settings.saltWorkFactor = 4; + // allow many User.afterRemote's to be called User.setMaxListeners(0); - }); - - beforeEach(function(done) { - app.enableAuth(); - app.use(loopback.token({model: AccessToken})); + app.enableAuth({ dataSource: 'db' }); + app.use(loopback.token({ model: AccessToken })); app.use(loopback.rest()); app.model(User); User.create(validCredentials, function(err, user) { + if (err) return done(err); User.create(validCredentialsEmailVerified, done); }); }); - afterEach(function(done) { - loopback.User.app = defaultApp; - User.destroyAll(function(err) { - User.accessToken.destroyAll(done); - }); - }); - describe('User.create', function() { it('Create a new user', function(done) { User.create({email: 'f@b.com', password: 'bar'}, function(err, user) { @@ -694,12 +702,19 @@ describe('User', function() { var User; var AccessToken; - before(function() { - User = loopback.User.extend('RealmUser', {}, - {realmRequired: true, realmDelimiter: ':'}); - AccessToken = loopback.AccessToken.extend('RealmAccessToken'); + beforeEach(function() { + User = app.registry.createModel('RealmUser', {}, { + base: 'TestUser', + realmRequired: true, + realmDelimiter: ':', + }); - loopback.autoAttach(); + AccessToken = app.registry.createModel('RealmAccessToken', {}, { + base: 'AccessToken', + }); + + app.model(AccessToken, { dataSource: 'db' }); + app.model(User, { dataSource: 'db' }); // Update the AccessToken relation to use the subclass of User AccessToken.belongsTo(User, {as: 'user', foreignKey: 'userId'}); @@ -770,15 +785,6 @@ describe('User', function() { }); }); - afterEach(function(done) { - User.deleteAll({realm: 'realm1'}, function(err) { - if (err) { - return done(err); - } - User.deleteAll({realm: 'realm2'}, done); - }); - }); - it('rejects a user by without realm', function(done) { User.login(credentialWithoutRealm, function(err, accessToken) { assert(err); @@ -828,11 +834,11 @@ describe('User', function() { }); describe('User.login with realmRequired but no realmDelimiter', function() { - before(function() { + beforeEach(function() { User.settings.realmDelimiter = undefined; }); - after(function() { + afterEach(function() { User.settings.realmDelimiter = ':'; }); @@ -1534,7 +1540,7 @@ describe('User', function() { describe('ctor', function() { it('exports default Email model', function() { expect(User.email, 'User.email').to.be.a('function'); - expect(User.email.modelName, 'modelName').to.eql('email'); + expect(User.email.modelName, 'modelName').to.eql('Email'); }); it('exports default AccessToken model', function() { From cae9786f0eb20bb4e2bb14efad961ad251542df4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 2 May 2016 14:30:11 +0200 Subject: [PATCH 22/27] Fix role.isOwner to support app-local registry --- common/models/role.js | 10 ++++------ test/role.test.js | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/common/models/role.js b/common/models/role.js index a176fec17..523e68982 100644 --- a/common/models/role.js +++ b/common/models/role.js @@ -147,12 +147,10 @@ module.exports = function(Role) { }); function isUserClass(modelClass) { - if (modelClass) { - return modelClass === loopback.User || - modelClass.prototype instanceof loopback.User; - } else { - return false; - } + if (!modelClass) return false; + var User = modelClass.modelBuilder.models.User; + if (!User) return false; + return modelClass == User || modelClass.prototype instanceof User; } /*! diff --git a/test/role.test.js b/test/role.test.js index 3053a0aff..8e5d1a129 100644 --- a/test/role.test.js +++ b/test/role.test.js @@ -407,4 +407,27 @@ describe('role model', function() { }); }); + describe('isOwner', function() { + it('supports app-local model registry', function(done) { + var app = loopback({ localRegistry: true, loadBuiltinModels: true }); + app.dataSource('db', { connector: 'memory' }); + // attach all auth-related models to 'db' datasource + app.enableAuth({ dataSource: 'db' }); + + var Role = app.models.Role; + var User = app.models.User; + + var u = app.registry.findModel('User'); + var credentials = { email: 'test@example.com', password: 'pass' }; + User.create(credentials, function(err, user) { + if (err) return done(err); + + Role.isOwner(User, user.id, user.id, function(err, result) { + if (err) return done(err); + expect(result, 'isOwner result').to.equal(true); + done(); + }); + }); + }); + }); }); From 53cd449c9c1b86f1147765a854d28f2b39be609c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 2 May 2016 14:53:21 +0200 Subject: [PATCH 23/27] test/rest.middleware: use local registry Rework tests in `test/rest.middleware.test.js` to not depend on `app.autoAttach()` and global shared registry of Models. Instead, each tests creates a fresh app instance with a new in-memory datasource and a new set of Models. --- test/rest.middleware.test.js | 78 +++++++++++++++++------------------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/test/rest.middleware.test.js b/test/rest.middleware.test.js index 5757c44b6..61b935644 100644 --- a/test/rest.middleware.test.js +++ b/test/rest.middleware.test.js @@ -1,11 +1,15 @@ var path = require('path'); describe('loopback.rest', function() { - var MyModel; + var app, MyModel; + beforeEach(function() { - var ds = app.dataSource('db', { connector: loopback.Memory }); - MyModel = ds.createModel('MyModel', {name: String}); - loopback.autoAttach(); + // override the global app object provided by test/support.js + // and create a local one that does not share state with other tests + app = loopback({ localRegistry: true, loadBuiltinModels: true }); + var db = app.dataSource('db', { connector: 'memory' }); + MyModel = app.registry.createModel('MyModel'); + MyModel.attachTo(db); }); it('works out-of-the-box', function(done) { @@ -101,7 +105,7 @@ describe('loopback.rest', function() { }); it('should honour `remoting.rest.supportedTypes`', function(done) { - var app = loopback(); + var app = loopback({ localRegistry: true }); // NOTE it is crucial to set `remoting` before creating any models var supportedTypes = ['json', 'application/javascript', 'text/javascript']; @@ -117,26 +121,24 @@ describe('loopback.rest', function() { }); it('allows models to provide a custom HTTP path', function(done) { - var ds = app.dataSource('db', { connector: loopback.Memory }); - var CustomModel = ds.createModel('CustomModel', + var CustomModel = app.registry.createModel('CustomModel', { name: String }, { http: { 'path': 'domain1/CustomModelPath' } }); - app.model(CustomModel); + app.model(CustomModel, { dataSource: 'db' }); app.use(loopback.rest()); request(app).get('/domain1/CustomModelPath').expect(200).end(done); }); it('should report 200 for url-encoded HTTP path', function(done) { - var ds = app.dataSource('db', { connector: loopback.Memory }); - var CustomModel = ds.createModel('CustomModel', + var CustomModel = app.registry.createModel('CustomModel', { name: String }, { http: { path: 'domain%20one/CustomModelPath' } }); - app.model(CustomModel); + app.model(CustomModel, { dataSource: 'db' }); app.use(loopback.rest()); request(app).get('/domain%20one/CustomModelPath').expect(200).end(done); @@ -144,12 +146,12 @@ describe('loopback.rest', function() { it('includes loopback.token when necessary', function(done) { givenUserModelWithAuth(); - app.enableAuth(); + app.enableAuth({ dataSource: 'db' }); app.use(loopback.rest()); givenLoggedInUser(function(err, token) { if (err) return done(err); - expect(token).instanceOf(app.models.accessToken); + expect(token).instanceOf(app.models.AccessToken); request(app).get('/users/' + token.userId) .set('Authorization', token.id) .expect(200) @@ -268,25 +270,25 @@ describe('loopback.rest', function() { it('should enable context using loopback.context', function(done) { app.use(loopback.context({ enableHttpContext: true })); - app.enableAuth(); + app.enableAuth({ dataSource: 'db' }); app.use(loopback.rest()); invokeGetToken(done); }); it('should enable context with loopback.rest', function(done) { - app.enableAuth(); - app.set('remoting', { context: { enableHttpContext: true } }); + app.enableAuth({ dataSource: 'db' }); + app.set('remoting', { context: { enableHttpContext: true }}); app.use(loopback.rest()); invokeGetToken(done); }); it('should support explicit context', function(done) { - app.enableAuth(); + app.enableAuth({ dataSource: 'db' }); app.use(loopback.context()); app.use(loopback.token( - { model: loopback.getModelByType(loopback.AccessToken) })); + { model: app.registry.getModelByType('AccessToken') })); app.use(function(req, res, next) { loopback.getCurrentContext().set('accessToken', req.accessToken); next(); @@ -321,32 +323,26 @@ describe('loopback.rest', function() { }); function givenUserModelWithAuth() { - // NOTE(bajtos) It is important to create a custom AccessToken model here, - // in order to overwrite the entry created by previous tests in - // the global model registry - app.model('accessToken', { - options: { - base: 'AccessToken' - }, - dataSource: 'db' - }); - return app.model('user', { - options: { - base: 'User', - relations: { - accessTokens: { - model: 'accessToken', - type: 'hasMany', - foreignKey: 'userId' - } - } - }, - dataSource: 'db' - }); + var AccessToken = app.registry.getModel('AccessToken'); + app.model(AccessToken, { dataSource: 'db' }); + var User = app.registry.getModel('User'); + app.model(User, { dataSource: 'db' }); + + // NOTE(bajtos) This is puzzling to me. The built-in User & AccessToken + // models should come with both relations already set up, i.e. the + // following two lines should not be neccessary. + // And it does behave that way when only tests in this file are run. + // However, when I run the full test suite (all files), the relations + // get broken. + AccessToken.belongsTo(User, { as: 'user', foreignKey: 'userId' }); + User.hasMany(AccessToken, { as: 'accessTokens', foreignKey: 'userId' }); + + return User; } + function givenLoggedInUser(cb, done) { var credentials = { email: 'user@example.com', password: 'pwd' }; - var User = app.models.user; + var User = app.models.User; User.create(credentials, function(err, user) { if (err) return done(err); From 6c59390754e776c9f831d06065aa0e7adbaa94b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 3 May 2016 10:19:45 +0200 Subject: [PATCH 24/27] Disable DEBUG output for eslint on Jenkins CI --- package.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/package.json b/package.json index 0ac007a69..148d8007a 100644 --- a/package.json +++ b/package.json @@ -103,5 +103,10 @@ "depd": "loopback-datasource-juggler/lib/browser.depd.js", "bcrypt": false }, + "config": { + "ci": { + "debug": "*,-mocha:*,-eslint:*" + } + }, "license": "MIT" } From bd7f2b6db1daf61b144a97e534bbdfcc5b0ee075 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 3 May 2016 15:55:37 +0200 Subject: [PATCH 25/27] travis: drop node@5, add node@6 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index e918e73fb..d431fa429 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,5 +4,5 @@ node_js: - "0.10" - "0.12" - "4" - - "5" + - "6" From da2fb0ae15c2b436a26467cde94173f2339eb439 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 3 May 2016 16:03:48 +0200 Subject: [PATCH 26/27] app: send port:0 instead of port:undefined Node v6 no longer supports port:undefined, this commit is fixing app.listen() to correctly send port:0 when no port is specified. --- lib/application.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/application.js b/lib/application.js index c5485fdb1..dad8f24f6 100644 --- a/lib/application.js +++ b/lib/application.js @@ -566,7 +566,11 @@ app.listen = function(cb) { (arguments.length == 1 && typeof arguments[0] == 'function'); if (useAppConfig) { - server.listen(this.get('port'), this.get('host'), cb); + var port = this.get('port'); + // NOTE(bajtos) port:undefined no longer works on node@6, + // we must pass port:0 explicitly + if (port === undefined) port = 0; + server.listen(port, this.get('host'), cb); } else { server.listen.apply(server, arguments); } From e2b1f78f1e801ac8f5dc7ed0a8bb594f39979207 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 3 May 2016 16:43:45 +0200 Subject: [PATCH 27/27] Upgrade phantomjs to 2.x --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 148d8007a..2eb05c8b3 100644 --- a/package.json +++ b/package.json @@ -78,12 +78,13 @@ "karma-html2js-preprocessor": "^0.1.0", "karma-junit-reporter": "^0.2.2", "karma-mocha": "^0.1.10", - "karma-phantomjs-launcher": "^0.1.4", + "karma-phantomjs-launcher": "^1.0.0", "karma-script-launcher": "^0.1.0", "loopback-boot": "^2.7.0", "loopback-datasource-juggler": "^2.19.1", "loopback-testing": "~1.1.0", "mocha": "^2.1.0", + "phantomjs-prebuilt": "^2.1.7", "sinon": "^1.13.0", "sinon-chai": "^2.8.0", "strong-task-emitter": "^0.0.6",