move helpers to util and fix param serialization for querystrings. write more tests

README.md

@@ -65,12 +65,12 @@ For API calls that require nested params (eg: http://api.challonge.com/v1/docume
 
 ```js
 {
-	tournament: {
-		name: 'new_tournament_name',
-		url: 'new_tournament_url',
-		tournamentType: 'single elimination',
-	},
-	callback: function(err, data){}
+  tournament: {
+    name: 'new_tournament_name',
+    url: 'new_tournament_url',
+    tournamentType: 'single elimination',
+  },
+  callback: function(err, data){}
 }
 ```
 

lib/api/client.js

 const qs = require('querystring');
 const https = require('https');
 const errorHandler = require('./error-handler');
+const util = require('../util');
 
 /**
  * @class Client(options)
@@ -31,56 +32,6 @@ const Client = exports.Client = function(options) {
 	}
 };
 
-// serialize nested params to tournament[name] style
-function serializeProperties(obj) {
-	let compiledParams = '';
-	let serializedProperties = [];
-	for (let prop in obj) {
-		if (obj.hasOwnProperty(prop) && typeof obj[prop] === 'object' && obj[prop] !== null) {
-			for (let attr in obj[prop]) {
-				compiledParams += '&' + prop + '[' + attr + ']=' + encodeURIComponent(obj[prop][attr]);
-			}
-			serializedProperties.push(prop);
-		}
-	}
-	return {
-		serialized: compiledParams,
-		properties: serializedProperties
-	};
-}
-
-// resources generate props internal to https requests
-let propertiesToDelete = ['callback', 'path', 'method'];
-
-function camelToUnderscore(str) {
-	return str.replace(/\W+/g, '-')
-		.replace(/([a-z\d])([A-Z])/g, '$1_$2')
-		.toLowerCase();
-}
-
-function UnderscoreToCamel(str) {
-	return str.replace(/_([a-z])/g, g => g[1].toUpperCase());
-}
-
-function convertProperties(obj, conversionFunction) {
-	// determine which we want to check with to see if we should convert
-	const checkRegex = conversionFunction === UnderscoreToCamel ? /_/ : /[A-Z]/;
-	for (let prop in obj) {
-		if (obj.hasOwnProperty(prop)) {
-			// objects recurse
-			if (typeof obj[prop] === 'object' && obj[prop] !== null) {
-				obj[conversionFunction(prop)] = convertProperties(obj[prop], conversionFunction);
-			} else if (prop.search(checkRegex) > -1) {
-				obj[conversionFunction(prop)] = obj[prop];
-				delete obj[prop]; // remove it
-			}
-
-		//otherwise leave it alone
-		}
-	}
-	return obj;
-}
-
 Client.prototype.setSubdomain = function(subdomain) {
 	// generate the subdomain URL string if there is one
 	if (!subdomain) {
@@ -95,28 +46,22 @@ Client.prototype.setSubdomain = function(subdomain) {
 // cleans the passed in object, generates the API url/query-string, makes the request, delegates errors and calls callbacks
 Client.prototype.makeRequest = function(obj) {
 	const self = this;
-	// cache vars that are about to be removed
+
+	const propertiesToDelete = ['callback', 'path', 'method'];
+
+	// cache vars that are to be removed
 	const callback = obj.callback;
 	const method = obj.method;
 	let path = obj.path;
 
 	// normalize the rest of the properties
-	obj = convertProperties(obj, camelToUnderscore);
+	obj = util.convert(obj, util.camelToUnderscore);
 
 	// Add on the api key
-	obj.api_key = this.options.get('apiKey'); //convert for url
+	obj.api_key = this.options.get('apiKey');
 	obj.cache_bust = Math.random();
 
-	// serialize the properties
-	const serialized = serializeProperties(obj);
-
-	// get the non-standard-formatted properties (this is to support the tournament[score] kind of params the api expects)
-	const compiledParams = serialized.serialized;
-
-	// merge the stuff to remove
-	propertiesToDelete = propertiesToDelete.concat(serialized.properties);
-
-	// remove params
+	// remove internal properties
 	propertiesToDelete.forEach((prop) => {
 		delete obj[prop];
 	});
@@ -125,7 +70,10 @@ Client.prototype.makeRequest = function(obj) {
 	const versionPaths = {
 		1: '/v1/tournaments'
 	};
-	path = versionPaths[this.options.get('version')] + (path ? path : '') + '.' + this.options.get('format') + '?' + qs.stringify(obj) + compiledParams;
+
+	const serialized = util.serializeToQSParams(obj);
+
+	path = versionPaths[this.options.get('version')] + (path ? path : '') + '.' + this.options.get('format') + '?' + serialized
 
 	// create options for the https call
 	const options = {
@@ -154,7 +102,7 @@ Client.prototype.makeRequest = function(obj) {
 			if (self.options.get('format') == 'json') {
 				resData = JSON.parse(resData);
 				if (self.options.get('massageProperties')) {
-					resData = convertProperties(resData, UnderscoreToCamel);
+					resData = util.convert(resData, util.underscoreToCamel);
 				}
 			}
 			callback(null, resData); //no error, so no err object

lib/api/client.spec.js

@@ -215,7 +215,7 @@ describe('Client Class', () => {
 
 			expect(httpsMock.opts).toEqual({
 				hostname: 'api.challonge.com',
-				path: '/v1/tournaments/some/path.json?randomprop=thingie&api_key=mykey&cache_bust=1&someProperty[another_property]=anotherthing&some_property[another_property]=anotherthing',
+				path: '/v1/tournaments/some/path.json?randomprop=thingie&some_property[another_property]=anotherthing&api_key=mykey&cache_bust=1',
 				method: 'GET',
 				headers: {
 					'Content-Length': 0

lib/param-serializer.js

+/**
+ * @function serialize
+ * @param  {object} a a javascript object
+ * @return {string} the jquery-like querystring to send to the api
+ * @description
+ *   Ripped from https://github.com/knowledgecode/jquery-param
+ */
+
+/* istanbul ignore next */
+function serialize(a) {
+	const s = [];
+	const rbracket = /\[\]$/;
+	const isArray = function(obj) {
+		return Object.prototype.toString.call(obj) === '[object Array]';
+	};
+
+	function add(k, v) {
+		v = typeof v === 'function' ? v() : v === null ? '' : v === undefined ? '' : v;
+		s[s.length] = encodeURIComponent(k) + '=' + encodeURIComponent(v);
+	}
+
+	function buildParams(prefix, obj) {
+		let i;
+		let len;
+		let key;
+
+		if (prefix) {
+			if (isArray(obj)) {
+				for (i = 0, len = obj.length; i < len; i++) {
+					if (rbracket.test(prefix)) {
+						add(prefix, obj[i]);
+					} else {
+						buildParams(prefix + '[' + (typeof obj[i] === 'object' ? i : '') + ']', obj[i]);
+					}
+				}
+			} else if (obj && String(obj) === '[object Object]') {
+				for (key in obj) {
+					buildParams(prefix + '[' + key + ']', obj[key]);
+				}
+			} else {
+				add(prefix, obj);
+			}
+		} else if (isArray(obj)) {
+			for (i = 0, len = obj.length; i < len; i++) {
+				add(obj[i].name, obj[i].value);
+			}
+		} else {
+			for (key in obj) {
+				buildParams(key, obj[key]);
+			}
+		}
+		return s;
+	}
+
+	return decodeURIComponent(buildParams('', a).join('&').replace(/%20/g, '+'));
+};
+
+module.exports = serialize;

lib/util.js

+function camelToUnderscore(str) {
+	return str.replace(/\W+/g, '-')
+		.replace(/([a-z\d])([A-Z])/g, '$1_$2')
+		.toLowerCase();
+}
+
+function underscoreToCamel(str) {
+	return str.replace(/_([a-z])/g, g => g[1].toUpperCase());
+}
+
+function convert(obj, conversionFn, newObject) {
+	if (!obj) {
+		return obj;
+	}
+	if (!newObject) {
+		newObject = {};
+	}
+
+	Object.keys(obj).forEach((prop) => {
+		if (typeof obj[prop] === 'object') {
+			const convertObj = newObject[conversionFn(prop)] = {};
+			convert(obj[prop], conversionFn, convertObj);
+		} else {
+			newObject[conversionFn(prop)] = obj[prop]
+		}
+	});
+	return newObject;
+}
+
+module.exports = {
+	convert: convert,
+	camelToUnderscore: camelToUnderscore,
+	serializeToQSParams: require('./param-serializer'),
+	underscoreToCamel: underscoreToCamel
+};

lib/util.spec.js

+const util = require('./util');
+
+describe('util methods', () => {
+	describe('serializeToQSParams', () => {
+		it('should return a serialized string and a properties array', () => {
+			expect(util.serializeToQSParams({})).toEqual('');
+		});
+
+		it('should work with flat values', () => {
+			expect(util.serializeToQSParams({
+				prop: 'value',
+				anotherProp: 123,
+				someOtherProp: true
+			})).toEqual('prop=value&anotherProp=123&someOtherProp=true');
+		});
+
+		it('should flatten nested objects', () => {
+			expect(util.serializeToQSParams({
+				prop: {
+					nestedProp: 'nestedVal'
+				}
+			})).toEqual('prop[nestedProp]=nestedVal');
+		});
+
+		it('should not fail on null values', () => {
+			expect(util.serializeToQSParams({
+				prop: {
+					nestedProp: null
+				}
+			})).toEqual('prop[nestedProp]=');
+		});
+
+		it('should flatten deep nested', () => {
+			expect(util.serializeToQSParams({
+				prop: {
+					nestedProp: {
+						deepNestedProp: true
+					},
+					nestedVal: false
+				}
+			})).toEqual('prop[nestedProp][deepNestedProp]=true&prop[nestedVal]=false');
+		});
+	});
+
+	const tests = [
+		['myProperty', 'my_property'],
+		['myOtherProperty', 'my_other_property'],
+		['withNumbers9Thing', 'with_numbers9_thing'],
+		['9Prop', '9_prop']
+	];
+	describe('camelToUnderscore', () => {
+		const extraTests = [
+			['withLOW', 'with_low'],
+			['some kind of string', 'some-kind-of-string'],
+			['some kind of stringWithCamel', 'some-kind-of-string_with_camel']
+		];
+		it('should convert a variety of cases', () => {
+			const allTests = tests.concat(extraTests);
+			allTests.forEach((item, index) => {
+				expect(util.camelToUnderscore(item[0])).toEqual(item[1]);
+			});
+		});
+	});
+
+	describe('underscoreToCamel', () => {
+		it('should invert the same cases as camelToUnderscore', () => {
+			tests.forEach((item, index) => {
+				expect(util.underscoreToCamel(tests[index][1])).toEqual(tests[index][0]);
+			});
+		});
+	});
+
+	describe('convert', () => {
+		it('should convert properties with a conversion function', () => {
+			expect(util.convert({}, util.underscoreToCamel)).toEqual({});
+		});
+
+		it('should convert properties with a conversion function', () => {
+			expect(util.convert({
+				some_prop: 123
+			}, util.underscoreToCamel)).toEqual({
+				someProp: 123
+			});
+		});
+
+		it('should convert properties that are nested', () => {
+			expect(util.convert({
+				some_prop: {
+					another_thing: 123
+				}
+			}, util.underscoreToCamel)).toEqual({
+				someProp: {
+					anotherThing: 123
+				}
+			});
+		});
+		
+		it('should convert properties that are deeply', () => {
+			expect(util.convert({
+				some_prop: {
+					another_thing: {
+						one_more_level: 'a thingie'
+					}
+				}
+			}, util.underscoreToCamel)).toEqual({
+				someProp: {
+					anotherThing: {
+						oneMoreLevel: 'a thingie'
+					}
+				}
+			});
+		});
+	});
+
+	describe('conversion', () => {
+		it('should return what was passed if it is falsy', () => {
+			expect(util.convert(undefined)).toEqual(undefined);
+		})
+		it('should work with no props', () => {
+			expect(util.convert({}, util.underscoreToCamel)).toEqual({});
+		});
+
+		it('should work with a single layer props', () => {
+			expect(util.convert({
+				top_prop: 123
+			}, util.underscoreToCamel)).toEqual({
+				topProp: 123
+			});
+		});
+
+		it('should work with a nested props', () => {
+			expect(util.convert({
+				top_prop: {
+					middle_prop: 123
+				}
+			}, util.underscoreToCamel)).toEqual({
+				topProp: {
+					middleProp: 123
+				}
+			});
+		});
+
+		it('should work with both top and nested properties', () => {
+			expect(util.convert({
+				top_prop: {
+					middle_prop: 123
+				},
+				also_top: 'a string'
+			}, util.underscoreToCamel)).toEqual({
+				topProp: {
+					middleProp: 123
+				},
+				alsoTop: 'a string'
+			});
+		});
+
+		it('should work with a different conversion function', () => {
+			expect(util.convert({
+				topProp: {
+					middleProp: 123
+				},
+				alsoTop: 'a string'
+			}, util.camelToUnderscore)).toEqual({
+				top_prop: {
+					middle_prop: 123
+				},
+				also_top: 'a string'
+			});
+		});
+	});
+});

test/full.js

@@ -6,7 +6,7 @@ var client = challonge.createClient({
 	version: 1,
 });
 
-var tourneyName = 'mytesttournament' + Math.floor(Math.random()*10000);
+var tourneyName = 'new_api_test' + Math.floor(Math.random()*10000);
 
 client.tournaments.create({
 	tournament: {
@@ -19,10 +19,31 @@ client.tournaments.create({
 		if (err) { console.log(err); return; }
 		console.log(data);
 
-		pcreate('player1');
+		update();
 	}
 });
 
+function update() {
+	client.tournaments.update({
+		id: tourneyName,
+		tournament: {
+			name: 'name-'+tourneyName,
+			url: tourneyName,
+			signupCap: 16,
+			tournamentType: 'double elimination',
+			description: 'some new description',
+			acceptAttachments: true
+
+		},
+		callback: function(err,data){
+			if (err) { console.log(err); return; }
+			console.log(data);
+
+			pcreate('player1');
+		}
+	});
+}
+
 function pcreate(name) {
 	client.participants.create({
 		id: tourneyName,

test/test.js

@@ -6,7 +6,7 @@ var client = challonge.createClient({
 	version: 1,
 });
 
-var tourneyName = 'nodeapite3stcamel24333341111';
+var tourneyName = 'nodeapite3stcamel24333342221111';
 
 function index() {
 	client.tournaments.index({