Early returns for mismatching name and transact values

We needed to avoid the unnecessary addition of handlers in the case where a name and transact value mismatched (different names, or true and a name, etc.)

Resolves #75

Author: Toni Sharpe

index.js

@@ -56,13 +56,13 @@ function transact (fn, cb) {
   })
 }
 
-function extractRequestClient (req, transactionConnection) {
-  if (transactionConnection.length) {
-    const requestClient = req.pg[transactionConnection]
+function extractRequestClient (req, transact) {
+  if (transact.length) {
+    const requestClient = req.pg[transact]
     if (!requestClient) {
-      throw new Error(`request client '${transactionConnection}' does not exist`)
+      throw new Error(`request client '${transact}' does not exist`)
     }
-    return req.pg[transactionConnection]
+    return req.pg[transact]
   }
 
   return req.pg
@@ -123,55 +123,63 @@ function fastifyPostgres (fastify, options, next) {
   }
 
   fastify.addHook('onRoute', routeOptions => {
-    const transactionConnection = routeOptions.pg && routeOptions.pg.transact
+    const transact = routeOptions && routeOptions.pg && routeOptions.pg.transact
 
-    if (transactionConnection) {
-      const preHandler = async (req, reply) => {
-        const client = await pool.connect()
+    if (!transact) {
+      return
+    }
+    if (typeof transact === 'string' && transact !== name) {
+      return
+    }
+    if (name && transact === true) {
+      return
+    }
 
-        if (name) {
-          if (!req.pg) {
-            req.pg = {}
-          }
+    const preHandler = async (req, reply) => {
+      const client = await pool.connect()
 
-          if (client[name]) {
-            throw new Error(`pg client '${name}' is a reserved keyword`)
-          } else if (req.pg[name]) {
-            throw new Error(`request client '${name}' has already been registered`)
-          }
+      if (name) {
+        if (!req.pg) {
+          req.pg = {}
+        }
 
-          req.pg[name] = client
-        } else {
-          if (req.pg) {
-            throw new Error('request client has already been registered')
-          } else {
-            req.pg = client
-          }
+        if (client[name]) {
+          throw new Error(`pg client '${name}' is a reserved keyword`)
+        } else if (req.pg[name]) {
+          throw new Error(`request client '${name}' has already been registered`)
         }
 
-        extractRequestClient(req, transactionConnection).query('BEGIN')
+        req.pg[name] = client
+      } else {
+        if (req.pg) {
+          throw new Error('request client has already been registered')
+        } else {
+          req.pg = client
+        }
       }
 
-      const onError = (req, reply, error, done) => {
-        req[transactionFailedSymbol] = true
-        extractRequestClient(req, transactionConnection).query('ROLLBACK', done)
-      }
+      extractRequestClient(req, transact).query('BEGIN')
+    }
+
+    const onError = (req, reply, error, done) => {
+      req[transactionFailedSymbol] = true
+      extractRequestClient(req, transact).query('ROLLBACK', done)
+    }
 
-      const onSend = async (req) => {
-        const requestClient = extractRequestClient(req, transactionConnection)
-        try {
-          if (!req[transactionFailedSymbol]) {
-            await requestClient.query('COMMIT')
-          }
-        } finally {
-          requestClient.release()
+    const onSend = async (req) => {
+      const requestClient = extractRequestClient(req, transact)
+      try {
+        if (!req[transactionFailedSymbol]) {
+          await requestClient.query('COMMIT')
         }
+      } finally {
+        requestClient.release()
       }
-
-      routeOptions.preHandler = addHandler(routeOptions.preHandler, preHandler)
-      routeOptions.onError = addHandler(routeOptions.onError, onError)
-      routeOptions.onSend = addHandler(routeOptions.onSend, onSend)
     }
+
+    routeOptions.preHandler = addHandler(routeOptions.preHandler, preHandler)
+    routeOptions.onError = addHandler(routeOptions.onError, onError)
+    routeOptions.onSend = addHandler(routeOptions.onSend, onSend)
   })
 
   next()

test/req-initialization.test.js

@@ -8,7 +8,7 @@ const { connectionString } = require('./helpers')
 
 const extractUserCount = response => parseInt(JSON.parse(response.payload).rows[0].userCount)
 
-test('fastify postgress useTransaction route option - ', t => {
+test('fastify postgress useTransaction route option', t => {
   test('queries that succeed provided', async t => {
     const fastify = Fastify()
     t.teardown(() => fastify.close())
@@ -19,13 +19,13 @@ test('fastify postgress useTransaction route option - ', t => {
 
     await fastify.pg.query('TRUNCATE users')
 
-    await fastify.get('/count-users', async (req, reply) => {
+    fastify.get('/count-users', async (req, reply) => {
       const result = await fastify.pg.query('SELECT COUNT(*) AS "userCount" FROM users WHERE username=\'pass-opt-in\'')
 
       reply.send(result)
     })
 
-    await fastify.get('/pass', { pg: { transact: true } }, async (req, reply) => {
+    fastify.get('/pass', { pg: { transact: true } }, async (req, reply) => {
       await req.pg.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['pass-opt-in'])
       await req.pg.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['pass-opt-in'])
       reply.send('complete')
@@ -54,13 +54,13 @@ test('fastify postgress useTransaction route option - ', t => {
 
     await fastify.pg.test.query('TRUNCATE users')
 
-    await fastify.get('/count-users', async (req, reply) => {
+    fastify.get('/count-users', async (req, reply) => {
       const result = await fastify.pg.test.query('SELECT COUNT(*) AS "userCount" FROM users WHERE username=\'pass-opt-in\'')
 
       reply.send(result)
     })
 
-    await fastify.get('/pass', { pg: { transact: 'test' } }, async (req, reply) => {
+    fastify.get('/pass', { pg: { transact: 'test' } }, async (req, reply) => {
       await req.pg.test.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['pass-opt-in'])
       await req.pg.test.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['pass-opt-in'])
 
@@ -89,13 +89,13 @@ test('fastify postgress useTransaction route option - ', t => {
 
     await fastify.pg.query('TRUNCATE users')
 
-    await fastify.get('/count-users', async (req, reply) => {
+    fastify.get('/count-users', async (req, reply) => {
       const result = await fastify.pg.query('SELECT COUNT(*) AS "userCount" FROM users WHERE username=\'fail-opt-in\'')
 
       reply.send(result)
     })
 
-    await fastify.get('/fail', { pg: { transact: true } }, async (req, reply) => {
+    fastify.get('/fail', { pg: { transact: true } }, async (req, reply) => {
       await req.pg.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['fail-opt-in'])
       await req.pg.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['fail-opt-in'])
       await req.pg.query('INSERT INTO nope(username) VALUES($1) RETURNING id', ['fail-opt-in'])
@@ -117,3 +117,105 @@ test('fastify postgress useTransaction route option - ', t => {
 
   t.end()
 })
+
+test('combinations of registrationOptions.name and routeOptions.pg.transact that should not add hooks', t => {
+  test('transact not set', t => {
+    t.plan(1)
+
+    const fastify = Fastify()
+    t.teardown(() => fastify.close())
+
+    fastify.register(fastifyPostgres, {
+      connectionString
+    })
+
+    fastify.get('/', (req, reply) => {
+      t.is(req.pg, null)
+    })
+
+    fastify.inject({
+      method: 'GET',
+      url: '/'
+    })
+  })
+  test('name set and transact not set', t => {
+    t.plan(1)
+
+    const fastify = Fastify()
+    t.teardown(() => fastify.close())
+
+    fastify.register(fastifyPostgres, {
+      connectionString,
+      name: 'test'
+    })
+
+    fastify.get('/', (req, reply) => {
+      t.is(req.pg, null)
+    })
+
+    fastify.inject({
+      method: 'GET',
+      url: '/'
+    })
+  })
+  test('name set and transact set to true', t => {
+    t.plan(1)
+
+    const fastify = Fastify()
+    t.teardown(() => fastify.close())
+
+    fastify.register(fastifyPostgres, {
+      connectionString,
+      name: 'test'
+    })
+
+    fastify.get('/', { pg: { transact: true } }, (req, reply) => {
+      t.is(req.pg, null)
+    })
+
+    fastify.inject({
+      method: 'GET',
+      url: '/'
+    })
+  })
+  test('name not set and transact set to string', t => {
+    t.plan(1)
+
+    const fastify = Fastify()
+    t.teardown(() => fastify.close())
+
+    fastify.register(fastifyPostgres, {
+      connectionString
+    })
+
+    fastify.get('/', { pg: { transact: 'test' } }, (req, reply) => {
+      t.is(req.pg, null)
+    })
+
+    fastify.inject({
+      method: 'GET',
+      url: '/'
+    })
+  })
+  test('name and transact set to different strings', t => {
+    t.plan(1)
+
+    const fastify = Fastify()
+    t.teardown(() => fastify.close())
+
+    fastify.register(fastifyPostgres, {
+      connectionString,
+      name: 'test'
+    })
+
+    fastify.get('/', { pg: { transact: 'different' } }, (req, reply) => {
+      t.is(req.pg, null)
+    })
+
+    fastify.inject({
+      method: 'GET',
+      url: '/'
+    })
+  })
+  t.end()
+})