Repository: ViacomInc/data-point
Pull request #280 (2018-05-06 17:44:25)

Fix stale while revalidate stale ttl logic
<!--
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**:
#275

<!-- How were these changes implemented? -->
**How**:

Attempting to correctly implement the RFC https://tools.ietf.org/html/rfc5861#section-3

Enables staleWhileRevalidate to be `string` (ms) or `number`, it is now calculated as an addition to the given **ttl**.

to give backward compatibility with `boolean`, when set as `true` it will resolve to a **ttl** of twice as the given `cache.ttl`

<!-- 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/280?src=pr&el=h1) Report
> Merging [#280](https://codecov.io/gh/ViacomInc/data-point/pull/280?src=pr&el=desc) into [master](https://codecov.io/gh/ViacomInc/data-point/commit/21429f308b4c415b2f32ffb20410614c5b9a5236?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/280/graphs/tree.svg?src=pr&token=LYxItnrx4i&height=150&width=650)](https://codecov.io/gh/ViacomInc/data-point/pull/280?src=pr&el=tree)

```diff
@@          Coverage Diff          @@
##           master   #280   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files         128    128           
  Lines        1764   1782   +18     
  Branches      157    162    +5     
=====================================
+ Hits         1764   1782   +18
```


| [Impacted Files](https://codecov.io/gh/ViacomInc/data-point/pull/280?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ackages/data-point-service/lib/cache-middleware.js](https://codecov.io/gh/ViacomInc/data-point/pull/280/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/280?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/280?src=pr&el=footer). Last update [21429f3...dc94786](https://codecov.io/gh/ViacomInc/data-point/pull/280?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/packages/data-point-service/README.md b/packages/data-point-service/README.md
index 3bce671..9a4f5ba 100644
--- a/packages/data-point-service/README.md
+++ b/packages/data-point-service/README.md
@@ -97,8 +97,8 @@ To configure an entity's cache settings you must set cache configuration through
 '<type>:<entity-name>': {
   params: {
     cache: {
-      ttl: String,
-      staleWhileRevalidate: Boolean
+      ttl: String|Number,
+      staleWhileRevalidate: String|Number
     }
   }
 }
@@ -106,6 +106,10 @@ To configure an entity's cache settings you must set cache configuration through
 
 ### cache.ttl
 
+```js
+ttl: String|Number
+```
+
 Use `cache.ttl` to set an entity's cache entry **Time To Live** value. This value is expected to be written as a string following the format supported by [ms](https://www.npmjs.com/package/ms).
 
 **Example:**
@@ -130,7 +134,11 @@ DataPointService.create({
 
 ### cache.staleWhileRevalidate
 
-Use `cache.staleWhileRevalidate` flag in conjunction with a valid `cache.ttl` to use the Stale While Revalidate caching pattern. When this flag is set to `true` your entity will be resolved to a stale value until the ttl expires, revalidation happens in the background and is triggered only upon the key being requested and its ttl being expired.
+`staleWhileRevalidate = delta-seconds`
+
+`staleWhileRevalidate` value is expected to be written as a string following the format supported by [ms](https://www.npmjs.com/package/ms). Alternately it may also be set to `true`, which tells the entity to use double the time of the value of its `ttl`.
+
+Use `cache.staleWhileRevalidate` in conjunction with a valid `cache.ttl` to use the Stale While Revalidate cache pattern. When present, caches MAY serve the response in which it appears after it becomes stale, up to the indicated `ttl`. The cache SHOULD attempt to revalidate it asynchronously while still serving stale responses. If delta-seconds passes without the cached entity being revalidated, it SHOULD NOT continue to be served stale.
 
 **Example:**
 
diff --git a/packages/data-point-service/examples/express-cache-stale-while-revalidate.js b/packages/data-point-service/examples/express-cache-stale-while-revalidate.js
index d92f830..7b0720f 100644
--- a/packages/data-point-service/examples/express-cache-stale-while-revalidate.js
+++ b/packages/data-point-service/examples/express-cache-stale-while-revalidate.js
@@ -45,8 +45,8 @@ function createService () {
         },
         params: {
           cache: {
-            ttl: '10s',
-            staleWhileRevalidate: true
+            ttl: '1m',
+            staleWhileRevalidate: '3m'
           }
         }
       }
diff --git a/packages/data-point-service/lib/cache-middleware.js b/packages/data-point-service/lib/cache-middleware.js
index 4f3f61f..557b056 100644
--- a/packages/data-point-service/lib/cache-middleware.js
+++ b/packages/data-point-service/lib/cache-middleware.js
@@ -3,6 +3,7 @@ const defaultTo = require('lodash/defaultTo')
 const set = require('lodash/fp/set')
 const logger = require('./logger')
 const { deprecate } = require('util')
+const ms = require('ms')
 
 /**
  * @param {Function} cacheKey function to generate a cache key
@@ -70,14 +71,15 @@ function setEntry (service, 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 {String} ttl time to live value supported by https://github.com/zeit/ms
+ * @param {Number|String} ttl time to live value supported by https://github.com/zeit/ms
  * @returns {Promise}
  */
 function setSWIStaleEntry (service, key, value, ttl) {
-  return setEntry(service, createSWIStaleKey(key), value, 0)
+  return setEntry(service, createSWIStaleKey(key), value, ttl)
 }
 
 /**
@@ -94,15 +96,16 @@ function setSWIControlEntry (service, key, ttl) {
  * @param {Service} service Service instance
  * @param {String} entryKey entry key
  * @param {Object} value entry value
- * @param {String} ttl time to live value supported by https://github.com/zeit/ms
+ * @param {Object} cache cache configuration
  * @returns {Promise}
  */
-function setStaleWhileRevalidateEntry (service, entryKey, value, ttl) {
-  // NOTE: stale entries have NO ttl, rethink if this is the right approach or
-  // if stale should just mean a much longer TTL?
-  return setSWIStaleEntry(service, entryKey, value).then(() =>
-    setSWIControlEntry(service, entryKey, ttl)
-  )
+function setStaleWhileRevalidateEntry (service, entryKey, value, cache) {
+  return setSWIStaleEntry(
+    service,
+    entryKey,
+    value,
+    cache.staleWhileRevalidateTtl
+  ).then(() => setSWIControlEntry(service, entryKey, cache.ttl))
 }
 
 /**
@@ -124,14 +127,20 @@ function revalidateSuccess (entityId, entryKey) {
 /**
  * @param {Service} service Service instance
  * @param {String} entryKey entity cache key
+ * @param {Object} cache cache configuration
  * @returns {Function}
  */
-function updateSWIEntry (service, entryKey, ttl) {
+function updateSWIEntry (service, entryKey, cache) {
   /**
    * @param {Accumulator} acc
    */
   return acc => {
-    return setStaleWhileRevalidateEntry(service, entryKey, acc.value, ttl)
+    return module.exports.setStaleWhileRevalidateEntry(
+      service,
+      entryKey,
+      acc.value,
+      cache
+    )
   }
 }
 
@@ -161,11 +170,11 @@ function handleRevalidateError (entityId, entryKey) {
 /**
  * @param {Service} service Service instance
  * @param {String} entryKey entry key
- * @param {String} ttl time to live value supported by https://github.com/zeit/ms
+ * @param {Object} cache cache configuration
  * @param {DataPoint.Accumulator} ctx DataPoint Accumulator object
  * @returns {Promise}
  */
-function revalidateEntry (service, entryKey, ttl, ctx) {
+function revalidateEntry (service, entryKey, cache, ctx) {
   const entityId = ctx.context.id
 
   const revalidatingCache = {
@@ -188,7 +197,7 @@ function revalidateEntry (service, entryKey, ttl, ctx) {
 
   return service.dataPoint
     .resolveFromAccumulator(entityId, revalidateContext)
-    .then(updateSWIEntry(service, entryKey, ttl))
+    .then(updateSWIEntry(service, entryKey, cache))
     .then(revalidateSuccess(entityId, entryKey))
     .catch(handleRevalidateError(entityId, entryKey))
 }
@@ -200,11 +209,11 @@ function revalidateEntry (service, entryKey, ttl, ctx) {
  *
  * @param {Service} service Service instance
  * @param {String} entryKey entry key
- * @param {String} ttl time to live value supported by https://github.com/zeit/ms
+ * @param {Object} cache cache configuration
  * @param {DataPoint.Accumulator} ctx DataPoint Accumulator object
  * @returns {Promise<Object|undefined>} cached stale value
  */
-function resolveStaleWhileRevalidateEntry (service, entryKey, ttl, ctx) {
+function resolveStaleWhileRevalidateEntry (service, entryKey, cache, ctx) {
   const revalidatingCache = ctx.locals.revalidatingCache
 
   if (revalidatingCache) {
@@ -227,7 +236,7 @@ function resolveStaleWhileRevalidateEntry (service, entryKey, ttl, ctx) {
     if (staleEntry && controlEntry === false) {
       // IMPORTANT: creates new thread
       // NOTE: using through module.exports allows us to test if it was called
-      module.exports.revalidateEntry(service, entryKey, ttl, ctx)
+      module.exports.revalidateEntry(service, entryKey, cache, ctx)
     }
     // return stale entry regardless
     return staleEntry
@@ -247,6 +256,35 @@ function warnLooseParamsCacheDeprecation (params) {
   }
 }
 
+/**
+ * @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
@@ -254,13 +292,36 @@ function warnLooseParamsCacheDeprecation (params) {
 function getCacheParams (params) {
   warnLooseParamsCacheDeprecation(params)
   const cache = defaultTo(params.cache, {})
-  return {
-    ttl: defaultTo(cache.ttl, params.ttl),
-    cacheKey: defaultTo(cache.cacheKey, params.cacheKey),
-    staleWhileRevalidate: defaultTo(
+  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
   }
 }
 
@@ -278,13 +339,13 @@ function before (service, ctx, next) {
 
   const entryKey = generateKey(cache.cacheKey, ctx)
 
-  Promise.resolve(cache.staleWhileRevalidate)
-    .then(isStaleWhileRevalidate => {
-      return isStaleWhileRevalidate
+  Promise.resolve(cache.useStaleWhileRevalidate)
+    .then(useStaleWhileRevalidate => {
+      return useStaleWhileRevalidate
         ? module.exports.resolveStaleWhileRevalidateEntry(
             service,
             entryKey,
-            cache.ttl,
+            cache,
             ctx
           )
         : getEntry(service, entryKey)
@@ -317,7 +378,7 @@ function after (service, ctx, next) {
   // from here below ttl is assumed to be true
   const entryKey = generateKey(cache.cacheKey, ctx)
 
-  if (cache.staleWhileRevalidate) {
+  if (cache.useStaleWhileRevalidate) {
     // if its at the process of revalidating, then lets skip any further calls
     if (ctx.locals.revalidatingCache) {
       return next()
@@ -325,7 +386,7 @@ function after (service, ctx, next) {
 
     // adds (or updates) the stale cache entry with the latest value
     return module.exports
-      .setStaleWhileRevalidateEntry(service, entryKey, ctx.value, cache.ttl)
+      .setStaleWhileRevalidateEntry(service, entryKey, ctx.value, cache)
       .then(() => {
         next()
       })
@@ -349,6 +410,9 @@ module.exports = {
   setStaleWhileRevalidateEntry,
   looseCacheParamsDeprecationWarning,
   warnLooseParamsCacheDeprecation,
+  parseMs,
+  getStaleWhileRevalidateTtl,
+  shouldUseStaleWhileRevalidate,
   getCacheParams,
   before,
   after
diff --git a/packages/data-point-service/package.json b/packages/data-point-service/package.json
index 10d21e7..b488a4b 100644
--- a/packages/data-point-service/package.json
+++ b/packages/data-point-service/package.json
@@ -18,7 +18,8 @@
     "bluebird": "3.5.1",
     "data-point-cache": "^3.2.0",
     "lodash": "4.x",
-    "winston": "2.x"
+    "winston": "2.x",
+    "ms": "2.x"
   },
   "peerDependencies": {
     "data-point": "^3.2.0"


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 9200ffe..2c171da 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
@@ -32,7 +32,7 @@ Array [
   Array [
     "SWI-STALE:key",
     undefined,
-    0,
+    400,
   ],
   Array [
     "SWI-CONTROL:key",
diff --git a/packages/data-point-service/lib/cache-middleware.test.js b/packages/data-point-service/lib/cache-middleware.test.js
index 816fb39..073928a 100644
--- a/packages/data-point-service/lib/cache-middleware.test.js
+++ b/packages/data-point-service/lib/cache-middleware.test.js
@@ -2,6 +2,7 @@
 
 const winston = require('winston')
 const util = require('util')
+const ms = require('ms')
 util.deprecate = jest.fn(fn => fn)
 
 const CacheMiddleware = require('./cache-middleware')
@@ -133,8 +134,8 @@ describe('setSWIStaleEntry', () => {
         set: jest.fn()
       }
     }
-    CacheMiddleware.setSWIStaleEntry(service, 'key', 'value')
-    expect(service.cache.set).toBeCalledWith('SWI-STALE:key', 'value', 0)
+    CacheMiddleware.setSWIStaleEntry(service, 'key', 'value', 100)
+    expect(service.cache.set).toBeCalledWith('SWI-STALE:key', 'value', 100)
   })
 })
 
@@ -178,13 +179,19 @@ describe('setStaleWhileRevalidateEntry', () => {
         set
       }
     }
+
+    const cache = {
+      ttl: 200,
+      staleWhileRevalidateTtl: 400
+    }
+
     return CacheMiddleware.setStaleWhileRevalidateEntry(
       service,
       'key',
       'value',
-      200
+      cache
     ).then(() => {
-      expect(set.mock.calls[0]).toEqual(['SWI-STALE:key', 'value', 0])
+      expect(set.mock.calls[0]).toEqual(['SWI-STALE:key', 'value', 400])
       expect(set.mock.calls[1]).toEqual(['SWI-CONTROL:key', 'SWI-CONTROL', 200])
     })
   })
@@ -193,6 +200,10 @@ describe('setStaleWhileRevalidateEntry', () => {
 describe('revalidateEntry', () => {
   let logDebug
   let logError
+  const cache = {
+    ttl: 200,
+    staleWhileRevalidateTtl: 400
+  }
 
   beforeEach(() => {
     const logger = winston.loggers.get('data-point-service')
@@ -211,10 +222,11 @@ describe('revalidateEntry', () => {
     const mocks = createMocks()
     const logger = winston.loggers.get('data-point-service')
     logger.debug = jest.fn()
+
     return CacheMiddleware.revalidateEntry(
       mocks.service,
       'key',
-      200,
+      cache,
       mocks.ctx
     ).then(() => {
       expect(logger.debug.mock.calls).toMatchSnapshot()
@@ -227,10 +239,11 @@ describe('revalidateEntry', () => {
     logger.error = jest.fn()
     mocks.service.dataPoint.resolveFromAccumulator = () =>
       Promise.reject(new Error('TestError'))
+
     return CacheMiddleware.revalidateEntry(
       mocks.service,
       'key',
-      200,
+      cache,
       mocks.ctx
     ).then(() => {
       expect(logger.error.mock.calls).toMatchSnapshot()
@@ -242,7 +255,7 @@ describe('revalidateEntry', () => {
     return CacheMiddleware.revalidateEntry(
       mocks.service,
       'key',
-      200,
+      cache,
       mocks.ctx
     ).then(() => {
       // original context should not be mutated
@@ -265,7 +278,7 @@ describe('revalidateEntry', () => {
     return CacheMiddleware.revalidateEntry(
       mocks.service,
       'key',
-      200,
+      cache,
       mocks.ctx
     ).then(() => {
       expect(mocks.resolveFromAccumulator).toBeCalled()
@@ -277,6 +290,10 @@ describe('revalidateEntry', () => {
 describe('resolveStaleWhileRevalidateEntry', () => {
   let logDebug
   let logError
+  const cache = {
+    ttl: 200,
+    staleWhileRevalidateTtl: 400
+  }
 
   beforeEach(() => {
     const logger = winston.loggers.get('data-point-service')
@@ -297,7 +314,7 @@ describe('resolveStaleWhileRevalidateEntry', () => {
     const result = CacheMiddleware.resolveStaleWhileRevalidateEntry(
       mocks.service,
       'key',
-      200,
+      cache,
       mocks.ctx
     )
     expect(result).toBeUndefined()
@@ -319,7 +336,7 @@ describe('resolveStaleWhileRevalidateEntry', () => {
     const result = CacheMiddleware.resolveStaleWhileRevalidateEntry(
       mocks.service,
       'key',
-      200,
+      cache,
       mocks.ctx
     )
     return result.then(value => {
@@ -342,10 +359,11 @@ describe('resolveStaleWhileRevalidateEntry', () => {
         }
       })
     })
+
     const result = CacheMiddleware.resolveStaleWhileRevalidateEntry(
       mocks.service,
       'key',
-      200,
+      cache,
       mocks.ctx
     )
     return result.then(value => {
@@ -373,14 +391,14 @@ describe('resolveStaleWhileRevalidateEntry', () => {
     const result = CacheMiddleware.resolveStaleWhileRevalidateEntry(
       mocks.service,
       'key',
-      200,
+      cache,
       mocks.ctx
     )
     return result.then(value => {
       expect(spyRevalidateEntry).toBeCalledWith(
         mocks.service,
         'key',
-        200,
+        cache,
         mocks.ctx
       )
       expect(value).toEqual('STALE')
@@ -431,40 +449,76 @@ describe('warnLooseParamsCacheDeprecation', () => {
   })
 })
 
+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: () => true
+      staleWhileRevalidate: '10s'
     }
     const cache = CacheMiddleware.getCacheParams(params)
     expect(cache).toEqual({
       ttl: params.ttl,
       cacheKey: params.cacheKey,
-      staleWhileRevalidate: params.staleWhileRevalidate
+      useStaleWhileRevalidate: true,
+      staleWhileRevalidateTtl: 30000
     })
   })
-  it('should have get values from params.cache property', () => {
+  it('should get values from params.cache property', () => {
     const params = {
       cache: {
         ttl: '20s',
         cacheKey: () => true,
-        staleWhileRevalidate: () => true
+        staleWhileRevalidate: '10s'
       }
     }
     const cache = CacheMiddleware.getCacheParams(params)
     expect(cache).toEqual({
       ttl: params.cache.ttl,
       cacheKey: params.cache.cacheKey,
-      staleWhileRevalidate: params.cache.staleWhileRevalidate
+      useStaleWhileRevalidate: true,
+      staleWhileRevalidateTtl: 30000
     })
   })
   it('should have params.cache priority over loose param cache settings', () => {
     const params = {
       ttl: '10s',
       cacheKey: () => true,
-      staleWhileRevalidate: () => true,
+      staleWhileRevalidate: '20ms',
       cache: {
         ttl: '20s',
         cacheKey: () => true
@@ -476,7 +530,34 @@ describe('getCacheParams', () => {
       cacheKey: params.cache.cacheKey,
       // this key will use the loose value since its not being set through
       // params.cache
-      staleWhileRevalidate: params.staleWhileRevalidate
+      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
     })
   })
 })
@@ -498,10 +579,10 @@ describe('before', () => {
     expect(next).toBeCalledWith() // with no arguments
   })
 
-  it('should attempt to resolve staleWhileRevalidate if ttl exists and staleWhileRevalidate is true', done => {
+  it('should attempt to resolve staleWhileRevalidate if ttl exists and staleWhileRevalidate is set', done => {
     const mocks = createMocks()
     mocks.ctx.context.params.ttl = '20m'
-    mocks.ctx.context.params.staleWhileRevalidate = true
+    mocks.ctx.context.params.staleWhileRevalidate = '10m'
 
     const spyResolveStaleWhileRevalidateEntry = jest.spyOn(
       CacheMiddleware,
@@ -509,11 +590,18 @@ describe('before', () => {
     )
     spyResolveStaleWhileRevalidateEntry.mockImplementation(() => true)
 
+    const cache = {
+      ttl: '20m',
+      cacheKey: undefined,
+      useStaleWhileRevalidate: true,
+      staleWhileRevalidateTtl: ms('20m') + ms('10m')
+    }
+
     const next = () => {
       expect(spyResolveStaleWhileRevalidateEntry).toHaveBeenCalledWith(
         mocks.service,
         'entity:model:Foo',
-        '20m',
+        cache,
         mocks.ctx
       )
       done()
@@ -600,7 +688,7 @@ describe('after', () => {
 
     mocks.ctx.value = 'VALUE'
     mocks.ctx.context.params.ttl = '20m'
-    mocks.ctx.context.params.staleWhileRevalidate = true
+    mocks.ctx.context.params.staleWhileRevalidate = '10m'
     mocks.ctx.locals.revalidatingCache = undefined
     const setStaleWhileRevalidateEntry = jest.spyOn(
       CacheMiddleware,
@@ -608,12 +696,19 @@ describe('after', () => {
     )
     setStaleWhileRevalidateEntry.mockImplementation(() => Promise.resolve())
 
+    const cache = {
+      ttl: '20m',
+      cacheKey: undefined,
+      useStaleWhileRevalidate: true,
+      staleWhileRevalidateTtl: ms('20m') + ms('10m')
+    }
+
     const next = () => {
       expect(setStaleWhileRevalidateEntry).toBeCalledWith(
         mocks.service,
         'entity:model:Foo',
         'VALUE',
-        '20m'
+        cache
       )
       setStaleWhileRevalidateEntry.mockReset()
       setStaleWhileRevalidateEntry.mockRestore()