refactored http-agent creation code

Added all the relevant code to the http-agent.js file and also updated
the unit tests for this module.

lib/http-agent.js

@@ -0,0 +1,82 @@
+/**
+ * This module limits the created of http.Agent instances to only one that
+ * allows connection pooling and another one that does not.
+ * The agents will be created on demand the first time and then the instances
+ * will be returned on subsequent calls to the getAgent method.
+ */
+const http = require('http');
+const https = require('https');
+
+const DEFAULT_KEEP_ALIVE_MSECS = 60000;
+const DEFAULT_KEEP_ALIVE_MAX_SOCKETS = 32;
+const DEFAULT_KEEP_ALIVE_MAX_FREE_SOCKETS = 32;
+
+let keepAliveAgent;
+let defaultAgent;
+
+/**
+ * Retuns a new instance of the http.Agent
+ * @param {Boolean} isSecure is https?
+ * @param {Object} [options] can contain
+ *  - options.keepAlive {Boolean}
+ *  - options.keepAliveMsecs {Integer}
+ *  - options.maxSockets  {Integer}
+ *  - options.maxFreeSockets {Integer}
+ *
+ * @returns {http.Agent} Agent
+ */
+function createAgent(isSecure, options = {}) {
+	return new (isSecure ? https : http).Agent(options);
+}
+
+/**
+ * Returns an instance of the 'keep-alive' http.Agent that _ALLOWS_ connection
+ * pooling
+ * It will create a new instance only the first time is invoked
+ * @param {Boolean} isSecure is https?
+ *
+ * @returns {http.Agent} Agent
+ */
+function getKeepAliveAgent(isSecure) {
+	if (!keepAliveAgent) {
+		keepAliveAgent = createAgent(isSecure, {
+			keepAlive: true,
+			keepAliveMsecs: DEFAULT_KEEP_ALIVE_MSECS,
+			maxSockets: DEFAULT_KEEP_ALIVE_MAX_SOCKETS,
+			maxFreeSockets: DEFAULT_KEEP_ALIVE_MAX_FREE_SOCKETS
+		});
+	}
+
+	return keepAliveAgent;
+}
+
+/**
+ * Returns an instance of the default http.Agent that _DOES_NOT_ allow
+ * connection pooling
+ * It will create a new instance only the first time is invoked
+ * @param {Boolean} isSecure is https?
+ *
+ * @returns {http.Agent} Agent
+ */
+function getDefaultAgent(isSecure) {
+	if (!defaultAgent) {
+		defaultAgent = createAgent(isSecure);
+	}
+
+	return defaultAgent;
+	}
+
+	module.exports = {
+	/**
+	 * Returns the appropriate agent depending on the specified params
+	 * @param {Boolean} isSecure is https?
+	 * @param {Bollean} allowPooling should we keep connection alive?
+	 *
+	 * @returns {http.Agent} Agent
+	 */
+	getAgent: function (isSecure, allowPooling) {
+		return allowPooling ?
+			getKeepAliveAgent(isSecure) :
+			getDefaultAgent(isSecure);
+	}
+};

lib/request.js

@@ -1,12 +1,12 @@
 var
-	coreUtil = require('util'),
 	events = require('events'),
 	http = require('http'),
+	httpAgent = require('./http-agent'),
 	https = require('https'),
 	qs = require('querystring'),
 	url = require('url'),
-	utils = require('./utils'),
-	validation = require('./validation');
+	validation = require('./validation'),
+	util = require('util');
 
 const
 	DEFAULT_KEEP_ALIVE = true,
@@ -168,11 +168,10 @@ module.exports = (function (self) {
 						DEFAULT_KEEP_ALIVE);
 
 				// here we make sure that the appropriate http.Agent is used
-				if (augmented.keepAlive) {
-					augmented.agent = utils.getHTTPAgent.keepAlive(augmented.secure);
-				} else {
-					augmented.agent = utils.getHTTPAgent.default(augmented.secure);
-				}
+				augmented.agent = httpAgent.getAgent(
+					augmented.secure,
+					augmented.keepAlive
+				);
 
 				// create `path` from pathname and query.
 				augmented.path = validation.coalesce(
@@ -454,7 +453,7 @@ module.exports = (function (self) {
 	}
 
 	// enable events
-	coreUtil.inherits(Request, events.EventEmitter);
+	util.inherits(Request, events.EventEmitter);
 
 	// return the ability to create a new request
 	self.Request = Request;

package.json

@@ -37,6 +37,10 @@
     {
       "name": "Philippe Vachon-Rivard",
       "url": "https://github.com/privard"
+    },
+    {
+      "name": "Ian Calderon",
+      "url": "https://github.com/iancl"
     }
   ],
   "description": "SDK to make accessing PlayNetwork APIs when writing code in Javascript (Node.js) much easier.",

test/lib/http-agent.js

@@ -0,0 +1,87 @@
+/*eslint no-magic-numbers: 0*/
+/*eslint no-unused-expressions: 0*/
+const { expect } = require('chai');
+const httpAgent = require('../../lib/http-agent');
+
+describe('http-agent', () => {
+	describe('connection pooling', () => {
+		describe('https', () => {
+			it('should allow connection pooling', () => {
+				const isSecure = true;
+				const allowPooling = true;
+				const agent = httpAgent.getAgent(isSecure, allowPooling);
+
+				// agent must allow connection pooling
+				expect(agent.options.keepAlive).to.be.true;
+			});
+
+			it('should _not_ allow connection pooling', () => {
+				const isSecure = true;
+				const allowPooling = false;
+				const agent = httpAgent.getAgent(isSecure, allowPooling);
+
+				// if connection pooling is disabled then the keep alive field
+				// will not exist
+				expect(agent.options.keepAlive).to.be.undefined;
+			});
+		});
+
+		describe('http', () => {
+			it('should allow connection pooling', () => {
+				const isSecure = false;
+				const allowPooling = true;
+				const agent = httpAgent.getAgent(isSecure, allowPooling);
+
+				// agent must allow connection pooling
+				expect(agent.options.keepAlive).to.be.true;
+			});
+
+			it('should _not_ allow connection pooling', () => {
+				const isSecure = false;
+				const allowPooling = false;
+				const agent = httpAgent.getAgent(isSecure, allowPooling);
+
+				// if connection pooling is disabled then the keep alive field
+				// will not exist
+				expect(agent.options.keepAlive).to.be.undefined;
+			});
+		});
+	});
+
+	describe('agents should be reused', () => {
+		it('should reuse https agent', () => {
+			const maxIterations = 5;
+			const isSecure = true;
+			const allowPooling = true;
+			// get agent that should be reused
+			const agent = httpAgent.getAgent(isSecure, allowPooling);
+
+			// set a dummy param so that we can verify that next time the same
+			// agent is returned.
+			agent.foo = true;
+
+			for (let i = 0; i < maxIterations; i++) {
+				let fooAgent = httpAgent.getAgent(isSecure, allowPooling);
+				expect(fooAgent.foo).to.be.true;
+			}
+
+		});
+
+		it('should reuse http agent', () => {
+			const maxIterations = 5;
+			const isSecure = false;
+			const allowPooling = true;
+
+			// get agent that should be reused
+			const agent = httpAgent.getAgent(isSecure, allowPooling);
+			// set a dummy param so that we can verify that next time the same
+			// agent is returned.
+			agent.foo = true;
+
+			for (let i = 0; i < maxIterations; i++) {
+				let fooAgent = httpAgent.getAgent(isSecure, allowPooling);
+				expect(fooAgent.foo).to.be.true;
+			}
+		});
+	});
+});