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
loopbackio · Sep 25, 2018 · deb2f05 · deb2f05
1 parent 8d105e4
commit deb2f05
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 8 deletions.
diff --git a/docs/site/Controllers.md b/docs/site/Controllers.md
@@ -214,6 +214,55 @@ export class HelloController {
 - `@param.query.number` specifies in the spec being generated that the route
   takes a parameter via query which will be a number.
 
+## Custom Response Writing
+
+By default, LoopBack serializes the return value from the controller methods
+into HTTP response based on the data type. For example, a string return value
+will be written to HTTP response as follows:
+
+status code: `200` Content-Type header: `text/plain` body: `the string value`
+
+In some cases, it's desirable for a controller method to customize how to
+produce the HTTP response. For example, we want to set the content type as
+`text/html`.
+
+```ts
+import {get} from '@loopback/openapi-v3';
+import {inject} from '@loopback/context';
+import {RestBindings, Response} from '@loopback/rest';
+
+export class HomePageController {
+  // ...
+  constructor(@inject(RestBindings.Http.RESPONSE) private res: Response) {
+    // ...
+  }
+  @get('/')
+  homePage() {
+    const homePage = '<html><body><h1>Hello</h1></body></html>';
+    this.res
+      .status(200)
+      .contentType('html')
+      .send(homePage);
+    // Return the `response` object itself to bypass further writing to HTTP
+    return this.res;
+  }
+}
+```
+
+Alternatively, the controller method can set status code and content type but
+leave the body to be populated by the LoopBack HTTP response writer.
+
+```ts
+@get('/')
+  homePage() {
+    const homePage = '<html><body><h1>Hello</h1></body></html>';
+    this.res
+      .status(200)
+      .contentType('html');
+    return homePage;
+  }
+```
+
 ## Handling Errors in Controllers
 
 In order to specify errors for controller methods to throw, the class

diff --git a/packages/rest/src/types.ts b/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.

diff --git a/packages/rest/src/writer.ts b/packages/rest/src/writer.ts
@@ -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 the response object itself, no writing should happen.
+  if (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;
   }

diff --git a/packages/rest/test/unit/writer.unit.ts b/packages/rest/test/unit/writer.unit.ts
@@ -23,6 +23,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 +41,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 +95,35 @@ describe('writer', () => {
     expect(result.payload).to.equal('');
   });
 
+  it('skips writing when the return value is the 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;