From e0aa441857797b042d6df2229ae512b4a78a4fad Mon Sep 17 00:00:00 2001 From: Cem Staveley Date: Tue, 30 Jan 2018 11:35:19 +0000 Subject: [PATCH] Fix: apply cache name when sending stats events --- index.js | 2 +- lib/cache.js | 17 +++--------- lib/events.js | 15 +++++++++++ lib/maxAge.js | 4 +++ lib/staleIfError.js | 6 ++--- test/cache.js | 64 +++++++++++--------------------------------- test/maxAge.js | 56 +++++++++++++++++++++++++++++++++++++- test/staleIfError.js | 32 +++++++++++++++++++++- 8 files changed, 128 insertions(+), 68 deletions(-) create mode 100644 lib/events.js diff --git a/index.js b/index.js index 93caa3a..b95e88b 100644 --- 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 index 181a4f2..09d2ac0 100644 --- 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 new file mode 100644 index 0000000..a5f2a45 --- /dev/null +++ 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 index ed04086..f601702 100644 --- 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 index 11894d7..bc430c9 100644 --- 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 index 38b4716..e477a05 100644 --- 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 index 1efd524..9e8ff8b 100644 --- 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 index c21442f..9c2f181 100644 --- 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); + }); + }); });