clean up implementation

package.json

@@ -1,6 +1,6 @@
 {
   "name": "spirit-express",
-  "version": "0.0.2",
+  "version": "0.1.0",
   "description": "use express middleware with spirit",
   "main": "index.js",
   "scripts": {
@@ -30,10 +30,10 @@
     "jasmine": "^2.4.1",
     "rewire": "^2.5.1",
     "superagent": "^2.0.0",
-    "spirit-router": ">=0.0.6"
+    "spirit-router": ">=0.0.8"
   },
   "dependencies": {
     "bluebird": "^3.4.1",
-    "spirit": ">=0.0.10"
+    "spirit": ">=0.0.12"
   }
 }

spec/examples/express-session-spec.js

@@ -18,9 +18,13 @@ describe("Middleware: express-session", () => {
   it("ok", (done) => {
     request.get("http://localhost:3009/")
       .end((err, res) => {
-        const cookie = res.header["set-cookie"]
+        let cookie = res.header["set-cookie"]
         expect(res.status).toBe(200)
         expect(res.body.counter).toBe(1)
+        if (!cookie) {
+          return done.fail(new Error("Didn't get a cookie back in express-session example"))
+        }
+
         request.get("http://localhost:3009/")
           .set("Cookie", cookie)
           .end((err, res) => {

spec/examples/passport-spec.js

@@ -21,6 +21,10 @@ describe("Middleware: passport", () => {
       .end((err, res) => {
         const cookie = res.header["set-cookie"]
 
+        if (!cookie) {
+          return done.fail("passport example did not get a cookie back on first GET request")
+        }
+
         request.post("http://localhost:3009/login")
           .set("Cookie", cookie)
           .send("username=testuser")

spec/express_compat-spec.js

@@ -1,8 +1,9 @@
 const compat = require("../lib/express-compat").compat
 const ExpressRes = require("../lib/express-res")
+const stream = require("stream")
 
 describe("express compat", () => {
-  const mock_res = {
+  const mock_req = {
     req: () => { return {} }
   }
 
@@ -14,7 +15,7 @@ describe("express compat", () => {
       expect(typeof res.redirect).toBe("function")
       done()
     }
-    compat(middleware)(handler)(mock_res)
+    compat(middleware)(handler)(mock_req)
   })
 
   it("errors in Exp Middleware get caught in Promise", (done) => {
@@ -23,7 +24,7 @@ describe("express compat", () => {
       next()
     }
 
-    compat(middleware)(handler)(mock_res).catch((err) => {
+    compat(middleware)(handler)(mock_req).catch((err) => {
       expect(err).toMatch(/TypeError/)
       done()
     })
@@ -33,10 +34,46 @@ describe("express compat", () => {
     const middleware = (req, res, next) => {
       next("an error")
     }
-    compat(middleware)(handler)(mock_res).catch((err) => {
+    compat(middleware)(handler)(mock_req).catch((err) => {
       expect(err).toBe("an error")
       done()
     })
   })
 
+  // Though technically not a middleware but a Express handler
+  it("example: Exp 'middleware' that streams", (done) => {
+    const middleware = (req, res, next) => {
+      res.writeHead(200, {
+        "Content-Type": "text/event-stream; charset=utf-8",
+        "Cache-Control": "no-cache, no-transform"
+      })
+
+      res.write("1")
+      res.write("2")
+      setTimeout(() => {
+        res.write("3")
+      }, 10)
+    }
+
+    compat(middleware)(handler)(mock_req).then((resp) => {
+      expect(resp.status).toBe(200)
+      expect(resp.headers).toEqual({
+        "Content-Type": "text/event-stream; charset=utf-8",
+        "Cache-Control": "no-cache, no-transform"
+      })
+
+      const _buf = []
+      const st = new stream.Writable({
+        write(chunk, encoding, callback) {
+          _buf.push(chunk.toString())
+          callback()
+          if (_buf.join("") === "1243") done()
+        }
+      })
+
+      resp.body.pipe(st)
+      resp.body.write("4")
+    })
+  })
+
 })

spec/express_res-spec.js

@@ -0,0 +1,61 @@
+const ExpressRes = require("../lib/express-res")
+const stream = require("stream")
+
+describe("ExpressRes", () => {
+
+  describe("writeHead", () => {
+    it("will return but not end and always converts the body to a stream", (done) => {
+      const _done = (response) => {
+        expect(response.status).toBe(123)
+        expect(response.headers).toEqual({ a: 123 })
+        expect(typeof response.body.pipe).toBe("function")
+
+        expect(res._state.headers).toBe(true)
+        expect(res._state.end).toBe(false)
+        done()
+      }
+      const res = new ExpressRes(_done)
+      res.writeHead(123, {
+        a: 123
+      })
+    })
+  })
+
+  describe("write", () => {
+    it("will convert resp body to stream if it isn't", (done) => {
+      const _done = (response) => {
+        expect(response.status).toBe(200)
+        expect(response.headers).toEqual({})
+        const _buf = []
+        const st = new stream.Writable({
+          write(chunk, encoding, callback) {
+            if (chunk.toString() === "hi") done()
+          }
+        })
+        response.body.pipe(st)
+        done()
+      }
+      const res = new ExpressRes(_done)
+      res.write("hi")
+    })
+
+    it("will write to resp body if it's a stream already")
+  })
+
+  describe("end", () => {
+    it("ending with body but no resp body, doesn't create a resp body stream", (done) => {
+      const _done = (response) => {
+        expect(response.status).toBe(200)
+        expect(response.headers).toEqual({ abc: 123 })
+        expect(response.body).toBe("hi")
+        done()
+      }
+      const res = new ExpressRes(_done)
+      // none of these trigger the response body to be a stream
+      res.status(200)
+      res.setHeader("abc", 123)
+      res.end("hi")
+    })
+  })
+
+})

src/express-compat.js

@@ -49,76 +49,73 @@ const init_req = (request) => {
 
 const partial_response = (res, response) => {
   if (res.status !== 0) response.status = res.status
-  if (res.body) response.body = res.body
+  if (res.body !== undefined) response.body = res.body
 
   Object.keys(res.headers).forEach((hdr) => {
     response.headers[hdr] = res.headers[hdr]
   })
   return response
 }
 
-const next = (resolve, reject, req, handler) => {
+const next = (resolve, reject) => {
   return (err) => {
     if (err) {
       return reject(err)
     }
-
-    // always keep url correct incase a Express middleware overwrites it
-    if (req.originalUrl) req.url = req.originalUrl
-
-    resolve(spirit.callp(handler, [req])
-            .then((response) => {
-              // some Express middleware modify the `res` obj
-              // but do not call `res.end`
-              // or they do a hack and wrap `res.end`
-              // or `res.writeHead` expecting it to always be
-              // called eventually
-              //
-              // both instances are considered partial responses
-              // so we apply the changes now as part of
-              // spirit middleware flowing back
-              //
-              // this doesn't guarantee the changes are the last
-              // that depends entirely on where this middleware
-              // is placed to determine the order
-              //
-              // this partial response being applied __only__
-              // happens once (first middleware hit on
-              // flow back), all other Express middleware
-              // will simply just return
-              if (req._res._end || !spirit.node.is_response(response)) {
-                return response
-              }
-
-              const p = new Promise((resolve, reject) => {
-                req._res._done = (resp) => {
-                  if (spirit.node.is_response(resp)) {
-                    response = partial_response(resp, response)
-                  }
-                  resolve(response)
-                }
-                // calling writeHead with 0 is a 'hack'
-                // to not actually create a response, if there
-                // wasn't already one, simply want to trigger
-                // any Express middleware wrappers
-                req._res.writeHead(0)
-                req._res.end()
-              })
-              return p
-            }))
+    return resolve()
   }
 }
 
 const compat = (exp_middleware) => {
   return (handler) => {
     return (request) => {
+      if (request && typeof request.req !== "function") {
+        throw new Error("Unable to use Express middleware. Expected request to have the raw node.js http req available")
+      }
+      const req = init_req(request)
+
       return new Promise((resolve, reject) => {
-        if (request && typeof request.req !== "function") {
-          throw new Error("Unable to use Express middleware. Expected request to have the raw node.js http req available")
-        }
-        const req = init_req(request)
         const res = init_res(req, resolve)
-        exp_middleware(req, res, next(resolve, reject, req, handler))
+        exp_middleware(req, res, next(resolve, reject))
+      }).then((resp) => {
+        // resp is the resolved value of the Exp. middleware
+        // and will only be populated if next() was NOT called
+        // or res.end, res.writeHead was called
+        // therefore we should exit early now
+        if (spirit.node.is_response(resp)) {
+          return resp
+        }
+
+        // always keep url correct incase a Express middleware overwrites it
+        if (req.originalUrl) req.url = req.originalUrl
+
+        // NOTE should really be handler(req).then
+        // handler should always return a promise anyway
+        // only our tests do not return a promise always
+        return spirit.callp(handler, [req]).then((response) => {
+          const res = req._res
+
+          // partially apply whatever was written to res
+          // (if any) this is only done once, and at the
+          // first Express middleware that gets flow back on
+          if (res._state.headers === true || res._state.end === true) return response
+
+          // following deals with Express middlewares hack wrap:
+          // some Express middlewares will wrap writeHead, end
+          // since Express has no way to flow back middleware
+          // thus it queues final changes over all other changes
+          // due to unknown if sync/async, use Promise
+          return new Promise((resolve, reject) => {
+            res._return = () => {
+              res._state.end = true
+              res._state.headers = true
+              resolve(partial_response(res._response, response))
+            }
+            res.writeHead(0)
+            res.end()
+          })
+
+        })
       })
     }
   }