session is now properly saved across requests and issues a set-cookie only when necessary

lib/cookie-session.js

@@ -38,6 +38,7 @@ function Session(req, res, cookies, opts) {
   this.opts = opts;
   this.content = {};
   this.loaded = false;
+  this.dirty = false;
 }
 
 Session.prototype = {
@@ -50,7 +51,8 @@ Session.prototype = {
   },
 
   updateCookie: function() {
-    this.cookies.set(this.opts.cookieName, base64urlencode(JSON.stringify(this.content)));
+    if (this.dirty)
+      this.cookies.set(this.opts.cookieName, base64urlencode(JSON.stringify(this.content)));
   },
 
   loadFromCookie: function() {
@@ -61,6 +63,7 @@ Session.prototype = {
     } else {
       this.content = {};
     }
+
     this.loaded = true;
   },
   
@@ -72,7 +75,7 @@ Session.prototype = {
 
     // all values from content except special values
     sessionHandler.get = function(rcvr, name) {
-      if (['clear', 'setExpires', 'updateCookie', 'loadFromCookie'].indexOf(name) > -1) {
+      if (['clear', 'setExpires'].indexOf(name) > -1) {
         return this.target[name].bind(this.target);
       } else {
         if (!this.target.loaded)
@@ -83,8 +86,13 @@ Session.prototype = {
 
     // set all values to content
     sessionHandler.set = function(rcvr, name, value) {
+      // we have to load existing content, otherwise it will later override
+      // the content that is written.
+      if (!this.target.loaded)
+        this.target.loadFromCookie();
+      
       this.target.content[name] = value;
-      this.target.updateCookie();
+      this.target.dirty = true;
     };
     
     var proxySession = Proxy.create(sessionHandler);
@@ -98,7 +106,19 @@ var cookieSession = function(opts) {
   
   return function(req, res, next) {
     var cookies = new Cookies(req, res);
-    req.session = new Session(req, res, cookies, opts).monitor();
+    var raw_session = new Session(req, res, cookies, opts);
+    req.session = raw_session.monitor();
+
+    // I wish we didn't have to do things this way, but
+    // I can find no other way of delaying the setting of
+    // the cookie until the end
+    var oldWriteHead = res.writeHead;
+    res.writeHead = function() {
+      raw_session.updateCookie();
+
+      oldWriteHead.apply(this, arguments);
+    };    
+
     next();
   };
 };

test/all-test.js

@@ -72,12 +72,48 @@ suite.addBatch({
       });
 
       var browser = tobi.createBrowser(app);
-      browser.get("/foo", function(res, $) {});
-      browser.get("/bar", function(res, $) {});
+      browser.get("/foo", function(res, $) {
+        browser.get("/bar", function(res, $) {
+        });
+      });
     },
     "session maintains state": function(err, req) {
       assert.equal(req.session.foo, 'foobar');
-      assert.equal(req.session.bar, [1,2,3]);
+      assert.equal(req.session.bar.length, 3);
+      assert.equal(req.session.bar[0], 1);
+      assert.equal(req.session.bar[1], 2);
+      assert.equal(req.session.bar[2], 3);
+    }
+  }
+});
+
+suite.addBatch({
+  "reading from a session" : {
+    topic: function() {
+      var self = this;
+
+      // simple app
+      var app = express.createServer();
+      app.use(middleware);
+      app.get("/foo", function(req, res) {
+        req.session.foo = 'foobar';
+        res.send("foo");
+      });
+
+      app.get("/bar", function(req, res) {
+        res.send(req.session.foo);
+      });
+
+      var browser = tobi.createBrowser(app);
+      browser.get("/foo", function(res, $) {
+        browser.get("/bar", function(res, $) {
+          // observe the response to the second request
+          self.callback(null, res);
+        });
+      });
+    },
+    "does not set a cookie": function(err, res) {
+      assert.isUndefined(res.headers['set-cookie']);
     }
   }
 });