Repository: ViacomInc/data-point
Pull request #304 (2018-09-11 20:56:10)

refactor(data-point-service): refactors modules to prepare for new revalidation feature
<!--
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**: prepare codebase for new revalidation feature

<!-- Why are these changes necessary? -->
**Why**: too much happening on cache-middleware.js to add a new feature, this PR refactors it so code can be more easily reasoned about

<!-- How were these changes implemented? -->
**How**: making code bit more modular

<!-- 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
- [ ] Documentation
- [X] Tests
- [X] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? -->
- [ ] Added username to **all-contributors** list

<!-- feel free to add additional comments -->


codecov[bot]: # [Codecov](https://codecov.io/gh/ViacomInc/data-point/pull/304?src=pr&el=h1) Report
> Merging [#304](https://codecov.io/gh/ViacomInc/data-point/pull/304?src=pr&el=desc) into [master](https://codecov.io/gh/ViacomInc/data-point/commit/01ebaad873a7c7a6eb3138afe11e5908bc38849d?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/ViacomInc/data-point/pull/304/graphs/tree.svg?width=650&token=LYxItnrx4i&height=150&src=pr)](https://codecov.io/gh/ViacomInc/data-point/pull/304?src=pr&el=tree)

```diff
@@          Coverage Diff          @@
##           master   #304   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         132    134    +2     
  Lines        1897   1901    +4     
  Branches      177    178    +1     
=====================================
+ Hits         1897   1901    +4
```


| [Impacted Files](https://codecov.io/gh/ViacomInc/data-point/pull/304?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ackages/data-point-service/lib/redis-controller.js](https://codecov.io/gh/ViacomInc/data-point/pull/304/diff?src=pr&el=tree#diff-cGFja2FnZXMvZGF0YS1wb2ludC1zZXJ2aWNlL2xpYi9yZWRpcy1jb250cm9sbGVyLmpz) | `100% <100%> (ø)` | |
| [...ages/data-point-service/lib/entity-cache-params.js](https://codecov.io/gh/ViacomInc/data-point/pull/304/diff?src=pr&el=tree#diff-cGFja2FnZXMvZGF0YS1wb2ludC1zZXJ2aWNlL2xpYi9lbnRpdHktY2FjaGUtcGFyYW1zLmpz) | `100% <100%> (ø)` | |
| [...ackages/data-point-service/lib/cache-middleware.js](https://codecov.io/gh/ViacomInc/data-point/pull/304/diff?src=pr&el=tree#diff-cGFja2FnZXMvZGF0YS1wb2ludC1zZXJ2aWNlL2xpYi9jYWNoZS1taWRkbGV3YXJlLmpz) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/ViacomInc/data-point/pull/304?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/ViacomInc/data-point/pull/304?src=pr&el=footer). Last update [01ebaad...22ca23d](https://codecov.io/gh/ViacomInc/data-point/pull/304?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/packages/data-point-service/lib/cache-middleware.js b/packages/data-point-service/lib/cache-middleware.js
index 0321562..cb4945d 100644
--- a/packages/data-point-service/lib/cache-middleware.js
+++ b/packages/data-point-service/lib/cache-middleware.js
@@ -1,11 +1,22 @@
 const Promise = require('bluebird')
-const defaultTo = require('lodash/defaultTo')
 const set = require('lodash/fp/set')
 const debug = require('debug')('data-point-service:cache')
-const { deprecate } = require('util')
-const ms = require('ms')
+const { getCacheParams } = require('./entity-cache-params')
 
-const SWR_CONTROL = 'SWR-CONTROL'
+const {
+  setEntry,
+  getEntry,
+  setSWRStaleEntry,
+  setSWRControlEntry,
+  getSWRStaleEntry,
+  getSWRControlEntry
+} = require('./redis-controller')
+
+/**
+ * Flag for redis control entry flag, when set it means the stored result should
+ * be considered stale
+ */
+const SWR_CONTROL_STALE = 'SWR-CONTROL-STALE'
 
 /**
  * @param {Function} cacheKey function to generate a cache key
@@ -16,84 +27,6 @@ function generateKey (cacheKey, ctx) {
   return cacheKey ? cacheKey(ctx) : `entity:${ctx.context.id}`
 }
 
-/**
- * @param {String} key cache key
- * @returns {String} key postfixed with "swr.stale"
- */
-function createSWRStaleKey (key) {
-  return `${key}:swr.stale`
-}
-
-/**
- * @param {String} key cache key
- * @returns {String} key postfixed with "swr.control"
- */
-function createSWRControlKey (key) {
-  return `${key}:swr.control`
-}
-
-/**
- * @param {Service} service Service instance
- * @param {String} key entry key
- * @returns {Promise<Object|undefined>} entry value
- */
-function getEntry (service, key) {
-  return service.cache.get(key)
-}
-
-/**
- * @param {Service} service Service instance
- * @param {String} key entry key
- * @returns {Promise<Object|undefined>} entry value
- */
-function getSWRStaleEntry (service, key) {
-  return service.cache.get(createSWRStaleKey(key))
-}
-
-/**
- * @param {Service} service Service instance
- * @param {String} key entry key
- * @returns {Promise<Object|undefined>} entry value
- */
-function getSWRControlEntry (service, key) {
-  return getEntry(service, createSWRControlKey(key)).then(
-    result => result === SWR_CONTROL
-  )
-}
-
-/**
- * @param {Service} service Service instance
- * @param {String} key entry key
- * @param {Object} value entry value
- * @param {String} ttl time to live value supported by https://github.com/zeit/ms
- * @returns {Promise}
- */
-function setEntry (service, key, value, ttl) {
-  return service.cache.set(key, value, ttl)
-}
-
-/**
- * When stale is provided the value is calculated as ttl + stale
- * @param {Service} service Service instance
- * @param {String} key entry key
- * @param {Object} value entry value
- * @param {Number|String} ttl time to live value supported by https://github.com/zeit/ms
- * @returns {Promise}
- */
-function setSWRStaleEntry (service, key, value, ttl) {
-  return setEntry(service, createSWRStaleKey(key), value, ttl)
-}
-
-/**
- * @param {Service} service Service instance
- * @param {String} key entry key
- * @param {String} ttl time to live value supported by https://github.com/zeit/ms
- * @returns {Promise}
- */
-function setSWRControlEntry (service, key, ttl) {
-  return setEntry(service, createSWRControlKey(key), SWR_CONTROL, ttl)
-}
-
 /**
  * @param {Service} service Service instance
  * @param {String} entryKey entry key
@@ -107,7 +40,9 @@ function setStaleWhileRevalidateEntry (service, entryKey, value, cache) {
     entryKey,
     value,
     cache.staleWhileRevalidateTtl
-  ).then(() => setSWRControlEntry(service, entryKey, cache.ttl))
+  ).then(() =>
+    setSWRControlEntry(service, entryKey, cache.ttl, SWR_CONTROL_STALE)
+  )
 }
 
 /**
@@ -169,6 +104,17 @@ function handleRevalidateError (entityId, entryKey) {
   }
 }
 
+/**
+ * @param {*} staleEntry value of entry stored in cache stores
+ * @param {RevalidationState} revalidationState cacheKey's revalidation state
+ */
+function shouldTriggerRevalidate (staleEntry, revalidationState) {
+  // we only want to revalidate if:
+  //  - stale response exists
+  //  - external control entry has expired
+  return staleEntry && typeof revalidationState === 'undefined'
+}
+
 /**
  * @param {Service} service Service instance
  * @param {String} entryKey entry key
@@ -184,12 +130,13 @@ function revalidateEntry (service, entryKey, cache, ctx) {
     entryKey
   }
 
-  // use this to bypass cache middleware
+  // this object serves as a flag is set to bypass cache middleware execution
   const revalidateContext = set(
     'locals.revalidatingCache',
     revalidatingCache,
     ctx
   )
+
   debug('Revalidating entityId: %s with cache key: %s', entityId, entryKey)
 
   return service.dataPoint
@@ -215,6 +162,10 @@ function isRevalidatingCacheKey (ctx, currentEntryKey) {
  * to resolve the entity, this thread is not meant to be chained to the main
  * Promise chain.
  *
+ * When this function returns undefined it is telling the caller it should
+ * make a standard resolution of the entity instead of using the
+ * stale-while-revalidate process.
+ *
  * @param {Service} service Service instance
  * @param {String} entryKey entry key
  * @param {Object} cache cache configuration
@@ -236,104 +187,20 @@ function resolveStaleWhileRevalidateEntry (service, entryKey, cache, ctx) {
   ]
 
   return Promise.all(tasks).then(results => {
-    const controlEntry = results[0]
+    const revalidationState = results[0]
     const staleEntry = results[1]
 
-    // we only want to revalidate if entry exprired but a stale response
-    // already exists, otherwise means its a cold start and must be resolved
-    // normally
-    if (staleEntry && controlEntry === false) {
-      // IMPORTANT: creates new thread
+    if (shouldTriggerRevalidate(staleEntry, revalidationState)) {
+      // IMPORTANT: revalidateEntry operates on a new thread
       // NOTE: using through module.exports allows us to test if it was called
       module.exports.revalidateEntry(service, entryKey, cache, ctx)
-    }
+    } // Otherwise means its a cold start and must be resolved outside
+
     // return stale entry regardless
     return staleEntry
   })
 }
 
-const looseCacheParamsDeprecationWarning = deprecate(() => {},
-'Usage of params.ttl, params.cacheKey and params.staleWhileRevalidate will be deprecated. Please configure through params.cache object instead')
-
-/**
- * Logs deprecation warning if loose cache params were used
- * @param {Object} params entity's custom params
- */
-function warnLooseParamsCacheDeprecation (params) {
-  if (params.ttl || params.cacheKey || params.staleWhileRevalidate) {
-    module.exports.looseCacheParamsDeprecationWarning()
-  }
-}
-
-/**
- * @param {String|Number} value value in string ms format, or number milliseconds
- * @returns {Number}
- */
-function parseMs (value) {
-  return typeof value === 'string' ? ms(value) : value
-}
-
-/**
- * @param {String|Number|Boolean} staleWhileRevalidate if true will calculate twice the ttl
- * @param {String|Number} ttl
- * @returns {Number} in milliseconds
- */
-function getStaleWhileRevalidateTtl (staleWhileRevalidate, ttl) {
-  return staleWhileRevalidate === true
-    ? parseMs(ttl) * 2 // default value
-    : parseMs(ttl) + parseMs(staleWhileRevalidate)
-}
-
-/**
- * @param {String|Number|Boolean}  value
- * @returns {Bolean}
- */
-function shouldUseStaleWhileRevalidate (value) {
-  return (
-    typeof value === 'string' || typeof value === 'number' || value === true
-  )
-}
-
-/**
- * @param {Object} params entity's custom params
- * @returns {Object} normalized values
- */
-function getCacheParams (params) {
-  warnLooseParamsCacheDeprecation(params)
-  const cache = defaultTo(params.cache, {})
-  const ttl = defaultTo(cache.ttl, params.ttl)
-
-  let useStaleWhileRevalidate
-  let staleWhileRevalidateTtl
-
-  // we only want to calculate below values if ttl is set
-  if (typeof ttl !== 'undefined') {
-    const staleWhileRevalidate = defaultTo(
-      cache.staleWhileRevalidate,
-      params.staleWhileRevalidate
-    )
-
-    useStaleWhileRevalidate = shouldUseStaleWhileRevalidate(
-      staleWhileRevalidate
-    )
-
-    // only calculate stale's ttl if we need to
-    if (useStaleWhileRevalidate) {
-      staleWhileRevalidateTtl = getStaleWhileRevalidateTtl(
-        staleWhileRevalidate,
-        ttl
-      )
-    }
-  }
-
-  return {
-    ttl,
-    cacheKey: defaultTo(cache.cacheKey, params.cacheKey),
-    useStaleWhileRevalidate,
-    staleWhileRevalidateTtl
-  }
-}
-
 /**
  * @param {Service} service Service instance
  * @param {DataPoint.Accumulator} ctx DataPoint Accumulator object
@@ -407,23 +274,10 @@ function after (service, ctx, next) {
 
 module.exports = {
   generateKey,
-  createSWRStaleKey,
-  createSWRControlKey,
-  getEntry,
-  getSWRControlEntry,
-  setEntry,
-  setSWRStaleEntry,
-  setSWRControlEntry,
   isRevalidatingCacheKey,
   resolveStaleWhileRevalidateEntry,
   revalidateEntry,
   setStaleWhileRevalidateEntry,
-  looseCacheParamsDeprecationWarning,
-  warnLooseParamsCacheDeprecation,
-  parseMs,
-  getStaleWhileRevalidateTtl,
-  shouldUseStaleWhileRevalidate,
-  getCacheParams,
   before,
   after
 }
diff --git a/packages/data-point-service/lib/entity-cache-params.js b/packages/data-point-service/lib/entity-cache-params.js
new file mode 100644
index 0000000..7a572a5
--- /dev/null
+++ b/packages/data-point-service/lib/entity-cache-params.js
@@ -0,0 +1,94 @@
+const defaultTo = require('lodash/defaultTo')
+const { deprecate } = require('util')
+const ms = require('ms')
+
+const looseCacheParamsDeprecationWarning = deprecate(() => {},
+'Usage of params.ttl, params.cacheKey and params.staleWhileRevalidate will be deprecated. Please configure through params.cache object instead')
+
+/**
+ * Logs deprecation warning if loose cache params were used
+ * @param {Object} params entity's custom params
+ */
+function warnLooseParamsCacheDeprecation (params) {
+  if (params.ttl || params.cacheKey || params.staleWhileRevalidate) {
+    module.exports.looseCacheParamsDeprecationWarning()
+  }
+}
+
+/**
+ * @param {String|Number} value value in string ms format, or number milliseconds
+ * @returns {Number}
+ */
+function parseMs (value) {
+  return typeof value === 'string' ? ms(value) : value
+}
+
+/**
+ * @param {String|Number|Boolean} staleWhileRevalidate if true will calculate twice the ttl
+ * @param {String|Number} ttl
+ * @returns {Number} in milliseconds
+ */
+function getStaleWhileRevalidateTtl (staleWhileRevalidate, ttl) {
+  return staleWhileRevalidate === true
+    ? parseMs(ttl) * 2 // default value
+    : parseMs(ttl) + parseMs(staleWhileRevalidate)
+}
+
+/**
+ * @param {String|Number|Boolean}  value
+ * @returns {Bolean}
+ */
+function shouldUseStaleWhileRevalidate (value) {
+  return (
+    typeof value === 'string' || typeof value === 'number' || value === true
+  )
+}
+
+/**
+ * @param {Object} params entity's custom params
+ * @returns {Object} normalized values
+ */
+function getCacheParams (params) {
+  warnLooseParamsCacheDeprecation(params)
+  const cache = defaultTo(params.cache, {})
+  const ttl = defaultTo(cache.ttl, params.ttl)
+
+  let useStaleWhileRevalidate
+  let staleWhileRevalidateTtl
+
+  // we only want to calculate below values if ttl is set
+  if (typeof ttl !== 'undefined') {
+    const staleWhileRevalidate = defaultTo(
+      cache.staleWhileRevalidate,
+      params.staleWhileRevalidate
+    )
+
+    useStaleWhileRevalidate = shouldUseStaleWhileRevalidate(
+      staleWhileRevalidate
+    )
+
+    // only calculate stale's ttl if we need to
+    if (useStaleWhileRevalidate) {
+      staleWhileRevalidateTtl = getStaleWhileRevalidateTtl(
+        staleWhileRevalidate,
+        ttl
+      )
+    }
+  }
+
+  return {
+    ttl,
+    cacheKey: defaultTo(cache.cacheKey, params.cacheKey),
+    useStaleWhileRevalidate,
+    staleWhileRevalidateTtl
+  }
+}
+
+module.exports = {
+  looseCacheParamsDeprecationWarning,
+  warnLooseParamsCacheDeprecation,
+  parseMs,
+  getStaleWhileRevalidateTtl,
+  shouldUseStaleWhileRevalidate,
+  getCacheParams
+}
diff --git a/packages/data-point-service/lib/redis-controller.js b/packages/data-point-service/lib/redis-controller.js
new file mode 100644
index 0000000..9d79992
--- /dev/null
+++ b/packages/data-point-service/lib/redis-controller.js
@@ -0,0 +1,86 @@
+/**
+ * @param {String} key cache key
+ * @returns {String} key postfixed with "swr.stale"
+ */
+function createSWRStaleKey (key) {
+  return `${key}:swr.stale`
+}
+
+/**
+ * @param {String} key cache key
+ * @returns {String} key postfixed with "swr.control"
+ */
+function createSWRControlKey (key) {
+  return `${key}:swr.control`
+}
+
+/**
+ * @param {Service} service Service instance
+ * @param {String} key entry key
+ * @returns {Promise<Object|undefined>} entry value
+ */
+function getEntry (service, key) {
+  return service.cache.get(key)
+}
+
+/**
+ * @param {Service} service Service instance
+ * @param {String} key entry key
+ * @returns {Promise<Object|undefined>} entry value
+ */
+function getSWRStaleEntry (service, key) {
+  return service.cache.get(createSWRStaleKey(key))
+}
+
+/**
+ * @param {Service} service Service instance
+ * @param {String} key entry key
+ * @returns {Promise<Object|undefined>} entry value
+ */
+function getSWRControlEntry (service, key) {
+  return getEntry(service, createSWRControlKey(key))
+}
+
+/**
+ * @param {Service} service Service instance
+ * @param {String} key entry key
+ * @param {Object} value entry value
+ * @param {String} ttl time to live value supported by https://github.com/zeit/ms
+ * @returns {Promise}
+ */
+function setEntry (service, key, value, ttl) {
+  return service.cache.set(key, value, ttl)
+}
+
+/**
+ * When stale is provided the value is calculated as ttl + stale
+ * @param {Service} service Service instance
+ * @param {String} key entry key
+ * @param {Object} value entry value
+ * @param {Number|String} ttl time to live value supported by https://github.com/zeit/ms
+ * @returns {Promise}
+ */
+function setSWRStaleEntry (service, key, value, ttl) {
+  return setEntry(service, createSWRStaleKey(key), value, ttl)
+}
+
+/**
+ * @param {Service} service Service instance
+ * @param {String} key entry key
+ * @param {String} ttl time to live value supported by https://github.com/zeit/ms
+ * @returns {Promise}
+ */
+function setSWRControlEntry (service, key, ttl, value) {
+  return setEntry(service, createSWRControlKey(key), value, ttl)
+}
+
+module.exports = {
+  createSWRControlKey,
+  createSWRStaleKey,
+  getEntry,
+  getSWRControlEntry,
+  getSWRStaleEntry,
+  setEntry,
+  setSWRControlEntry,
+  setSWRStaleEntry
+}


Test patch:
diff --git a/packages/data-point-service/lib/__snapshots__/cache-middleware.test.js.snap b/packages/data-point-service/lib/__snapshots__/cache-middleware.test.js.snap
index 24068f5..fee4bb2 100644
--- a/packages/data-point-service/lib/__snapshots__/cache-middleware.test.js.snap
+++ b/packages/data-point-service/lib/__snapshots__/cache-middleware.test.js.snap
@@ -36,7 +36,7 @@ Array [
   ],
   Array [
     "key:swr.control",
-    "SWR-CONTROL",
+    "SWR-CONTROL-STALE",
     200,
   ],
 ]
diff --git a/packages/data-point-service/lib/cache-middleware.test.js b/packages/data-point-service/lib/cache-middleware.test.js
index 4632e41..16ee7bb 100644
--- a/packages/data-point-service/lib/cache-middleware.test.js
+++ b/packages/data-point-service/lib/cache-middleware.test.js
@@ -66,115 +66,6 @@ describe('genrateKey', () => {
   })
 })
 
-describe('createSWRStaleKey', () => {
-  it('should create stale key', () => {
-    expect(CacheMiddleware.createSWRStaleKey('key')).toEqual('key:swr.stale')
-  })
-})
-
-describe('createSWRStaleKey', () => {
-  it('should create control key', () => {
-    expect(CacheMiddleware.createSWRControlKey('key')).toEqual(
-      'key:swr.control'
-    )
-  })
-})
-
-describe('getEntry', () => {
-  it('should call cache.get from service to fetch a key', () => {
-    const service = {
-      cache: {
-        get: key => Promise.resolve(`${key}:value`)
-      }
-    }
-    return CacheMiddleware.getEntry(service, 'test').then(result => {
-      expect(result).toEqual('test:value')
-    })
-  })
-})
-
-describe('getSWRControlEntry', () => {
-  it('should return true if key exists with valid wrapper ', () => {
-    const service = {
-      cache: {
-        get: jest.fn(key => Promise.resolve('SWR-CONTROL'))
-      }
-    }
-    return CacheMiddleware.getSWRControlEntry(service, 'test').then(result => {
-      expect(service.cache.get).toBeCalledWith('test:swr.control')
-      expect(result).toEqual(true)
-    })
-  })
-
-  it('should return true if key exists with valid wrapper ', () => {
-    const service = {
-      cache: {
-        get: jest.fn(key => Promise.resolve(undefined))
-      }
-    }
-    return CacheMiddleware.getSWRControlEntry(service, 'test').then(result => {
-      expect(service.cache.get).toBeCalledWith('test:swr.control')
-      expect(result).toEqual(false)
-    })
-  })
-})
-
-describe('setEntry', () => {
-  it('should pass all arguments to cache.set', () => {
-    const service = {
-      cache: {
-        set: jest.fn()
-      }
-    }
-    CacheMiddleware.setEntry(service, 'key', 'value', 200)
-    expect(service.cache.set).toBeCalledWith('key', 'value', 200)
-  })
-})
-
-describe('setSWRStaleEntry', () => {
-  it('should create a key that has no ttl', () => {
-    const service = {
-      cache: {
-        set: jest.fn()
-      }
-    }
-    CacheMiddleware.setSWRStaleEntry(service, 'key', 'value', 100)
-    expect(service.cache.set).toBeCalledWith('key:swr.stale', 'value', 100)
-  })
-})
-
-describe('setSWRControlEntry', () => {
-  it('should create a key that has no ttl', () => {
-    const service = {
-      cache: {
-        set: jest.fn()
-      }
-    }
-    CacheMiddleware.setSWRControlEntry(service, 'key', 100)
-    expect(service.cache.set).toBeCalledWith(
-      'key:swr.control',
-      'SWR-CONTROL',
-      100
-    )
-  })
-})
-
-describe('setSWRControlEntry', () => {
-  it('should create a key that has no ttl', () => {
-    const service = {
-      cache: {
-        set: jest.fn()
-      }
-    }
-    CacheMiddleware.setSWRControlEntry(service, 'key', 200)
-    expect(service.cache.set).toBeCalledWith(
-      'key:swr.control',
-      'SWR-CONTROL',
-      200
-    )
-  })
-})
-
 describe('setStaleWhileRevalidateEntry', () => {
   it('should store stale entry and SWR control entry', () => {
     const set = jest.fn(() => Promise.resolve(true))
@@ -196,7 +87,11 @@ describe('setStaleWhileRevalidateEntry', () => {
       cache
     ).then(() => {
       expect(set.mock.calls[0]).toEqual(['key:swr.stale', 'value', 400])
-      expect(set.mock.calls[1]).toEqual(['key:swr.control', 'SWR-CONTROL', 200])
+      expect(set.mock.calls[1]).toEqual([
+        'key:swr.control',
+        'SWR-CONTROL-STALE',
+        200
+      ])
     })
   })
 })
@@ -399,6 +294,7 @@ describe('resolveStaleWhileRevalidateEntry', () => {
     return result.then(value => {
       expect(spyRevalidateEntry).not.toBeCalled()
       expect(value).toEqual('STALE')
+      spyRevalidateEntry.mockRestore()
     })
   })
 
@@ -438,160 +334,6 @@ describe('resolveStaleWhileRevalidateEntry', () => {
   })
 })
 
