From a9a0a3d203a1a4d5ba3a7aacde0238c109b8ad54 Mon Sep 17 00:00:00 2001
From: Bryan Donovan <bdondo@gmail.com>
Date: Sun, 17 May 2015 15:08:16 -0700
Subject: [PATCH] requiring options arg to be accepted in underlying cache
 stores instead of handling ttl ints and options. Removed deprecated methods.

---
 History.md                            | 11 ++++-
 examples/redis_example/redis_store.js |  5 +-
 lib/caching.js                        |  2 +-
 lib/index.js                          |  4 --
 lib/multi_caching.js                  | 59 +++++++---------------
 lib/stores/memory.js                  |  3 ++
 test/caching.unit.js                  | 71 ++++++++++++++-------------
 test/multi_caching.unit.js            | 12 ++---
 8 files changed, 75 insertions(+), 92 deletions(-)

diff --git a/History.md b/History.md
index 6e5cf0a..e9b8285 100644
--- a/History.md
+++ b/History.md
@@ -1,6 +1,13 @@
-- {next release} 2015-05-17
-  - By default, cache falsey values like `false`, `0`, and `null`, but not `undefined` (#25).
+- 1.0.0 2015-05-XX
+  - Added JSDOC generation (`make docs`)
+  - (Breaking change) 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.
+  - (Breaking change) Removed deprecated lower-case `multi_caching` export (use `multiCaching` instead).
+  - (Breaking change) Removed `multiCaching#get_and_pass_up` (use `getAndPassUp` instead).
+  - (Breaking change) Cache store methods must accept an `options` param (which can be ignored). Eg.,
+       `function set(key, val, options, cb) { }`
+  - (Breaking change) caching/multicaching methods no longer accept a `ttl` param. You must instead pass
+     in an options object which will be passed to the cache store's `set` method.
 
 - 0.19.0 2015-03-29
   - Pass dispose, length & stale options to lru-cache (#22). - @gmaclennan
diff --git a/examples/redis_example/redis_store.js b/examples/redis_example/redis_store.js
index 93c922b..247eca5 100644
--- a/examples/redis_example/redis_store.js
+++ b/examples/redis_example/redis_store.js
@@ -81,7 +81,10 @@ function redisStore(args) {
         });
     };
 
