Merge pull request #218 from strongloop/feature/declare-status

Add support for declaring status code defaults

lib/http-context.js

@@ -38,6 +38,7 @@ function HttpContext(req, res, method, options) {
   this.args = this.buildArgs(method);
   this.methodString = method.stringName;
   this.supportedTypes = this.options.supportedTypes || DEFAULT_SUPPORTED_TYPES;
+  this.result = {};
 
   if (this.supportedTypes === DEFAULT_SUPPORTED_TYPES && !this.options.xml) {
     // Disable all XML-based types by default
@@ -304,6 +305,9 @@ HttpContext.prototype.invoke = function(scope, method, fn, isCtor) {
   var pipe = http && http.pipe;
   var pipeDest = pipe && pipe.dest;
   var pipeSrc = pipe && pipe.source;
+  var ctx = this;
+  var defaultErrorStatus = http && http.errorStatus;
+  var res = this.res;
 
   if (pipeDest) {
     // only support response for now
@@ -313,7 +317,7 @@ HttpContext.prototype.invoke = function(scope, method, fn, isCtor) {
         this.res.header('Content-Type', 'application/json');
         this.res.header('Transfer-Encoding', 'chunked');
 
-        var stream = method.invoke(scope, args, this.options, fn);
+        var stream = method.invoke(scope, args, this.options, ctx, fn);
         stream.pipe(this.res);
         break;
       default:
@@ -324,15 +328,50 @@ HttpContext.prototype.invoke = function(scope, method, fn, isCtor) {
     // only support request for now
     switch (pipeDest) {
       case 'req':
-        this.req.pipe(method.invoke(scope, args, this.options, fn));
+        this.req.pipe(method.invoke(scope, args, this.options, ctx, fn));
         break;
       default:
         fn(new Error('unsupported pipe source'));
         break;
     }
   } else {
     // simple invoke
-    method.invoke(scope, args, this.options, fn);
+    method.invoke(scope, args, this.options, ctx, function(err, result) {
+      if (err) {
+        if (defaultErrorStatus &&
+          (res.statusCode === undefined || res.statusCode === 200)) {
+          res.status(err.status || err.statusCode || defaultErrorStatus);
+        }
+        return fn(err);
+      }
+      fn(null, result);
+    });
+  }
+};
+
+HttpContext.prototype.setReturnArgByName = function(name, value) {
+  var returnDesc = this.method.getReturnArgDescByName(name);
+  var result = this.result;
+  var res = this.res;
+
+  if (!returnDesc) {
+    return debug('warning: cannot set return value for arg' +
+      ' (%s) without description!', name);
+  }
+
+  if (returnDesc.root) {
+    this.result = value;
+  } else if (returnDesc.http) {
+    switch (returnDesc.http.target) {
+      case 'status':
+        res.status(value);
+        break;
+      case 'header':
+        res.set(returnDesc.http.header || name, value);
+        break;
+    }
+  } else {
+    result[name] = value;
   }
 };
 
@@ -384,6 +423,11 @@ HttpContext.prototype.done = function() {
   var data = this.result;
   var res = this.res;
   var accepts = this.req.accepts(this.supportedTypes);
+  var defaultStatus = this.method.http.status;
+
+  if (defaultStatus) {
+    res.status(defaultStatus);
+  }
 
   if (this.req.query._format) {
     accepts = this.req.query._format.toLowerCase();

lib/rest-adapter.js

@@ -359,7 +359,9 @@ RestAdapter.errorHandler = function(options) {
         err.status = err.statusCode = 500;
       }
 
-      res.statusCode = err.statusCode || err.status || 500;
+      if (res.statusCode === undefined || res.statusCode < 400) {
+        res.statusCode = err.statusCode || err.status || 500;
+      }
 
       debug('Error in %s %s: %s', req.method, req.url, err.stack);
       var data = {

lib/shared-method.js

@@ -24,6 +24,9 @@ var assert = require('assert');
  * @param {Boolean} [options.isStatic] Is the method a static method or a
  * a `prototype` method
  * @param {Array} [options.aliases] A list of aliases for the
+ * @param {Array} [options.http] HTTP only options
+ * @param {Number} [options.http.status] The default status code
+ * @param {Number} [options.http.errorStatus] The default error status code
  * `sharedMethod.name`
  * @param {Array|Object} [options.accepts] An `Array` of argument definitions
  * that describe the arguments of the `SharedMethod`.
@@ -144,6 +147,17 @@ SharedMethod.fromFunction = function(fn, name, sharedClass, isStatic) {
   });
 };
 
+SharedMethod.prototype.getReturnArgDescByName = function(name) {
+  var returns = this.returns;
+  var desc;
+  for (var i = 0; i < returns.length; i++) {
+    desc = returns[i];
+    if (desc && ((desc.arg || desc.name) === name)) {
+      return desc;
+    }
+  }
+};
+
 /**
  * Execute the remote method using the given arg data.
  *
@@ -153,14 +167,19 @@ SharedMethod.fromFunction = function(fn, name, sharedClass, isStatic) {
  * @param {Function} cb callback `fn(err, result)` containing named result data
  */
 
-SharedMethod.prototype.invoke = function(scope, args, remotingOptions, cb) {
+SharedMethod.prototype.invoke = function(scope, args, remotingOptions, ctx, cb) {
   var accepts = this.accepts;
   var returns = this.returns;
   var errors = this.errors;
   var method = this.getFunction();
   var sharedMethod = this;
   var formattedArgs = [];
 
+  if (typeof ctx === 'function') {
+    cb = ctx;
+    ctx = undefined;
+  }
+
   if (cb === undefined && typeof remotingOptions === 'function') {
     cb = remotingOptions;
     remotingOptions = {};
@@ -189,8 +208,9 @@ SharedMethod.prototype.invoke = function(scope, args, remotingOptions, cb) {
     if (err) {
       return cb(err);
     }
-
-    var result = SharedMethod.toResult(returns, [].slice.call(arguments, 1));
+    // args without err
+    var rawArgs = [].slice.call(arguments, 1);
+    var result = SharedMethod.toResult(returns, rawArgs, ctx);
 
     debug('- %s - result %j', sharedMethod.name, result);
 
@@ -460,7 +480,7 @@ SharedMethod.convertArg = function(accept, raw) {
  * results from a remoting function, based on `returns`.
  */
 
-SharedMethod.toResult = function(returns, raw) {
+SharedMethod.toResult = function(returns, raw, ctx) {
   var result = {};
 
   if (!returns.length) {
@@ -474,14 +494,18 @@ SharedMethod.toResult = function(returns, raw) {
 
     if (item.root) {
       result = convert(raw[index]);
+      if (ctx) ctx.setReturnArgByName(item.name || item.arg, raw[index]);
       return false;
     }
 
     return true;
   });
 
   returns.forEach(function(item, index) {
-    result[item.name || item.arg] = convert(raw[index]);
+    var value = convert(raw[index]);
+    var name = item.name || item.arg;
+    result[name] = value;
+    if (ctx) ctx.setReturnArgByName(name, value);
   });
 
   return result;

test/authorize-hook.test.js

@@ -55,4 +55,3 @@ describe('authorization hook', function() {
     remotes.invoke(method, args, callback);
   }
 });
-

test/rest.test.js

@@ -1813,17 +1813,113 @@ describe('strong-remoting-rest', function() {
     });
   });
 
-  it('returns 404 for unknown method of a shared class', function(done) {
-    var classUrl = givenSharedStaticMethod().classUrl;
+  describe('status codes', function() {
+    describe('using a custom satus code', function() {
+      it('returns a custom status code', function(done) {
+        var method = givenSharedStaticMethod(
+          function fn(cb) {
+            cb();
+          },
+          {
+            http: { status: 201 }
+          }
+        );
+        json(method.url)
+          .expect(201, done);
+      });
+      it('returns a custom error status code', function(done) {
+        var method = givenSharedStaticMethod(
+          function fn(cb) {
+            cb(new Error('test error'));
+          },
+          {
+            http: { status: 201, errorStatus: 508 }
+          }
+        );
+        json(method.url)
+          .expect(508, done);
+      });
+      it('returns a custom error status code (using the err object)', function(done) {
+        var method = givenSharedStaticMethod(
+          function fn(cb) {
+            var err = new Error('test error');
+            err.status = 555;
+            cb(err);
+          },
+          {
+            http: { status: 201, errorStatus: 508 }
+          }
+        );
+        json(method.url)
+          .expect(555, done);
+      });
+      it('returns a custom status code from a callback arg', function(done) {
+        var exampleStatus = 222;
+        var method = givenSharedStaticMethod(
+          function fn(status, cb) {
+            cb(null, status);
+          },
+          {
+            accepts: { arg: 'status', type: 'number' },
+            returns: {
+              arg: 'status',
+              http: { target: 'status' }
+            }
+          }
+        );
+        json(method.url + '?status=' + exampleStatus)
+          .expect(exampleStatus, done);
+      });
+    });
+    it('returns 404 for unknown method of a shared class', function(done) {
+      var classUrl = givenSharedStaticMethod().classUrl;
+
+      json(classUrl + '/unknown-method')
+        .expect(404, done);
+    });
 
-    json(classUrl + '/unknown-method')
-      .expect(404, done);
+    it('returns 404 with standard JSON body for uknown URL', function(done) {
+      json('/unknown-url')
+        .expect(404)
+        .end(expectErrorResponseContaining({status: 404}, done));
+    });
   });
 
-  it('returns 404 with standard JSON body for uknown URL', function(done) {
-    json('/unknown-url')
-      .expect(404)
-      .end(expectErrorResponseContaining({status: 404}, done));
+  describe('result args as headers', function() {
+    it('sets the header using the callback arg', function(done) {
+      var val = 'foobar';
+      var method = givenSharedStaticMethod(
+        function fn(input, cb) {
+          cb(null, input);
+        },
+        {
+          accepts: {arg: 'input', type: 'string'},
+          returns: {arg: 'output', type: 'string', http: { target: 'header' } }
+        }
+      );
+      json(method.url + '?input=' + val)
+        .expect('output', val)
+        .expect(200, done);
+    });
+    it('sets the custom header using the callback arg', function(done) {
+      var val = 'foobar';
+      var method = givenSharedStaticMethod(
+        function fn(input, cb) {
+          cb(null, input);
+        },
+        {
+          accepts: {arg: 'input', type: 'string'},
+          returns: {arg: 'output', type: 'string', http: {
+              target: 'header',
+              header: 'X-Custom-Header'
+            }
+          }
+        }
+      );
+      json(method.url + '?input=' + val)
+        .expect('X-Custom-Header', val)
+        .expect(200, done);
+    });
   });
 
   it('returns correct error response body', function(done) {

test/streams.js

@@ -31,7 +31,10 @@ describe('strong-remoting', function() {
   it('should stream the file output', function(done) {
     remotes.fs = fs;
     fs.createReadStream.shared = true;
-    fs.createReadStream.accepts = [{arg: 'path', type: 'string'}];
+    fs.createReadStream.accepts = [
+      { arg: 'path', type: 'string' },
+      { arg: 'encoding', type: 'string' }
+    ];
     fs.createReadStream.returns = {arg: 'res', type: 'stream'};
     fs.createReadStream.http = {
       verb: 'get',
@@ -41,7 +44,7 @@ describe('strong-remoting', function() {
       }
     };
 
-    json('get', '/fs/createReadStream?path=' + __dirname + '/data/foo.json')
+    json('get', '/fs/createReadStream?path=' + __dirname + '/data/foo.json&encoding=utf8')
       .expect({bar: 'baz'}, done);
   });
 });