Repository: chimurai/http-proxy-middleware
Pull request #397 (2020-02-13 21:46:31)

feat: async pathRewrite
Related issue: https://github.com/chimurai/http-proxy-middleware/issues/374

deepcode-ci-bot[bot]: ## DeepCode's analysis on [#5be845](https://www.deepcode.ai/app/gh/chimurai/http-proxy-middleware/483911aafe14efa02d4980aa78d289d5351c5deb/chimurai/http-proxy-middleware/5be8454f74077dcb6e7a6f12089b03f58fe38c72/_/%2F/code/?utm_source=gh_review) found:

- :x: **0** critical issues.  :warning: **0** warnings  and **0** minor issues.  :heavy_check_mark: **0** issues were fixed.

:speech_balloon: This comment has been generated by the [**DeepCode bot**](https://github.com/apps/deepcode-ci-bot/), installed by the owner of the repository. The DeepCode bot **protects** your repository by detecting and commenting on security vulnerabilities or other critical issues.

---
:relaxed: If you want to provide feedback on our bot, here is how to [contact us](https://www.deepcode.ai/feedback?select=4&utm_source=gh_review).

coveralls: 
[![Coverage Status](https://coveralls.io/builds/28722187/badge)](https://coveralls.io/builds/28722187)

Coverage remained the same at 87.778% when pulling **5be8454f74077dcb6e7a6f12089b03f58fe38c72 on rsethc:master** into **483911aafe14efa02d4980aa78d289d5351c5deb on chimurai:master**.

chimurai: Nice. Looks good.

The following needs to be in place before this PR can be accepted:

- Updated documentation README.md
- unit test(s) for pathRewriter 
- unit test types.spec.ts (testing for the async syntax)

You can have a look at:
Similar PR, but async router: https://github.com/chimurai/http-proxy-middleware/pull/379/files

Unit tests for path rewriter:
https://github.com/chimurai/http-proxy-middleware/blob/master/test/unit/path-rewriter.spec.ts

Types test:
https://github.com/chimurai/http-proxy-middleware/blob/master/test/types.spec.ts
example of async router: https://github.com/chimurai/http-proxy-middleware/blob/master/test/types.spec.ts#L74
rsethc: Alright, I'll take a look at solving these requirements. Thanks
rsethc: I think this is ready to have another look. One change I made by accident affected the `yarn.lock` file, let me know if it's preferable to add a commit that undoes that.
chimurai: Yes, please undo the changes to `yarn.lock`

I'll checkout the PR to do some additional testing.

Patch:
diff --git a/README.md b/README.md
index 9bfd3a2..8b7841d 100644
--- a/README.md
+++ b/README.md
@@ -199,6 +199,14 @@ Providing an alternative way to decide which requests should be proxied; In case
 
   // custom rewriting
   pathRewrite: function (path, req) { return path.replace('/api', '/base/api') }
+
+  // custom rewriting, returning Promise
+  pathRewrite: async function (path, req) {
+    var should_add_something = await httpRequestToDecideSomething(path);
+    if (should_add_something) path += "something";
+    return path;
+  }
+
   ```
 
 - **option.router**: object/function, re-target `option.target` for specific requests.
diff --git a/src/http-proxy-middleware.ts b/src/http-proxy-middleware.ts
index ca11332..c6fe759 100644
--- a/src/http-proxy-middleware.ts
+++ b/src/http-proxy-middleware.ts
@@ -116,7 +116,7 @@ export class HttpProxyMiddleware {
     // 1. option.router
     // 2. option.pathRewrite
     await this.applyRouter(req, newProxyOptions);
-    this.applyPathRewrite(req, this.pathRewriter);
+    await this.applyPathRewrite(req, this.pathRewriter);
 
     // debug logging for both http(s) and websockets
     if (this.proxyOptions.logLevel === 'debug') {
@@ -157,9 +157,9 @@ export class HttpProxyMiddleware {
   };
 
   // rewrite path
-  private applyPathRewrite = (req: IRequest, pathRewriter) => {
+  private applyPathRewrite = async (req: IRequest, pathRewriter) => {
     if (pathRewriter) {
-      const path = pathRewriter(req.url, req);
+      const path = await pathRewriter(req.url, req);
 
       if (typeof path === 'string') {
         req.url = path;
diff --git a/src/types.ts b/src/types.ts
index 7aef997..06ff4f1 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -18,7 +18,8 @@ export type Filter =
 export interface Options extends httpProxy.ServerOptions {
   pathRewrite?:
     | { [regexp: string]: string }
-    | ((path: string, req: IRequest) => string);
+    | ((path: string, req: IRequest) => string)
+    | ((path: string, req: IRequest) => Promise<string>);
   router?:
     | { [hostOrPath: string]: string }
     | ((req: IRequest) => string)


Test patch:
diff --git a/test/types.spec.ts b/test/types.spec.ts
index 64a0fdb..671ae47 100644
--- a/test/types.spec.ts
+++ b/test/types.spec.ts
@@ -58,6 +58,11 @@ describe('http-proxy-middleware TypeScript Types', () => {
         options = { pathRewrite: (path, req) => '/path' };
         expect(options).toBeDefined();
       });
+
+      it('should have pathRewrite Type with async function', () => {
+        options = { pathRewrite: async (path, req) => '/path' };
+        expect(options).toBeDefined();
+      });
     });
 
     describe('router', () => {
diff --git a/test/unit/path-rewriter.spec.ts b/test/unit/path-rewriter.spec.ts
index f42d7a8..b848782 100644
--- a/test/unit/path-rewriter.spec.ts
+++ b/test/unit/path-rewriter.spec.ts
@@ -102,6 +102,44 @@ describe('Path rewriting', () => {
 
       expect(rewriter(rewriteFn)).toBe('/123/789');
     });
+
+    // Same tests as the above three, but async
+
+    it('is async and should return unmodified path', () => {
+      const rewriteFn = async path => {
+        const promise = new Promise((resolve, reject) => {
+          resolve(path);
+        });
+        const changed = await promise;
+        return changed;
+      };
+
+      expect(rewriter(rewriteFn)).resolves.toBe('/123/456');
+    });
+
+    it('is async and should return alternative path', () => {
+      const rewriteFn = async path => {
+        const promise = new Promise((resolve, reject) => {
+          resolve('/foo/bar');
+        });
+        const changed = await promise;
+        return changed;
+      };
+
+      expect(rewriter(rewriteFn)).resolves.toBe('/foo/bar');
+    });
+
+    it('is async and should return replaced path', () => {
+      const rewriteFn = async path => {
+        const promise = new Promise((resolve, reject) => {
+          resolve(path.replace('/456', '/789'));
+        });
+        const changed = await promise;
+        return changed;
+      };
+
+      expect(rewriter(rewriteFn)).resolves.toBe('/123/789');
+    });
   });
 
   describe('Invalid configuration', () => {
@@ -136,5 +174,10 @@ describe('Path rewriting', () => {
       // tslint:disable-next-line: no-empty
       expect(badFn(() => {})).not.toThrowError(Error);
     });
+
+    it('should not throw when async function config is provided', () => {
+      // tslint:disable-next-line: no-empty
+      expect(badFn(async () => {})).not.toThrowError(Error);
+    });
   });
 });