From b40f057594d51ae64e9d638d3b5877e544214b53 Mon Sep 17 00:00:00 2001
From: Chocobozzz <me@florianbigard.com>
Date: Wed, 23 May 2018 10:03:26 +0200
Subject: [PATCH] Handle concurrent requests in cache middleware

---
 package.json                |  2 ++
 server/lib/redis.ts         | 24 ++++++++---------
 server/middlewares/cache.ts | 53 +++++++++++++++++++++++--------------
 yarn.lock                   |  8 ++++++
 4 files changed, 55 insertions(+), 32 deletions(-)

diff --git a/package.json b/package.json
index bf69c4ce0..910de3f39 100644
--- a/package.json
+++ b/package.json
@@ -69,6 +69,7 @@
   },
   "dependencies": {
     "async": "^2.0.0",
+    "async-lock": "^1.1.2",
     "async-lru": "^1.1.1",
     "bcrypt": "^2.0.1",
     "bittorrent-tracker": "^9.0.0",
@@ -120,6 +121,7 @@
   },
   "devDependencies": {
     "@types/async": "^2.0.40",
+    "@types/async-lock": "^1.1.0",
     "@types/bcrypt": "^2.0.0",
     "@types/body-parser": "^1.16.3",
     "@types/chai": "^4.0.4",
diff --git a/server/lib/redis.ts b/server/lib/redis.ts
index 97ff3598b..5bd55109c 100644
--- a/server/lib/redis.ts
+++ b/server/lib/redis.ts
@@ -88,6 +88,18 @@ class Redis {
     })
   }
 
+  generateResetPasswordKey (userId: number) {
+    return 'reset-password-' + userId
+  }
+
+  buildViewKey (ip: string, videoUUID: string) {
+    return videoUUID + '-' + ip
+  }
+
+  buildCachedRouteKey (req: express.Request) {
+    return req.method + '-' + req.originalUrl
+  }
+
   private getValue (key: string) {
     return new Promise<string>((res, rej) => {
       this.client.get(this.prefix + key, (err, value) => {
@@ -146,18 +158,6 @@ class Redis {
     })
   }
 
-  private generateResetPasswordKey (userId: number) {
-    return 'reset-password-' + userId
-  }
-
-  private buildViewKey (ip: string, videoUUID: string) {
-    return videoUUID + '-' + ip
-  }
-
-  private buildCachedRouteKey (req: express.Request) {
-    return req.method + '-' + req.originalUrl
-  }
-
   static get Instance () {
     return this.instance || (this.instance = new this())
   }
diff --git a/server/middlewares/cache.ts b/server/middlewares/cache.ts
index c589ef683..bf6659687 100644
--- a/server/middlewares/cache.ts
+++ b/server/middlewares/cache.ts
@@ -1,39 +1,52 @@
 import * as express from 'express'
+import * as AsyncLock from 'async-lock'
 import { Redis } from '../lib/redis'
 import { logger } from '../helpers/logger'
 
+const lock = new AsyncLock({ timeout: 5000 })
+
 function cacheRoute (lifetime: number) {
   return async function (req: express.Request, res: express.Response, next: express.NextFunction) {
-    const cached = await Redis.Instance.getCachedRoute(req)
+    const redisKey = Redis.Instance.buildCachedRouteKey(req)
 
-    // Not cached
-    if (!cached) {
-      logger.debug('Not cached result for route %s.', req.originalUrl)
+    await lock.acquire(redisKey, async (done) => {
+      const cached = await Redis.Instance.getCachedRoute(req)
 
-      const sendSave = res.send.bind(res)
+      // Not cached
+      if (!cached) {
+        logger.debug('Not cached result for route %s.', req.originalUrl)
 
-      res.send = (body) => {
-        if (res.statusCode >= 200 && res.statusCode < 400) {
-          const contentType = res.getHeader('content-type').toString()
-          Redis.Instance.setCachedRoute(req, body, lifetime, contentType, res.statusCode)
-               .catch(err => logger.error('Cannot cache route.', { err }))
+        const sendSave = res.send.bind(res)
+
+        res.send = (body) => {
+          if (res.statusCode >= 200 && res.statusCode < 400) {
+            const contentType = res.getHeader('content-type').toString()
+            Redis.Instance.setCachedRoute(req, body, lifetime, contentType, res.statusCode)
+                 .then(() => done())
+                 .catch(err => {
+                   logger.error('Cannot cache route.', { err })
+                   return done(err)
+                 })
+          }
+
+          return sendSave(body)
         }
 
-        return sendSave(body)
+        return next()
       }
 
-      return next()
-    }
+      if (cached.contentType) res.contentType(cached.contentType)
 
-    if (cached.contentType) res.contentType(cached.contentType)
+      if (cached.statusCode) {
+        const statusCode = parseInt(cached.statusCode, 10)
+        if (!isNaN(statusCode)) res.status(statusCode)
+      }
 
-    if (cached.statusCode) {
-      const statusCode = parseInt(cached.statusCode, 10)
-      if (!isNaN(statusCode)) res.status(statusCode)
-    }
+      logger.debug('Use cached result for %s.', req.originalUrl)
+      res.send(cached.body).end()
 
-    logger.debug('Use cached result for %s.', req.originalUrl)
-    return res.send(cached.body).end()
+      return done()
+    })
   }
 }
 
diff --git a/yarn.lock b/yarn.lock
index 49af4df03..a8660dbab 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -16,6 +16,10 @@
     esutils "^2.0.2"
     js-tokens "^3.0.0"
 
+"@types/async-lock@^1.1.0":
+  version "1.1.0"
+  resolved "https://registry.yarnpkg.com/@types/async-lock/-/async-lock-1.1.0.tgz#002b1ebeebd382aff66b68bed70a74c7bdd06e3e"
+
 "@types/async@^2.0.40":
   version "2.0.49"
   resolved "https://registry.yarnpkg.com/@types/async/-/async-2.0.49.tgz#92e33d13f74c895cb9a7f38ba97db8431ed14bc0"
@@ -618,6 +622,10 @@ async-limiter@~1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/async-limiter/-/async-limiter-1.0.0.tgz#78faed8c3d074ab81f22b4e985d79e8738f720f8"
 
+async-lock@^1.1.2:
+  version "1.1.2"
+  resolved "https://registry.yarnpkg.com/async-lock/-/async-lock-1.1.2.tgz#d552b3f8fe93018bf917efcf66d3154b9035282a"
+
 async-lru@^1.1.1:
   version "1.1.1"
   resolved "https://registry.yarnpkg.com/async-lru/-/async-lru-1.1.1.tgz#3edbf7e96484d5c2dd852a8bf9794fc07f5e7274"