Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup WPRequest and add tests #22

Merged
merged 8 commits into from
Jun 23, 2014
82 changes: 51 additions & 31 deletions lib/WPRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ function WPRequest( options ) {
*/
function noop() {}

/**
* Identity function for use within invokeAndPromisify()
*
* @private
*/
function identity( value ) {
return value;
}

/**
* If fn is a function, return it; else, return a no-op function
*
Expand All @@ -74,28 +83,54 @@ function ensureFunction( fn ) {
*
* @param {Object} request A superagent request object
* @param {Function} callback A callback function (optional)
* @param {Function} transform A function to transform the result data (optional)
* @return {Promise} A promise to the superagent request
*/
function invokeAndPromisify( request, callback ) {
function invokeAndPromisify( request, callback, transform ) {
callback = ensureFunction( callback );
transform = transform || identity;

// Return a promise, to enable chaining if so desired
return new Promise(function( resolve, reject ) {
// Fire off the result
request.end(function( err, result ) {

// Invoke the callback (if provided), to conform to standard Node pattern
callback( err, result );
callback( err, transform( result ) );

// Resolve the returned promise
// Return the results as a promise
if ( err || result.error ) {
reject( err || result.error );
} else {
resolve( result );
resolve( transform( result ) );
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bmac, @carldanley and I were talking about these lines—we've both seen code like this in which the actual reject and resolve are return'd, rather than just called. Is there any issue of which you are aware why we'd want to return these statements, likc

if ( err || result.error ) {
  return reject( err || result.error );
} else {
  return resolve( transform( result ) );
}

instead of just calling the functions?

}
});
});
}

/**
* Extract and return the body property from a superagent response object
*
* @private
*
* @param {Object} result The results from the HTTP request
* @return {Object} The "body" property of the result
*/
function returnBody( result ) {
return result.body;
}

/**
* Extract and return the headers property from a superagent response object
*
* @private
*
* @param {Object} result The results from the HTTP request
* @return {Object} The "headers" property of the result
*/
function returnHeaders( result ) {
return result.headers;
}

// Prototype Methods
// =================

Expand Down Expand Up @@ -178,9 +213,7 @@ WPRequest.prototype.get = function( callback ) {
this._checkMethodSupport( 'get' );
var url = this.generateRequestUri();

return invokeAndPromisify( agent.get( url ), function( err, result ) {
callback( err, result.body );
});
return invokeAndPromisify( agent.get( url ), callback, returnBody );
};

/**
Expand All @@ -202,9 +235,7 @@ WPRequest.prototype.post = function( data, callback ) {
.set( 'Authorization', auth )
.send( data );

return invokeAndPromisify( request, function( err, result ) {
callback( err, result.body );
});
return invokeAndPromisify( request, callback, returnBody );
};

/**
Expand All @@ -226,9 +257,7 @@ WPRequest.prototype.put = function( data, callback ) {
.set( 'Authorization', auth )
.send( data );

return invokeAndPromisify( request, function( err, result ) {
callback( err, result.body );
});
return invokeAndPromisify( request, callback, returnBody );
};

/**
Expand All @@ -246,24 +275,21 @@ WPRequest.prototype.patch = function( callback ) {
// todo: no idea what this method is supposed to do but it's documented in the WP-API docs.
};

// Cannot use `delete`: reserved word
/**
* @method remove
* @method delete
* @async
* @param {Function} [callback] A callback to invoke with the results of the DELETE request
* @param {Error|Object} callback.err Any errors encountered during the request
* @param {Object} callback.result The body of the server response
* @return {Promise} A promise to the results of the HTTP request
*/
WPRequest.prototype.remove = function( callback ) {
WPRequest.prototype.delete = function( callback ) {
this._checkMethodSupport( 'delete' );
var url = this.generateRequestUri();
var auth = this._options.username + ':' + this._options.password;
var request = agent.del( url ).set( 'Authorization', auth );

invokeAndPromisify( request, function( err, result ) {
callback( err, result.body );
});
return invokeAndPromisify( request, callback, returnBody );
};

/**
Expand All @@ -278,27 +304,21 @@ WPRequest.prototype.head = function( callback ) {
this._checkMethodSupport( 'head' );
var url = this.generateRequestUri();

return invokeAndPromisify( agent.head( url ), function( err, result ) {
callback( err, result.headers );
});
return invokeAndPromisify( agent.head( url ), callback, returnHeaders );
};

// Calling .then on a query chain will invoke it as a GET and return a promise

/**
* Calling .then on a query chain will invoke it as a GET and return a promise
*
* @method then
* @async
* @param {Function} [callback] A callback to invoke with the results of the GET request
* @param {Object} callback.results The body of the server response
* @return {Promise} A promise to the results of the HTTP request
*/
WPRequest.prototype.then = function( callback ) {
this._checkMethodSupport( 'get' );
var url = this.generateRequestUri();

return invokeAndPromisify( agent.get( url ) )
.then(function( result ) {
return callback( result.body );
});
return this.get().then( callback );
};

module.exports = WPRequest;
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"jshint-stylish": "^0.2.0",
"load-grunt-tasks": "^0.6.0",
"mocha": "^1.20.1",
"sandboxed-module": "^1.0.0",
"sinon": "^1.10.2",
"sinon-chai": "^2.5.0"
}
Expand Down
Loading