Merge pull request #1322 from Real-Dev-Squad/fix/cache-success

Cache new response if successful

Author: ankushdharkar

test/unit/middlewares/cache.test.js

@@ -19,6 +19,7 @@ describe("Middleware | Utils | cache", function () {
     };
 
     const response = {
+      statusCode: 200,
       send: sinon.spy(),
     };
 
@@ -39,14 +40,16 @@ describe("Middleware | Utils | cache", function () {
     expect(response.send.callCount).to.equal(2);
   });
 
-  it("should invalidate stale the response", function () {
+  it("should invalidate stale response", function () {
     const cacheTestKey = "__cache__2";
 
     const request = {
       method: "GET",
       originalUrl: "/test2",
     };
     const response = {
+      on: sinon.spy(),
+      statusCode: 200,
       send: sinon.spy(),
     };
 
@@ -66,6 +69,9 @@ describe("Middleware | Utils | cache", function () {
     cacheMiddlewareForInvalidation(request, response, nextSpy);
     response.send(responseBody);
 
+    response.on.withArgs("finish").yield();
+
+    expect(response.on.callCount).to.equal(1);
     expect(nextSpy.callCount).to.equal(2);
     expect(response.send.callCount).to.equal(2);
 
@@ -75,4 +81,79 @@ describe("Middleware | Utils | cache", function () {
     expect(nextSpy.callCount).to.equal(3);
     expect(response.send.callCount).to.equal(3);
   });
+
+  it("should not cache the response", function () {
+    const cacheTestKey = "__cache__3";
+    const request = {
+      method: "GET",
+      originalUrl: "/test3",
+    };
+
+    const response = {
+      statusCode: 400,
+      send: sinon.spy(),
+    };
+
+    const nextSpy = sinon.spy();
+
+    const cacheMiddleware = cacheResponse({ invalidationKey: cacheTestKey });
+
+    cacheMiddleware(request, response, nextSpy);
+
+    response.send(responseBody);
+
+    expect(nextSpy.callCount).to.equal(1);
+    expect(response.send.callCount).to.equal(1);
+
+    cacheMiddleware(request, response, nextSpy);
+    response.send(responseBody);
+
+    expect(nextSpy.callCount).to.equal(2);
+
+    expect(response.send.callCount).to.equal(2);
+  });
+
+  it("should not invalidate stale the response if theres an error", function () {
+    const cacheTestKey = "__cache__4";
+
+    const request = {
+      method: "GET",
+      originalUrl: "/test4",
+    };
+    const response = {
+      on: sinon.spy(),
+      statusCode: 200,
+      send: sinon.spy(),
+    };
+
+    const nextSpy = sinon.spy();
+
+    const cacheMiddlewareForCache = cacheResponse({ invalidationKey: cacheTestKey });
+
+    cacheMiddlewareForCache(request, response, nextSpy);
+
+    response.send(responseBody);
+
+    expect(nextSpy.callCount).to.equal(1);
+    expect(response.send.callCount).to.equal(1);
+
+    response.statusCode = 400;
+
+    const cacheMiddlewareForInvalidation = invalidateCache({ invalidationKeys: [cacheTestKey] });
+
+    cacheMiddlewareForInvalidation(request, response, nextSpy);
+    response.send(responseBody);
+
+    response.on.withArgs("finish").yield();
+
+    expect(response.on.callCount).to.equal(1);
+    expect(nextSpy.callCount).to.equal(2);
+    expect(response.send.callCount).to.equal(2);
+
+    response.statusCode = 200;
+    cacheMiddlewareForCache(request, response, nextSpy);
+
+    expect(nextSpy.callCount).to.equal(2);
+    expect(response.send.callCount).to.equal(3);
+  });
 });

utils/cache.js

@@ -160,6 +160,11 @@ const cacheResponse = (options = {}) => {
         const oldSend = res.send;
 
         res.send = (body) => {
+          if (res.statusCode < 200 || res.statusCode >= 300) {
+            res.send = oldSend;
+            return res.send(body);
+          }
+
           const cacheValue = {
             priority: priority,
             response: body,
@@ -206,19 +211,23 @@ const invalidateCache = (options = {}) => {
   }
 
   return async (req, res, next) => {
-    try {
-      for (const key of keys) {
-        const cachedKeysSet = cachedKeys.getCachedKeys(key);
-        for (const cachedKey of cachedKeysSet) {
-          pool.evict(cachedKey);
+    res.on("finish", () => {
+      if (res.statusCode < 200 || res.statusCode >= 300) {
+        return;
+      }
+      try {
+        for (const key of keys) {
+          const cachedKeysSet = cachedKeys.getCachedKeys(key);
+          for (const cachedKey of cachedKeysSet) {
+            pool.evict(cachedKey);
+          }
+          cachedKeys.removeModelKey(key);
         }
-        cachedKeys.removeModelKey(key);
+      } catch (err) {
+        logger.error(`Error while removing cached response ${err}`);
       }
-    } catch (err) {
-      logger.error(`Error while removing cached response ${err}`);
-    } finally {
-      next();
-    }
+    });
+    next();
   };
 };