Repository: ViacomInc/data-point (js)
Issue: resetCache should clear stale data
<!--
Thanks for your interest in the project. We appreciate bugs filed and PRs submitted!
Please make sure that you are familiar with and follow the Code of Conduct for
this project (found in the CODE_OF_CONDUCT.md file).

Please fill/un-comment out this template with all the relevant information so we can
understand what's going on and fix the issue.
-->

**Problem description**:
When resetCache is true the promise chain ignore saved values but stale data is not removed


**Suggested solution**:
Clear stale data during the promise chain


<!-- 
REPORTING BUG:

If you are reporting a bug, please provide the full error message/screenshots/anything.
-->

**What happened**:



**Reproduction**: 



<!--If you have a link or example that reproduces your issue please include it. A reproduction with as little code as possible is better.-->

**Environment**:

- `data-point` version: 
- `node` version: 
- `npm` version:

Pull request description: fix(data-point-service): resetCache should clear stale data
resetCache should clear stale data

fix #492

<!--
Thanks for your interest in the project. We appreciate bugs filed and PRs submitted!

Please make sure that you are familiar with and follow the Code of Conduct for
this project (found in the CODE_OF_CONDUCT.md file).

Also, please make sure you're familiar with and follow the instructions in the
contributing guidelines (found in the CONTRIBUTING.md file).

Please fill out the information below to expedite the review and (hopefully)
merge of your pull request!

BREAKING CHANGES:

If your PR includes a breaking change, please submit it with a codemod under
data-point-codemods that will help users upgrade their codebase.

Breaking changes without a codemod will not be accepted unless a codemod is not
viable or does not apply to the specific situation.
-->

<!-- What changes are being made? (What feature/bug is being fixed here?) -->
**What**: resetCache doesn't remove stale data so, it happens that for removed contents it still provide old data 

<!-- Why are these changes necessary? -->
**Why**:

<!-- How were these changes implemented? -->
**How**: removing stale data on promise chain 

<!-- Have you done all of these things?  -->
**Checklist**:
<!-- add "N/A" to the end of each line that's irrelevant to your changes -->
<!-- to check an item, place an "x" in the box like so: "- [x] Documentation" -->
- [ ] Has Breaking changes N/A
- [x] Documentation
- [x] Tests
- [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->
- [x] Added username to **all-contributors** list

<!-- feel free to add additional comments -->


Patch:
diff --git a/.all-contributorsrc b/.all-contributorsrc
index 19a4300..b318bbe 100644
--- a/.all-contributorsrc
+++ b/.all-contributorsrc
@@ -107,6 +107,17 @@
         "question",
         "code"
       ]
+    },
+    {
+      "login": "schiavig",
+      "name": "Giampiero Schiavi",
+      "avatar_url": "https://avatars.githubusercontent.com/u/85735845?v=4",
+      "profile": "https://github.com/schiavig",
+      "contributions": [
+        "code",
+        "test",
+        "bug"
+      ]
     }
   ],
   "repoType": "github"
diff --git a/packages/data-point-service/lib/cache-middleware.js b/packages/data-point-service/lib/cache-middleware.js
index f403e1c..f667bb1 100644
--- a/packages/data-point-service/lib/cache-middleware.js
+++ b/packages/data-point-service/lib/cache-middleware.js
@@ -243,6 +243,10 @@ function before(service, ctx, next) {
   const cache = EntityCacheParams.getCacheParams(ctx.context.params);
 
   if (!cache.ttl || ctx.locals.resetCache === true) {
+    if (ctx.locals.resetCache === true) {
+      const currentEntryKey = module.exports.generateKey(cache.cacheKey, ctx);
+      RedisController.deleteSWRStaleEntry(service, currentEntryKey);
+    }
     next();
     return false;
   }
diff --git a/packages/data-point-service/lib/redis-controller.js b/packages/data-point-service/lib/redis-controller.js
index a403a0d..83e486f 100644
--- a/packages/data-point-service/lib/redis-controller.js
+++ b/packages/data-point-service/lib/redis-controller.js
@@ -93,11 +93,23 @@ function deleteSWRControlEntry(service, key) {
   return deleteEntry(service, createSWRControlKey(key));
 }
 
+/**
+ * Delete stale entry by key
+ * @param {Service} service Service instance
+ * @param {String} key entry key
+ * @returns {Promise}
+ */
+function deleteSWRStaleEntry(service, key) {
+  const staleKey = createSWRStaleKey(key);
+  return deleteEntry(service, staleKey);
+}
+
 module.exports = {
   createSWRControlKey,
   createSWRStaleKey,
   deleteEntry,
   deleteSWRControlEntry,
+  deleteSWRStaleEntry,
   getEntry,
   getSWRControlEntry,
   getSWRStaleEntry,


Test patch:
diff --git a/packages/data-point-service/lib/cache-middleware.test.js b/packages/data-point-service/lib/cache-middleware.test.js
index acf59a0..0293004 100644
--- a/packages/data-point-service/lib/cache-middleware.test.js
+++ b/packages/data-point-service/lib/cache-middleware.test.js
@@ -549,6 +549,10 @@ describe("before", () => {
       .spyOn(RedisController, "getEntry")
       .mockReturnValue("noTTLEntry");
 
+    mocks.deleteSWRStaleEntry = jest
+      .spyOn(RedisController, "deleteSWRStaleEntry")
+      .mockReturnValue(Promise.resolve());
+
     return mocks;
   }
 
@@ -564,7 +568,7 @@ describe("before", () => {
       expect(result).toEqual(false);
       expect(mocks.next).toBeCalledWith();
     });
-    it("should call next and return false if resetCache is true", () => {
+    it("should delete stale and call next returning false if resetCache is true", () => {
       const mocks = createMocks();
       mocks.ctx.locals.resetCache = true;
       const result = CacheMiddleware.before(
@@ -572,6 +576,7 @@ describe("before", () => {
         mocks.ctx,
         mocks.next
       );
+      expect(mocks.deleteSWRStaleEntry).toBeCalled();
       expect(result).toEqual(false);
       expect(mocks.next).toBeCalledWith();
     });
diff --git a/packages/data-point-service/lib/redis-controller.test.js b/packages/data-point-service/lib/redis-controller.test.js
index 126664b..b020458 100644
--- a/packages/data-point-service/lib/redis-controller.test.js
+++ b/packages/data-point-service/lib/redis-controller.test.js
@@ -147,3 +147,15 @@ describe("deleteSWRControlEntry", () => {
     expect(service.cache.del).toBeCalledWith("key:swr.control");
   });
 });
+
+describe("deleteSWRStaleEntry", () => {
+  it("should delete a stale entry", () => {
+    const service = {
+      cache: {
+        del: jest.fn()
+      }
+    };
+    RedisController.deleteSWRStaleEntry(service, "key");
+    expect(service.cache.del).toBeCalledWith("key:swr.stale");
+  });
+});