Cache duplicate HTTP requests

index.js

@@ -1,11 +1,17 @@
 const nodeFetch = require( 'node-fetch' );
+const getCacheHandler = require( './lib/cache-handler' );
 const { fetchNotifications, getAdditionalDataFetcher } = require( './lib/fetchers' );
 const { makeConverter } = require( './lib/converter' );
+const noop = () => {};
 
 function createNoteGetter( options = {} ) {
+	const cacheHandler = getCacheHandler();
 	const defaultOptions = {
+		getCachedResponseFor: cacheHandler.getCachedResponseFor,
+		cacheResponseFor: cacheHandler.cacheResponseFor,
 		fetch: nodeFetch,
-		log: () => {},
+		log: noop,
+		notificationsUrl: 'https://api.github.com/notifications',
 	};
 	const getterOptions = Object.assign( {}, defaultOptions, options );
 	const convertToGitnews = makeConverter();

lib/cache-handler.js

@@ -0,0 +1,25 @@
+const debugFactory = require( 'debug' );
+
+const debug = debugFactory( 'cache-handler' );
+
+function getCacheHandler() {
+	const responseCache = {};
+
+	function getCachedResponseFor( url ) {
+		const cachedResponse = responseCache[ url ];
+		cachedResponse ? debug( `< using cached response for url '${ url }'` ) : debug( `x no cached response for url '${ url }'` );
+		return cachedResponse;
+	}
+
+	function cacheResponseFor( url, data ) {
+		debug( `> caching response for url '${ url }'` );
+		responseCache[ url ] = data;
+	}
+
+	return {
+		getCachedResponseFor,
+		cacheResponseFor,
+	};
+}
+
+module.exports = getCacheHandler;

lib/fetchers.js

@@ -15,22 +15,50 @@ function convertToJson( result ) {
 	return result.json();
 }
 
+/**
+ * Return a Promise of notifications
+ *
+ * Requires `getter` to have the following properties:
+ * - log Function A function that logs status output
+ * - getCachedResponseFor Function A function that returns a cached response for a URL
+ * - cacheResponseFor Function A function that caches a response for a URL
+ * - fetch Function A function that implements the HTTP fetch API
+ * - notificationsUrl String the GitHub API notifications URL
+ *
+ * @param {Object} getter See above
+ * @param {String} token The API token to send to the getter
+ * @param {Object} params params object to include in the fetch URL
+ * @return {Promise} An array of notifications
+ */
 function fetchNotifications( getter, token, params = {} ) {
 	if ( ! token ) {
 		const err = new Error( 'GitHub token is not available' );
 		err.code = 'GitHubTokenNotFound';
 		return Promise.reject( err );
 	}
 	getter.log( 'fetching notifications...' );
-	return getter.fetch( withQuery( 'https://api.github.com/notifications', params ), getFetchInit( token ) )
+	const url = withQuery( getter.notificationsUrl, params );
+	const cachedResponse = getter.getCachedResponseFor( url );
+	if ( cachedResponse ) {
+		return Promise.resolve( cachedResponse );
+	}
+	return getter.fetch( url, getFetchInit( token ) )
 		.then( checkForHttpErrors )
 		.then( convertToJson )
 		.then( checkForErrors )
-		.then( checkForArray );
+		.then( checkForArray )
+		.then( response => {
+			getter.cacheResponseFor( url, response );
+			return response;
+		} );
 }
 
 function fetchNotificationSubjectUrl( getter, token, note ) {
 	const url = get( note, 'api.notification.subject.url', '' );
+	const cachedResponse = getter.getCachedResponseFor( url );
+	if ( cachedResponse ) {
+		return Promise.resolve( cachedResponse );
+	}
 	getter.log( `fetching subject data for ${ url }` );
 	return getter.fetch( url, getFetchInit( token ) )
 		.then( checkForHttpErrors )
@@ -40,12 +68,20 @@ function fetchNotificationSubjectUrl( getter, token, note ) {
 			note.api.subject = subject;
 			note.subjectUrl = get( subject, 'html_url' );
 			return note;
+		} )
+		.then( response => {
+			getter.cacheResponseFor( url, response );
+			return response;
 		} );
 }
 
 function fetchNotificationCommentData( getter, token, note ) {
 	const subject = get( note, 'api.notification.subject', {} );
 	const url = subject.latest_comment_url || subject.url || '';
+	const cachedResponse = getter.getCachedResponseFor( url );
+	if ( cachedResponse ) {
+		return Promise.resolve( cachedResponse );
+	}
 	getter.log( `fetching comment data for ${ url }` );
 	return getter.fetch( url, getFetchInit( token ) )
 		.then( checkForHttpErrors )
@@ -57,6 +93,10 @@ function fetchNotificationCommentData( getter, token, note ) {
 			note.commentUrl = get( comment, 'html_url' );
 			note.commentAvatar = get( comment, 'user.avatar_url' );
 			return note;
+		} )
+		.then( response => {
+			getter.cacheResponseFor( url, response );
+			return response;
 		} );
 }
 
