From 9c2ae65bc4e4a4350cb51532ec6cb1e2eaed1f4a Mon Sep 17 00:00:00 2001
From: Jonathan <jmuller@dial-once.com>
Date: Mon, 18 Jan 2016 12:59:27 +0100
Subject: [PATCH] More coverage + refactor promise usage on multi cache getters

---
 lib/multi_caching.js       | 36 +++++++++++++++---------------------
 test/multi_caching.unit.js | 28 +++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/lib/multi_caching.js b/lib/multi_caching.js
index 4444bf4..dd552a4 100644
--- a/lib/multi_caching.js
+++ b/lib/multi_caching.js
@@ -53,6 +53,8 @@ var multiCaching = function(caches, options) {
             options = {};
         }
 
+        var defer = Promise.defer();
+
         var i = 0;
         async.eachSeries(caches, function(cache, next) {
             var callback = function(err, result) {
@@ -66,9 +68,8 @@ var multiCaching = function(caches, options) {
                     // break out of async loop.
                     if (typeof cb === 'function') {
                         return cb(err, result, i);
-                    } else {
-                        return cb.resolve(result);
                     }
+                    return defer.resolve(result);
                 }
 
                 i += 1;
@@ -77,16 +78,17 @@ var multiCaching = function(caches, options) {
 
             cache.store.get(key, options, callback);
         }, function(err, result) {
-            if (typeof cb === 'object') {
-                if (err) {
-                    cb.reject(err);
-                } else {
-                    cb.resolve(result);
-                }
-            } else if (typeof cb === 'function') {
+            if (typeof cb === 'function') {
                 cb(err, result);
             }
+
+            if (err) {
+                return defer.reject(err);
+            }
+            defer.resolve(result);
         });
+
+        return defer.promise;
     }
 
     function setInMultipleCaches(caches, opts, cb) {
@@ -120,19 +122,15 @@ var multiCaching = function(caches, options) {
      * @param {function} cb
      */
     self.getAndPassUp = function(key, cb) {
-        getFromHighestPriorityCache(key, function(err, result, index) {
+        return getFromHighestPriorityCache(key, function(err, result, index) {
             if (err) {
-                if (typeof cb === 'function') {
+                if (cb) {
                     return cb(err);
-                } else {
-                    return cb.reject(err);
                 }
             }
 
-            if (typeof cb === 'function') {
+            if (cb) {
                 cb(err, result);
-            } else {
-                cb.resolve(result);
             }
 
             if (index) {
@@ -282,11 +280,7 @@ var multiCaching = function(caches, options) {
             options = {};
         }
 
-        var defer = Promise.defer();
-
-        getFromHighestPriorityCache(key, options, cb || defer);
-
-        return defer.promise;
+        return getFromHighestPriorityCache(key, options, cb);
     };
 
     /**
diff --git a/test/multi_caching.unit.js b/test/multi_caching.unit.js
index c50247e..955e623 100644
--- a/test/multi_caching.unit.js
+++ b/test/multi_caching.unit.js
@@ -183,7 +183,7 @@ describe("multiCaching", function() {
             });
 
             describe('using promises', function() {
-                it('should return a promise and resolve it', function(done) {
+                it('gets data from first cache that has it', function(done) {
                     memoryCache3.set(key, value)
                     .then(function() {
                         return multiCache.get(key);
@@ -193,6 +193,23 @@ describe("multiCaching", function() {
                     })
                     .then(done);
                 });
+
+                it("passes any options to underlying caches", function(done) {
+                    var opts = {foo: 'bar'};
+
+                    multiCache.set(key, value)
+                    .then(function() {
+                        sinon.spy(memoryCache.store, 'get');
+                        return multiCache.get(key, opts);
+                    })
+                    .then(function(result) {
+                        assert.equal(result, value);
+                        assert.ok(memoryCache.store.get.calledWith(key, opts));
+
+                        memoryCache.store.get.restore();
+                    })
+                    .then(done);
+                });
             });
         });
 
@@ -360,6 +377,15 @@ describe("multiCaching", function() {
                         done();
                     });
                 });
+
+                it("bubbles up errors from caches and reject promise", function(done) {
+                    multiCache.getAndPassUp(key)
+                    .catch(function(err) {
+                        assert.ok(memoryStoreStub.get.called);
+                        assert.equal(err, fakeError);
+                        done();
+                    });
+                });
             });
         });
     });