feat: Applying code review

nodeshift · Mar 7, 2017 · 8445a24 · 8445a24
1 parent 0b717f6
commit 8445a24
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 15 deletions.
diff --git a/lib/circuit.js b/lib/circuit.js
@@ -170,11 +170,21 @@ class CircuitBreaker extends EventEmitter {
    */
   fire () {
     if (CACHE.get(this) !== undefined) {
-      this.emit('cacheHit');
+      /**
+       * Emitted when the circuit breaker is using the cache
+       * @event CircuitBreaker#cacheHits
+       */
+      this.emit('cacheHits');
       return CACHE.get(this);
-    } else {
-      this.emit('cacheMiss');
+    } else if (this.options.cache) {
+      /**
+       * Emitted when the circuit breaker is not using the cache but
+       * the cache option is enabled.
+       * @event CircuitBreaker#cacheHits
+       */
+      this.emit('cacheMisses');
     }
+
     const args = Array.prototype.slice.call(arguments);
     /**
      * Emitted when the circuit breaker action is executed
@@ -195,7 +205,7 @@ class CircuitBreaker extends EventEmitter {
 
     let timeout;
     let timeoutError = false;
-    const value = new this.Promise((resolve, reject) => {
+    return new this.Promise((resolve, reject) => {
       timeout = setTimeout(
         () => {
           timeoutError = true;
@@ -223,6 +233,9 @@ class CircuitBreaker extends EventEmitter {
                */
               this.emit('success', result);
               resolve(result);
+              if (this.options.cache) {
+                CACHE.set(this, promise);
+              }
               clearTimeout(timeout);
             }
           })
@@ -232,10 +245,6 @@ class CircuitBreaker extends EventEmitter {
         handleError(error, this, timeout, args, resolve, reject);
       }
     });
-    if (this.options.cache) {
-      CACHE.set(this, value);
-    }
-    return value;
   }
 
   /**

diff --git a/lib/status.js b/lib/status.js
@@ -36,20 +36,20 @@ class Status {
     /**
      * The number of times the cache is used
      */
-    this.cacheHit = 0;
+    this.cacheHits = 0;
     /**
      * The number of times the cache is missed
      */
-    this.cacheMiss = 0;
+    this.cacheMisses = 0;
     this[CIRCUIT_BREAKER] = circuit;
     circuit.on('success', () => this.successes++);
     circuit.on('failure', () => this.failures++);
     circuit.on('fallback', () => this.fallbacks++);
     circuit.on('timeout', () => this.timeouts++);
     circuit.on('fire', () => this.fires++);
     circuit.on('reject', () => this.rejects++);
-    circuit.on('cacheHit', () => this.cacheHit++);
-    circuit.on('cacheMiss', () => this.cacheMiss++);
+    circuit.on('cacheHits', () => this.cacheHits++);
+    circuit.on('cacheMisses', () => this.cacheMisses++);
   }
 }
 

diff --git a/test/test.js b/test/test.js
@@ -49,19 +49,19 @@ test('Using cache', (t) => {
   const breaker = cb(passFail, options);
 
   breaker.fire(expected)
-    .then((arg) => t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHit}:${breaker.status.cacheMiss}`))
+    .then((arg) => t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHits}:${breaker.status.cacheMisses}`))
     .catch(t.fail)
     .then(() => {
       breaker.fire(expected)
         .then((arg) => {
-          t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHit}:${breaker.status.cacheMiss}`);
+          t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHits}:${breaker.status.cacheMisses}`);
           breaker.clearCache();
         })
         .catch(t.fail)
         .then(() => {
           breaker.fire(expected)
             .then((arg) => {
-              t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHit}:${breaker.status.cacheMiss}`);
+              t.equals(arg, expected, `cache hits:misses ${breaker.status.cacheHits}:${breaker.status.cacheMisses}`);
             })
             .then(t.end)
             .catch(t.fail);
@@ -512,6 +512,23 @@ test('CircuitBreaker fallback as a CircuitBreaker that fails', (t) => {
     .catch((e) => t.equals(e, 'Error: -1 is < 0', 'Breaker should fail'))
     .then(t.end);
 });
+
+test('CircuitBreaker fallback as a CircuitBreaker', (t) => {
+  t.plan(1);
+  const options = {
+    maxFailures: 1,
+    resetTimeout: 100
+  };
+
+  const input = -1;
+  const breaker = cb(passFail, options);
+  breaker.fallback(cb((x) => x, options));
+
+  breaker.fire(input)
+    .then((v) => t.equals(v, input, 'Fallback value equals input'))
+    .then(t.end);
+});
+
 /**
  * Returns a promise that resolves if the parameter
  * 'x' evaluates to >= 0. Otherwise the returned promise fails.