Merge branch 'feature/specify-what-to-cache' into develop

History.md

@@ -1,3 +1,7 @@
+- {next release} 2015-05-17
+  - By default, cache falsey values like `false`, `0`, and `null`, but not `undefined` (#25).
+  - Allow users to pass in callback function `isCacheableValue` to specify what to cache.
+
 - 0.19.0 2015-03-29
   - Pass dispose, length & stale options to lru-cache (#22). - @gmaclennan
 

README.md

@@ -207,6 +207,42 @@ multiCache.wrap(key2, function (cb) {
 });
 ```
 
+### Specifying What to Cache
+
+Both the `caching` and `multicaching` modules allow you to pass in a callback function called
+`isCacheableValue` which is called with every value returned from cache or from a wrapped function.
+This lets you specify which values should and should not be cached. If the function returns true, it will be
+stored in cache. By default the caches cache everything except `undefined`.
+
+For example, if you don't want to cache `false` and `null`, you can pass in a function like this:
+
+```javascript
+
+var isCacheableValue = function(value) {
+    return value !== null && value !== false && value !== undefined;
+};
+
+```
+
+Then pass it to `caching` like this:
+
+```javascript
+
+var memoryCache = cacheManager.caching({store: 'memory', isCacheableValue: isCacheableValue};
+
+```
+
+And pass it to `multicaching` like this:
+
+```javascript
+
+var multiCache = cacheManager.multiCaching([memoryCache, someOtherCache], {
+    isCacheableValue: isCacheableValue
+});
+
+```
+
+
 ## Tests
 
 To run tests, first run:

lib/caching.js

@@ -2,6 +2,18 @@
 var domain = require('domain');
 var CallbackFiller = require('./callback_filler');
 
+/**
+ * Generic caching interface that wraps any caching library with a compatible interface.
+ *
+ * @param {object} args
+ * @param {object|string} args.store - The store must have at least the following functions:
+ *   - set
+ *   - get
+ * @param {function} [args.isCacheableValue] - A callback function which is called
+ *   with every value returned from cache or from a wrapped function. This lets you specify
+ *   which values should and should not be cached. If the function returns true, it will be
+ *   stored in cache. By default it caches everything except undefined.
+ */
 var caching = function(args) {
     args = args || {};
     var self = {};
@@ -23,6 +35,14 @@ var caching = function(args) {
 
     var callbackFiller = new CallbackFiller();
 
+    if (typeof args.isCacheableValue === 'function') {
+        self._isCacheableValue = args.isCacheableValue;
+    } else {
+        self._isCacheableValue = function(value) {
+            return value !== undefined;
+        };
+    }
+
     /**
      * Wraps a function in cache. I.e., the first time the function is run,
      * its results are stored in cache so subsequent calls retrieve from cache
@@ -50,7 +70,7 @@ var caching = function(args) {
         self.store.get(key, options, function(err, result) {
             if (err && (!self.ignoreCacheErrors)) {
                 callbackFiller.fill(key, err);
-            } else if (result) {
+            } else if (self._isCacheableValue(result)) {
                 callbackFiller.fill(key, null, result);
             } else {
                 domain
@@ -63,6 +83,11 @@ var caching = function(args) {
                         callbackFiller.fill(key, err);
                         return;
                     }
+
+                    if (!self._isCacheableValue(data)) {
+                        return cb();
+                    }
+
                     self.store.set(key, data, options, function(err) {
                         if (err && (!self.ignoreCacheErrors)) {
                             callbackFiller.fill(key, err);

lib/multi_caching.js

@@ -3,16 +3,33 @@ var domain = require('domain');
 var CallbackFiller = require('./callback_filler');
 
 /**
+ *
  * Module that lets you specify a hierarchy of caches.
+ * @param {array} caches - Array of caching objects.
+ * @param {object} [options]
+ * @param {function} [options.isCacheableValue] - A callback function which is called
+ *   with every value returned from cache or from a wrapped function. This lets you specify
+ *   which values should and should not be cached. If the function returns true, it will be
+ *   stored in cache. By default it caches everything except undefined.
  */
-var multiCaching = function(caches) {
+var multiCaching = function(caches, options) {
     var self = {};
+    options = options || {};
+
     if (!Array.isArray(caches)) {
         throw new Error('multiCaching requires an array of caches');
     }
 
     var callbackFiller = new CallbackFiller();
 
+    if (typeof options.isCacheableValue === 'function') {
+        self._isCacheableValue = options.isCacheableValue;
+    } else {
+        self._isCacheableValue = function(value) {
+            return value !== undefined;
+        };
+    }
+
     function getFromHighestPriorityCache(key, options, cb) {
         if (typeof options === 'function') {
             cb = options;
@@ -54,8 +71,10 @@ var multiCaching = function(caches) {
 
     /**
      * Looks for an item in cache tiers.
+     * When a key is found in a lower cache, all higher levels are updated.
      *
-     * When a key is found in a lower cache, all higher levels are updated
+     * @param {string} key
+     * @param {function} cb
      */
     self.getAndPassUp = function(key, cb) {
         getFromHighestPriorityCache(key, function(err, result, index) {
@@ -91,6 +110,11 @@ var multiCaching = function(caches) {
      * without getting set in other lower-priority caches.
      * If a key doesn't exist in a higher-priority cache but exists in a lower-priority
      * cache, it gets set in all higher-priority caches.
+     *
+     * @param {string} key - The cache key to use in cache operations
+     * @param {function} work - The function to wrap
+     * @param {object} [options] - options passed to `set` function
+     * @param {function} cb
      */
     self.wrap = function(key, work, options, cb) {
         if (typeof options === 'function') {
@@ -119,7 +143,7 @@ var multiCaching = function(caches) {
         getFromHighestPriorityCache(key, function(err, result, index) {
             if (err) {
                 return callbackFiller.fill(key, err);
-            } else if (result) {
+            } else if (self._isCacheableValue(result)) {
                 var cachesToUpdate = caches.slice(0, index);
                 var opts = getOptsForSet(result);
 
@@ -137,6 +161,10 @@ var multiCaching = function(caches) {
                         return callbackFiller.fill(key, err);
                     }
 
+                    if (!self._isCacheableValue(data)) {
+                        return cb();
+                    }
+
                     var opts = getOptsForSet(data);
 
                     setInMultipleCaches(caches, opts, function(err) {
@@ -147,6 +175,13 @@ var multiCaching = function(caches) {
         });
     };
 
+    /**
+     * Set value in all caches
+     * @param {string} key
+     * @param {*} value
+     * @param {object} [options] to pass to underlying set function.
+     * @param {function} cb
+     */
     self.set = function(key, value, options, cb) {
         var opts = {
             key: key,
@@ -159,6 +194,12 @@ var multiCaching = function(caches) {
         setInMultipleCaches(caches, opts, cb);
     };
 
+    /**
+     * Get value from highest level cache that has stored it.
+     * @param {string} key
+     * @param {object} [options] to pass to underlying get function.
+     * @param {function} cb
+     */
     self.get = function(key, options, cb) {
         if (typeof options === 'function') {
             cb = options;
@@ -167,6 +208,12 @@ var multiCaching = function(caches) {
         getFromHighestPriorityCache(key, options, cb);
     };
 
+    /**
+     * Delete value from all caches.
+     * @param {string} key
+     * @param {object} [options] to pass to underlying del function.
+     * @param {function} cb
+     */
     self.del = function(key, options, cb) {
         if (typeof options === 'function') {
             cb = options;

test/caching.unit.js

@@ -35,7 +35,9 @@ describe("caching", function() {
                 it("lets us set and get data in cache", function(done) {
                     cache.set(key, value, ttl, function(err) {
                         checkErr(err);
+
                         cache.get(key, function(err, result) {
+                            checkErr(err);
                             assert.equal(result, value);
                             done();
                         });
@@ -44,6 +46,7 @@ describe("caching", function() {
 
                 it("lets us set and get data without a callback", function(done) {
                     cache.set(key, value, ttl);
+
                     setTimeout(function() {
                         var result = cache.get(key);
                         assert.equal(result, value);
@@ -53,6 +56,7 @@ describe("caching", function() {
 
                 it("lets us set and get data without a ttl or callback", function(done) {
                     cache.set(key, value);
+
                     setTimeout(function() {
                         var result = cache.get(key);
                         assert.equal(result, value);
@@ -351,6 +355,116 @@ describe("caching", function() {
                 });
             });
 
+            var falseyValues = [false, null, 0];
+
+            falseyValues.forEach(function(falseyValue) {
+                context("when cached value is `" + falseyValue + "`", function() {
+                    function getFalseyValue(cb) {
+                        process.nextTick(function() {
+                            cb(null, falseyValue);
+                        });
+                    }
+
+                    function getCachedFalseyValue(cb) {
+                        cache.wrap(key, function(cacheCb) {
+                            getFalseyValue(cacheCb);
+                        }, ttl, cb);
+                    }
+
+                    beforeEach(function(done) {
+                        getCachedFalseyValue(function(err, result) {
+                            checkErr(err);
+                            assert.strictEqual(result, falseyValue);
+
+                            memoryStoreStub.get(key, function(err, result) {
+                                checkErr(err);
+                                assert.strictEqual(result, falseyValue);
+
+                                sinon.spy(memoryStoreStub, 'get');
+
+                                done();
+                            });
+                        });
+                    });
+
+                    afterEach(function() {
+                        memoryStoreStub.get.restore();
+                    });
+
+                    it("retrieves data from cache", function(done) {
+                        getCachedFalseyValue(function(err, value) {
+                            checkErr(err);
+                            assert.strictEqual(value, falseyValue);
+                            assert.ok(memoryStoreStub.get.calledWith(key));
+                            done();
+                        });
+                    });
+                });
+            });
+
+            context("when we pass in an isCacheableValue function to the caching constructor", function() {
+                var testCallbacks = {
+                    isCacheableValue: function(value) {
+                        return value !== 'do_not_store_this' && value !== undefined;
+                    }
+                };
+
+                function getValue(name, cb) {
+                    process.nextTick(function() {
+                        if (name === 'foo') {
+                            cb(null, 'store_this');
+                        } else {
+                            cb(null, 'do_not_store_this');
+                        }
+                    });
+                }
+
+                function getCachedValue(name, cb) {
+                    cache.wrap(key, function(cacheCb) {
+                        getValue(name, function(err, result) {
+                            cacheCb(err, result);
+                        });
+                    }, ttl, cb);
+                }
+
+                beforeEach(function() {
+                    sinon.spy(testCallbacks, 'isCacheableValue');
+                    cache = caching({store: 'memory', isCacheableValue: testCallbacks.isCacheableValue});
+                    sinon.spy(memoryStoreStub, 'set');
+                });
+
+                afterEach(function() {
+                    memoryStoreStub.set.restore();
+                    testCallbacks.isCacheableValue.restore();
+                });
+
+                it("stores allowed values", function(done) {
+                    var name = 'foo';
+
+                    getCachedValue(name, function(err) {
+                        checkErr(err);
+                        assert.ok(memoryStoreStub.set.called);
+                        assert.ok(testCallbacks.isCacheableValue.called);
+
+                        getCachedValue(name, function(err) {
+                            checkErr(err);
+                            done();
+                        });
+                    });
+                });
+
+                it("does not store non-allowed values", function(done) {
+                    var name = 'bar';
+
+                    getCachedValue(name, function(err) {
+                        checkErr(err);
+                        assert.ok(memoryStoreStub.set.notCalled);
+                        assert.ok(testCallbacks.isCacheableValue.called);
+                        done();
+                    });
+                });
+            });
+
             it("lets us make nested calls", function(done) {
                 function getCachedWidget(name, cb) {
                     cache.wrap(key, function(cacheCb) {

test/multi_caching.unit.js

@@ -369,6 +369,121 @@ describe("multiCaching", function() {
                     });
                 });
             });
+
+            var falseyValues = [false, null, 0];
+
+            falseyValues.forEach(function(falseyValue) {
+                context("when cached value is `" + falseyValue + "`", function() {
+                    function getFalseyValue(cb) {
+                        process.nextTick(function() {
+                            cb(null, falseyValue);
+                        });
+                    }
+
+                    function getCachedFalseyValue(cb) {
+                        multiCache.wrap(key, function(cacheCb) {
+                            getFalseyValue(cacheCb);
+                        }, ttl, cb);
+                    }
+
+                    beforeEach(function(done) {
+                        multiCache = multiCaching([memoryCache3]);
+                        sinon.spy(memoryCache3.store, 'set');
+
+                        getCachedFalseyValue(function(err, result) {
+                            checkErr(err);
+                            assert.strictEqual(result, falseyValue);
+
+                            memoryCache3.get(key, function(err, result) {
+                                checkErr(err);
+                                assert.strictEqual(result, falseyValue);
+
+                                sinon.spy(memoryCache3.store, 'get');
+
+                                done();
+                            });
+                        });
+                    });
+
+                    afterEach(function() {
+                        memoryCache3.store.set.restore();
+                        memoryCache3.store.get.restore();
+                    });
+
+                    it("sets data in and retrieves data from cache", function(done) {
+                        getCachedFalseyValue(function(err, value) {
+                            checkErr(err);
+                            assert.strictEqual(value, falseyValue);
+                            assert.ok(memoryCache3.store.set.calledWith(key));
+                            assert.ok(memoryCache3.store.get.calledWith(key));
+                            done();
+                        });
+                    });
+                });
+            });
+
+            context("when we pass in an isCacheableValue function to the caching constructor", function() {
+                var testCallbacks = {
+                    isCacheableValue: function(value) {
+                        return value !== 'do_not_store_this' && value !== undefined;
+                    }
+                };
+
+                function getValue(name, cb) {
+                    process.nextTick(function() {
+                        if (name === 'foo') {
+                            cb(null, 'store_this');
+                        } else {
+                            cb(null, 'do_not_store_this');
+                        }
+                    });
+                }
+
+                function getCachedValue(name, cb) {
+                    multiCache.wrap(key, function(cacheCb) {
+                        getValue(name, function(err, result) {
+                            cacheCb(err, result);
+                        });
+                    }, ttl, cb);
+                }
+
+                beforeEach(function() {
+                    sinon.spy(testCallbacks, 'isCacheableValue');
+                    multiCache = multiCaching([memoryCache3], {isCacheableValue: testCallbacks.isCacheableValue});
+                    sinon.spy(memoryCache3.store, 'set');
+                });
+
+                afterEach(function() {
+                    memoryCache3.store.set.restore();
+                    testCallbacks.isCacheableValue.restore();
+                });
+
+                it("stores allowed values", function(done) {
+                    var name = 'foo';
+
+                    getCachedValue(name, function(err) {
+                        checkErr(err);
+                        assert.ok(memoryCache3.store.set.called);
+
+                        assert.ok(testCallbacks.isCacheableValue.called);
+
+                        getCachedValue(name, function(err) {
+                            checkErr(err);
+                            done();
+                        });
+                    });
+                });
+
+                it("does not store non-allowed values", function(done) {
+                    var name = 'bar';
+
+                    getCachedValue(name, function(err) {
+                        checkErr(err);
+                        assert.ok(memoryCache3.store.set.notCalled);
+                        done();
+                    });
+                });
+            });
         });
 
         describe("using two cache stores", function() {