Skip to content

Commit b9a417f

Browse files
author
Toni Sharpe
committed
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 fastify#75
1 parent 3d5c5ca commit b9a417f

File tree

2 files changed

+159
-49
lines changed

2 files changed

+159
-49
lines changed

‎index.js

+50-42
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ function transact (fn, cb) {
5656
})
5757
}
5858

59-
function extractRequestClient (req, transactionConnection) {
60-
if (transactionConnection.length) {
61-
const requestClient = req.pg[transactionConnection]
59+
function extractRequestClient (req, transact) {
60+
if (transact.length) {
61+
const requestClient = req.pg[transact]
6262
if (!requestClient) {
63-
throw new Error(`request client '${transactionConnection}' does not exist`)
63+
throw new Error(`request client '${transact}' does not exist`)
6464
}
65-
return req.pg[transactionConnection]
65+
return req.pg[transact]
6666
}
6767

6868
return req.pg
@@ -123,55 +123,63 @@ function fastifyPostgres (fastify, options, next) {
123123
}
124124

125125
fastify.addHook('onRoute', routeOptions => {
126-
const transactionConnection = routeOptions.pg && routeOptions.pg.transact
126+
const transact = routeOptions && routeOptions.pg && routeOptions.pg.transact
127127

128-
if (transactionConnection) {
129-
const preHandler = async (req, reply) => {
130-
const client = await pool.connect()
128+
if (!transact) {
129+
return
130+
}
131+
if (typeof transact === 'string' && transact !== name) {
132+
return
133+
}
134+
if (name && transact === true) {
135+
return
136+
}
131137

132-
if (name) {
133-
if (!req.pg) {
134-
req.pg = {}
135-
}
138+
const preHandler = async (req, reply) => {
139+
const client = await pool.connect()
136140

137-
if (client[name]) {
138-
throw new Error(`pg client '${name}' is a reserved keyword`)
139-
} else if (req.pg[name]) {
140-
throw new Error(`request client '${name}' has already been registered`)
141-
}
141+
if (name) {
142+
if (!req.pg) {
143+
req.pg = {}
144+
}
142145

143-
req.pg[name] = client
144-
} else {
145-
if (req.pg) {
146-
throw new Error('request client has already been registered')
147-
} else {
148-
req.pg = client
149-
}
146+
if (client[name]) {
147+
throw new Error(`pg client '${name}' is a reserved keyword`)
148+
} else if (req.pg[name]) {
149+
throw new Error(`request client '${name}' has already been registered`)
150150
}
151151

152-
extractRequestClient(req, transactionConnection).query('BEGIN')
152+
req.pg[name] = client
153+
} else {
154+
if (req.pg) {
155+
throw new Error('request client has already been registered')
156+
} else {
157+
req.pg = client
158+
}
153159
}
154160

155-
const onError = (req, reply, error, done) => {
156-
req[transactionFailedSymbol] = true
157-
extractRequestClient(req, transactionConnection).query('ROLLBACK', done)
158-
}
161+
extractRequestClient(req, transact).query('BEGIN')
162+
}
163+
164+
const onError = (req, reply, error, done) => {
165+
req[transactionFailedSymbol] = true
166+
extractRequestClient(req, transact).query('ROLLBACK', done)
167+
}
159168

160-
const onSend = async (req) => {
161-
const requestClient = extractRequestClient(req, transactionConnection)
162-
try {
163-
if (!req[transactionFailedSymbol]) {
164-
await requestClient.query('COMMIT')
165-
}
166-
} finally {
167-
requestClient.release()
169+
const onSend = async (req) => {
170+
const requestClient = extractRequestClient(req, transact)
171+
try {
172+
if (!req[transactionFailedSymbol]) {
173+
await requestClient.query('COMMIT')
168174
}
175+
} finally {
176+
requestClient.release()
169177
}
170-
171-
routeOptions.preHandler = addHandler(routeOptions.preHandler, preHandler)
172-
routeOptions.onError = addHandler(routeOptions.onError, onError)
173-
routeOptions.onSend = addHandler(routeOptions.onSend, onSend)
174178
}
179+
180+
routeOptions.preHandler = addHandler(routeOptions.preHandler, preHandler)
181+
routeOptions.onError = addHandler(routeOptions.onError, onError)
182+
routeOptions.onSend = addHandler(routeOptions.onSend, onSend)
175183
})
176184

177185
next()

‎test/req-initialization.test.js

+109-7
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const { connectionString } = require('./helpers')
88

99
const extractUserCount = response => parseInt(JSON.parse(response.payload).rows[0].userCount)
1010

11-
test('fastify postgress useTransaction route option - ', t => {
11+
test('fastify postgress useTransaction route option', t => {
1212
test('queries that succeed provided', async t => {
1313
const fastify = Fastify()
1414
t.teardown(() => fastify.close())
@@ -19,13 +19,13 @@ test('fastify postgress useTransaction route option - ', t => {
1919

2020
await fastify.pg.query('TRUNCATE users')
2121

22-
await fastify.get('/count-users', async (req, reply) => {
22+
fastify.get('/count-users', async (req, reply) => {
2323
const result = await fastify.pg.query('SELECT COUNT(*) AS "userCount" FROM users WHERE username=\'pass-opt-in\'')
2424

2525
reply.send(result)
2626
})
2727

28-
await fastify.get('/pass', { pg: { transact: true } }, async (req, reply) => {
28+
fastify.get('/pass', { pg: { transact: true } }, async (req, reply) => {
2929
await req.pg.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['pass-opt-in'])
3030
await req.pg.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['pass-opt-in'])
3131
reply.send('complete')
@@ -54,13 +54,13 @@ test('fastify postgress useTransaction route option - ', t => {
5454

5555
await fastify.pg.test.query('TRUNCATE users')
5656

57-
await fastify.get('/count-users', async (req, reply) => {
57+
fastify.get('/count-users', async (req, reply) => {
5858
const result = await fastify.pg.test.query('SELECT COUNT(*) AS "userCount" FROM users WHERE username=\'pass-opt-in\'')
5959

6060
reply.send(result)
6161
})
6262

63-
await fastify.get('/pass', { pg: { transact: 'test' } }, async (req, reply) => {
63+
fastify.get('/pass', { pg: { transact: 'test' } }, async (req, reply) => {
6464
await req.pg.test.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['pass-opt-in'])
6565
await req.pg.test.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['pass-opt-in'])
6666

@@ -89,13 +89,13 @@ test('fastify postgress useTransaction route option - ', t => {
8989

9090
await fastify.pg.query('TRUNCATE users')
9191

92-
await fastify.get('/count-users', async (req, reply) => {
92+
fastify.get('/count-users', async (req, reply) => {
9393
const result = await fastify.pg.query('SELECT COUNT(*) AS "userCount" FROM users WHERE username=\'fail-opt-in\'')
9494

9595
reply.send(result)
9696
})
9797

98-
await fastify.get('/fail', { pg: { transact: true } }, async (req, reply) => {
98+
fastify.get('/fail', { pg: { transact: true } }, async (req, reply) => {
9999
await req.pg.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['fail-opt-in'])
100100
await req.pg.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['fail-opt-in'])
101101
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 => {
117117

118118
t.end()
119119
})
120+
121+
test('combinations of registrationOptions.name and routeOptions.pg.transact that should not add hooks', t => {
122+
test('transact not set', t => {
123+
t.plan(1)
124+
125+
const fastify = Fastify()
126+
t.teardown(() => fastify.close())
127+
128+
fastify.register(fastifyPostgres, {
129+
connectionString
130+
})
131+
132+
fastify.get('/', (req, reply) => {
133+
t.is(req.pg, null)
134+
})
135+
136+
fastify.inject({
137+
method: 'GET',
138+
url: '/'
139+
})
140+
})
141+
test('name set and transact not set', t => {
142+
t.plan(1)
143+
144+
const fastify = Fastify()
145+
t.teardown(() => fastify.close())
146+
147+
fastify.register(fastifyPostgres, {
148+
connectionString,
149+
name: 'test'
150+
})
151+
152+
fastify.get('/', (req, reply) => {
153+
t.is(req.pg, null)
154+
})
155+
156+
fastify.inject({
157+
method: 'GET',
158+
url: '/'
159+
})
160+
})
161+
test('name set and transact set to true', t => {
162+
t.plan(1)
163+
164+
const fastify = Fastify()
165+
t.teardown(() => fastify.close())
166+
167+
fastify.register(fastifyPostgres, {
168+
connectionString,
169+
name: 'test'
170+
})
171+
172+
fastify.get('/', { pg: { transact: true } }, (req, reply) => {
173+
t.is(req.pg, null)
174+
})
175+
176+
fastify.inject({
177+
method: 'GET',
178+
url: '/'
179+
})
180+
})
181+
test('name not set and transact set to string', t => {
182+
t.plan(1)
183+
184+
const fastify = Fastify()
185+
t.teardown(() => fastify.close())
186+
187+
fastify.register(fastifyPostgres, {
188+
connectionString
189+
})
190+
191+
fastify.get('/', { pg: { transact: 'test' } }, (req, reply) => {
192+
t.is(req.pg, null)
193+
})
194+
195+
fastify.inject({
196+
method: 'GET',
197+
url: '/'
198+
})
199+
})
200+
test('name and transact set to different strings', t => {
201+
t.plan(1)
202+
203+
const fastify = Fastify()
204+
t.teardown(() => fastify.close())
205+
206+
fastify.register(fastifyPostgres, {
207+
connectionString,
208+
name: 'test'
209+
})
210+
211+
fastify.get('/', { pg: { transact: 'different' } }, (req, reply) => {
212+
t.is(req.pg, null)
213+
})
214+
215+
fastify.inject({
216+
method: 'GET',
217+
url: '/'
218+
})
219+
})
220+
t.end()
221+
})

0 commit comments

Comments
 (0)