DRY: Don't Repeat Yourself (thelinmichael#123)

* Moved the optional callback logic to "performRequest" * Refactored header and auth request bulding * Removed “addHeaders” and made repeatable calls to “withHeaders” possible * Made withAuth part of the request builder * Refactored legacy options/callback handler * Don't assign empty object * Refactored query parameters handling * Refactored body parameters handling * Allow multiple arguments to with[body|Query|Headers] builders * Made token part of WebApiRequest builder * A built request can now execute itself * Limit repetition in base-request with factories * Refactored getQueryParameterString and wrote tests for it
atoami · Sep 4, 2017 · e464b88 · e464b88
1 parent 9578ca7
commit e464b88
Show file tree

Hide file tree

Showing 7 changed files with 421 additions and 1,304 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## Change log
 
+
 #### 2.5.0 (4 Sep 2017)
 - Change README to reflect new authorization. Thanks [@arirawr](https://github.com/arirawr) for the [PR](https://github.com/thelinmichael/spotify-web-api-node/pull/146).
 - Add support for 'show_dialog' parameter when creating authorization url. Thanks [@ajhaupt7](https://github.com/ajhaupt7) for [the PR](https://github.com/thelinmichael/spotify-web-api-node/pull/101).

diff --git a/src/base-request.js b/src/base-request.js
@@ -14,33 +14,23 @@ var Request = function(builder) {
  this.path = builder.path;
 };
 
-Request.prototype.getHost = function() {
- return this.host;
+Request.prototype._getter = function (key) {
+ return function () { return this[key]; };
 };
 
-Request.prototype.getPort = function() {
- return this.port;
-};
+Request.prototype.getHost = Request.prototype._getter('host');
 
-Request.prototype.getScheme = function() {
- return this.scheme;
-};
+Request.prototype.getPort = Request.prototype._getter('port');
 
-Request.prototype.getPath = function() {
- return this.path;
-};
+Request.prototype.getScheme = Request.prototype._getter('scheme');
 
-Request.prototype.getQueryParameters = function() {
- return this.queryParameters;
-};
+Request.prototype.getPath = Request.prototype._getter('path');
 
-Request.prototype.getBodyParameters = function() {
- return this.bodyParameters;
-};
+Request.prototype.getQueryParameters = Request.prototype._getter('queryParameters');
 
-Request.prototype.getHeaders = function() {
- return this.headers;
-};
+Request.prototype.getBodyParameters = Request.prototype._getter('bodyParameters');
+
+Request.prototype.getHeaders = Request.prototype._getter('headers');
 
 Request.prototype.getURI = function() {
  if (!this.scheme || !this.host || !this.port) {
@@ -66,99 +56,82 @@ Request.prototype.getURL = function() {
  }
 };
 
-Request.prototype.addQueryParameters = function(queryParameters) {
- for (var key in queryParameters) {
- this.addQueryParameter(key, queryParameters[key]);
- }
-};
-
-Request.prototype.addQueryParameter = function(key, value) {
- if (!this.queryParameters) {
- this.queryParameters = {};
- }
- this.queryParameters[key] = value;
-};
-
-Request.prototype.addBodyParameters = function(bodyParameters) {
- for (var key in bodyParameters) {
- this.addBodyParameter(key, bodyParameters[key]);
- }
-};
-
-Request.prototype.addBodyParameter = function(key, value) {
- if (!this.bodyParameters) {
- this.bodyParameters = {};
- }
- this.bodyParameters[key] = value;
-};
-
-Request.prototype.addHeaders = function(headers) {
- if (!this.headers) {
- this.headers = headers;
- } else {
- for (var key in headers) {
- this.headers[key] = headers[key];
- }
+Request.prototype.getQueryParameterString = function() {
+ var queryParameters = this.getQueryParameters();
+ if (queryParameters) {
+ return '?' + Object.keys(queryParameters).filter(function (key) {
+ return queryParameters[key] !== undefined;
+ }).map(function (key) {
+ return key + '=' + queryParameters[key];
+ }).join('&');
  }
 };
 
-Request.prototype.getQueryParameterString = function() {
- var queryParameters = this.getQueryParameters();
- if (!queryParameters) {
+Request.prototype.execute = function (method, callback) {
+ if (callback) {
+  method(this, callback);
  return;
  }
- var queryParameterString = '?';
- var first = true;
- for (var key in queryParameters) {
- if (queryParameters.hasOwnProperty(key)) {
- if (!first) {
- queryParameterString += '&';
+ var _self = this;
+
+ return new Promise(function(resolve, reject) {
+ method(_self, function(error, result) {
+ if (error) {
+ reject(error);
  } else {
- first = false;
+ resolve(result);
  }
- queryParameterString += key + '=' + queryParameters[key];
- }
- }
- return queryParameterString;
+ });
+ });
 };
 
 var Builder = function() {
- var host, port, scheme, queryParameters, bodyParameters, headers, jsonBody;
 };
 
-Builder.prototype.withHost = function(host) {
- this.host = host;
- return this;
+Builder.prototype._setter = function (key) {
+ return function (value) {
+ this[key] = value;
+ return this;
+ };
 };
 
-Builder.prototype.withPort = function(port) {
- this.port = port;
- return this;
-};
+Builder.prototype.withHost = Builder.prototype._setter('host');
 
-Builder.prototype.withScheme = function(scheme) {
- this.scheme = scheme;
- return this;
-};
+Builder.prototype.withPort = Builder.prototype._setter('port');
 
-Builder.prototype.withQueryParameters = function(queryParameters) {
- this.queryParameters = queryParameters;
- return this;
-};
+Builder.prototype.withScheme = Builder.prototype._setter('scheme');
 
-Builder.prototype.withPath = function(path) {
- this.path = path;
- return this;
+Builder.prototype.withPath = Builder.prototype._setter('path');
+
+Builder.prototype._assigner = function (key) {
+ return function () {
+ for (var i = 0; i < arguments.length; i++) {
+ this[key] = this._assign(this[key], arguments[i]);
+ }
+ return this;
+ };
 };
 
-Builder.prototype.withBodyParameters = function(bodyParameters) {
- this.bodyParameters = bodyParameters;
+Builder.prototype.withQueryParameters = Builder.prototype._assigner('queryParameters');
+
+Builder.prototype.withBodyParameters = Builder.prototype._assigner('bodyParameters');
+
+Builder.prototype.withHeaders = Builder.prototype._assigner('headers');
+
+Builder.prototype.withAuth = function(accessToken) {
+ if (accessToken) {
+ this.withHeaders(
+ {'Authorization' : 'Bearer ' + accessToken}
+ );
+ }
  return this;
 };
 
-Builder.prototype.withHeaders = function(headers) {
- this.headers = headers;
- return this;
+Builder.prototype._assign = function(src, obj) {
+ if (obj && Object.keys(obj).length > 0) {
+ return Object.assign(src || {}, obj);
+ }
+ return src;
 };
 
 Builder.prototype.build = function() {

diff --git a/src/server-methods.js b/src/server-methods.js
@@ -13,29 +13,17 @@ module.exports = {
  * token type and time to expiration. If rejected, it contains an error object. Not returned if a callback is given.
  */
  clientCredentialsGrant: function(options, callback) {
- var request = AuthenticationRequest.builder()
+ return AuthenticationRequest.builder()
  .withPath('/api/token')
  .withBodyParameters({
  'grant_type' : 'client_credentials'
  })
+ .withBodyParameters(options)
  .withHeaders({
  Authorization : ('Basic ' + new Buffer(this.getClientId() + ':' + this.getClientSecret()).toString('base64'))
  })
- .build();
-
- this._addBodyParameters(request, options);
-
- var promise = this._performRequest(HttpManager.post, request);
-
- if (callback) {
- promise.then(function(data) {
- callback(null, data);
- }, function(err) {
- callback(err);
- });
- } else {
- return promise;
- }
+ .build()
+ .execute(HttpManager.post, callback);
  },
 
  /**
@@ -48,7 +36,7 @@ module.exports = {
  * Not returned if a callback is given.
  */
  authorizationCodeGrant: function(code, callback) {
- var request = AuthenticationRequest.builder()
+ return AuthenticationRequest.builder()
  .withPath('/api/token')
  .withBodyParameters({
  'grant_type' : 'authorization_code',
@@ -57,19 +45,8 @@ module.exports = {
  'client_id' : this.getClientId(),
  'client_secret' : this.getClientSecret()
  })
- .build();
-
- var promise = this._performRequest(HttpManager.post, request);
-
- if (callback) {
- promise.then(function(data) {
- callback(null, data);
- }, function(err) {
- callback(err);
- });
- } else {
- return promise;
- }
+ .build()
+ .execute(HttpManager.post, callback);
  },
 
  /**
@@ -81,7 +58,7 @@ module.exports = {
  * Not returned if a callback is given.
  */
  refreshAccessToken: function(callback) {
- var request = AuthenticationRequest.builder()
+ return AuthenticationRequest.builder()
  .withPath('/api/token')
  .withBodyParameters({
  'grant_type' : 'refresh_token',
@@ -90,18 +67,7 @@ module.exports = {
  .withHeaders({
  Authorization : ('Basic ' + new Buffer(this.getClientId() + ':' + this.getClientSecret()).toString('base64'))
  })
- .build();
-
- var promise = this._performRequest(HttpManager.post, request);
-
- if (callback) {
- promise.then(function(data) {
- callback(null, data);
- }, function(err) {
- callback(err);
- });
- } else {
- return promise;
- }
+ .build()
+ .execute(HttpManager.post, callback);
  }
 };