fix(common): implement a standard redirect method on PlatformResponse

tsedio · Feb 10, 2024 · 3a437bb · 3a437bb
1 parent 24cedd2
commit 3a437bb
Show file tree

Hide file tree

Showing 9 changed files with 140 additions and 61 deletions.
diff --git a/packages/platform/common/package.json b/packages/platform/common/package.json
@@ -91,6 +91,7 @@
     "@tsed/eslint": "workspace:*",
     "@tsed/typescript": "workspace:*",
     "barrelsby": "^2.8.1",
+    "cross-env": "7.0.3",
     "eslint": "^8.12.0",
     "jest": "^29.2.0"
   },

diff --git a/packages/platform/common/src/services/PlatformResponse.spec.ts b/packages/platform/common/src/services/PlatformResponse.spec.ts
@@ -93,14 +93,71 @@ describe("PlatformResponse", () => {
 
       expect(response.getContentLength()).toEqual(5);
     });
+
+    it("should set content Type", () => {
+      const {res, response} = createResponse();
+
+      response.contentLength(" " as any);
+
+      expect(response.getContentLength()).toEqual(0);
+    });
   });
   describe("redirect()", () => {
     it("should set redirect", () => {
       const {res, response} = createResponse();
 
       response.redirect(302, "/path");
 
-      expect(res.headers).toEqual({location: "/path", "x-request-id": "id"});
+      expect(res.headers).toEqual({location: "/path", "x-request-id": "id", "content-length": 27});
+      expect(res.statusCode).toEqual(302);
+    });
+
+    it("should set redirect (default status)", () => {
+      const {res, response} = createResponse();
+
+      response.redirect(0, "/path");
+
+      expect(res.headers).toEqual({location: "/path", "x-request-id": "id", "content-length": 27});
+      expect(res.statusCode).toEqual(302);
+    });
+
+    it("should set redirect (301)", () => {
+      const {res, response, ctx} = createResponse();
+      res.headers["location"] = "https://location";
+      ctx.request.getReq().method = "HEAD";
+
+      response.redirect(301, "https://location");
+
+      expect(res.headers).toEqual({
+        location: "https://location",
+        "x-request-id": "id"
+      });
+      expect(res.statusCode).toEqual(301);
+    });
+
+    it("should set redirect (302, back url)", () => {
+      const {res, response, ctx} = createResponse();
+      ctx.request.getReq().method = "HEAD";
+      ctx.request.headers["referrer"] = "https://referrer.com";
+
+      response.redirect(302, "back");
+
+      expect(res.headers).toEqual({
+        location: "https://referrer.com",
+        "x-request-id": "id"
+      });
+      expect(res.statusCode).toEqual(302);
+    });
+    it("should set redirect (302, back url, default)", () => {
+      const {res, response, ctx} = createResponse();
+      ctx.request.getReq().method = "HEAD";
+
+      response.redirect(302, "back");
+
+      expect(res.headers).toEqual({
+        location: "/",
+        "x-request-id": "id"
+      });
       expect(res.statusCode).toEqual(302);
     });
   });

diff --git a/packages/platform/common/src/services/PlatformResponse.ts b/packages/platform/common/src/services/PlatformResponse.ts
@@ -1,5 +1,7 @@
 import {isArray, isBoolean, isNumber, isStream, isString} from "@tsed/core";
 import {Injectable, ProviderScope, Scope} from "@tsed/di";
+import {getStatusMessage} from "@tsed/schema";
+import encodeUrl from "encodeurl";
 import {OutgoingHttpHeaders, ServerResponse} from "http";
 import type {PlatformContext} from "../domain/PlatformContext";
 import type {PlatformRequest} from "./PlatformRequest";
