Merge pull request #519 from fabien/fix/error-response

Fix embedsMany/findById to return proper 404 response

lib/models/model.js

@@ -403,6 +403,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,
@@ -411,7 +421,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];

test/relations.integration.js

@@ -114,6 +114,37 @@ describe('relations - integration', function () {
     });
   });
   
+  describe('/stores/:id/widgets/:fk - 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/:fk', 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/:fk - 404', function () {
+    beforeEach(function () {
+      this.url = '/api/stores/' + this.store.id + '/widgets/123456';
+    });
+    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);
+      });
+    });
+  });
+  
   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) {
+    it.skip('checks if an referenced model exists - fail', function(done) {
-    //   var url = '/api/recipes/' + this.recipe.id + '/ingredients/';
+      var url = '/api/recipes/' + this.recipe.id + '/ingredients/';
-    //   url += this.ingredient1;
+      url += this.ingredient3;
-    //   
+      
-    //   this.head(url)
+      this.head(url)
-    //     .expect(200, function(err, res) {
+        .expect(404, function(err, res) {
-    //       done();
+          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();
-    //     });
-    // });
     
   });