Merge branch 'release/1.2.1'

History.md

@@ -1,3 +1,6 @@
+- 1.2.1 2015-10-17
+  - Bugfix: multi-caching: using underlying store's isCacheableValue function when it exists (#34).
+
 - 1.2.0 2015-10-07
   - using `isCacheableValue` in `getFromHighestPriorityCache` and `getAndPassUp` (#32).
 

lib/multi_caching.js

@@ -12,6 +12,9 @@ var CallbackFiller = require('./callback_filler');
  *   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.
+ *
+ *   If an underlying cache specifies its own isCacheableValue function, that function will
+ *   be used instead of the multiCaching's _isCacheableValue function.
  */
 var multiCaching = function(caches, options) {
     var self = {};
@@ -31,6 +34,19 @@ var multiCaching = function(caches, options) {
         };
     }
 
+    /**
+     * If the underlying cache specifies its own isCacheableValue function (such
+     * as how node-cache-manager-redis does), use that function, otherwise use
+     * self._isCacheableValue function.
+     */
+    function getIsCacheableValueFunction(cache) {
+        if (cache.store && typeof cache.store.isCacheableValue === 'function') {
+            return cache.store.isCacheableValue;
+        } else {
+            return self._isCacheableValue;
+        }
+    }
+
     function getFromHighestPriorityCache(key, options, cb) {
         if (typeof options === 'function') {
             cb = options;
@@ -43,7 +59,10 @@ var multiCaching = function(caches, options) {
                 if (err) {
                     return next(err);
                 }
-                if (self._isCacheableValue(result)) {
+
+                var _isCacheableValue = getIsCacheableValueFunction(cache);
+
+                if (_isCacheableValue(result)) {
                     // break out of async loop.
                     return cb(err, result, i);
                 }
@@ -59,7 +78,13 @@ var multiCaching = function(caches, options) {
     function setInMultipleCaches(caches, opts, cb) {
         opts.options = opts.options || {};
         async.each(caches, function(cache, next) {
-            cache.store.set(opts.key, opts.value, opts.options, next);
+            var _isCacheableValue = getIsCacheableValueFunction(cache);
+
+            if (_isCacheableValue(opts.value)) {
+                cache.store.set(opts.key, opts.value, opts.options, next);
+            } else {
+                next();
+            }
         }, cb);
     }
 
@@ -78,11 +103,14 @@ var multiCaching = function(caches, options) {
 
             cb(err, result);
 
-            if (self._isCacheableValue(result) && index) {
+            if (index) {
                 var cachesToUpdate = caches.slice(0, index);
                 async.each(cachesToUpdate, function(cache, next) {
-                    // We rely on the cache module's default TTL
-                    cache.set(key, result, next);
+                    var _isCacheableValue = getIsCacheableValueFunction(cache);
+                    if (_isCacheableValue(result)) {
+                        // We rely on the cache module's default TTL
+                        cache.set(key, result, next);
+                    }
                 });
             }
         });

package.json

@@ -1,6 +1,6 @@
 {
   "name": "cache-manager",
-  "version": "1.2.0",
+  "version": "1.2.1",
   "description": "Cache module for Node.js",
   "main": "index.js",
   "scripts": {

test/multi_caching.unit.js

@@ -534,6 +534,81 @@ describe("multiCaching", function() {
                     });
                 });
             });
+
+            context("when an underlying store has its own isCacheableValue function", function() {
+                var memoryCache4;
+
+                var testCallbacks = {
+                    isCacheableValue: function(value) {
+                        var x = value !== 'do_not_store_this' && value !== undefined;
+                        return x;
+                    }
+                };
+
+                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);
+                        });
+                    }, cb);
+                }
+
+                beforeEach(function() {
+                    sinon.spy(testCallbacks, 'isCacheableValue');
+                    memoryCache4 = caching({
+                        store: 'memory',
+                        ttl: memoryTtl
+                    });
+
+                    // This simulates how node-cache-manager-redis sets its
+                    // isCacheableValue function:
+                    memoryCache4.store.isCacheableValue = testCallbacks.isCacheableValue;
+
+                    multiCache = multiCaching([memoryCache4]);
+                    sinon.spy(memoryCache4.store, 'set');
+                });
+
+                afterEach(function() {
+                    memoryCache4.store.set.restore();
+                    testCallbacks.isCacheableValue.restore();
+                });
+
+                it("stores allowed values", function(done) {
+                    var name = 'foo';
+
+                    getCachedValue(name, function(err) {
+                        checkErr(err);
+                        assert.ok(memoryCache4.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(memoryCache4.store.set.notCalled);
+                        done();
+                    });
+                });
+            });
         });
 
         describe("using two cache stores", function() {