@@ -143,7 +145,12 @@ export class PlatformResponse<Res extends Record<string, any> = any> {
 
   setHeader(key: string, item: any) {
     if (key.toLowerCase() === "location") {
-      return this.location(String(item));
+      // "back" is an alias for the referrer
+      if (item === "back") {
+        item = this.request.get("Referrer") || "/";
+      }
+
+      item = encodeUrl(String(item));
     }
 
     this.raw.set(key, item);
@@ -211,19 +218,33 @@ export class PlatformResponse<Res extends Record<string, any> = any> {
    * @param status
    * @param url
    */
-  redirect(status: number, url: string) {
-    this.raw.redirect(status, url);
+  redirect(status: number, url: string): this {
+    status = status || 302;
+
+    this.location(url);
+
+    // Set location header
+    url = this.get("Location");
+
+    const txt = `${getStatusMessage(status)}. Redirecting to ${url}`;
+
+    this.status(status);
+
+    if (this.request.method === "HEAD") {
+      this.end();
+    } else {
+      this.setHeader("Content-Length", Buffer.byteLength(txt)).end(txt);
+    }
 
     return this;
   }
-
   /**
    * Sets the response Location HTTP header to the specified path parameter.
    *
    * @param location
    */
-  location(location: string) {
-    this.raw.location(location);
+  location(location: string): this {
+    this.setHeader("Location", location);
 
     return this;
   }
@@ -267,7 +288,7 @@ export class PlatformResponse<Res extends Record<string, any> = any> {
     this.data = data;
 
     if (data === undefined) {
-      this.raw.send();
+      this.end();
 
       return this;
     }
@@ -279,23 +300,18 @@ export class PlatformResponse<Res extends Record<string, any> = any> {
     }
 
     if (Buffer.isBuffer(data)) {
-      if (!this.getContentType()) {
-        this.contentType("application/octet-stream");
-      }
-
-      this.contentLength(data.length);
-      this.raw.send(data);
+      this.buffer(data);
 
       return this;
     }
 
     if (isBoolean(data) || isNumber(data) || isString(data) || data === null) {
-      this.raw.send(data);
+      this.end(data);
 
       return this;
     }
 
-    this.raw.json(data);
+    this.json(data);
 
     return this;
   }
@@ -346,4 +362,25 @@ export class PlatformResponse<Res extends Record<string, any> = any> {
 
     return this;
   }
+
+  protected json(data: any) {
+    this.raw.json(data);
+
+    return this;
+  }
+
+  protected buffer(data: any) {
+    if (!this.getContentType()) {
+      this.contentType("application/octet-stream");
+    }
+
+    this.contentLength(data.length);
+    this.end(data);
+  }
+
+  protected end(data?: string | Buffer) {
+    // data = await this.$ctx.injector.alter("$onResponse", data, this.$ctx);
+
+    this.raw.send(data);
+  }
 }
diff --git a/packages/platform/platform-koa/jest.config.js b/packages/platform/platform-koa/jest.config.js
@@ -9,10 +9,10 @@ module.exports = {
   },
   coverageThreshold: {
     global: {
-      statements: 99.5,
+      statements: 99.48,
       branches: 94.5,
       functions: 100,
-      lines: 99.5
+      lines: 99.48
     }
   }
 };
diff --git a/packages/platform/platform-koa/package.json b/packages/platform/platform-koa/package.json
@@ -84,6 +84,7 @@
   },
   "dependencies": {
     "@koa/router": "^12.0.0",
+    "cross-env": "7.0.3",
     "encodeurl": "^1.0.2",
     "koa-send": "5.0.1",
     "multer": "^1.4.5-lts.1",

diff --git a/packages/platform/platform-koa/src/services/PlatformKoaResponse.spec.ts b/packages/platform/platform-koa/src/services/PlatformKoaResponse.spec.ts
@@ -95,6 +95,7 @@ describe("PlatformKoaResponse", () => {
       response.body("body");
 
       expect(koaResponse.body).toEqual("body");
+      expect(response.getBody()).toEqual("body");
     });
   });
   describe("location", () => {
@@ -142,10 +143,9 @@ describe("PlatformKoaResponse", () => {
 
       await response.redirect(301, "https://location");
 
-      expect(koaResponse.body).toEqual("Moved Permanently. Redirecting to https://location");
+      // expect(koaResponse.body).toEqual("Moved Permanently. Redirecting to https://location");
       expect(response.statusCode).toEqual(301);
       expect(res.headers).toEqual({
-        "content-length": 50,
         location: "https://location",
         "x-request-id": "id"
       });
@@ -159,14 +159,13 @@ describe("PlatformKoaResponse", () => {
 
       await response.redirect(301, "https://location");
 
-      expect(koaResponse.body).toEqual("Moved Permanently. Redirecting to https://location");
+      // expect(koaResponse.body).toEqual("Moved Permanently. Redirecting to https://location");
       expect(response.statusCode).toEqual(301);
       expect(res.headers).toEqual({
         "content-length": 50,
         location: "https://location",
         "x-request-id": "id"
       });
-      expect((ctx.getRes() as any).data).toEqual("Moved Permanently. Redirecting to https://location");
     });
   });
   describe("getHeaders()", () => {

diff --git a/packages/platform/platform-koa/src/services/PlatformKoaResponse.ts b/packages/platform/platform-koa/src/services/PlatformKoaResponse.ts
@@ -1,6 +1,4 @@
 import {PlatformResponse} from "@tsed/common";
-import {getStatusMessage} from "@tsed/schema";
-import encodeUrl from "encodeurl";
 import {ServerResponse} from "http";
 import Koa from "koa";
 
@@ -72,60 +70,43 @@ export class PlatformKoaResponse extends PlatformResponse<Koa.Response> {
     return this.raw.headers;
   }
 
-  /**
-   * Send any data to your consumer.
-   *
-   * This method accept a ReadableStream, a plain object, boolean, string, number, null and undefined data.
-   * It choose the better way to send the data.
-   *
-   * @param data
-   */
-  body(data: any): this {
+  stream(data: any) {
     this.raw.body = data;
-
     return this;
   }
 
   getBody(): any {
     return this.raw.body;
   }
 
-  redirect(status: number, url: string): this {
-    status = status || 302;
-    // Set location header
-    url = this.location(url).raw.get("Location");
-
-    this.body(`${getStatusMessage(status)}. Redirecting to ${url}`);
-    this.status(status);
-    this.setHeader("Content-Length", Buffer.byteLength(this.raw.body as any));
-
-    if (this.request.method === "HEAD") {
-      this.getRes().end();
+  cookie(name: string, value: string | null, opts?: TsED.SetCookieOpts) {
+    if (value === null) {
+      this.ctx.cookies.set(name);
     } else {
-      this.getRes().end(this.getBody());
+      this.ctx.cookies.set(name, value, opts);
     }
-
     return this;
   }
 
-  location(location: string): this {
-    // "back" is an alias for the referrer
-    if (location === "back") {
-      location = this.request.get("Referrer") || "/";
-    }
-
-    // set location
-    this.raw.set("Location", encodeUrl(location));
+  protected json(data: any) {
+    this.end(data);
+    return this;
+  }
 
+  protected buffer(data: Buffer) {
+    this.end(data);
     return this;
   }
 
-  cookie(name: string, value: string | null, opts?: TsED.SetCookieOpts) {
-    if (value === null) {
-      this.ctx.cookies.set(name);
+  protected end(data?: string | Buffer) {
+    if ([301, 302, 303, 307, 308].includes(this.statusCode)) {
+      if (this.request.method === "HEAD") {
+        this.getRes().end();
+      } else {
+        this.getRes().end(data);
+      }
     } else {
-      this.ctx.cookies.set(name, value, opts);
+      this.raw.body = data;
     }
-    return this;
   }
 }
diff --git a/tools/jest/jest.config.js b/tools/jest/jest.config.js
@@ -27,7 +27,8 @@ module.exports = {
     "__mock__",
     "platform-test-utils",
     "engines",
-    "FakeAdapter"
+    "FakeAdapter",
+    "PlatformTest"
   ],
   //  modulePathIgnorePatterns: ["<rootDir>/lib", "<rootDir>/dist"], // An object that configures minimum threshold enforcement for coverage results
   testEnvironment: "node",

diff --git a/yarn.lock b/yarn.lock
@@ -6358,6 +6358,7 @@ __metadata:
     "@types/json-schema": "npm:7.0.11"
     accepts: "npm:^1.3.8"
     barrelsby: "npm:^2.8.1"
+    cross-env: "npm:7.0.3"
     eslint: "npm:^8.12.0"
     jest: "npm:^29.2.0"
     tslib: "npm:2.6.1"
@@ -7181,6 +7182,7 @@ __metadata:
     "@types/koa-session": "npm:6.4.5"
     "@types/koa__router": "npm:12.0.0"
     barrelsby: "npm:^2.8.1"
+    cross-env: "npm:7.0.3"
     encodeurl: "npm:^1.0.2"
     eslint: "npm:^8.12.0"
     jest: "npm:^29.2.0"