Skip to content

Commit

Permalink
feat(rest): allow bypassing http response writing and custom content …
Browse files Browse the repository at this point in the history
…type

We discover an issue to return html from a controller:
See loopbackio/loopback4-example-shopping#16
  • Loading branch information
raymondfeng committed Sep 24, 2018
1 parent d65a17f commit 7af3258
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 8 deletions.
14 changes: 13 additions & 1 deletion packages/rest/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,13 @@ export type InvokeMethod = (
*
* @param response The response the response to send to.
* @param result The operation result to send.
* @param contentType The content type to send
*/
export type Send = (response: Response, result: OperationRetval) => void;
export type Send = (
response: Response,
result: OperationRetval,
contentType?: string,
) => void;

/**
* Reject the request with an error.
Expand Down Expand Up @@ -88,5 +93,12 @@ export type OperationArgs = any[];
export type OperationRetval = any;
// tslint:enable:no-any

/**
* A special return value that can be used by controller methods to bypass
* response writing as the controller has sent the response directly using
* injected `Response` object.
*/
export const BypassResponse = Symbol('Bypass HTTP response writing');

export type GetFromContext = (key: string) => Promise<BoundValue>;
export type BindElement = (key: string) => Binding;
26 changes: 19 additions & 7 deletions packages/rest/src/writer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT

import {OperationRetval, Response} from './types';
import {OperationRetval, Response, BypassResponse} from './types';
import {Readable} from 'stream';

/**
Expand All @@ -12,21 +12,33 @@ import {Readable} from 'stream';
*
* @param response HTTP Response
* @param result Result from the API to write into HTTP Response
* @param contentType Optional content type for the response
*/
export function writeResultToResponse(
// not needed and responsibility should be in the sequence.send
response: Response,
// result returned back from invoking controller method
result: OperationRetval,
// content type for the response
contentType?: string,
): void {
if (!result) {
if (result === BypassResponse) return;

if (result === undefined) {
response.statusCode = 204;
response.end();
return;
}

if (result instanceof Readable || typeof result.pipe === 'function') {
response.setHeader('Content-Type', 'application/octet-stream');
function setContentType(defaultType: string = 'application/json') {
response.setHeader('Content-Type', contentType || defaultType);
}

if (
result instanceof Readable ||
typeof (result && result.pipe) === 'function'
) {
setContentType('application/octet-stream');
// Stream
result.pipe(response);
return;
Expand All @@ -37,17 +49,17 @@ export function writeResultToResponse(
case 'number':
if (Buffer.isBuffer(result)) {
// Buffer for binary data
response.setHeader('Content-Type', 'application/octet-stream');
setContentType('application/octet-stream');
} else {
// TODO(ritch) remove this, should be configurable
// See https://github.com/strongloop/loopback-next/issues/436
response.setHeader('Content-Type', 'application/json');
setContentType();
// TODO(bajtos) handle errors - JSON.stringify can throw
result = JSON.stringify(result);
}
break;
default:
response.setHeader('Content-Type', 'text/plain');
setContentType('text/plain');
result = result.toString();
break;
}
Expand Down
35 changes: 35 additions & 0 deletions packages/rest/test/unit/writer.unit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import {Response, writeResultToResponse} from '../..';
import {Duplex} from 'stream';
import {expect, ObservedResponse, stubExpressContext} from '@loopback/testlab';
import {BypassResponse} from '../../src';

describe('writer', () => {
let response: Response;
Expand All @@ -23,6 +24,16 @@ describe('writer', () => {
expect(result.payload).to.equal('Joe');
});

it('writes string result to response as html', async () => {
writeResultToResponse(response, 'Joe', 'text/html');
const result = await observedResponse;

// content-type should be 'application/json' since it's set
// into the response in writer.writeResultToResponse()
expect(result.headers['content-type']).to.eql('text/html');
expect(result.payload).to.equal('Joe');
});

it('writes object result to response as JSON', async () => {
writeResultToResponse(response, {name: 'Joe'});
const result = await observedResponse;
Expand All @@ -31,6 +42,14 @@ describe('writer', () => {
expect(result.payload).to.equal('{"name":"Joe"}');
});

it('writes null object result to response as JSON', async () => {
writeResultToResponse(response, null);
const result = await observedResponse;

expect(result.headers['content-type']).to.eql('application/json');
expect(result.payload).to.equal('null');
});

it('writes boolean result to response as json', async () => {
writeResultToResponse(response, true);
const result = await observedResponse;
Expand Down Expand Up @@ -77,6 +96,22 @@ describe('writer', () => {
expect(result.payload).to.equal('');
});

it('skips response writing when the return value is ByPassResponse', async () => {
response
.status(200)
.contentType('text/html; charset=utf-8')
.send('<html><body>Hi</body></html>');
writeResultToResponse(response, BypassResponse);
const result = await observedResponse;

expect(result.statusCode).to.equal(200);
expect(result.headers).to.have.property(
'content-type',
'text/html; charset=utf-8',
);
expect(result.payload).to.equal('<html><body>Hi</body></html>');
});

function setupResponseMock() {
const responseMock = stubExpressContext();
response = responseMock.response;
Expand Down

0 comments on commit 7af3258

Please sign in to comment.