allow request id to be set. remove reqIdHeaders option (#1256)

docs/pages/components/request.md

@@ -119,12 +119,16 @@ server.get('/:x/bar', function(req, res, next) {
 ```
 
 
-## id()
+## id([reqId])
+
+* `[reqId]` {String}
 
 __Returns__ {String}
 
-Returns the request id. If a request id header was not specified for incoming
-requests, this will return a UUID.
+Returns the request id. If a `reqId` value is passed in, this will
+become the request's new id. The request id is immutable, and can only be
+set once. Attempting to set the request id more than once will cause
+restify to throw.
 
 
 ## path()

docs/pages/components/server.md

@@ -23,7 +23,6 @@ The `createServer()` method takes the following options:
 |version|String|Array|A default version to set for all routes|
 |handleUpgrades|Boolean|Hook the `upgrade` event from the node HTTP server, pushing `Connection: Upgrade` requests through the regular request handling chain; defaults to `false`|
 |httpsServerOptions|Object|Any options accepted by [node-https Server](http://nodejs.org/api/https.html#https_https). If provided the following restify server options will be ignored: spdy, ca, certificate, key, passphrase, rejectUnauthorized, requestCert and ciphers; however these can all be specified on httpsServerOptions.|
-|reqIdHeaders|Array|an optional array of request id header names that will be used to set the request id (i.e., the value for req.getId())|
 |strictRouting|Boolean|(Default=`false`). If set, Restify will treat "/foo" and "/foo/" as different paths.|
 
 

lib/request.js

@@ -218,6 +218,7 @@ Request.prototype.href = Request.prototype.getHref;
  * @returns  {String}
  */
 Request.prototype.getId = function getId() {
+
     if (this._id !== undefined) {
         return (this._id);
     }
@@ -226,7 +227,22 @@ Request.prototype.getId = function getId() {
 
     return (this._id);
 };
-Request.prototype.id = Request.prototype.getId;
+Request.prototype.id = function id(reqId) {
+
+    var self = this;
+
+    if (reqId) {
+        if (self._id) {
+            throw new Error('request id is immutable, cannot be set again!');
+        } else {
+            assert.string(reqId, 'reqId');
+            self._id = reqId;
+            return self._id;
+        }
+    }
+
+    return self.getId();
+};
 
 
 /**

lib/server.js

@@ -248,15 +248,12 @@ function optionsError(err, req, res) {
  * @public
  * @class
  * @param {Object} options an options object
- * @param {Array} [options.reqIdHeaders] an array of request id headers to
- * re-use from incoming requests
  */
 function Server(options) {
     assert.object(options, 'options');
     assert.object(options.log, 'options.log');
     assert.object(options.router, 'options.router');
     assert.string(options.name, 'options.name');
-    assert.optionalArrayOfString(options.reqIdHeaders, 'options.reqIdHeaders');
 
     var self = this;
 
@@ -274,11 +271,6 @@ function Server(options) {
     this.socketio = options.socketio || false;
     this._unfinishedRequests = 0;
 
-    var defaultReqIdHeaders = ['request-id', 'x-request-id'];
-    this.reqIdHeaders = (options.reqIdHeaders) ?
-                    options.reqIdHeaders.concat(defaultReqIdHeaders) :
-                    defaultReqIdHeaders;
-
     var fmt = mergeFormatters(options.formatters);
     this.acceptable = fmt.acceptable;
     this.formatters = fmt.formatters;
@@ -1123,19 +1115,6 @@ Server.prototype._setupRequest = function _setupRequest(req, res) {
         res.header('Server', self.name);
     }
     res.version = self.router.versions[self.router.versions.length - 1];
-
-    // GH-1086: set request id based on array of headers, lowest index
-    // is highest priority.
-    var i = 0;
-
-    for (i = 0; i < self.reqIdHeaders.length; i++) {
-        var idName = self.reqIdHeaders[i];
-
-        if (req.headers.hasOwnProperty(idName) && req.headers[idName]) {
-            req._id = req.headers[idName];
-            break;
-        }
-    }
 };
 
 

test/request.test.js

@@ -1,6 +1,7 @@
 'use strict';
 
 var restifyClients = require('restify-clients');
+var validator = require('validator');
 
 var restify = require('../lib');
 
@@ -90,3 +91,82 @@ test('query should return raw query string string', function (t) {
     });
 });
 
+
+test('should generate request id on first req.id() call', function (t) {
+
+    SERVER.get('/ping', function (req, res, next) {
+        t.equal(typeof req.id(), 'string');
+        t.equal(validator.isUUID(req.id(), 4), true);
+        res.send();
+        return next();
+    });
+
+    CLIENT.get('/ping', function (err, _, res) {
+        t.ifError(err);
+        t.equal(res.statusCode, 200);
+        t.end();
+    });
+});
+
+
+test('should set request id', function (t) {
+
+    SERVER.pre(function setId(req, res, next) {
+        var newId = req.id('lagavulin');
+        t.equal(newId, 'lagavulin');
+        return next();
+    });
+
+    SERVER.get('/ping', function (req, res, next) {
+        t.equal(typeof req.id(), 'string');
+        t.equal(req.id(), 'lagavulin');
+        res.send();
+        return next();
+    });
+
+    CLIENT.get('/ping', function (err, _, res) {
+        t.ifError(err);
+        t.equal(res.statusCode, 200);
+        t.end();
+    });
+});
+
+
+test('should throw when setting request id after autogeneration', function (t) {
+
+    SERVER.get('/ping', function (req, res, next) {
+        t.equal(typeof req.id(), 'string');
+        t.equal(validator.isUUID(req.id(), 4), true);
+        t.throws(function () {
+            req.id('blowup');
+        }, Error, 'request id is immutable, cannot be set again!');
+        res.send();
+        return next();
+    });
+
+    CLIENT.get('/ping', function (err, _, res) {
+        t.ifError(err);
+        t.equal(res.statusCode, 200);
+        t.end();
+    });
+});
+
+
+test('should throw when setting request id twice', function (t) {
+
+    SERVER.get('/ping', function (req, res, next) {
+        req.id('lagavulin');
+        t.throws(function () {
+            req.id('blowup');
+        }, Error, 'request id is immutable, cannot be set again!');
+        res.send();
+        return next();
+    });
+
+    CLIENT.get('/ping', function (err, _, res) {
+        t.ifError(err);
+        t.equal(res.statusCode, 200);
+        t.end();
+    });
+});
+

test/server.test.js

@@ -13,7 +13,6 @@ var filed = require('filed');
 var plugins = require('restify-plugins');
 var restifyClients = require('restify-clients');
 var uuid = require('uuid');
-var validator = require('validator');
 
 var RestError = errors.RestError;
 var restify = require('../lib');
@@ -44,8 +43,7 @@ before(function (cb) {
             dtrace: helper.dtrace,
             handleUncaughtExceptions: true,
             log: helper.getLog('server'),
-            version: ['2.0.0', '0.5.4', '1.4.3'],
-            reqIdHeaders: ['x-req-id-a', 'x-req-id-b']
+            version: ['2.0.0', '0.5.4', '1.4.3']
         });
         SERVER.listen(PORT, '127.0.0.1', function () {
             PORT = SERVER.address().port;
@@ -2263,106 +2261,6 @@ test('calling next(false) should early exit from pre handlers', function (t) {
 });
 
 
-test('GH-1086: should reuse request id when available', function (t) {
-
-    SERVER.get('/1', function (req, res, next) {
-        // the 12345 value is set when the client is created.
-        t.ok(req.headers.hasOwnProperty('x-req-id-a'));
-        t.equal(req.getId(), req.headers['x-req-id-a']);
-        res.send('hello world');
-        return next();
-    });
-
-    // create new client since we new specific headers
-    CLIENT = restifyClients.createJsonClient({
-        url: 'http://127.0.0.1:' + PORT,
-        headers:{
-            'x-req-id-a': 12345
-        }
-    });
-
-    CLIENT.get('/1', function (err, req, res, data) {
-        t.ifError(err);
-        t.equal(data, 'hello world');
-        t.end();
-    });
-});
-
-
-test('GH-1086: should use second request id when available', function (t) {
-
-    SERVER.get('/1', function (req, res, next) {
-        t.ok(req.headers.hasOwnProperty('x-req-id-b'));
-        t.equal(req.getId(), req.headers['x-req-id-b']);
-        res.send('hello world');
-        return next();
-    });
-
-    // create new client since we new specific headers
-    CLIENT = restifyClients.createJsonClient({
-        url: 'http://127.0.0.1:' + PORT,
-        headers:{
-            'x-req-id-b': 678910
-        }
-    });
-
-    CLIENT.get('/1', function (err, req, res, data) {
-        t.ifError(err);
-        t.equal(data, 'hello world');
-        t.end();
-    });
-});
-
-
-test('GH-1086: should use default uuid request id if none provided',
-function (t) {
-
-    SERVER.get('/1', function (req, res, next) {
-        t.ok(req.getId());
-        t.ok(validator.isUUID(req.getId()));
-        res.send('hello world');
-        return next();
-    });
-
-    // create new client since we new specific headers
-    CLIENT = restifyClients.createJsonClient({
-        url: 'http://127.0.0.1:' + PORT
-    });
-
-    CLIENT.get('/1', function (err, req, res, data) {
-        t.ifError(err);
-        t.equal(data, 'hello world');
-        t.end();
-    });
-});
-
-
-test('GH-1086: empty request id should be ignored', function (t) {
-
-    SERVER.get('/1', function (req, res, next) {
-        t.ok(req.headers.hasOwnProperty('x-req-id-b'));
-        t.equal(req.getId(), req.headers['x-req-id-b']);
-        res.send('hello world');
-        return next();
-    });
-
-    // create new client since we new specific headers
-    CLIENT = restifyClients.createJsonClient({
-        url: 'http://127.0.0.1:' + PORT,
-        headers:{
-            'x-req-id-a': '',
-            'x-req-id-b': 12345
-        }
-    });
-
-    CLIENT.get('/1', function (err, req, res, data) {
-        t.ifError(err);
-        t.equal(data, 'hello world');
-        t.end();
-    });
-});
-
-
 test('GH-1078: server name should default to restify', function (t) {
 
     var myServer = restify.createServer();