Various fixes (#5)

* Bumps of various dependencies * changelog * allow for overrides * changelog link * fix nesting, stylistic changes to use async over explicit Promise operations * Use getValue, gross milk should be one less new * Update lib/multi-level.js Co-Authored-By: msluther <[email protected]> * make fixes
godaddy · Feb 13, 2019 · dac5651 · dac5651
1 parent 28ca8d0
commit dac5651
Show file tree

Hide file tree

Showing 5 changed files with 222 additions and 56 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,6 @@
 # Change Log
 
+- [#5] Allow for call-level age overrides as specified in the README.
 - [#4] Bump various dev-dependencies
 
 ### 1.1.0
@@ -17,3 +18,4 @@
 [#1]: https://github.com/godaddy/out-of-band-cache/pull/1
 [#2]: https://github.com/godaddy/out-of-band-cache/pull/2
 [#4]: https://github.com/godaddy/out-of-band-cache/pull/4
+[#5]: https://github.com/godaddy/out-of-band-cache/pull/5
diff --git a/lib/multi-level.js b/lib/multi-level.js
@@ -1,3 +1,20 @@
+const debug = require('diagnostics')('out-of-band-cache:multi-level');
+
+
+/**
+ * Gets a property from either the options object if defined there, otherwise the default value.
+ *
+ * @param {Object} opts An options object
+ * @param {String} key The name of the property to get
+ * @param {Any} defaultValue The default value
+ *
+ * @private
+ * @returns {Any} The value of the property
+ */
+function getValue(opts, key, defaultValue) {
+  return key in opts ? opts[key] : defaultValue;
+}
+
 /**
  * @typedef Cache
  * @prop {AsyncFunction} init Initialization function
@@ -17,11 +34,11 @@
 /**
  * A multi-level cache.
  *
- * @param {Object} opts                             - Options for the Client instance
- * @param {String} [opts.maxAge=600,000]            - The duration, in milliseconds, before a cached item expires
- * @param {String} [opts.maxStaleness=0]            - The duration, in milliseconds, in which expired cache items are still served
- * @param {ShouldCache} [opts.shouldCache=()=>true] - Function to determine whether to not we should cache the result
- * @param {Cache[]}  opts.caches                    - An Array of cache objects. See `./fs` and `./memory` for sample caches.
+ * @param {Object} opts Options for the Client instance
+ * @param {Number} [opts.maxAge=600000] The duration, in milliseconds, before a cached item expires (defaults to 600,000)
+ * @param {Number} [opts.maxStaleness=0] The duration, in milliseconds, in which expired cache items are still served
+ * @param {ShouldCache} [opts.shouldCache=()=>true] Function to determine whether to not we should cache the result
+ * @param {Cache[]}  opts.caches An Array of cache objects. See `./fs` and `./memory` for sample caches.
  */
 class MultiLevelCache {
   constructor(opts) {
@@ -36,28 +53,26 @@ class MultiLevelCache {
     this.shouldCache = opts.shouldCache || (() => true);
 
     this._pendingRefreshes = {};
-    this._initTask = Promise.all(opts.caches.map(cache => {
-      return cache.init().catch(err => {
-        throw err;
-      });
-    }));
+    this._initTask = Promise.all(opts.caches.map(cache => cache.init()));
   }
 
   /**
    * Attempts a cache get
    *
-   * @param {String}   key                    - The cache key
-   * @param {Object}   opts                   - Options for this particular read
-   * @param {Boolean}  [opts.skipCache=false] - Whether the cache should be bypassed (default false)
-   * @param {String}   [opts.maxAge]          - The duration in milliseconds before a cached item expires
-   * @param {ShouldCache} [opts.shouldCache]  - A function to determine whether or not we should cache the item
-   * @param {UpdateFn} updateFn               - async function that defines how to get a new value
+   * @param {String} key The cache key
+   * @param {Object} [opts={}] Options for this particular read
+   * @param {Boolean} [opts.skipCache=false] Whether the cache should be bypassed (default false)
+   * @param {String} [opts.maxAge] The duration in milliseconds before a cached item expires
+   * @param {Number} [opts.maxStaleness] The duration, in milliseconds, in which expired cache items are still served
+   * @param {ShouldCache} [opts.shouldCache] A function to determine whether or not we should cache the item
+   * @param {UpdateFn} updateFn async function that defines how to get a new value
    *
    * @async
    * @returns {Promise<GetResult>} a Promise which resolves to an object containing
    * a `value` property and a `fromCache` boolean indicator.
    */
   async get(key, opts, updateFn) {
+    opts = opts || {};
     if (opts.skipCache) {
       const value = await updateFn(key, null);
       return {
@@ -73,16 +88,20 @@ class MultiLevelCache {
         return getChain.catch(() => cache.get(key));
       }, Promise.reject());
     } catch (e) { // cache miss
-      return this._refresh(key, null, updateFn, opts.shouldCache);
+      return this._refresh(key, null, updateFn, opts);
     }
 
     // cache hit
     const now = Date.now();
     if (item.expiry < now) {
-      const refreshTask = this._refresh(key, item, updateFn, opts.shouldCache);
-      if (item.expiry + this._maxStaleness < now) {
-        return refreshTask;
+      if (item.expiry + getValue(opts, 'maxStaleness', this._maxStaleness) < now) {
+        return this._refresh(key, item, updateFn, opts);
       }
+
+      // Update the cache, but ignore failures
+      this._refresh(key, item, updateFn, opts).catch(err => {
+        debug('background refresh failed for %s with %s', key && JSON.stringify(key), err && err.message);
+      });
     }
 
     return {
@@ -103,16 +122,18 @@ class MultiLevelCache {
 
   /**
    * Refresh the cache for a given key value pair
-   * @param  {String} key       cache key
-   * @param  {JSONSerializable} staleItem cache value
-   * @param  {UpdateFn} updateFn  async function that defines how to get a new value
-   * @param  {ShouldCache} [shouldCache] function that determines whether or not we should cache the item
+   * @param {String} key cache key
+   * @param {JSONSerializable} staleItem cache value
+   * @param {UpdateFn} updateFn async function that defines how to get a new value
+   * @param {Object} opts An options object
+   * @param {ShouldCache} [opts.shouldCache] Function that determines whether or not we should cache the item
+   * @param {Number} [opts.maxAge] The duration, in milliseconds, before a cached item expires
    *
    * @private
    * @async
    * @returns {Promise<any>} a promise that resolves once we have refreshed the correct key
    */
-  async _refresh(key, staleItem, updateFn, shouldCache) {
+  async _refresh(key, staleItem, updateFn, opts = {}) {
     if (!(key in this._pendingRefreshes)) {
       try {
         const task = updateFn(key, staleItem && staleItem.value);
@@ -121,16 +142,16 @@ class MultiLevelCache {
         const value = await task;
         const cacheItem = {
           value,
-          expiry: Date.now() + this._maxAge
+          expiry: Date.now() + getValue(opts, 'maxAge', this._maxAge)
         };
 
-        if (shouldCache && typeof shouldCache !== 'function') {
+        const shouldCache = getValue(opts, 'shouldCache', this.shouldCache);
+        if (typeof shouldCache !== 'function') {
           throw new TypeError('shouldCache has to be a function');
         }
 
-        const willCache = shouldCache || this.shouldCache;
         // Given that we are not ignoring this value, perform an out-of-band cache update
-        if (willCache(cacheItem.value)) {
+        if (shouldCache(cacheItem.value)) {
           // NB: an in-band update would `await` this Promise.all block
           Promise.all(this._caches.map(cache => cache.set(key, cacheItem))).catch(err => {
             throw new Error(`Error caching ${key}`, err);
@@ -143,7 +164,6 @@ class MultiLevelCache {
         }
 
         return { value, fromCache: false };
-
       } catch (err) {
         delete this._pendingRefreshes[key];
         throw err;