@@ -73,7 +113,7 @@ function checkForHttpErrors( response ) {
 	if ( ! response.ok ) {
 		return Promise.reject( response );
 	}
-	return Promise.resolve( response );
+	return response;
 }
 
 function checkForArray( result ) {

package.json

@@ -9,15 +9,18 @@
   "author": "Payton Swick <[email protected]>",
   "license": "MIT",
   "dependencies": {
+    "debug": "^3.0.1",
     "lodash.get": "^4.4.2",
     "md5-hex": "^2.0.0",
-    "node-fetch": "^1.6.3",
+    "node-fetch": "1.7.3",
     "with-query": "^1.0.2"
   },
   "devDependencies": {
     "chai": "^4.1.0",
     "chai-subset": "^1.5.0",
-    "mocha": "^3.4.2"
+    "mocha": "^3.4.2",
+    "sinon": "^3.2.1",
+    "sinon-chai": "^2.13.0"
   },
   "repository": {
     "type": "git",

test/cache-handler.js

@@ -0,0 +1,18 @@
+/* global describe, it */
+const chai = require( 'chai' );
+const getCacheHandler = require( '../lib/cache-handler' );
+
+const { expect } = chai;
+
+describe( 'getCachedResponseFor()', function() {
+	it( 'returns undefined if there is no cache for a url', function() {
+		const { getCachedResponseFor } = getCacheHandler();
+		expect( getCachedResponseFor( 'url' ) ).to.be.undefined;
+	} );
+
+	it( 'returns cached data if there is a cache for a url', function() {
+		const { getCachedResponseFor, cacheResponseFor } = getCacheHandler();
+		cacheResponseFor( 'url', 'abcd' );
+		expect( getCachedResponseFor( 'url' ) ).to.eql( 'abcd' );
+	} );
+} );

test/fetchers.js

@@ -0,0 +1,37 @@
+/* global describe, it */
+const chai = require( 'chai' );
+const chaiSubset = require( 'chai-subset' );
+const sinon = require( 'sinon' );
+const sinonChai = require( 'sinon-chai' );
+const { fetchNotifications } = require( '../lib/fetchers' );
+const { Response } = require( 'node-fetch' );
+
+chai.use( chaiSubset );
+chai.use( sinonChai );
+const { expect } = chai;
+const noop = () => {};
+
+describe( 'fetchNotifications()', function() {
+	it( 'calls the fetch function if there is no cached response for that url', function() {
+		const mockFetch = sinon.stub().returns( Promise.resolve( new Response( '[{"foo":"bar"}]' ) ) );
+		return fetchNotifications( { fetch: mockFetch, log: noop, getCachedResponseFor: noop, cacheResponseFor: noop }, 'token' )
+		.then( () => {
+			expect( mockFetch ).to.have.been.called;
+		} );
+	} );
+
+	it( 'returns fetched Response correctly if there is no cached response for that url', function() {
+		const mockFetch = sinon.stub().returns( Promise.resolve( new Response( '[{"foo":"bar"}]' ) ) );
+		return fetchNotifications( { fetch: mockFetch, log: noop, getCachedResponseFor: noop, cacheResponseFor: noop }, 'token' )
+		.then( response => expect( response ).to.eql( [ { foo: 'bar' } ] ) );
+	} );
+
+	it( 'does not call the fetch function if there is a cached response for that url', function() {
+		const mockFetch = sinon.stub().returns( Promise.resolve( new Response( '[{"foo":"bar"}]' ) ) );
+		const getCachedResponseFor = () => ( [ { foo: 'bar' } ] );
+		return fetchNotifications( { fetch: mockFetch, log: noop, getCachedResponseFor, cacheResponseFor: noop }, 'token' )
+		.then( () => {
+			expect( mockFetch ).to.not.have.been.called;
+		} );
+	} );
+} );