From b542b74bd92563b76b3514690c10075f1bdcc014 Mon Sep 17 00:00:00 2001
From: Malte-Thorben Bruns <skenqbx@gmail.com>
Date: Sat, 25 Apr 2015 04:34:00 +0200
Subject: [PATCH] lib: new configuration management

---
 README.markdown               |   8 +--
 doc/plugin-api.markdown       |   4 +-
 lib/call.js                   | 120 ++++------------------------------
 lib/configuration.js          |  53 +++++++++++++++
 lib/index.js                  |   3 +
 lib/plugins/redirect.js       |  35 +++++-----
 lib/plugins/retry.js          |   2 +-
 lib/request-options.js        |  56 ++++++++++++++++
 lib/tools.js                  |  62 ++++++++++++++++++
 test/test-0-http.js           |   8 ++-
 test/test-http-redirect.js    |  10 ---
 test/test-http-send-buffer.js |  20 ++++++
 test/test-public-api.js       |  30 +++------
 tools/cross-test.sh           |  10 ++-
 14 files changed, 252 insertions(+), 169 deletions(-)
 create mode 100644 lib/configuration.js
 create mode 100644 lib/request-options.js
 create mode 100644 lib/tools.js

diff --git a/README.markdown b/README.markdown
index 352c06e..8c722fd 100644
--- a/README.markdown
+++ b/README.markdown
@@ -169,10 +169,10 @@ firefox coverage/lcov-report/index.html
 ### Coverage
 
 ```
-Statements   : 94.88% ( 667/703 )
-Branches     : 88.92% ( 353/397 )
-Functions    : 96.63% ( 86/89 )
-Lines        : 94.88% ( 667/703 )
+Statements   : 95.79% ( 705/736 )
+Branches     : 89.19% ( 330/370 )
+Functions    : 96.84% ( 92/95 )
+Lines        : 95.79% ( 705/736 )
 ```
 
 [back to top](#table-of-contents)
diff --git a/doc/plugin-api.markdown b/doc/plugin-api.markdown
index 6ac32bc..639def3 100644
--- a/doc/plugin-api.markdown
+++ b/doc/plugin-api.markdown
@@ -118,7 +118,9 @@ Emitted on an error.
 All request configurations are stored in `call._stack`, the current configuration is referenced by `call._pointer`.
 
 ### call.\_\_configure(options)
-Creates a new request configuration from the given options and increments the internal pointer.
+Creates a new request configuration and increments the internal pointer.
+
+The current configuration is always the default, meaning `options` only needs to contain changes.
 
 _Note_: Request options are _copied_, plugin options are _referenced_ when not primitive.
 
diff --git a/lib/call.js b/lib/call.js
index da46561..b1e0dc0 100644
--- a/lib/call.js
+++ b/lib/call.js
@@ -1,8 +1,7 @@
 'use strict';
 var util = require('util');
 var stream = require('stream');
-var assert = require('assert');
-var parseURL = require('url').parse;
+var Configuration = require('./configuration');
 var ReplayBuffer = require('./replay-buffer');
 
 var protocols = {
@@ -12,12 +11,6 @@ var protocols = {
   http2: require('http2')
 };
 
-var ports = {
-  http: 80,
-  https: 443,
-  http2: 443
-};
-
 
 
 /**
@@ -71,84 +64,12 @@ module.exports = Call;
  * @return {Object}
  */
 Call.prototype.__configure = function(options) {
-  var i, keys, req;
-  var defaults = this.rail.defaults;
-
-  if (typeof options === 'string') {
-    options = this._urlToOptions({}, options);
-  } else if (typeof options.url === 'string') {
-    options = this._urlToOptions(options, options.url);
-  }
-
-  req = options.request;
-
-  if (!req) {
-    req = options;
-  }
-
-  var request = {
-    method: req.method || defaults.method,
-    path: req.path || defaults.path,
-    host: req.host || defaults.host,
-    port: req.port || defaults.port,
-    headers: {},
-    auth: req.auth || defaults.auth,
-    agent: req.agent !== undefined ? req.agent : defaults.agent,
-    keepAlive: req.keepAlive || false,
-    keepAliveMsecs: req.keepAliveMsecs || 1000
-  };
-
-  if (defaults.headers) {
-    keys = Object.keys(defaults.headers);
-
-    for (i = 0; i < keys.length; ++i) {
-      request.headers[keys[i]] = defaults.headers[keys[i]];
-    }
-  }
-
-  if (req.headers) {
-    keys = Object.keys(req.headers);
-
-    for (i = 0; i < keys.length; ++i) {
-      request.headers[keys[i]] = req.headers[keys[i]];
-    }
-  }
-
-  var opts = {
-    proto: options.proto || req.proto || this.rail.proto,
-    request: request
-  };
-
-  if (!request.port) {
-    request.port = ports[opts.proto];
-  }
+  var configuration = new Configuration(this, options);
 
-  if (opts.proto === 'https' || opts.proto === 'http2') {
-    request.ca = req.ca || defaults.ca;
-    request.pfx = req.pfx || defaults.pfx;
-    request.key = req.key || defaults.key;
-    request.cert = req.cert || defaults.cert;
-    request.ciphers = req.ciphers || defaults.ciphers;
-    request.passphrase = req.passphrase || defaults.passphrase;
-    request.servername = req.servername || defaults.servername;
-    request.secureProtocol = req.secureProtocol || defaults.secureProtocol;
-    request.rejectUnauthorized =
-        req.rejectUnauthorized || defaults.rejectUnauthorized;
-  }
-
-  // apply plugin configuration
-  keys = Object.keys(this.rail.plugins);
-
-  for (i = 0; i < keys.length; ++i) {
-    if (options[keys[i]]) {
-      opts[keys[i]] = options[keys[i]];
-    }
-  }
+  this.rail.emit('plugin-configure', this, configuration);
+  this._pointer = this._stack.push(configuration) - 1;
 
-  this.rail.emit('plugin-configure', this, opts);
-  this._pointer = this._stack.push(opts) - 1;
-
-  return opts;
+  return configuration;
 };
 
 
@@ -292,12 +213,6 @@ Call.prototype.__abort = function() {
 
         if (self.aborted) {
           self.emit('abort');
-
-          if (self._buffer) {
-            self._buffer.close();
-            self._buffer.dump();
-            self._buffer = null;
-          }
         }
       });
     }