-    self.del = function(key, cb) {
+    self.del = function(key, options, cb) {
+        if (typeof options === 'function') {
+            cb = options;
+        }
         connect(function(err, conn) {
             if (err) { return cb(err); }
             conn.del(key, handleResponse(conn, cb));
diff --git a/lib/caching.js b/lib/caching.js
index 90ce10d..2927806 100644
--- a/lib/caching.js
+++ b/lib/caching.js
@@ -66,7 +66,7 @@ var caching = function(args) {
     self.wrap = function(key, work, options, cb) {
         if (typeof options === 'function') {
             cb = options;
-            options = undefined;
+            options = {};
         }
 
         var hasKey = callbackFiller.has(key);
diff --git a/lib/index.js b/lib/index.js
index b6c2ab1..4e482ee 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -1,10 +1,6 @@
 /** @namespace cacheManager */
 var cacheManager = {
     caching: require('./caching'),
-    // Deprecate
-    //jscs:disable requireCamelCaseOrUpperCaseIdentifiers
-    multi_caching: require('./multi_caching'), //backward compat
-    //jscs:enable requireCamelCaseOrUpperCaseIdentifiers
     multiCaching: require('./multi_caching')
 };
 
diff --git a/lib/multi_caching.js b/lib/multi_caching.js
index 55164c8..2d534c3 100644
--- a/lib/multi_caching.js
+++ b/lib/multi_caching.js
@@ -6,8 +6,6 @@ var CallbackFiller = require('./callback_filler');
 /**
  * Module that lets you specify a hierarchy of caches.
  *
- * @memberof cacheManager
- *
  * @param {array} caches - Array of caching objects.
  * @param {object} [options]
  * @param {function} [options.isCacheableValue] - A callback function which is called
@@ -36,7 +34,7 @@ var multiCaching = function(caches, options) {
     function getFromHighestPriorityCache(key, options, cb) {
         if (typeof options === 'function') {
             cb = options;
-            options = undefined;
+            options = {};
         }
 
         var i = 0;
@@ -54,21 +52,14 @@ var multiCaching = function(caches, options) {
                 next();
             };
 
-            if (typeof options === 'object') {
-                cache.store.get(key, options, callback);
-            } else {
-                cache.store.get(key, callback);
-            }
+            cache.store.get(key, options, callback);
         }, cb);
     }
 
     function setInMultipleCaches(caches, opts, cb) {
+        opts.options = opts.options || {};
         async.each(caches, function(cache, next) {
-            if (typeof opts.options === 'object') {
-                cache.store.set(opts.key, opts.value, opts.options, next);
-            } else {
-                cache.store.set(opts.key, opts.value, opts.ttl, next);
-            }
+            cache.store.set(opts.key, opts.value, opts.options, next);
         }, cb);
     }
 
@@ -97,13 +88,6 @@ var multiCaching = function(caches, options) {
         });
     };
 
-    /**
-     * This is for backward-compatibility
-     */
-    //jscs:disable requireCamelCaseOrUpperCaseIdentifiers
-    self.get_and_pass_up = self.getAndPassUp;
-    //jscs:enable requireCamelCaseOrUpperCaseIdentifiers
-
     /**
      * Wraps a function in one or more caches.
      * Has same API as regular caching module.
@@ -122,21 +106,15 @@ var multiCaching = function(caches, options) {
     self.wrap = function(key, work, options, cb) {
         if (typeof options === 'function') {
             cb = options;
-            options = undefined;
+            options = {};
         }
 
-        function getOptsForSet(result) {
-            var opts = {
+        function getOptsForSet(value) {
+            return {
                 key: key,
-                value: result,
+                value: value,
                 options: options
             };
-
-            if (typeof options !== 'object') {
-                opts.ttl = options;
-            }
-
-            return opts;
         }
 
         var hasKey = callbackFiller.has(key);
@@ -190,14 +168,17 @@ var multiCaching = function(caches, options) {
      * @param {function} cb
      */
     self.set = function(key, value, options, cb) {
+        if (typeof options === 'function') {
+            cb = options;
+            options = {};
+        }
+
         var opts = {
             key: key,
             value: value,
             options: options
         };
-        if (typeof options !== 'object') {
-            opts.ttl = options;
-        }
+
         setInMultipleCaches(caches, opts, cb);
     };
 
@@ -214,8 +195,9 @@ var multiCaching = function(caches, options) {
     self.get = function(key, options, cb) {
         if (typeof options === 'function') {
             cb = options;
-            options = false;
+            options = {};
         }
+
         getFromHighestPriorityCache(key, options, cb);
     };
 
@@ -232,14 +214,11 @@ var multiCaching = function(caches, options) {
     self.del = function(key, options, cb) {
         if (typeof options === 'function') {
             cb = options;
-            options = false;
+            options = {};
         }
+
         async.each(caches, function(cache, next) {
-            if (typeof options === 'object') {
-                cache.store.del(key, options, next);
-            } else {
-                cache.store.del(key, next);
-            }
+            cache.store.del(key, options, next);
         }, cb);
     };
 
diff --git a/lib/stores/memory.js b/lib/stores/memory.js
index defa2ea..cece561 100644
--- a/lib/stores/memory.js
+++ b/lib/stores/memory.js
@@ -16,6 +16,9 @@ var memoryStore = function(args) {
     var lruCache = new Lru(lruOpts);
 
     self.set = function(key, value, options, cb) {
+        if (typeof options === 'function') {
+            cb = options;
+        }
         lruCache.set(key, value);
         if (cb) {
             process.nextTick(cb);
diff --git a/test/caching.unit.js b/test/caching.unit.js
index e98f5ae..2a8d7e4 100644
--- a/test/caching.unit.js
+++ b/test/caching.unit.js
@@ -19,7 +19,7 @@ var methods = {
 describe("caching", function() {
     var cache;
     var key;
-    var ttl = 1;
+    var defaultTtl = 1;
     var name;
     var value;
 
@@ -33,7 +33,7 @@ describe("caching", function() {
                 });
 
                 it("lets us set and get data in cache", function(done) {
-                    cache.set(key, value, ttl, function(err) {
+                    cache.set(key, value, {ttl: defaultTtl}, function(err) {
                         checkErr(err);
 
                         cache.get(key, function(err, result) {
@@ -45,7 +45,7 @@ describe("caching", function() {
                 });
 
                 it("lets us set and get data without a callback", function(done) {
-                    cache.set(key, value, ttl);
+                    cache.set(key, value, {ttl: defaultTtl});
 
                     setTimeout(function() {
                         var result = cache.get(key);
@@ -54,7 +54,7 @@ describe("caching", function() {
                     }, 20);
                 });
 
-                it("lets us set and get data without a ttl or callback", function(done) {
+                it("lets us set and get data without options object or callback", function(done) {
                     cache.set(key, value);
 
                     setTimeout(function() {
@@ -74,7 +74,7 @@ describe("caching", function() {
                     cache = caching({store: store});
                     key = support.random.string(20);
                     value = support.random.string();
-                    cache.set(key, value, ttl, function(err) {
+                    cache.set(key, value, {ttl: defaultTtl}, function(err) {
                         checkErr(err);
                         done();
                     });
@@ -121,13 +121,13 @@ describe("caching", function() {
             cache = caching({store: 'memory'});
             key = support.random.string(20);
             value = support.random.string();
-            cache.set(key, value, ttl, function(err) {
+            cache.set(key, value, function(err) {
                 checkErr(err);
 
                 key2 = support.random.string(20);
                 value2 = support.random.string();
 
-                cache.set(key2, value2, ttl, done);
+                cache.set(key2, value2, done);
             });
         });
 
@@ -241,7 +241,7 @@ describe("caching", function() {
                 key = support.random.string(20);
                 savedKeys.push(key);
                 value = support.random.string();
-                cache.set(key, value, ttl, cb);
+                cache.set(key, value, cb);
             }, done);
         });
 
@@ -262,14 +262,15 @@ describe("caching", function() {
     describe("wrap()", function() {
         describe("using memory (lru-cache) store", function() {
             var memoryStoreStub;
+            var opts;
 
             beforeEach(function() {
-                ttl = 0.1;
-                memoryStoreStub = memoryStore.create({ttl: ttl});
+                opts = {ttl: 0.1};
+                memoryStoreStub = memoryStore.create(opts);
 
                 sinon.stub(memoryStore, 'create').returns(memoryStoreStub);
 
-                cache = caching({store: 'memory', ttl: ttl, ignoreCacheErrors: false});
+                cache = caching({store: 'memory', ttl: opts.ttl, ignoreCacheErrors: false});
                 key = support.random.string(20);
                 name = support.random.string();
             });
@@ -290,10 +291,10 @@ describe("caching", function() {
                 it("when a ttl is passed in", function(done) {
                     cache.wrap(key, function(cb) {
                         methods.getWidget(name, cb);
-                    }, ttl, function(err, widget) {
+                    }, opts, function(err, widget) {
                         checkErr(err);
                         assert.deepEqual(widget, {name: name});
-                        sinon.assert.calledWith(memoryStoreStub.set, key, {name: name}, ttl);
+                        sinon.assert.calledWith(memoryStoreStub.set, key, {name: name}, opts);
                         done();
                     });
                 });
@@ -304,7 +305,7 @@ describe("caching", function() {
                     }, function(err, widget) {
                         checkErr(err);
                         assert.deepEqual(widget, {name: name});
-                        sinon.assert.calledWith(memoryStoreStub.set, key, {name: name}, undefined);
+                        sinon.assert.calledWith(memoryStoreStub.set, key, {name: name}, {});
                         done();
                     });
                 });
@@ -314,7 +315,7 @@ describe("caching", function() {
                 function getCachedWidget(name, cb) {
                     cache.wrap(key, function(cacheCb) {
                         methods.getWidget(name, cacheCb);
-                    }, ttl, cb);
+                    }, opts, cb);
                 }
 
                 beforeEach(function(done) {
@@ -345,7 +346,7 @@ describe("caching", function() {
                             funcCalled = true;
                             cb(err, result);
                         });
-                    }, ttl, function(err, widget) {
+                    }, function(err, widget) {
                         checkErr(err);
                         assert.deepEqual(widget, {name: name});
                         assert.ok(memoryStoreStub.get.calledWith(key));
@@ -368,7 +369,7 @@ describe("caching", function() {
                     function getCachedFalseyValue(cb) {
                         cache.wrap(key, function(cacheCb) {
                             getFalseyValue(cacheCb);
-                        }, ttl, cb);
+                        }, cb);
                     }
 
                     beforeEach(function(done) {
@@ -424,7 +425,7 @@ describe("caching", function() {
                         getValue(name, function(err, result) {
                             cacheCb(err, result);
                         });
-                    }, ttl, cb);
+                    }, {ttl: defaultTtl}, cb);
                 }
 
                 beforeEach(function() {
@@ -490,9 +491,11 @@ describe("caching", function() {
             });
 
             it("expires cached result after ttl seconds", function(done) {
+                var ttl = 0.1;
+
                 cache.wrap(key, function(cb) {
                     methods.getWidget(name, cb);
-                }, ttl, function(err, widget) {
+                }, {ttl: ttl}, function(err, widget) {
                     checkErr(err);
                     assert.deepEqual(widget, {name: name});
 
@@ -529,7 +532,7 @@ describe("caching", function() {
                 it("bubbles up that error", function(done) {
                     cache.wrap(key, function() {
                         throw fakeError;
-                    }, ttl, function(err) {
+                    }, function(err) {
                         assert.equal(err, fakeError);
                         done();
                     });
@@ -547,7 +550,7 @@ describe("caching", function() {
 
                         cache.wrap(key, function(cb) {
                             methods.getWidget(name, cb);
-                        }, ttl, function(err) {
+                        }, function(err) {
                             assert.equal(err, fakeError);
                             memoryStoreStub.get.restore();
                             done();
@@ -557,7 +560,7 @@ describe("caching", function() {
 
                 context("and ignoreCacheErrors is set to true", function() {
                     it("does not bubble up that error", function(done) {
-                        cache = caching({store: 'memory', ttl: ttl, ignoreCacheErrors: true});
+                        cache = caching({store: 'memory', ttl: defaultTtl, ignoreCacheErrors: true});
 
                         var fakeError = new Error(support.random.string());
 
@@ -567,7 +570,7 @@ describe("caching", function() {
 
                         cache.wrap(key, function(cb) {
                             methods.getWidget(name, cb);
-                        }, ttl, function(err) {
+                        }, function(err) {
                             assert.equal(err, null);
                             memoryStoreStub.get.restore();
                             done();
@@ -587,7 +590,7 @@ describe("caching", function() {
 
                         cache.wrap(key, function(cb) {
                             methods.getWidget(name, cb);
-                        }, ttl, function(err) {
+                        }, function(err) {
                             assert.equal(err, fakeError);
                             memoryStoreStub.set.restore();
                             done();
@@ -597,16 +600,14 @@ describe("caching", function() {
 
                 context("and ignoreCacheErrors is set to true", function() {
                     it("does not bubbles up that error", function(done) {
-                        cache = caching({store: 'memory', ttl: ttl, ignoreCacheErrors: true});
+                        cache = caching({store: 'memory', ttl: defaultTtl, ignoreCacheErrors: true});
                         var fakeError = new Error(support.random.string());
 
-                        sinon.stub(memoryStoreStub, 'set', function(key, val, ttl, cb) {
-                            cb(fakeError);
-                        });
+                        sinon.stub(memoryStoreStub, 'set').yields(fakeError);
 
                         cache.wrap(key, function(cb) {
                             methods.getWidget(name, cb);
-                        }, ttl, function(err) {
+                        }, function(err) {
                             assert.equal(err, null);
                             memoryStoreStub.set.restore();
                             done();
@@ -624,7 +625,7 @@ describe("caching", function() {
 
                     cache.wrap(key, function(cb) {
                         methods.getWidget(name, cb);
-                    }, ttl, function(err, widget) {
+                    }, function(err, widget) {
                         methods.getWidget.restore();
                         assert.equal(err, fakeError);
                         assert.ok(!widget);
@@ -661,7 +662,7 @@ describe("caching", function() {
                 async.each(values, function(val, next) {
                     cache.wrap('key', function(cb) {
                         construct(val, cb);
-                    }, ttl, function(err, result) {
+                    }, function(err, result) {
                         assert.equal(result, 'value');
                         next(err);
                     });
@@ -683,9 +684,9 @@ describe("caching", function() {
 
     describe("instantiating with custom store", function() {
         it("allows us to pass in our own store object", function(done) {
-            var store = memoryStore.create({ttl: ttl});
+            var store = memoryStore.create({ttl: defaultTtl});
             cache = caching({store: store});
-            cache.set(key, value, ttl, function(err) {
+            cache.set(key, value, function(err) {
                 checkErr(err);
                 cache.get(key, function(err, result) {
                     assert.equal(result, value);
@@ -697,7 +698,7 @@ describe("caching", function() {
         it("allows us to pass in a path to our own store", function(done) {
             var storePath = '../lib/stores/memory';
             cache = caching({store: storePath});
-            cache.set(key, value, ttl, function(err) {
+            cache.set(key, value, {ttl: defaultTtl}, function(err) {
                 checkErr(err);
                 cache.get(key, function(err, result) {
                     assert.equal(result, value);
@@ -709,7 +710,7 @@ describe("caching", function() {
         it("allows us to pass in a module (uninstantiated)", function(done) {
             var store = memoryStore;
             cache = caching({store: store});
-            cache.set(key, value, ttl, function(err) {
+            cache.set(key, value, {ttl: defaultTtl}, function(err) {
                 checkErr(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 740bc14..c8e8d8a 100644
--- a/test/multi_caching.unit.js
+++ b/test/multi_caching.unit.js
@@ -355,9 +355,7 @@ describe("multiCaching", function() {
             context("when wrapped function calls back with an error", function() {
                 it("calls back with that error", function(done) {
                     var fakeError = new Error(support.random.string());
-                    sinon.stub(methods, 'getWidget', function(name, cb) {
-                        cb(fakeError, {name: name});
-                    });
+                    sinon.stub(methods, 'getWidget').yields(fakeError, {name: name});
 
                     multiCache.wrap(key, function(cb) {
                         methods.getWidget(name, cb);
@@ -745,9 +743,7 @@ describe("multiCaching", function() {
                 it("bubbles up that error", function(done) {
                     var fakeError = new Error(support.random.string());
 
-                    sinon.stub(memoryStoreStub, 'get', function(key, cb) {
-                        cb(fakeError);
-                    });
+                    sinon.stub(memoryStoreStub, 'get').yields(fakeError);
 
                     multiCache.wrap(key, function(cb) {
                         methods.getWidget(name, cb);
@@ -763,9 +759,7 @@ describe("multiCaching", function() {
                 it("bubbles up that error", function(done) {
                     var fakeError = new Error(support.random.string());
 
-                    sinon.stub(memoryStoreStub, 'set', function(key, val, ttl, cb) {
-                        cb(fakeError);
-                    });
+                    sinon.stub(memoryStoreStub, 'set').yields(fakeError);
 
                     multiCache.wrap(key, function(cb) {
                         methods.getWidget(name, cb);