-describe('warnLooseParamsCacheDeprecation', () => {
-  const looseCacheParamsDeprecationWarning =
-    CacheMiddleware.looseCacheParamsDeprecationWarning
-  afterAll(() => {
-    CacheMiddleware.looseCacheParamsDeprecationWarning = looseCacheParamsDeprecationWarning
-  })
-  it('should call deprecate if params.ttl is set', () => {
-    CacheMiddleware.looseCacheParamsDeprecationWarning = jest.fn()
-    expect(
-      CacheMiddleware.warnLooseParamsCacheDeprecation({
-        ttl: true
-      })
-    )
-    expect(CacheMiddleware.looseCacheParamsDeprecationWarning).toBeCalled()
-  })
-
-  it('should call deprecate if params.cacheKey is set', () => {
-    CacheMiddleware.looseCacheParamsDeprecationWarning = jest.fn()
-
-    expect(
-      CacheMiddleware.warnLooseParamsCacheDeprecation({
-        cacheKey: true
-      })
-    )
-
-    expect(CacheMiddleware.looseCacheParamsDeprecationWarning).toBeCalled()
-  })
-
-  it('should call deprecate if params.staleWhileRevalidate is set', () => {
-    CacheMiddleware.looseCacheParamsDeprecationWarning = jest.fn()
-
-    expect(
-      CacheMiddleware.warnLooseParamsCacheDeprecation({
-        staleWhileRevalidate: true
-      })
-    )
-
-    expect(CacheMiddleware.looseCacheParamsDeprecationWarning).toBeCalled()
-  })
-})
-
-describe('parseMs', () => {
-  it('should return number if ms string is provided', () => {
-    expect(CacheMiddleware.parseMs('1s')).toEqual(1000)
-  })
-  it('should return number if number is provided', () => {
-    expect(CacheMiddleware.parseMs(1000)).toEqual(1000)
-  })
-})
-
-describe('getStaleWhileRevalidateTtl', () => {
-  it('should return double the value of ttl if staleWhileRevalidate is true', () => {
-    expect(CacheMiddleware.getStaleWhileRevalidateTtl(true, 1000)).toEqual(2000)
-  })
-  it('should return addition of ttl and staleWhileRevalidate if staleWhileRevalidate different to true (string or number)', () => {
-    expect(CacheMiddleware.getStaleWhileRevalidateTtl(500, 1000)).toEqual(1500)
-    expect(CacheMiddleware.getStaleWhileRevalidateTtl('5s', 1000)).toEqual(6000)
-  })
-})
-
-describe('shouldUseStaleWhileRevalidate', () => {
-  it('should be true for "true", number and string', () => {
-    expect(CacheMiddleware.shouldUseStaleWhileRevalidate(true)).toEqual(true)
-    expect(CacheMiddleware.shouldUseStaleWhileRevalidate(200)).toEqual(true)
-    expect(CacheMiddleware.shouldUseStaleWhileRevalidate('20m')).toEqual(true)
-  })
-  it('should be false for "false" and undefined', () => {
-    expect(CacheMiddleware.shouldUseStaleWhileRevalidate(false)).toEqual(false)
-    expect(CacheMiddleware.shouldUseStaleWhileRevalidate(undefined)).toEqual(
-      false
-    )
-    expect(CacheMiddleware.shouldUseStaleWhileRevalidate(null)).toEqual(false)
-  })
-})
-
-describe('getCacheParams', () => {
-  it('should have backwards compatability with params loose properties', () => {
-    const params = {
-      ttl: '20s',
-      cacheKey: () => true,
-      staleWhileRevalidate: '10s'
-    }
-    const cache = CacheMiddleware.getCacheParams(params)
-    expect(cache).toEqual({
-      ttl: params.ttl,
-      cacheKey: params.cacheKey,
-      useStaleWhileRevalidate: true,
-      staleWhileRevalidateTtl: 30000
-    })
-  })
-  it('should get values from params.cache property', () => {
-    const params = {
-      cache: {
-        ttl: '20s',
-        cacheKey: () => true,
-        staleWhileRevalidate: '10s'
-      }
-    }
-    const cache = CacheMiddleware.getCacheParams(params)
-    expect(cache).toEqual({
-      ttl: params.cache.ttl,
-      cacheKey: params.cache.cacheKey,
-      useStaleWhileRevalidate: true,
-      staleWhileRevalidateTtl: 30000
-    })
-  })
-  it('should have params.cache priority over loose param cache settings', () => {
-    const params = {
-      ttl: '10s',
-      cacheKey: () => true,
-      staleWhileRevalidate: '20ms',
-      cache: {
-        ttl: '20s',
-        cacheKey: () => true
-      }
-    }
-    const cache = CacheMiddleware.getCacheParams(params)
-    expect(cache).toEqual({
-      ttl: params.cache.ttl,
-      cacheKey: params.cache.cacheKey,
-      // this key will use the loose value since its not being set through
-      // params.cache
-      useStaleWhileRevalidate: true,
-      staleWhileRevalidateTtl: 20020
-    })
-  })
-  it('should not calculate stale values if ttl is not set', () => {
-    const params = {
-      cache: {}
-    }
-    const cache = CacheMiddleware.getCacheParams(params)
-    expect(cache).toEqual({
-      ttl: undefined,
-      cacheKey: undefined,
-      useStaleWhileRevalidate: undefined,
-      staleWhileRevalidateTtl: undefined
-    })
-  })
-  it('should not calculate stale ttl if staleWhileRevalidate is not set', () => {
-    const params = {
-      cache: {
-        ttl: '20s'
-      }
-    }
-    const cache = CacheMiddleware.getCacheParams(params)
-    expect(cache).toEqual({
-      ttl: '20s',
-      cacheKey: undefined,
-      useStaleWhileRevalidate: false,
-      staleWhileRevalidateTtl: undefined
-    })
-  })
-})
-
 describe('before', () => {
   it('should call next if no  ttl', () => {
     const mocks = createMocks()
diff --git a/packages/data-point-service/lib/entity-cache-params.test.js b/packages/data-point-service/lib/entity-cache-params.test.js
new file mode 100644
index 0000000..5c6dfaa
--- /dev/null
+++ b/packages/data-point-service/lib/entity-cache-params.test.js
@@ -0,0 +1,165 @@
+/* eslint-env jest */
+
+const EntityCacheParams = require('./entity-cache-params')
+
+describe('warnLooseParamsCacheDeprecation', () => {
+  const looseCacheParamsDeprecationWarning =
+    EntityCacheParams.looseCacheParamsDeprecationWarning
+  afterAll(() => {
+    EntityCacheParams.looseCacheParamsDeprecationWarning = looseCacheParamsDeprecationWarning
+  })
+  it('should call deprecate if params.ttl is set', () => {
+    EntityCacheParams.looseCacheParamsDeprecationWarning = jest.fn()
+    expect(
+      EntityCacheParams.warnLooseParamsCacheDeprecation({
+        ttl: true
+      })
+    )
+    expect(EntityCacheParams.looseCacheParamsDeprecationWarning).toBeCalled()
+  })
+
+  it('should call deprecate if params.cacheKey is set', () => {
+    EntityCacheParams.looseCacheParamsDeprecationWarning = jest.fn()
+
+    expect(
+      EntityCacheParams.warnLooseParamsCacheDeprecation({
+        cacheKey: true
+      })
+    )
+
+    expect(EntityCacheParams.looseCacheParamsDeprecationWarning).toBeCalled()
+  })
+
+  it('should call deprecate if params.staleWhileRevalidate is set', () => {
+    EntityCacheParams.looseCacheParamsDeprecationWarning = jest.fn()
+
+    expect(
+      EntityCacheParams.warnLooseParamsCacheDeprecation({
+        staleWhileRevalidate: true
+      })
+    )
+
+    expect(EntityCacheParams.looseCacheParamsDeprecationWarning).toBeCalled()
+  })
+})
+
+describe('parseMs', () => {
+  it('should return number if ms string is provided', () => {
+    expect(EntityCacheParams.parseMs('1s')).toEqual(1000)
+  })
+  it('should return number if number is provided', () => {
+    expect(EntityCacheParams.parseMs(1000)).toEqual(1000)
+  })
+})
+
+describe('getStaleWhileRevalidateTtl', () => {
+  it('should return double the value of ttl if staleWhileRevalidate is true', () => {
+    expect(EntityCacheParams.getStaleWhileRevalidateTtl(true, 1000)).toEqual(
+      2000
+    )
+  })
+  it('should return addition of ttl and staleWhileRevalidate if staleWhileRevalidate different to true (string or number)', () => {
+    expect(EntityCacheParams.getStaleWhileRevalidateTtl(500, 1000)).toEqual(
+      1500
+    )
+    expect(EntityCacheParams.getStaleWhileRevalidateTtl('5s', 1000)).toEqual(
+      6000
+    )
+  })
+})
+
+describe('shouldUseStaleWhileRevalidate', () => {
+  it('should be true for "true", number and string', () => {
+    expect(EntityCacheParams.shouldUseStaleWhileRevalidate(true)).toEqual(true)
+    expect(EntityCacheParams.shouldUseStaleWhileRevalidate(200)).toEqual(true)
+    expect(EntityCacheParams.shouldUseStaleWhileRevalidate('20m')).toEqual(true)
+  })
+  it('should be false for "false" and undefined', () => {
+    expect(EntityCacheParams.shouldUseStaleWhileRevalidate(false)).toEqual(
+      false
+    )
+    expect(EntityCacheParams.shouldUseStaleWhileRevalidate(undefined)).toEqual(
+      false
+    )
+    expect(EntityCacheParams.shouldUseStaleWhileRevalidate(null)).toEqual(false)
+  })
+})
+
+describe('getCacheParams', () => {
+  it('should have backwards compatability with params loose properties', () => {
+    const params = {
+      ttl: '20s',
+      cacheKey: () => true,
+      staleWhileRevalidate: '10s'
+    }
+    const cache = EntityCacheParams.getCacheParams(params)
+    expect(cache).toEqual({
+      ttl: params.ttl,
+      cacheKey: params.cacheKey,
+      useStaleWhileRevalidate: true,
+      staleWhileRevalidateTtl: 30000
+    })
+  })
+  it('should get values from params.cache property', () => {
+    const params = {
+      cache: {
+        ttl: '20s',
+        cacheKey: () => true,
+        staleWhileRevalidate: '10s'
+      }
+    }
+    const cache = EntityCacheParams.getCacheParams(params)
+    expect(cache).toEqual({
+      ttl: params.cache.ttl,
+      cacheKey: params.cache.cacheKey,
+      useStaleWhileRevalidate: true,
+      staleWhileRevalidateTtl: 30000
+    })
+  })
+  it('should have params.cache priority over loose param cache settings', () => {
+    const params = {
+      ttl: '10s',
+      cacheKey: () => true,
+      staleWhileRevalidate: '20ms',
+      cache: {
+        ttl: '20s',
+        cacheKey: () => true
+      }
+    }
+    const cache = EntityCacheParams.getCacheParams(params)
+    expect(cache).toEqual({
+      ttl: params.cache.ttl,
+      cacheKey: params.cache.cacheKey,
+      // this key will use the loose value since its not being set through
+      // params.cache
+      useStaleWhileRevalidate: true,
+      staleWhileRevalidateTtl: 20020
+    })
+  })
+  it('should not calculate stale values if ttl is not set', () => {
+    const params = {
+      cache: {}
+    }
+    const cache = EntityCacheParams.getCacheParams(params)
+    expect(cache).toEqual({
+      ttl: undefined,
+      cacheKey: undefined,
+      useStaleWhileRevalidate: undefined,
+      staleWhileRevalidateTtl: undefined
+    })
+  })
+  it('should not calculate stale ttl if staleWhileRevalidate is not set', () => {
+    const params = {
+      cache: {
+        ttl: '20s'
+      }
+    }
+    const cache = EntityCacheParams.getCacheParams(params)
+    expect(cache).toEqual({
+      ttl: '20s',
+      cacheKey: undefined,
+      useStaleWhileRevalidate: false,
+      staleWhileRevalidateTtl: undefined
+    })
+  })
+})
diff --git a/packages/data-point-service/lib/redis-controller.test.js b/packages/data-point-service/lib/redis-controller.test.js
new file mode 100644
index 0000000..85663df
--- /dev/null
+++ b/packages/data-point-service/lib/redis-controller.test.js
@@ -0,0 +1,110 @@
+/* eslint-env jest */
+
+const RedisController = require('./redis-controller')
+
+describe('createSWRStaleKey', () => {
+  it('should create stale key', () => {
+    expect(RedisController.createSWRStaleKey('key')).toEqual('key:swr.stale')
+  })
+})
+
+describe('createSWRStaleKey', () => {
+  it('should create control key', () => {
+    expect(RedisController.createSWRControlKey('key')).toEqual(
+      'key:swr.control'
+    )
+  })
+})
+
+describe('getEntry', () => {
+  it('should call cache.get from service to fetch a key', () => {
+    const service = {
+      cache: {
+        get: key => Promise.resolve(`${key}:value`)
+      }
+    }
+    return RedisController.getEntry(service, 'test').then(result => {
+      expect(result).toEqual('test:value')
+    })
+  })
+})
+
+describe('getSWRControlEntry', () => {
+  it('should return true if key exists with valid wrapper ', () => {
+    const service = {
+      cache: {
+        get: jest.fn(key => Promise.resolve('SWR-CONTROL'))
+      }
+    }
+    return RedisController.getSWRControlEntry(service, 'test').then(result => {
+      expect(service.cache.get).toBeCalledWith('test:swr.control')
+      expect(result).toEqual('SWR-CONTROL')
+    })
+  })
+
+  it('should return true if key exists with valid wrapper ', () => {
+    const service = {
+      cache: {
+        get: jest.fn(key => Promise.resolve(undefined))
+      }
+    }
+    return RedisController.getSWRControlEntry(service, 'test').then(result => {
+      expect(service.cache.get).toBeCalledWith('test:swr.control')
+      expect(result).toEqual(undefined)
+    })
+  })
+})
+
+describe('setEntry', () => {
+  it('should pass all arguments to cache.set', () => {
+    const service = {
+      cache: {
+        set: jest.fn()
+      }
+    }
+    RedisController.setEntry(service, 'key', 'value', 200)
+    expect(service.cache.set).toBeCalledWith('key', 'value', 200)
+  })
+})
+
+describe('setSWRStaleEntry', () => {
+  it('should create a key that has no ttl', () => {
+    const service = {
+      cache: {
+        set: jest.fn()
+      }
+    }
+    RedisController.setSWRStaleEntry(service, 'key', 'value', 100)
+    expect(service.cache.set).toBeCalledWith('key:swr.stale', 'value', 100)
+  })
+})
+
+describe('setSWRControlEntry', () => {
+  it('should create a key that has no ttl', () => {
+    const service = {
+      cache: {
+        set: jest.fn()
+      }
+    }
+    RedisController.setSWRControlEntry(service, 'key')
+    expect(service.cache.set).toBeCalledWith(
+      'key:swr.control',
+      undefined,
+      undefined
+    )
+  })
+
+  it('should create a key that with ttl and value', () => {
+    const service = {
+      cache: {
+        set: jest.fn()
+      }
+    }
+    RedisController.setSWRControlEntry(service, 'key', 200, 'SWR-CONTROL')
+    expect(service.cache.set).toBeCalledWith(
+      'key:swr.control',
+      'SWR-CONTROL',
+      200
+    )
+  })
+})