@@ -312,6 +227,12 @@ Call.prototype.abort = function() {
   if (!this.aborted) {
     this.aborted = true;
     this.__abort(true);
+
+    if (this._buffer) {
+      this._buffer.close();
+      this._buffer.dump();
+      this._buffer = null;
+    }
   }
 };
 
@@ -401,22 +322,3 @@ Call.prototype._write = function(chunk, encoding, callback) {
     });
   }
 };
-
-
-Call.prototype._urlToOptions = function(options, url) {
-  var parsed = parseURL(url);
-
-  options.proto = parsed.protocol.substr(0, parsed.protocol.length - 1);
-
-  if (options.request) {
-    options.request.host = parsed.hostname;
-    options.request.port = parsed.port;
-    options.request.path = parsed.path;
-  } else {
-    options.host = parsed.hostname;
-    options.port = parsed.port ? parseInt(parsed.port, 10) : null;
-    options.path = parsed.path;
-  }
-
-  return options;
-};
diff --git a/lib/configuration.js b/lib/configuration.js
new file mode 100644
index 0000000..6a7bc26
--- /dev/null
+++ b/lib/configuration.js
@@ -0,0 +1,53 @@
+'use strict';
+var tools = require('./tools');
+var RequestOptions = require('./request-options');
+
+var ports = {
+  http: 80,
+  https: 443,
+  http2: 443
+};
+
+
+
+function Configuration(call, options) {
+  Object.defineProperty(this, 'call', {
+    value: call
+  });
+  var defaults, i, keys;
+
+  if (typeof options === 'string') {
+    options = tools.parseURL(options);
+  } else if (!options) {
+    options = {};
+  }
+
+  if (call._pointer === -1) {
+    defaults = call.rail.defaults;
+    this.proto = options.proto || call.rail.proto;
+  } else {
+    defaults = call._stack[call._pointer].request;
+    this.proto = options.proto || call._stack[call._pointer].proto;
+  }
+
+  if (typeof options.url === 'string') {
+    this.proto = tools.applyURL(options.request || options, options.url);
+  }
+
+  this.request = new RequestOptions(options.request || options, defaults);
+
+  if (!this.request.port) {
+    this.request.port = ports[this.proto];
+  }
+
+  keys = Object.keys(call.rail.plugins);
+
+  if (call._pointer > -1) {
+    options = call._stack[call._pointer];
+  }
+
+  for (i = 0; i < keys.length; ++i) {
+    this[keys[i]] = options[keys[i]] !== undefined ? options[keys[i]] : null;
+  }
+}
+module.exports = Configuration;
diff --git a/lib/index.js b/lib/index.js
index 207612e..5863eae 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -5,8 +5,11 @@ var globalClient = new RAIL();
 module.exports = exports = RAIL;
 
 exports.Call = require('./call');
