Merge pull request #1520 from simov/refactor-defaults

Refactor defaults
request · Apr 5, 2015 · cfe9f5d · cfe9f5d
2 parents 204dbe0 + 98e47fb
commit cfe9f5d
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 112 deletions.
diff --git a/index.js b/index.js
@@ -19,22 +19,27 @@ var extend                = require('util')._extend
   , helpers               = require('./lib/helpers')
 
 var isFunction            = helpers.isFunction
-  , constructObject       = helpers.constructObject
-  , filterForCallback     = helpers.filterForCallback
-  , constructOptionsFrom  = helpers.constructOptionsFrom
   , paramsHaveRequestBody = helpers.paramsHaveRequestBody
 
 
 // organize params for patch, post, put, head, del
 function initParams(uri, options, callback) {
-  callback = filterForCallback([options, callback])
-  options = constructOptionsFrom(uri, options)
-
-  return constructObject()
-    .extend({callback: callback})
-    .extend({options: options})
-    .extend({uri: options.uri})
-    .done()
+  if (typeof options === 'function') {
+    callback = options
+  }
+
+  var params = {}
+  if (typeof options === 'object') {
+    params = extend({}, options)
+    params = extend(params, {uri: uri})
+  } else if (typeof uri === 'string') {
+    params = extend({}, {uri: uri})
+  } else {
+    params = extend({}, uri)
+  }
+
+  params.callback = callback
+  return params
 }
 
 function request (uri, options, callback) {
@@ -43,25 +48,22 @@ function request (uri, options, callback) {
   }
 
   var params = initParams(uri, options, callback)
-  options = params.options
-  options.callback = params.callback
-  options.uri = params.uri
 
-  if (params.options.method === 'HEAD' && paramsHaveRequestBody(params)) {
+  if (params.method === 'HEAD' && paramsHaveRequestBody(params)) {
     throw new Error('HTTP HEAD requests MUST NOT include a request body.')
   }
 
-  return new request.Request(options)
+  return new request.Request(params)
 }
 
 var verbs = ['get', 'head', 'post', 'put', 'patch', 'del']
 
-verbs.forEach(function(verb){
+verbs.forEach(function(verb) {
   var method = verb === 'del' ? 'DELETE' : verb.toUpperCase()
-  request[verb] = function(uri, options, callback){
+  request[verb] = function (uri, options, callback) {
     var params = initParams(uri, options, callback)
-    params.options.method = method
-    return (this || request)(params.uri || null, params.options, params.callback)
+    params.method = method
+    return request(params, params.callback)
   }
 })
 
@@ -73,77 +75,65 @@ request.cookie = function (str) {
   return cookies.parse(str)
 }
 
-request.defaults = function (options, requester) {
+function wrapRequestMethod (method, options, requester) {
 
-  if (typeof options === 'function') {
-    requester = options
-    options = {}
-  }
+  return function (uri, opts, callback) {
+    var params = initParams(uri, opts, callback)
 
-  var self = this
-  var wrap = function (method) {
-    var headerlessOptions = function (options) {
-      options = extend({}, options)
-      delete options.headers
-      return options
+    var headerlessOptions = extend({}, options)
+    delete headerlessOptions.headers
+    params = extend(headerlessOptions, params)
+
+    if (options.headers) {
+      var headers = extend({}, options.headers)
+      params.headers = extend(headers, params.headers)
     }
 
-    var getHeaders = function (params, options) {
-      return constructObject()
-        .extend(options.headers)
-        .extend(params.options.headers)
-        .done()
+    if (typeof method === 'string') {
+      params.method = (method === 'del' ? 'DELETE' : method.toUpperCase())
+      method = request[method]
     }
 
-    return function (uri, opts, callback) {
-      var params = initParams(uri, opts, callback)
-      params.options = extend(headerlessOptions(options), params.options)
+    if (isFunction(requester)) {
+      method = requester
+    }
 
-      if (typeof method === 'string') {
-        params.options.method = (method === 'del' ? 'DELETE' : method.toUpperCase())
-        method = request[method]
-      }
+    return method(params, params.callback)
+  }
+}
 
-      if (params.options.method === 'HEAD' && paramsHaveRequestBody(params)) {
-        throw new Error('HTTP HEAD requests MUST NOT include a request body.')
-      }
+request.defaults = function (options, requester) {
+  var self = this
 
-      if (options.headers) {
-        params.options.headers = getHeaders(params, options)
-      }
+  if (typeof options === 'function') {
+    requester = options
+    options = {}
+  }
 
-      if (isFunction(requester)) {
-        method = requester
-      }
+  var defaults      = wrapRequestMethod(self, options, requester)
 
-      return method(params.options, params.callback)
-    }
-  }
+  var verbs = ['get', 'head', 'post', 'put', 'patch', 'del']
+  verbs.forEach(function(verb) {
+    defaults[verb]  = wrapRequestMethod(verb, options, requester)
+  })
 
-  var defaults      = wrap(self)
-  defaults.get      = wrap('get')
-  defaults.patch    = wrap('patch')
-  defaults.post     = wrap('post')
-  defaults.put      = wrap('put')
-  defaults.head     = wrap('head')
-  defaults.del      = wrap('del')
-  defaults.cookie   = wrap(self.cookie)
+  defaults.cookie   = wrapRequestMethod(self.cookie, options, requester)
   defaults.jar      = self.jar
   defaults.defaults = self.defaults
   return defaults
 }
 
 request.forever = function (agentOptions, optionsArg) {
-  var options = constructObject()
+  var options = {}
   if (optionsArg) {
-    options.extend(optionsArg)
+    options = extend({}, optionsArg)
   }
   if (agentOptions) {
     options.agentOptions = agentOptions
   }
 
-  options.extend({forever: true})
-  return request.defaults(options.done())
+  options.forever = true
+  return request.defaults(options)
 }
 
 // Exports

diff --git a/lib/helpers.js b/lib/helpers.js
@@ -1,7 +1,6 @@
 'use strict'
 
-var extend = require('util')._extend
-  , jsonSafeStringify = require('json-stringify-safe')
+var jsonSafeStringify = require('json-stringify-safe')
   , crypto = require('crypto')
 
 function deferMethod() {
@@ -12,46 +11,16 @@ function deferMethod() {
   return setImmediate
 }
 
-function constructObject(initialObject) {
-  initialObject = initialObject || {}
-
-  return {
-    extend: function (object) {
-      return constructObject(extend(initialObject, object))
-    },
-    done: function () {
-      return initialObject
-    }
-  }
-}
-
-function constructOptionsFrom(uri, options) {
-  var params = constructObject()
-  if (typeof options === 'object') {
-    params.extend(options).extend({uri: uri})
-  } else if (typeof uri === 'string') {
-    params.extend({uri: uri})
-  } else {
-    params.extend(uri)
-  }
-  return params.done()
-}
-
 function isFunction(value) {
   return typeof value === 'function'
 }
 
-function filterForCallback(values) {
-  var callbacks = values.filter(isFunction)
-  return callbacks[0]
-}
-
 function paramsHaveRequestBody(params) {
   return (
-    params.options.body ||
-    params.options.requestBodyStream ||
-    (params.options.json && typeof params.options.json !== 'boolean') ||
-    params.options.multipart
+    params.body ||
+    params.requestBodyStream ||
+    (params.json && typeof params.json !== 'boolean') ||
+    params.multipart
   )
 }
 
@@ -78,9 +47,6 @@ function toBase64 (str) {
 }
 
 exports.isFunction            = isFunction
-exports.constructObject       = constructObject
-exports.constructOptionsFrom  = constructOptionsFrom
-exports.filterForCallback     = filterForCallback
 exports.paramsHaveRequestBody = paramsHaveRequestBody
 exports.safeStringify         = safeStringify
 exports.md5                   = md5

diff --git a/tests/test-defaults.js b/tests/test-defaults.js
@@ -273,9 +273,8 @@ tape('test custom request handler function', function(t) {
     body: 'TESTING!'
   }, function(uri, options, callback) {
     var params = request.initParams(uri, options, callback)
-    options = params.options
-    options.headers.x = 'y'
-    return request(params.uri, params.options, params.callback)
+    params.headers.x = 'y'
+    return request(params.uri, params, params.callback)
   })
 
   t.throws(function() {
@@ -294,11 +293,11 @@ tape('test custom request handler function without options', function(t) {
 
   var customHandlerWithoutOptions = request.defaults(function(uri, options, callback) {
     var params = request.initParams(uri, options, callback)
-    var headers = params.options.headers || {}
+    var headers = params.headers || {}
     headers.x = 'y'
     headers.foo = 'bar'
-    params.options.headers = headers
-    return request(params.uri, params.options, params.callback)
+    params.headers = headers
+    return request(params.uri, params, params.callback)
   })
 
   customHandlerWithoutOptions.get(s.url + '/get_custom', function(e, r, b) {