Skip to content

Commit

Permalink
Add support for declaring status code defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
ritch committed Jun 18, 2015
1 parent 8b0f04c commit 04314ed
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 19 deletions.
51 changes: 48 additions & 3 deletions lib/http-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -324,15 +328,51 @@ 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) {
if(!(err.status || err.statusCode)) {
res.status(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.header || name, value);
break;
}
} else {
result[name] = value;
}
};

Expand Down Expand Up @@ -384,6 +424,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();
Expand Down
4 changes: 3 additions & 1 deletion lib/rest-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
34 changes: 29 additions & 5 deletions lib/shared-method.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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.
*
Expand All @@ -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 = {};
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -457,7 +477,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) {
Expand All @@ -471,14 +491,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;
Expand Down
1 change: 0 additions & 1 deletion test/authorize-hook.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,3 @@ describe('authorization hook', function() {
remotes.invoke(method, args, callback);
}
});

78 changes: 69 additions & 9 deletions test/rest.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1753,17 +1753,77 @@ 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);
});
});

json(classUrl + '/unknown-method')
.expect(404, done);
});
it('returns 404 for unknown method of a shared class', function(done) {
var classUrl = givenSharedStaticMethod().classUrl;

it('returns 404 with standard JSON body for uknown URL', function(done) {
json('/unknown-url')
.expect(404)
.end(expectErrorResponseContaining({status: 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 correct error response body', function(done) {
Expand Down

0 comments on commit 04314ed

Please sign in to comment.