+exports.Configuration = require('./configuration');
+exports.RequestOptions = require('./request-options');
 exports.ReplayBuffer = require('./replay-buffer');
 exports.plugins = require('./plugins');
+exports.tools = require('./tools');
 
 globalClient.use('buffer');
 globalClient.use('json');
diff --git a/lib/plugins/redirect.js b/lib/plugins/redirect.js
index 1c3a736..36f1d24 100644
--- a/lib/plugins/redirect.js
+++ b/lib/plugins/redirect.js
@@ -1,6 +1,7 @@
 'use strict';
 var parseURL = require('url').parse;
 var path = require('path');
+var tools = require('../tools');
 
 
 
@@ -63,52 +64,48 @@ RedirectPlugin.prototype._setup = function() {
 
 RedirectPlugin.prototype._interceptResponse = function(call, options, response) {
   var self = this;
-  var p, config, url, proto1, proto2;
+  var p, url;
   var location = response.headers.location;
 
   if (!location) {
     return call.__emit('response', response);
   }
-
   // create a new request configuration on the stack
   p = location.indexOf('://');
 
   if (p > 0) {
-    url = parseURL(location);
-    proto1 = options.proto;
-    proto2 = url.protocol.substr(0, url.protocol.length - 1);
+    url = tools.parseURL(location);
 
-    if (url.hostname !== options.request.host &&
+    if (url.host !== options.request.host &&
         options.redirect.sameHost === true) {
       call.emit('warn', 'redirect', 'blocked', 'different host');
       return call.__emit('response', response);
 
-    } else if (proto1 !== proto2) {
+    } else if (options.proto !== url.proto) {
 
-      if (proto2 === 'http' &&
+      if (url.proto === 'http' &&
           options.redirect.allowDowngrade === false) {
         call.emit('warn', 'redirect', 'blocked', 'protocol downgrade');
         return call.__emit('response', response);
 
-      } else if (proto1 === 'http' &&
+      } else if (options.proto === 'http' &&
           options.redirect.allowUpgrade === false) {
         call.emit('warn', 'redirect', 'blocked', 'protocol upgrade');
         return call.__emit('response', response);
       }
-
-      options.proto = proto2;
     }
 
-    config = call.__configure(options);
-    config.request.host = url.hostname;
-    config.request.port = url.port; // TODO: vaildate port
-    config.request.path = url.path;
-    // XXX: revert options to original proto
-    options.proto = proto1;
+    call.__configure({
+      proto: url.proto,
+      host: url.host,
+      port: url.port,
+      path: url.path
+    });
 
   } else {
-    config = call.__configure(options);
-    config.request.path = path.resolve(options.request.path, location);
+    call.__configure({
+      path: path.resolve(options.request.path, location)
+    });
   }
 
   function onEnd() {
diff --git a/lib/plugins/retry.js b/lib/plugins/retry.js
index 0ad9376..bbcadfc 100644
--- a/lib/plugins/retry.js
+++ b/lib/plugins/retry.js
@@ -74,7 +74,7 @@ RetryPlugin.prototype._interceptError = function(call, options, err) {
     // TODO: alternate protocols, hosts and/or ports
 
     // create a new request configuration
-    config = call.__configure(options);
+    call.__configure();
 
     call.emit('retry', options);
 
diff --git a/lib/request-options.js b/lib/request-options.js
new file mode 100644
index 0000000..cc9e727
--- /dev/null
+++ b/lib/request-options.js
@@ -0,0 +1,56 @@
+'use strict';
+var parseURL = require('url').parse;
+var tools = require('./tools');
+
+var requestOptions = [
+  'method',
+  'port',
+  'host',
+  'path',
+  'auth',
+  'agent',
+  'keepAlive',
+  'keepAliveMsecs',
+  'ca',
+  'pfx',
+  'key',
+  'cert',
+  'ciphers',
+  'passphrase',
+  'servername',
+  'secureProtocol',
+  'rejectUnauthorized'
+];
+
+
+
+function RequestOptions(options, defaults) {
+  // http
+  this.method = 'GET';
+  this.port = 0;
+  this.host = 'localhost';
+  this.path = '/';
+  this.auth = null;
+  this.agent = null;
+  this.keepAlive = null;
+  this.keepAliveMsecs = null;
+
+  // https, http2
+  this.ca = null;
+  this.pfx = null;
+  this.key = null;
+  this.cert = null;
+  this.ciphers = null;
+  this.passphrase = null;
+  this.servername = null;
+  this.secureProtocol = null;
+  this.rejectUnauthorized = null;
+
+  // all
+  this.headers = {};
+
+  // apply options or defaults
+  tools.copy(this, options, defaults, requestOptions);
+  tools.copyHeaders(this.headers, options.headers, defaults.headers);
+}
+module.exports = RequestOptions;
diff --git a/lib/tools.js b/lib/tools.js
new file mode 100644
index 0000000..f5cc48c
--- /dev/null
+++ b/lib/tools.js
@@ -0,0 +1,62 @@
+'use strict';
+var parse_url = require('url').parse;
+
+
+function copyHeaders(target, source, defaults) {
+  var i, keys;
+
+  source = source || {};
+  defaults = defaults || {};
+
+  keys = Object.keys(source).concat(Object.keys(defaults));
+
+  for (i = 0; i < keys.length; ++i) {
+    if (source[keys[i]] !== undefined) {
+      target[keys[i]] = source[keys[i]];
+    } else if (defaults[keys[i]] !== undefined) {
+      target[keys[i]] = defaults[keys[i]];
+    }
+  }
+}
+exports.copyHeaders = copyHeaders;
+
+
+function copy(target, source, defaults, keys) {
+  var i;
+
+  source = source || {};
+
+  for (i = 0; i < keys.length; ++i) {
+    if (source[keys[i]] !== undefined) {
+      target[keys[i]] = source[keys[i]];
+    } else if (defaults[keys[i]] !== undefined) {
+      target[keys[i]] = defaults[keys[i]];
+    }
+  }
+}
+exports.copy = copy;
+
+
+function parseURL(url) {
+  var parsed = parse_url(url);
+
+  return {
+    proto: parsed.protocol.substr(0, parsed.protocol.length - 1),
+    host: parsed.hostname,
+    port: parsed.port ? parseInt(parsed.port, 10) : undefined,
+    path: parsed.path
+  };
+}
+exports.parseURL = parseURL;
+
+
+function applyURL(target, url) {
+  var parsed = parseURL(url);
+
+  target.host = parsed.host;
+  target.port = parsed.port;
+  target.path = parsed.path;
+
+  return parsed.proto;
+}
+exports.applyURL = applyURL;
diff --git a/test/test-0-http.js b/test/test-0-http.js
index d55c883..38246f1 100644
--- a/test/test-0-http.js
+++ b/test/test-0-http.js
@@ -60,7 +60,13 @@ suite('http', function() {
         assert.strictEqual(body.toString(), 'pong');
         done();
       });
-    }).end('ping');
+    });
+
+    call.write('start');
+
+    setImmediate(function() {
+      call.end('ping');
+    });
 
     assert(call);
     assert.strictEqual(typeof call.end, 'function');
diff --git a/test/test-http-redirect.js b/test/test-http-redirect.js
index efbad47..578eb28 100644
--- a/test/test-http-redirect.js
+++ b/test/test-http-redirect.js
@@ -84,8 +84,6 @@ suite('http:redirect', function() {
       done();
     }).on('warn', function(plugin, status, message) {
       console.log('warn', plugin, status, message);
-    }).on('error', function(err) {
-      console.log('TEST CALL ERROR', err.stack);
     }).end();
   });
 
