perf: cache parsed req.query per request

lib/request.js

@@ -29,6 +29,13 @@ var proxyaddr = require('proxy-addr');
 
 var req = Object.create(http.IncomingMessage.prototype)
 
+/**
+ * Per-request cache of the parsed query string.
+ * @private
+ */
+
+var queryCache = Symbol('express.query')
+
 /**
  * Module exports.
  * @public
@@ -235,9 +242,19 @@ defineGetter(req, 'query', function query(){
     return Object.create(null);
   }
 
-  var querystring = parse(this).query;
+  // reuse the parsed result while the url and parser are unchanged
+  var cached = this[queryCache];
+  var url = this.url;
+
+  if (cached !== undefined && cached.url === url && cached.parser === queryparse) {
+    return cached.value;
+  }
+
+  var value = queryparse(parse(this).query);
+
+  this[queryCache] = { url: url, parser: queryparse, value: value };
 
-  return queryparse(querystring);
+  return value;
 });
 
 /**

test/req.query.js

@@ -88,6 +88,177 @@ describe('req', function(){
           /unknown value.*query parser/)
       });
     });
+
+    describe('when accessed more than once', function () {
+      it('should parse once per request', function (done) {
+        var app = express();
+        var calls = 0;
+
+        app.set('query parser', function (str) {
+          calls++;
+          return { str: str };
+        });
+
+        app.use(function (req, res) {
+          var first = req.query;
+          res.send(String(first === req.query) + ' ' + calls);
+        });
+
+        request(app)
+        .get('/?a=1')
+        .expect(200, 'true 1', done);
+      });
+
+      it('should not share between requests', function (done) {
+        var app = createApp();
+
+        request(app)
+        .get('/?a=1')
+        .expect(200, '{"a":"1"}', function (err) {
+          if (err) return done(err);
+          request(app)
+          .get('/?a=2')
+          .expect(200, '{"a":"2"}', done);
+        });
+      });
+
+      it('should reparse when req.url changes', function (done) {
+        var app = express();
+
+        app.use(function (req, res, next) {
+          req.query;
+          req.url = '/?b=2';
+          next();
+        });
+
+        app.use(function (req, res) {
+          res.send(req.query);
+        });
+
+        request(app)
+        .get('/?a=1')
+        .expect(200, '{"b":"2"}', done);
+      });
+
+      it('should reparse when the "query parser" setting changes', function (done) {
+        var app = express();
+
+        app.use(function (req, res) {
+          var simple = req.query;
+          req.app.set('query parser', 'extended');
+          res.send({ simple: simple, extended: req.query });
+        });
+
+        request(app)
+        .get('/?a[b]=1')
+        .expect(200, '{"simple":{"a[b]":"1"},"extended":{"a":{"b":"1"}}}', done);
+      });
+
+      it('should keep changes made to the returned object', function (done) {
+        var app = express();
+
+        app.use(function (req, res) {
+          req.query.extra = 'yes';
+          res.send(req.query);
+        });
+
+        request(app)
+        .get('/?a=1')
+        .expect(200, '{"a":"1","extra":"yes"}', done);
+      });
+
+      it('should reparse when the "query parser" function changes', function (done) {
+        var app = express();
+
+        app.use(function (req, res) {
+          var before = req.query;
+          req.app.set('query parser', function () {
+            return { custom: true };
+          });
+          res.send({ before: before, after: req.query });
+        });
+
+        request(app)
+        .get('/?a=1')
+        .expect(200, '{"before":{"a":"1"},"after":{"custom":true}}', done);
+      });
+
+      it('should not cache when parsing is disabled', function (done) {
+        var app = express();
+
+        app.set('query parser', false);
+
+        app.use(function (req, res) {
+          res.send(String(req.query === req.query) + ' ' + Object.keys(req.query).length);
+        });
+
+        request(app)
+        .get('/?a=1')
+        .expect(200, 'false 0', done);
+      });
+
+      it('should retry after the parser throws', function (done) {
+        var app = express();
+        var calls = 0;
+
+        app.set('query parser', function () {
+          if (++calls === 1) throw new Error('boom');
+          return { ok: true };
+        });
+
+        app.use(function (req, res) {
+          var err;
+          try {
+            req.query;
+          } catch (e) {
+            err = e.message;
+          }
+          res.send({ err: err, query: req.query, calls: calls });
+        });
+
+        request(app)
+        .get('/?a=1')
+        .expect(200, '{"err":"boom","query":{"ok":true},"calls":2}', done);
+      });
+
+      it('should work across a mounted sub-app', function (done) {
+        var app = express();
+        var sub = express();
+
+        app.use(function (req, res, next) {
+          req.query;
+          next();
+        });
+
+        sub.use(function (req, res) {
+          res.send(req.query);
+        });
+
+        app.use('/sub', sub);
+
+        request(app)
+        .get('/sub/path?a=1&b=2')
+        .expect(200, '{"a":"1","b":"2"}', done);
+      });
+
+      it('should reflect the new query after req.url is rewritten with a mount path', function (done) {
+        var app = express();
+
+        app.use('/api', function (req, res, next) {
+          req.query;
+          req.url = req.url + '&added=1';
+          next();
+        });
+
+        app.use(function (req, res) {
+          res.send(req.query);
+        });
+
+        request(app)
+        .get('/api/x?a=1')
+        .expect(200, '{"a":"1","added":"1"}', done);
+      });
+    });
   })
 })