feat(rest): allow bypassing http response writing and custom content …

…type

We discover an issue to return html from a controller:
See loopbackio/loopback4-example-shopping#16

packages/rest/src/types.ts

@@ -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.
@@ -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;

packages/rest/src/writer.ts

@@ -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';
 
 /**
@@ -12,21 +12,37 @@ 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) {
-    response.statusCode = 204;
+  // Check if response writing should be bypassed when the return value
+  // is ByPassResponse or the response object
+  if (result === BypassResponse || result === response) return;
+
+  if (result === undefined) {
+    response.status(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') {
+    if (response.getHeader('Content-Type') == null) {
+      response.setHeader('Content-Type', contentType || defaultType);
+    }
+  }
+
+  if (
+    result instanceof Readable ||
+    typeof (result && result.pipe) === 'function'
+  ) {
+    setContentType('application/octet-stream');
     // Stream
     result.pipe(response);
     return;
@@ -37,17 +53,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;
   }

packages/rest/test/unit/writer.unit.ts

@@ -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;
@@ -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;
@@ -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;
@@ -77,6 +96,51 @@ 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>');
+  });
+
+  it('skips response writing when the return value is a Response', async () => {
+    response
+      .status(200)
+      .contentType('text/html; charset=utf-8')
+      .send('<html><body>Hi</body></html>');
+    writeResultToResponse(response, response);
+    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>');
+  });
+
+  it('does not override content type', async () => {
+    response.status(200).contentType('text/html; charset=utf-8');
+    writeResultToResponse(response, '<html><body>Hi</body></html>');
+    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;