From 33f3ba45491e8d997b35e124a414c519dbd4d64d Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 30 Aug 2014 16:54:14 +0200 Subject: [PATCH 1/2] Fix embedsMany/findById to return proper 404 response --- lib/models/model.js | 13 +++++- test/relations.integration.js | 78 +++++++++++++++++++++++++---------- 2 files changed, 69 insertions(+), 22 deletions(-) diff --git a/lib/models/model.js b/lib/models/model.js index e490804c..1c2f162a 100644 --- a/lib/models/model.js +++ b/lib/models/model.js @@ -388,6 +388,16 @@ Model.hasManyRemoting = function (relationName, relation, define) { var pathName = (relation.options.http && relation.options.http.path) || relationName; var toModelName = relation.modelTo.modelName; + function convertNullToNotFoundError(ctx, cb) { + if (ctx.result !== null) return cb(); + + var fk = ctx.getArgByName('fk'); + var msg = 'Unknown "' + toModelName + '" id "' + fk + '".'; + var error = new Error(msg); + error.statusCode = error.status = 404; + cb(error); + } + var findByIdFunc = this.prototype['__findById__' + relationName]; define('__findById__' + relationName, { isStatic: false, @@ -396,7 +406,8 @@ Model.hasManyRemoting = function (relationName, relation, define) { description: 'Foreign key for ' + relationName, required: true, http: {source: 'path'}}, description: 'Find a related item by id for ' + relationName, - returns: {arg: 'result', type: toModelName, root: true} + returns: {arg: 'result', type: toModelName, root: true}, + rest: {after: convertNullToNotFoundError} }, findByIdFunc); var destroyByIdFunc = this.prototype['__destroyById__' + relationName]; diff --git a/test/relations.integration.js b/test/relations.integration.js index e8eea9be..ee183162 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -114,6 +114,37 @@ describe('relations - integration', function () { }); }); + describe('/stores/:id/widgets/:fkId - 200', function () { + beforeEach(function (done) { + var self = this; + this.store.widgets.create({ + name: this.widgetName + }, function(err, widget) { + self.widget = widget; + self.url = '/api/stores/' + self.store.id + '/widgets/' + widget.id; + done(); + }); + }); + lt.describe.whenCalledRemotely('GET', '/stores/:id/widgets/:fkId', function () { + it('should succeed with statusCode 200', function () { + assert.equal(this.res.statusCode, 200); + assert.equal(this.res.body.id, this.widget.id); + }); + }); + }); + + describe('/stores/:id/widgets/:fkId - 404', function () { + beforeEach(function () { + this.url = '/api/stores/' + this.store.id + '/widgets/123456'; + }); + lt.describe.whenCalledRemotely('GET', '/stores/:id/widgets/:fkId', function () { + it('should fail with statusCode 404', function () { + assert.equal(this.res.statusCode, 404); + assert.equal(this.res.body.error.status, 404); + }); + }); + }); + describe('/store/:id/widgets/count', function () { beforeEach(function() { this.url = '/api/stores/' + this.store.id + '/widgets/count'; @@ -729,7 +760,14 @@ describe('relations - integration', function () { }); }); - // TODO - this.head is undefined + 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) { + expect(res.body.error.status).to.be.equal(404); + expect(res.body.error.message).to.be.equal('Unknown "todoItem" id "2".'); + done(); + }); + }); it.skip('checks if an embedded model exists - ok', function(done) { var url = '/api/todo-lists/' + this.todoList.id + '/items/3'; @@ -1034,27 +1072,25 @@ describe('relations - integration', function () { }); }); - // TODO - this.head is undefined + it.skip('checks if a referenced model exists - ok', function(done) { + var url = '/api/recipes/' + this.recipe.id + '/ingredients/'; + url += this.ingredient1; + + this.head(url) + .expect(200, function(err, res) { + done(); + }); + }); - // it.skip('checks if a referenced model exists - ok', function(done) { - // var url = '/api/recipes/' + this.recipe.id + '/ingredients/'; - // url += this.ingredient1; - // - // this.head(url) - // .expect(200, function(err, res) { - // done(); - // }); - // }); - - // it.skip('checks if an referenced model exists - fail', function(done) { - // var url = '/api/recipes/' + this.recipe.id + '/ingredients/'; - // url += this.ingredient3; - // - // this.head(url) - // .expect(404, function(err, res) { - // done(); - // }); - // }); + it.skip('checks if an referenced model exists - fail', function(done) { + var url = '/api/recipes/' + this.recipe.id + '/ingredients/'; + url += this.ingredient3; + + this.head(url) + .expect(404, function(err, res) { + done(); + }); + }); }); From 1067c94bf643e81782504e32a973184f1e426415 Mon Sep 17 00:00:00 2001 From: Fabien Franzen Date: Sat, 30 Aug 2014 17:00:36 +0200 Subject: [PATCH 2/2] Tiny fix: correct url format --- test/relations.integration.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/relations.integration.js b/test/relations.integration.js index ee183162..49d6093e 100644 --- a/test/relations.integration.js +++ b/test/relations.integration.js @@ -114,7 +114,7 @@ describe('relations - integration', function () { }); }); - describe('/stores/:id/widgets/:fkId - 200', function () { + describe('/stores/:id/widgets/:fk - 200', function () { beforeEach(function (done) { var self = this; this.store.widgets.create({ @@ -125,7 +125,7 @@ describe('relations - integration', function () { done(); }); }); - lt.describe.whenCalledRemotely('GET', '/stores/:id/widgets/:fkId', function () { + lt.describe.whenCalledRemotely('GET', '/stores/:id/widgets/:fk', function () { it('should succeed with statusCode 200', function () { assert.equal(this.res.statusCode, 200); assert.equal(this.res.body.id, this.widget.id); @@ -133,11 +133,11 @@ describe('relations - integration', function () { }); }); - describe('/stores/:id/widgets/:fkId - 404', function () { + describe('/stores/:id/widgets/:fk - 404', function () { beforeEach(function () { this.url = '/api/stores/' + this.store.id + '/widgets/123456'; }); - lt.describe.whenCalledRemotely('GET', '/stores/:id/widgets/:fkId', function () { + lt.describe.whenCalledRemotely('GET', '/stores/:id/widgets/:fk', function () { it('should fail with statusCode 404', function () { assert.equal(this.res.statusCode, 404); assert.equal(this.res.body.error.status, 404);