start adding tests

Author: chrisdickinson

db-session.js

@@ -8,6 +8,13 @@ const AtomicSessionConnectionPair = require('./lib/atomic-session-connpair.js')
 const TxSessionConnectionPair = require('./lib/tx-session-connpair.js')
 const SessionConnectionPair = require('./lib/session-connpair.js')
 
+class NoSessionAvailable extends Error {
+  constructor () {
+    super('No session available')
+    Error.captureStackTrace(this, NoSessionAvailable)
+  }
+}
+
 const api = module.exports = {
   install (domain, getConnection, opts) {
     opts = opts || {}
@@ -19,15 +26,19 @@ const api = module.exports = {
 
   atomic (operation) {
     return function atomic$operation () {
-      const args = [].slice.call(arguments)
-      return api.session.atomic(operation, args)
+      return Promise.try(() => {
+        const args = [].slice.call(arguments)
+        return api.session.atomic(operation, args)
+      })
     }
   },
 
   transaction (operation) {
     return function transaction$operation () {
-      const args = [].slice.call(arguments)
-      return api.session.transaction(operation, args)
+      return Promise.try(() => {
+        const args = [].slice.call(arguments)
+        return api.session.transaction(operation, args)
+      })
     }
   },
 
@@ -37,14 +48,16 @@ const api = module.exports = {
 
   get session () {
     var current = DOMAIN_TO_SESSION.get(process.domain)
-    if (!current) {
-      throw new Error('no session active')
+    if (!current || !process.domain) {
+      throw new NoSessionAvailable()
     }
     while (current.inactive && current.parent) {
       current = current.parent
     }
     return current
-  }
+  },
+
+  NoSessionAvailable
 }
 
 // how does this nest:
@@ -70,7 +83,7 @@ class Session {
       return pending.promise
     }
 
-    const connPair = this._getConnection()
+    const connPair = Promise.resolve(this._getConnection())
     ++this._activeConnections
 
     return connPair.then(
@@ -210,12 +223,12 @@ function getSavepointName (operation) {
 getSavepointName.ID = 0
 
 function markInactive (session) {
-  // XXX(chrisdickinson): is this a good idea? this
-  // means that DB requests after the operation completes
-  // will still work, but will operate outside of the
-  // transaction.
   return domain => {
+    domain.exit()
     DOMAIN_TO_SESSION.get(domain).inactive = true
+
+    // if, somehow, we get a reference to this domain again, point
+    // it at the parent session.
     DOMAIN_TO_SESSION.set(domain, session)
   }
 }

package.json

@@ -4,8 +4,8 @@
   "description": "domain-attached database sessions",
   "main": "db-session.js",
   "scripts": {
-    "test": "tape test/*-test.js && standard",
-    "test-cov": "nyc tape test/*-test.js && standard"
+    "test": "tap test/*-test.js && standard",
+    "test-cov": "nyc tap test/*-test.js && standard"
   },
   "repository": {
     "type": "git",
@@ -24,11 +24,10 @@
   },
   "homepage": "https://github.com/npm/pg-db-session#readme",
   "devDependencies": {
-    "faucet": "0.0.1",
     "nyc": "^5.3.0",
     "pg": "^4.4.3",
     "standard": "^5.4.1",
-    "tape": "^4.4.0"
+    "tap": "^5.0.0"
   },
   "dependencies": {
     "bluebird": "^3.1.1"

test/basic-api-errors-test.js

@@ -0,0 +1,37 @@
+'use strict'
+
+const Promise = require('bluebird')
+const domain = require('domain')
+const test = require('tap').test
+
+const db = require('../db-session.js')
+
+test('test transaction outside of session', assert => {
+  const testTransaction = db.transaction(function testTransaction () {
+    return
+  })
+
+  testTransaction()
+    .then(() => { throw new Error('expected error') })
+    .catch(db.NoSessionAvailable, () => assert.end())
+    .catch(err => assert.end(err))
+})
+
+test('test atomic outside of session', assert => {
+  const testAtomic = db.atomic(function testAtomic () {
+    return
+  })
+
+  testAtomic()
+    .then(() => { throw new Error('expected error') })
+    .catch(db.NoSessionAvailable, () => assert.end())
+    .catch(err => assert.end(err))
+})
+
+if (false)
+test('test atomic after release', assert => {
+})
+
+if (false)
+test('test transaction after release', assert => {
+})

test/basic-interleaved-test.js

@@ -0,0 +1,55 @@
+'use strict'
+
+const domain = require('domain')
+const test = require('tap').test
+const fs = require('fs')
+
+const db = require('../db-session.js')
+
+// this is pretty much just testing domains
+test('test of interleaved requests', assert => {
+  const domain1 = domain.create()
+  const domain2 = domain.create()
+
+  db.install(domain1, getFakeConnection)
+  db.install(domain2, getFakeConnection)
+
+  var pending = 3
+
+  domain1.run(() => {
+    const firstSession = db.session
+    fs.readFile(__filename, () => {
+      assert.equal(firstSession, db.session)
+      domain2.run(() => {
+        assert.ok(firstSession !== db.session)
+        const secondSession = db.session
+        setTimeout(() => {
+          assert.equal(secondSession, db.session)
+          !--pending && end()
+        }, 100)
+        fs.readFile(__filename, () => {
+          assert.equal(secondSession, db.session)
+          !--pending && end()
+        })
+      })
+      setTimeout(() => {
+        assert.equal(firstSession, db.session)
+        !--pending && end()
+      }, 100)
+    })
+  })
+
+  function end () {
+    process.domain.exit()
+    assert.end()
+  }
+
+  function getFakeConnection () {
+    return {
+      connection: {query () {
+      }},
+      release () {
+      }
+    }
+  }
+})

test/basic-session-concurrency-test.js

@@ -0,0 +1,171 @@
+'use strict'
+
+const Promise = require('bluebird')
+const domain = require('domain')
+const test = require('tap').test
+
+const db = require('../db-session.js')
+
+const LOGS = []
+
+test('test root session concurrency=0', assert => {
+  const domain1 = domain.create()
+  db.install(domain1, innerGetConnection, {maxConcurrency: 0})
+  domain1.run(() => {
+    return runOperations()
+  }).then(() => {
+    domain1.exit()
+  }).then(() => {
+    assert.equal(LOGS.join('\n'), `
+load 0
+load 1
+load 2
+load 3
+load 4
+load 5
+load 6
+load 7
+release 0
+release
+release 1
+release
+release 2
+release
+release 3
+release
+release 4
+release
+release 5
+release
+release 6
+release
+release 7
+release
+`.trim())
+    assert.equal(process.domain, undefined)
+  }).then(() => assert.end())
+    .catch(assert.end)
+})
+
+test('test root session concurrency=1', assert => {
+  const domain1 = domain.create()
+  db.install(domain1, innerGetConnection, {maxConcurrency: 1})
+  domain1.run(() => {
+    return runOperations()
+  }).then(() => {
+    domain1.exit()
+  }).then(() => {
+    assert.equal(LOGS.join('\n'), `
+load 0
+release 0
+load 1
+release 1
+load 2
+release 2
+load 3
+release 3
+load 4
+release 4
+load 5
+release 5
+load 6
+release 6
+load 7
+release 7
+release
+`.trim())
+    assert.equal(process.domain, undefined)
+  }).then(() => assert.end())
+    .catch(assert.end)
+})
+
+test('test root session concurrency=2', assert => {
+  const domain1 = domain.create()
+  db.install(domain1, innerGetConnection, {maxConcurrency: 2})
+  domain1.run(() => {
+    return runOperations()
+  }).then(() => {
+    domain1.exit()
+  }).then(() => {
+    assert.equal(LOGS.join('\n'), `
+load 0
+load 1
+release 0
+release 1
+load 2
+load 3
+release 2
+release 3
+load 4
+load 5
+release 4
+release 5
+load 6
+load 7
+release 6
+release
+release 7
+release
+`.trim())
+    assert.equal(process.domain, undefined)
+  }).then(() => assert.end())
+    .catch(assert.end)
+})
+
+test('test root session concurrency=4', assert => {
+  const domain1 = domain.create()
+  db.install(domain1, innerGetConnection, {maxConcurrency: 4})
+  domain1.run(() => {
+    return runOperations()
+  }).then(() => {
+    domain1.exit()
+  }).then(() => {
+    assert.equal(LOGS.join('\n'), `
+load 0
+load 1
+load 2
+load 3
+release 0
+release 1
+release 2
+release 3
+load 4
+load 5
+load 6
+load 7
+release 4
+release
+release 5
+release
+release 6
+release
+release 7
+release
+`.trim())
+    assert.equal(process.domain, undefined)
+  }).then(() => assert.end())
+    .catch(assert.end)
+})
+
+function innerGetConnection () {
+  return {
+    connection: {query () {
+    }},
+    release () {
+      LOGS.push(`release`)
+    }
+  }
+}
+
+function runOperations () {
+  LOGS.length = 0
+  return Promise.all(Array.from(Array(8)).map((_, idx) => {
+    return db.getConnection().then(connPair => {
+      LOGS.push(`load ${idx}`)
+      return Promise.delay(5).then(() => {
+        LOGS.push(`release ${idx}`)
+        connPair.release()
+      })
+    })
+  }))
+}