From cca63f92d16cef59466d70c5b55f97ce28567ac6 Mon Sep 17 00:00:00 2001
From: nguyenchr <nguyenchr@tabcorp.com.au>
Date: Wed, 15 Oct 2014 20:55:21 +1100
Subject: [PATCH] Allow ttl to be passed into wrap()

---
 README.md                  | 28 +++++++-----
 lib/caching.js             | 10 ++++-
 lib/multi_caching.js       | 35 ++++++++++++---
 lib/stores/memory.js       |  2 +-
 test/caching.unit.js       | 89 ++++++++++++++++++++++++++------------
 test/multi_caching.unit.js | 82 +++++++++++++++++++++++++++--------
 test/support.js            |  3 +-
 7 files changed, 181 insertions(+), 68 deletions(-)

diff --git a/README.md b/README.md
index b071539..a81961b 100644
--- a/README.md
+++ b/README.md
@@ -15,7 +15,7 @@ tiered caches, and a consistent interface.
 * Tiered caches -- data gets stored in each cache and fetched from the highest
 priority cache(s) first.
 * Use any cache you want, as long as it has the same API.
-* 100% test coverage via [mocha](https://github.com/visionmedia/mocha), 
+* 100% test coverage via [mocha](https://github.com/visionmedia/mocha),
   [istanbul](https://github.com/yahoo/istanbul), and [sinon](http://sinonjs.org).
 
 
@@ -57,14 +57,14 @@ function get_cached_user(id, cb) {
 function get_cached_user(id, cb) {
     memory_cache.wrap(id, function (cache_callback) {
         get_user(id, cache_callback);
-    }, cb);
+    }, ttl, cb);
 }
 ```
 
 Second, node-cache-manager features a built-in memory cache (using [node-lru-cache](https://github.com/isaacs/node-lru-cache)),
 with the standard functions you'd expect in most caches:
 
-    set(key, val, cb)
+    set(key, val, ttl, cb)
     get(key, cb)
     del(key, cb)
 
@@ -88,10 +88,10 @@ Redis cache store with connection pooling.
 ```javascript
 var cache_manager = require('cache-manager');
 var memory_cache = cache_manager.caching({store: 'memory', max: 100, ttl: 10/*seconds*/});
-
+var ttl = 5;
 // Note: callback is optional in set() and del().
 
-memory_cache.set('foo', 'bar', function(err) {
+memory_cache.set('foo', 'bar', ttl, function(err) {
     if (err) { throw err; }
 
     memory_cache.get('foo', function(err, result) {
@@ -109,11 +109,12 @@ function get_user(id, cb) {
 }
 
 var user_id = 123;
-var key = 'user_' + user_id; 
+var key = 'user_' + user_id;
 
+// Note: ttl is optional in wrap()
 memory_cache.wrap(key, function (cb) {
     get_user(user_id, cb);
-}, function (err, user) {
+}, ttl, function (err, user) {
     console.log(user);
 
     // Second time fetches user from memory_cache
@@ -134,7 +135,7 @@ Here's a very basic example of how you could use this in an Express app:
 
 ```javascript
 function respond(res, err, data) {
-    if (err) { 
+    if (err) {
         res.json(500, err);
     } else {
         res.json(200, data);
@@ -143,9 +144,10 @@ function respond(res, err, data) {
 
 app.get('/foo/bar', function(req, res) {
     var cache_key = 'foo-bar:' + JSON.stringify(req.query);
+    var ttl = 10;
     memory_cache.wrap(cache_key, function(cache_cb) {
         DB.find(req.query, cache_cb);
-    }, function(err, result) {
+    }, ttl, function(err, result) {
         respond(res, err, result);
     });
 });
@@ -171,10 +173,11 @@ var cache = cache_manager.caching({store: '/path/to/your/store'});
 ```javascript
 var multi_cache = cache_manager.multi_caching([memory_cache, some_other_cache]);
 user_id2 = 456;
-key2 = 'user_' + user_id; 
+key2 = 'user_' + user_id;
+ttl = 5;
 
 // Sets in all caches.
-multi_cache.set('foo2', 'bar2', function(err) {
+multi_cache.set('foo2', 'bar2', ttl, function(err) {
     if (err) { throw err; }
 
     // Fetches from highest priority cache that has the key.
@@ -187,9 +190,10 @@ multi_cache.set('foo2', 'bar2', function(err) {
     });
 });
 
+// Note: ttl is optional in wrap()
 multi_cache.wrap(key2, function (cb) {
     get_user(user_id2, cb);
-}, function (err, user) {
+}, ttl, function (err, user) {
     console.log(user);
 
     // Second time fetches user from memory_cache, since it's highest priority.
diff --git a/lib/caching.js b/lib/caching.js
index 9827641..517246b 100644
--- a/lib/caching.js
+++ b/lib/caching.js
@@ -34,7 +34,13 @@ var caching = function (args) {
      *       console.log(user);
      *   });
      */
-    self.wrap = function (key, work, cb) {
+    self.wrap = function (key, work, ttl, cb) {
+
+        if(typeof(ttl) == 'function') {
+          cb = ttl;
+          ttl = undefined;
+        }
+
         self.store.get(key, function (err, result) {
             if (err && (!self.ignoreCacheErrors)) {
                 cb(err);
@@ -55,7 +61,7 @@ var caching = function (args) {
                         return;
                     }
                     // Subsequently assume second arg is result.
-                    self.store.set(key, work_args[1], function (err) {
+                    self.store.set(key, work_args[1], ttl, function (err) {
                         if (err && (!self.ignoreCacheErrors)) {
                             self.queues[key].forEach(function (done) {
                                 done.call(null, err);
diff --git a/lib/multi_caching.js b/lib/multi_caching.js
index 15b90bb..8d1f130 100644
--- a/lib/multi_caching.js
+++ b/lib/multi_caching.js
@@ -25,9 +25,9 @@ var multi_caching = function (caches) {
         }, cb);
     }
 
-    function set_in_multiple_caches(caches, key, value, cb) {
+    function set_in_multiple_caches(caches, opts, cb) {
         async.forEach(caches, function (cache, async_cb) {
-            cache.store.set(key, value, async_cb);
+            cache.store.set(opts.key, opts.value, opts.ttl, async_cb);
         }, cb);
     }
 
@@ -41,13 +41,24 @@ var multi_caching = function (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.
      */
-    self.wrap = function (key, work, cb) {
+    self.wrap = function (key, work, ttl, cb) {
+
+        if(typeof(ttl) == 'function') {
+          cb = ttl;
+          ttl = undefined;
+        }
+
         get_from_highest_priority_cache(key, function (err, result, index) {
             if (err) {
                 return cb(err);
             } else if (result) {
                 var caches_to_update = caches.slice(0, index);
-                set_in_multiple_caches(caches_to_update, key, result, function (err) {
+                var opts = {
+                  key: key,
+                  value: result,
+                  ttl: ttl
+                };
+                set_in_multiple_caches(caches_to_update, opts, function (err) {
                     cb(err, result);
                 });
             } else if (self.queues[key]) {
@@ -63,7 +74,12 @@ var multi_caching = function (caches) {
                         delete self.queues[key];
                         return;
                     }
-                    set_in_multiple_caches(caches, key, work_args[1], function (err) {
+                    var opts = {
+                      key: key,
+                      value: work_args[1],
+                      ttl: ttl
+                    };
+                    set_in_multiple_caches(caches, opts, function (err) {
                         if (err) {
                             self.queues[key].forEach(function (done) {
                                 done.call(null, err);
@@ -81,8 +97,13 @@ var multi_caching = function (caches) {
         });
     };
 
-    self.set = function (key, value, cb) {
-        set_in_multiple_caches(caches, key, value, cb);
+    self.set = function (key, value, ttl, cb) {
+        var opts = {
+          key: key,
+          value: value,
+          ttl: ttl
+        };
+        set_in_multiple_caches(caches, opts, cb);
     };
 
     self.get = function (key, cb) {
diff --git a/lib/stores/memory.js b/lib/stores/memory.js
index 1b02648..d36bb36 100644
--- a/lib/stores/memory.js
+++ b/lib/stores/memory.js
@@ -12,7 +12,7 @@ var memory_store = function (args) {
 
     var lru_cache = new Lru(lru_opts);
 
-    self.set = function (key, value, cb) {
+    self.set = function (key, value, ttl, cb) {
         lru_cache.set(key, value);
         if (cb) {
             process.nextTick(cb);
diff --git a/test/caching.unit.js b/test/caching.unit.js
index 16d22b5..0e7dafa 100644
--- a/test/caching.unit.js
+++ b/test/caching.unit.js
@@ -17,7 +17,7 @@ var methods = {
 describe("caching", function () {
     var cache;
     var key;
-    var ttl;
+    var ttl = 1;
     var name;
     var value;
 
@@ -31,7 +31,7 @@ describe("caching", function () {
                 });
 
                 it("lets us set and get data in cache", function (done) {
-                    cache.set(key, value, function (err) {
+                    cache.set(key, value, ttl, function (err) {
                         check_err(err);
                         cache.get(key, function (err, result) {
                             assert.equal(result, value);
@@ -41,6 +41,15 @@ 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);
+                        done();
+                    }, 20);
+                });
+
+                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);
@@ -59,7 +68,7 @@ describe("caching", function () {
                     cache = caching({store: store});
                     key = support.random.string(20);
                     value = support.random.string();
-                    cache.set(key, value, function (err) {
+                    cache.set(key, value, ttl, function (err) {
                         check_err(err);
                         done();
                     });
@@ -106,14 +115,13 @@ describe("caching", function () {
             cache = caching({store: 'memory'});
             key = support.random.string(20);
             value = support.random.string();
-
-            cache.set(key, value, function (err) {
+            cache.set(key, value, ttl, function (err) {
                 check_err(err);
 
                 key2 = support.random.string(20);
                 value2 = support.random.string();
 
-                cache.set(key2, value2, done);
+                cache.set(key2, value2, ttl, done);
             });
         });
 
@@ -205,7 +213,7 @@ describe("caching", function () {
                 key = support.random.string(20);
                 saved_keys.push(key);
                 value = support.random.string();
-                cache.set(key, value, cb);
+                cache.set(key, value, ttl, cb);
             }, done);
         });
 
@@ -242,21 +250,46 @@ describe("caching", function () {
                 memory_store.create.restore();
             });
 
-            it("calls back with the result of the wrapped function", function (done) {
-                cache.wrap(key, function (cb) {
-                    methods.get_widget(name, cb);
-                }, function (err, widget) {
-                    check_err(err);
-                    assert.deepEqual(widget, {name: name});
-                    done();
+            context("calls back with the result of the wrapped function", function () {
+
+                beforeEach(function () {
+                    sinon.spy(memory_store_stub, 'set');
                 });
+
+                afterEach(function () {
+                    memory_store_stub.set.restore();
+                });
+
+                it("when a ttl is passed in", function(done) {
+                    cache.wrap(key, function (cb) {
+                        methods.get_widget(name, cb);
+                    }, ttl, function (err, widget) {
+                        check_err(err);
+                        assert.deepEqual(widget, {name: name});
+                        sinon.assert.calledWith(memory_store_stub.set, key, {name: name}, ttl);
+                        done();
+                    });
+
+                });
+
+                it("when a ttl is not passed in", function(done) {
+                    cache.wrap(key, function (cb) {
+                        methods.get_widget(name, cb);
+                    }, function (err, widget) {
+                        check_err(err);
+                        assert.deepEqual(widget, {name: name});
+                        sinon.assert.calledWith(memory_store_stub.set, key, {name: name}, undefined);
+                        done();
+                    });
+                });
+
             });
 
             context("when result is already cached", function () {
                 function get_cached_widget(name, cb) {
                     cache.wrap(key, function (cache_cb) {
                         methods.get_widget(name, cache_cb);
-                    }, cb);
+                    }, ttl, cb);
                 }
 
                 beforeEach(function (done) {
@@ -287,7 +320,7 @@ describe("caching", function () {
                             func_called = true;
                             cb(err, result);
                         });
-                    }, function (err, widget) {
+                    }, ttl, function (err, widget) {
                         check_err(err);
                         assert.deepEqual(widget, {name: name});
                         assert.ok(memory_store_stub.get.calledWith(key));
@@ -300,7 +333,7 @@ describe("caching", function () {
             it("expires cached result after ttl seconds", function (done) {
                 cache.wrap(key, function (cb) {
                     methods.get_widget(name, cb);
-                }, function (err, widget) {
+                }, ttl, function (err, widget) {
                     check_err(err);
                     assert.deepEqual(widget, {name: name});
 
@@ -338,7 +371,7 @@ describe("caching", function () {
 
                         cache.wrap(key, function (cb) {
                             methods.get_widget(name, cb);
-                        }, function (err) {
+                        }, ttl, function (err) {
                             assert.equal(err, fake_error);
                             memory_store_stub.get.restore();
                             done();
@@ -358,7 +391,7 @@ describe("caching", function () {
 
                         cache.wrap(key, function (cb) {
                             methods.get_widget(name, cb);
-                        }, function (err) {
+                        }, ttl, function (err) {
                             assert.equal(err, null);
                             memory_store_stub.get.restore();
                             done();
@@ -372,13 +405,13 @@ describe("caching", function () {
                     it("bubbles up that error", function (done) {
                         var fake_error = new Error(support.random.string());
 
-                        sinon.stub(memory_store_stub, 'set', function (key, val, cb) {
+                        sinon.stub(memory_store_stub, 'set', function (key, val, ttl, cb) {
                             cb(fake_error);
                         });
 
                         cache.wrap(key, function (cb) {
                             methods.get_widget(name, cb);
-                        }, function (err) {
+                        }, ttl, function (err) {
                             assert.equal(err, fake_error);
                             memory_store_stub.set.restore();
                             done();
@@ -391,13 +424,13 @@ describe("caching", function () {
                         cache = caching({store: 'memory', ttl: ttl, ignoreCacheErrors: true});
                         var fake_error = new Error(support.random.string());
 
-                        sinon.stub(memory_store_stub, 'set', function (key, val, cb) {
+                        sinon.stub(memory_store_stub, 'set', function (key, val, ttl, cb) {
                             cb(fake_error);
                         });
 
                         cache.wrap(key, function (cb) {
                             methods.get_widget(name, cb);
-                        }, function (err) {
+                        }, ttl, function (err) {
                             assert.equal(err, null);
                             memory_store_stub.set.restore();
                             done();
@@ -415,7 +448,7 @@ describe("caching", function () {
 
                     cache.wrap(key, function (cb) {
                         methods.get_widget(name, cb);
-                    }, function (err, widget) {
+                    }, ttl, function (err, widget) {
                         methods.get_widget.restore();
                         assert.equal(err, fake_error);
                         assert.ok(!widget);
@@ -452,7 +485,7 @@ describe("caching", function () {
                 async.each(values, function (val, async_cb) {
                     cache.wrap('key', function (cb) {
                         construct(val, cb);
-                    }, function (err, result) {
+                    }, ttl, function (err, result) {
                         assert.equal(result, 'value');
                         async_cb(err);
                     });
@@ -476,7 +509,7 @@ describe("caching", function () {
         it("allows us to pass in our own store object", function (done) {
             var store = memory_store.create({ttl: ttl});
             cache = caching({store: store});
-            cache.set(key, value, function (err) {
+            cache.set(key, value, ttl, function (err) {
                 check_err(err);
                 cache.get(key, function (err, result) {
                     assert.equal(result, value);
@@ -488,7 +521,7 @@ describe("caching", function () {
         it("allows us to pass in a path to our own store", function (done) {
             var store_path = '../lib/stores/memory';
             cache = caching({store: store_path});
-            cache.set(key, value, function (err) {
+            cache.set(key, value, ttl, function (err) {
                 check_err(err);
                 cache.get(key, function (err, result) {
                     assert.equal(result, value);
@@ -500,7 +533,7 @@ describe("caching", function () {
         it("allows us to pass in a module (uninstantiated)", function (done) {
             var store = memory_store;
             cache = caching({store: store});
-            cache.set(key, value, function (err) {
+            cache.set(key, value, ttl, function (err) {
                 check_err(err);
                 cache.get(key, function (err, result) {
                     assert.equal(result, value);
diff --git a/test/multi_caching.unit.js b/test/multi_caching.unit.js
index 6521d9a..3311ab7 100644
--- a/test/multi_caching.unit.js
+++ b/test/multi_caching.unit.js
@@ -21,6 +21,7 @@ describe("multi_caching", function () {
     var key;
     var memory_ttl;
     var name;
+    var ttl = 5;
 
     beforeEach(function () {
         memory_ttl = 0.1;
@@ -44,7 +45,7 @@ describe("multi_caching", function () {
 
         describe("set()", function () {
             it("lets us set data in all caches", function (done) {
-                multi_cache.set(key, value, function (err) {
+                multi_cache.set(key, value, ttl, function (err) {
                     check_err(err);
                     memory_cache.get(key, function (err, result) {
                         assert.equal(result, value);
@@ -64,6 +65,29 @@ describe("multi_caching", function () {
             });
 
             it("lets us set data without a callback", function (done) {
+                multi_cache.set(key, value, ttl);
+                setTimeout(function () {
+                    multi_cache.get(key, function (err, result) {
+                        assert.equal(result, value);
+                        memory_cache.get(key, function (err, result) {
+                            assert.equal(result, value);
+
+                            memory_cache2.get(key, function (err, result) {
+                                check_err(err);
+                                assert.equal(result, value);
+
+                                memory_cache3.get(key, function (err, result) {
+                                    check_err(err);
+                                    assert.equal(result, value);
+                                    done();
+                                });
+                            });
+                        });
+                    });
+                }, 20);
+            });
+
+            it("lets us set data without a ttl or callback", function (done) {
                 multi_cache.set(key, value);
                 setTimeout(function () {
                     multi_cache.get(key, function (err, result) {
@@ -89,7 +113,7 @@ describe("multi_caching", function () {
 
         describe("get()", function () {
             it("gets data from first cache that has it", function (done) {
-                memory_cache3.set(key, value, function (err) {
+                memory_cache3.set(key, value, ttl, function (err) {
                     check_err(err);
 
                     multi_cache.get(key, function (err, result) {
@@ -103,7 +127,7 @@ describe("multi_caching", function () {
 
         describe("del()", function () {
             it("lets us delete data in all caches", function (done) {
-                multi_cache.set(key, value, function (err) {
+                multi_cache.set(key, value, ttl, function (err) {
                     check_err(err);
 
                     multi_cache.del(key, function (err) {
@@ -128,7 +152,7 @@ describe("multi_caching", function () {
             });
 
             it("lets us delete data without a callback", function (done) {
-                multi_cache.set(key, value, function (err) {
+                multi_cache.set(key, value, ttl, function (err) {
                     check_err(err);
 
                     multi_cache.del(key);
@@ -160,14 +184,38 @@ describe("multi_caching", function () {
                 multi_cache = multi_caching([memory_cache3]);
             });
 
-            it("calls back with the result of a function", function (done) {
-                multi_cache.wrap(key, function (cb) {
-                    methods.get_widget(name, cb);
-                }, function (err, widget) {
-                    check_err(err);
-                    assert.deepEqual(widget, {name: name});
-                    done();
+            context("calls back with the result of a function", function () {
+
+                beforeEach(function () {
+                    sinon.spy(memory_cache3.store, 'set');
+                });
+
+                afterEach(function () {
+                    memory_cache3.store.set.restore();
                 });
+
+                it('when a ttl is passed in', function (done) {
+                    multi_cache.wrap(key, function (cb) {
+                        methods.get_widget(name, cb);
+                    }, ttl, function (err, widget) {
+                        check_err(err);
+                        assert.deepEqual(widget, {name: name});
+                        sinon.assert.calledWith(memory_cache3.store.set, key, {name: name}, ttl);
+                        done();
+                    });
+                });
+
+                it('when a ttl is not passed in', function (done) {
+                    multi_cache.wrap(key, function (cb) {
+                        methods.get_widget(name, cb);
+                    }, function (err, widget) {
+                        check_err(err);
+                        assert.deepEqual(widget, {name: name});
+                        sinon.assert.calledWith(memory_cache3.store.set, key, {name: name});
+                        done();
+                    });
+                });
+
             });
 
             context("when wrapped function calls back with an error", function () {
@@ -226,7 +274,7 @@ describe("multi_caching", function () {
 
             context("when value exists in first store but not second", function () {
                 it("returns value from first store, does not set it in second", function (done) {
-                    memory_cache.set(key, {name: name}, function (err) {
+                    memory_cache.set(key, {name: name}, ttl, function (err) {
                         check_err(err);
 
                         multi_cache.wrap(key, function (cb) {
@@ -247,7 +295,7 @@ describe("multi_caching", function () {
 
             context("when value exists in second store but not first", function () {
                 it("returns value from second store, sets it in first store", function (done) {
-                    memory_cache3.set(key, {name: name}, function (err) {
+                    memory_cache3.set(key, {name: name}, ttl, function (err) {
                         check_err(err);
 
                         multi_cache.wrap(key, function (cb) {
@@ -309,7 +357,7 @@ describe("multi_caching", function () {
 
             context("when value exists in first store only", function () {
                 it("returns value from first store, does not set it in second or third", function (done) {
-                    memory_cache.set(key, {name: name}, function (err) {
+                    memory_cache.set(key, {name: name}, ttl, function (err) {
                         check_err(err);
 
                         multi_cache.wrap(key, function (cb) {
@@ -335,7 +383,7 @@ describe("multi_caching", function () {
 
             context("when value exists in second store only", function () {
                 it("returns value from second store, sets it in first store, does not set third store", function (done) {
-                    memory_cache3.set(key, {name: name}, function (err) {
+                    memory_cache3.set(key, {name: name}, ttl, function (err) {
                         check_err(err);
 
                         multi_cache.wrap(key, function (cb) {
@@ -361,7 +409,7 @@ describe("multi_caching", function () {
 
             context("when value exists in third store only", function () {
                 it("returns value from third store, sets it in first and second stores", function (done) {
-                    memory_cache2.set(key, {name: name}, function (err) {
+                    memory_cache2.set(key, {name: name}, ttl, function (err) {
                         check_err(err);
 
                         multi_cache.wrap(key, function (cb) {
@@ -425,7 +473,7 @@ describe("multi_caching", function () {
                 it("bubbles up that error", function (done) {
                     var fake_error = new Error(support.random.string());
 
-                    sinon.stub(memory_store_stub, 'set', function (key, val, cb) {
+                    sinon.stub(memory_store_stub, 'set', function (key, val, ttl, cb) {
                         cb(fake_error);
                     });
 
diff --git a/test/support.js b/test/support.js
index 84c6db7..eccab71 100644
--- a/test/support.js
+++ b/test/support.js
@@ -90,8 +90,9 @@ var support = {
     test_set_get_del: function (cache, cb) {
         var key = 'TEST' + support.random.string();
         var val = support.random.string();
+        var ttl;
 
-        cache.set(key, val, function (err) {
+        cache.set(key, val, ttl, function (err) {
             if (err) { return cb(err); }
 
             cache.get(key, function (err, result) {