@@ -122,8 +120,6 @@ suite('http:redirect', function() {
       done();
     }).on('warn', function(plugin, status, message) {
       console.log('warn', plugin, status, message);
-    }).on('error', function(err) {
-      console.log('TEST CALL ERROR', err.stack);
     }).end();
   });
 
@@ -147,8 +143,6 @@ suite('http:redirect', function() {
       assert.strictEqual(response.statusCode, 302);
       assert.strictEqual(response.buffer, null);
       done();
-    }).on('error', function(err) {
-      console.log('TEST CALL ERROR', err.stack);
     }).end();
   });
 
@@ -176,8 +170,6 @@ suite('http:redirect', function() {
       assert.strictEqual(response.buffer, null);
       assert.deepEqual(['redirect', 'blocked', 'different host'], warn);
       done();
-    }).on('error', function(err) {
-      console.log('TEST CALL ERROR', err.stack);
     }).on('warn', function(plugin, status, message) {
       warn = [plugin, status, message];
     }).end();
@@ -207,8 +199,6 @@ suite('http:redirect', function() {
       assert.strictEqual(response.buffer, null);
       assert.deepEqual(['redirect', 'blocked', 'protocol upgrade'], warn);
       done();
-    }).on('error', function(err) {
-      console.log('TEST CALL ERROR', err.stack);
     }).on('warn', function(plugin, status, message) {
       warn = [plugin, status, message];
     }).end();
