Fix: apply cache name when sending stats events

bbc · Jan 30, 2018 · e0aa441 · e0aa441
1 parent ef6187c
commit e0aa441
Show file tree

Hide file tree

Showing 8 changed files with 128 additions and 68 deletions.
diff --git a/index.js b/index.js
@@ -2,7 +2,7 @@
 
 const maxAge = require('./lib/maxAge');
 const staleIfError = require('./lib/staleIfError');
-const events = require('./lib/cache').events;
+const events = require('./lib/events').emitter;
 
 module.exports = {
   maxAge,

diff --git a/lib/cache.js b/lib/cache.js
@@ -2,18 +2,12 @@
 
 const _ = require('lodash');
 const bluebird = require('bluebird');
-const EventEmitter = require('events');
+const events = require('./events');
 
 const VERSION = require('../package').version;
 
-const events = new EventEmitter();
 const TimeoutError = bluebird.TimeoutError;
 
-function emitEvent(event, name) {
-  const type = name ? `cache.${name}.${event}` : `cache.${event}`;
-  events.emit(type);
-}
-
 function createCacheKey(segment, id) {
   const versionedSegment = `http-transport:${VERSION}:${segment}`;
 
@@ -27,15 +21,10 @@ function getFromCache(cache, segment, id, opts) {
   let pending = new Promise((resolve, reject) => {
     cache.get(createCacheKey(segment, id), (err, cached) => {
       if (err) {
-        emitEvent('error', cache.name);
+        events.emitCacheEvent('error', opts);
         return reject(err);
       }
 
-      if (!cached) {
-        emitEvent('miss', cache.name);
-      } else {
-        emitEvent('hit', cache.name);
-      }
       resolve(cached);
     });
   });
@@ -49,7 +38,7 @@ function getFromCache(cache, segment, id, opts) {
 
   return pending.catch((err) => {
     if (err instanceof TimeoutError) {
-      emitEvent('timeout', cache.name);
+      events.emitCacheEvent('timeout', opts);
     }
 
     if (_.get(opts, 'ignoreCacheErrors', false)) {

diff --git a/lib/events.js b/lib/events.js
@@ -0,0 +1,15 @@
+'use strict';
+
+const EventEmitter = require('events');
+const events = new EventEmitter();
+
+function emitCacheEvent(event, opts) {
+  const name = opts && opts.name ? opts.name : null;
+  const type = name ? `cache.${name}.${event}` : `cache.${event}`;
+  events.emit(type);
+}
+
+module.exports = {
+  emitter: events,
+  emitCacheEvent
+};
diff --git a/lib/maxAge.js b/lib/maxAge.js
@@ -7,6 +7,7 @@ const parseCacheControl = require('./parseCacheControl');
 const getFromCache = require('./cache').getFromCache;
 const storeInCache = require('./cache').storeInCache;
 const toResponse = require('./toResponse');
+const events = require('./events');
 
 const MAX_AGE = 'max-age';
 const STALE_WHILST_REVALIDATE = 'stale-while-revalidate';
@@ -52,9 +53,12 @@ module.exports = (cache, opts) => {
           revalidate(cached, ctx.req.getUrl(), cache, opts);
         }
         ctx.res = toResponse(cached);
+        events.emitCacheEvent('hit', opts);
         return;
       }
 
+      events.emitCacheEvent('miss', opts);
+
       return next().then(() => {
         if (ctx.isStale || ctx.res.statusCode >= 400) return;
 

diff --git a/lib/staleIfError.js b/lib/staleIfError.js
@@ -5,7 +5,7 @@ const parseCacheControl = require('./parseCacheControl');
 const getFromCache = require('./cache').getFromCache;
 const storeInCache = require('./cache').storeInCache;
 const toResponse = require('./toResponse');
-const events = require('./cache').events;
+const events = require('./events');
 
 const STALE_IF_ERROR = 'stale-if-error';
 const MAX_AGE = 'max-age';
@@ -34,7 +34,7 @@ module.exports = (cache, opts) => {
         }
       })
       .catch((err) => {
-        return getFromCache(cache, SEGMENT, ctx.req.getUrl())
+        return getFromCache(cache, SEGMENT, ctx.req.getUrl(), opts)
           .catch((cacheErr) => {
             if (_.get(opts, 'ignoreCacheErrors', false)) throw err;
             throw cacheErr;
@@ -43,7 +43,7 @@ module.exports = (cache, opts) => {
             if (cached) {
               ctx.isStale = true;
               ctx.res = toResponse(cached);
-              events.emit('cache.stale');
+              events.emitCacheEvent('stale', opts);
               return;
             }
 

diff --git a/test/cache.js b/test/cache.js
@@ -7,7 +7,8 @@ const bluebird = require('bluebird');
 const sinon = require('sinon');
 const sandbox = sinon.sandbox.create();
 
-const { getFromCache, storeInCache, events } = require('../lib/cache');
+const { getFromCache, storeInCache } = require('../lib/cache');
+const events = require('../').events;
 
 const SEGMENT = 'body';
 const VERSION = require('../package').version;
@@ -113,60 +114,27 @@ describe('Cache', () => {
   });
 
   describe('events', () => {
-    it('emits events with the cache id when present', () => {
+    it('emits events with the cache name when present', () => {
       const cache = createCache();
-      cache.name = 'memory';
-
-      let cacheHit = false;
-      events.on('cache.memory.hit', () => {
-        cacheHit = true;
+      sandbox.stub(cache, 'get').callsFake(() => {
+        setTimeout(() => { }, 100);
       });
 
-      return cache
-        .startAsync()
-        .then(() => cache.setAsync(bodySegment, cachedResponse, 600))
-        .then(() => {
-          return getFromCache(cache, SEGMENT, ID)
-            .catch(assert.ifError)
-            .then(() => {
-              assert.ok(cacheHit);
-            });
-        });
-    });
-
-    it('emits a cache hit event', () => {
-      const cache = createCache();
-
-      let cacheHit = false;
-      events.on('cache.hit', () => {
-        cacheHit = true;
+      let cacheTimeout = false;
+      events.on('cache.ceych.timeout', () => {
+        cacheTimeout = true;
       });
 
-      return cache
-        .startAsync()
-        .then(() => cache.setAsync(bodySegment, cachedResponse, 600))
-        .then(() => {
-          return getFromCache(cache, SEGMENT, ID)
-            .catch(assert.ifError)
-            .then(() => {
-              assert.ok(cacheHit);
-            });
-        });
-    });
-
-    it('emits a cache miss event', () => {
-      const cache = createCache();
-
-      let cacheMiss = false;
-      events.on('cache.miss', () => {
-        cacheMiss = true;
-      });
+      const opts = {
+        name: 'ceych',
+        timeout: 50
+      };
 
       return cache.startAsync().then(() => {
-        return getFromCache(cache, SEGMENT, ID)
-          .catch(assert.ifError)
-          .then(() => {
-            assert.ok(cacheMiss);
+        return getFromCache(cache, SEGMENT, ID, opts)
+          .then(assert.ifError)
+          .catch(() => {
+            assert.ok(cacheTimeout);
           });
       });
     });

diff --git a/test/maxAge.js b/test/maxAge.js
@@ -11,6 +11,7 @@ const sinon = require('sinon');
 
 const sandbox = sinon.sandbox.create();
 const cache = require('../');
+const events = require('../').events;
 
 const api = nock('http://www.example.com');
 
@@ -100,7 +101,6 @@ describe('Max-Age', () => {
   });
 
   it('creates cache entries for item fetcher from another cache with the correct ttl', async () => {
-
     const nearCache = createCache();
     const farCache = createCache();
 
@@ -548,4 +548,58 @@ describe('Max-Age', () => {
         return cache.drop(bodySegment);
       });
   });
+
+  describe('Events', () => {
+    it('emits events with name when name option is present', () => {
+      const cache = createCache();
+      api.get('/').reply(200, defaultResponse.body, defaultHeaders);
+
+      let cacheMiss = false;
+      events.on('cache.ceych.miss', () => {
+        cacheMiss = true;
+      });
+
+      const opts = {
+        name: 'ceych'
+      };
+
+      return requestWithCache(cache, opts)
+        .then(() => {
+          assert.ok(cacheMiss);
+        });
+    });
+
+    it('emits a cache miss event', () => {
+      const cache = createCache();
+      api.get('/').reply(200, defaultResponse.body, defaultHeaders);
+
+      let cacheMiss = false;
+      events.on('cache.miss', () => {
+        cacheMiss = true;
+      });
+
+      return requestWithCache(cache)
+        .then(() => {
+          assert.ok(cacheMiss);
+        });
+    });
+
+    it('emits a cache hit event', () => {
+      const cache = createCache();
+      api.get('/').reply(200, defaultResponse.body, defaultHeaders);
+
+      let cacheHit = false;
+      events.on('cache.hit', () => {
+        cacheHit = true;
+      });
+
+      return requestWithCache(cache)
+        .then(() => {
+          return requestWithCache(cache)
+            .then(() => {
+              assert.ok(cacheHit);
+            });
+        });
+    });
+  });
 });
diff --git a/test/staleIfError.js b/test/staleIfError.js
@@ -12,7 +12,7 @@ const httpTransport = require('@bbc/http-transport');
 const toError = require('@bbc/http-transport-to-error');
 
 const cache = require('../');
-const events = require('../lib/cache').events;
+const events = require('../').events;
 
 const VERSION = require('../package').version;
 const api = nock('http://www.example.com');
@@ -274,4 +274,34 @@ describe('Stale-If-Error', () => {
         assert.ok(cacheStale);
       });
   });
+
+  it('emits a stale cache event with cache name when present', () => {
+    const opts = {
+      name: 'ceych'
+    };
+
+    let cacheStale = false;
+    events.on('cache.ceych.stale', () => {
+      cacheStale = true;
+    });
+
+    const cachedResponse = {
+      body: 'http-transport',
+      headers: defaultHeaders,
+      elapsedTime: 40,
+      url: 'http://www.example.com/',
+      statusCode: 200
+    };
+    const cache = createCache();
+
+    api.get('/').reply(500, defaultResponse.body, {});
+
+    return cache
+      .startAsync()
+      .then(() => cache.setAsync(bodySegment, cachedResponse, 7200))
+      .then(() => requestWithCache(cache, opts))
+      .then(() => {
+        assert.ok(cacheStale);
+      });
+  });
 });