diff --git a/test/test-http-send-buffer.js b/test/test-http-send-buffer.js
index 4a329af..5ef6e85 100644
--- a/test/test-http-send-buffer.js
+++ b/test/test-http-send-buffer.js
@@ -89,6 +89,26 @@ suite('http:send-buffer', function() {
   });
 
 
+  test('abort', function() {
+    onrequest = function(request, response) {
+    };
+
+    var call = rail.call({
+      proto: 'http',
+      port: common.port,
+      method: 'POST'
+    });
+
+    call.__buffer();
+
+    call.write('a');
+    call.write('b');
+    call.write('c');
+    call.abort();
+    assert(call._buffer === null);
+  });
+
+
   suiteTeardown(function(done) {
     server.close(done);
   });
diff --git a/test/test-public-api.js b/test/test-public-api.js
index 8b5edaa..20478d8 100644
--- a/test/test-public-api.js
+++ b/test/test-public-api.js
@@ -57,6 +57,15 @@ suite('public-api', function() {
   });
 
 
+  test('call.abort() -> __request()', function() {
+    var client = rail();
+    var call = client.call();
+    call.abort();
+    var ret = call.__request();
+    assert(ret === false);
+  });
+
+
   test('call.__intercept "interceptor should be a function"', function() {
     var client = rail();
     var call = client.call();
@@ -80,25 +89,4 @@ suite('public-api', function() {
     ++call._pointer;
     call.__request();
   });
-
-  test('call._urlToOptions', function() {
-    var client = rail();
-    var call = client.call();
-
-    var options = call._urlToOptions({
-      request: {
-        method: 'POST'
-      }
-    }, 'http://github.com');
-
-    assert.deepEqual(options, {
-      proto: 'http',
-      request: {
-        method: 'POST',
-        host: 'github.com',
-        path: '/',
-        port: null
-      }
-    });
-  });
 });
diff --git a/tools/cross-test.sh b/tools/cross-test.sh
index 40390ab..7af9faf 100755
--- a/tools/cross-test.sh
+++ b/tools/cross-test.sh
@@ -8,14 +8,18 @@
 # $ git clone https://github.com/creationix/nvm.git ~/.nvm && cd ~/.nvm && git checkout `git describe --abbrev=0 --tags`
 # $ echo "source ~/.nvm/nvm.sh" >> ~/.bashrc
 #
+# $ nvm install -s 0.10
+# $ nvm install -s 0.12
+# $ nvm install iojs
+#
 
 source ~/.nvm/nvm.sh
 
-nvm install iojs
+nvm use iojs
 node_modules/.bin/_mocha ./test
 
-nvm install 0.12.2
+nvm use 0.12
 node_modules/.bin/_mocha ./test
 
-nvm install 0.10.38
+nvm use 0.10
 node_modules/.bin/_mocha ./test