From 81d58e0ccaa3a255ce04d1a7fbe8be5cb63f9808 Mon Sep 17 00:00:00 2001 From: darkgl0w <31093081+darkgl0w@users.noreply.github.com> Date: Mon, 20 Sep 2021 18:10:54 +0200 Subject: [PATCH 1/7] chore: upgrade package dependencies --- package.json | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 9ff49df..8d9bfb8 100644 --- a/package.json +++ b/package.json @@ -36,15 +36,15 @@ "fastify-plugin": "^3.0.0" }, "devDependencies": { - "@tsconfig/node10": "^1.0.7", - "@types/pg": "^8.6.0", - "fastify": "^3.0.0", - "pg": "^8.2.1", + "@tsconfig/node10": "^1.0.8", + "@types/pg": "^8.6.1", + "fastify": "^3.21.3", + "pg": "^8.7.1", "pg-native": "^3.0.0", - "standard": "^16.0.0", - "tap": "^15.0.2", + "standard": "^16.0.3", + "tap": "^15.0.9", "tsd": "^0.17.0", - "typescript": "^4.0.2" + "typescript": "^4.4.3" }, "peerDependencies": { "pg": ">=6.0.0" From 156de789cad58b84cc331365a1f4deb9016c9ba1 Mon Sep 17 00:00:00 2001 From: darkgl0w <31093081+darkgl0w@users.noreply.github.com> Date: Mon, 20 Sep 2021 18:11:44 +0200 Subject: [PATCH 2/7] chore: add a `lint:fix` command --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index 8d9bfb8..5dddec2 100644 --- a/package.json +++ b/package.json @@ -6,6 +6,7 @@ "types": "index.d.ts", "scripts": { "check-examples": "tsc --build examples/typescript/*", + "lint:fix": "standard --fix", "load-data": "docker exec -it fastify-postgres psql -c 'CREATE TABLE users(id serial PRIMARY KEY, username VARCHAR (50) NOT NULL);' -U postgres -d postgres", "postgres": "docker run -p 5432:5432 --name fastify-postgres -e POSTGRES_PASSWORD=postgres -d postgres:11-alpine", "test": "standard && tap -J test/*.test.js && npm run test:typescript", From 304e9b51c909ae6b089a24cc6b9ca8201256fad7 Mon Sep 17 00:00:00 2001 From: darkgl0w <31093081+darkgl0w@users.noreply.github.com> Date: Mon, 20 Sep 2021 18:16:09 +0200 Subject: [PATCH 3/7] test: add new test cases and make test names more explicit --- test/add-handler.test.js | 13 +++--- test/initialization.test.js | 38 +++++++++++++++- test/req-initialization.test.js | 77 +++++++++++++++++++++++++-------- 3 files changed, 103 insertions(+), 25 deletions(-) diff --git a/test/add-handler.test.js b/test/add-handler.test.js index 1ae4eba..72ab91e 100644 --- a/test/add-handler.test.js +++ b/test/add-handler.test.js @@ -1,11 +1,10 @@ 'use strict' -const t = require('tap') -const test = t.test +const { test } = require('tap') const addHandler = require('../lib/add-handler') -test('addHandler - ', t => { - test('when existing handler is not defined', t => { +test('The addHandler lib should return the right handlers structure', t => { + t.test('When the existingHandler argument is undefined', t => { t.plan(1) const handlers = addHandler( @@ -15,7 +14,8 @@ test('addHandler - ', t => { t.same(handlers, ['test']) }) - test('when existing handler is a array', t => { + + t.test('When the existingHandler argument is an array', t => { t.plan(1) const handlers = addHandler( @@ -25,7 +25,8 @@ test('addHandler - ', t => { t.same(handlers, ['test', 'again']) }) - test('when existing handler is a function', t => { + + t.test('When the existingHandler argument is a function', t => { t.plan(2) const stub = () => 'test' diff --git a/test/initialization.test.js b/test/initialization.test.js index 0a49788..ea9d847 100644 --- a/test/initialization.test.js +++ b/test/initialization.test.js @@ -1,7 +1,6 @@ 'use strict' -const t = require('tap') -const test = t.test +const { test } = require('tap') const Fastify = require('fastify') const pg = require('pg') const fastifyPostgres = require('../index') @@ -247,3 +246,38 @@ test('fastify.pg custom namespace should exist if a name is set', (t) => { t.ok(fastify.pg.test.Client) }) }) + +test('fastify.pg and a fastify.pg custom namespace should exist when registering a named instance before an unnamed instance)', (t) => { + t.plan(11) + + const fastify = Fastify() + t.teardown(() => fastify.close()) + + fastify.register(fastifyPostgres, { + connectionString, + name: 'one' + }) + + fastify.register(fastifyPostgres, { + connectionString + }) + + fastify.ready(async (err) => { + t.error(err) + + t.ok(fastify.pg) + t.ok(fastify.pg.connect) + t.ok(fastify.pg.pool) + t.ok(fastify.pg.Client) + + t.ok(fastify.pg.one) + t.ok(fastify.pg.one.connect) + t.ok(fastify.pg.one.pool) + t.ok(fastify.pg.one.Client) + + const result = await fastify.pg.query('SELECT NOW()') + const resultOne = await fastify.pg.one.query('SELECT NOW()') + t.same(result.rowCount, 1) + t.same(resultOne.rowCount, 1) + }) +}) diff --git a/test/req-initialization.test.js b/test/req-initialization.test.js index 707f54a..2c1fd01 100644 --- a/test/req-initialization.test.js +++ b/test/req-initialization.test.js @@ -1,15 +1,14 @@ 'use strict' -const t = require('tap') -const test = t.test +const { test } = require('tap') const Fastify = require('fastify') const fastifyPostgres = require('../index') const { connectionString } = require('./helpers') const extractUserCount = response => parseInt(JSON.parse(response.payload).rows[0].userCount) -test('fastify postgress useTransaction route option', t => { - test('queries that succeed provided', async t => { +test('When we use the fastify-postgres transaction route option', t => { + t.test('Should be able to execute queries provided to the request pg decorator', async t => { const fastify = Fastify() t.teardown(() => fastify.close()) @@ -40,7 +39,8 @@ test('fastify postgress useTransaction route option', t => { t.equal(extractUserCount(response), 2) }) - test('queries that succeed provided to a namespace', async t => { + + t.test('Should be able to execute queries provided to a namespaced request pg decorator', async t => { const fastify = Fastify() t.teardown(() => fastify.close()) @@ -73,7 +73,8 @@ test('fastify postgress useTransaction route option', t => { t.equal(extractUserCount(response), 2) }) - test('queries that fail provided', async t => { + + t.test('Should trigger a rollback when failing to execute a query provided to the request pg decorator', async t => { const fastify = Fastify() t.teardown(() => fastify.close()) @@ -92,7 +93,43 @@ test('fastify postgress useTransaction route option', t => { 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']) + // This one should fail (unknown_table does no exists) and trigger a rollback + await req.pg.query('INSERT INTO unknown_table(username) VALUES($1) RETURNING id', ['fail-opt-in']) + reply.send('complete') + }) + + await fastify.inject({ url: '/fail' }) + + const response = await fastify.inject({ + method: 'GET', + url: '/count-users' + }) + + t.equal(extractUserCount(response), 0) + }) + + t.test('Should trigger a rollback when failing to execute a query provided to a namespaced request pg decorator', async t => { + const fastify = Fastify() + t.teardown(() => fastify.close()) + + await fastify.register(fastifyPostgres, { + connectionString, + name: 'test' + }) + + await fastify.pg.test.query('TRUNCATE users') + + fastify.get('/count-users', async (req, reply) => { + const result = await fastify.pg.test.query('SELECT COUNT(*) AS "userCount" FROM users WHERE username=\'fail-opt-in\'') + + reply.send(result) + }) + + fastify.get('/fail', { pg: { transact: true } }, async (req, reply) => { + await req.pg.test.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['fail-opt-in']) + await req.pg.test.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['fail-opt-in']) + // This one should fail (unknown_table does no exists) and trigger a rollback + await req.pg.test.query('INSERT INTO unknown_table(username) VALUES($1) RETURNING id', ['fail-opt-in']) reply.send('complete') }) @@ -109,8 +146,8 @@ 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 => { +test('Should not add hooks with combinations of registration `options.name` and route options `pg.transact`', t => { + t.test('Should not add hooks when `transact` is not set', t => { t.plan(1) const fastify = Fastify() @@ -126,7 +163,8 @@ test('combinations of registrationOptions.name and routeOptions.pg.transact that fastify.inject({ url: '/' }) }) - test('name set and transact not set', t => { + + t.test('Should not add hooks when `name` is set and `transact` is not set', t => { t.plan(1) const fastify = Fastify() @@ -143,7 +181,8 @@ test('combinations of registrationOptions.name and routeOptions.pg.transact that fastify.inject({ url: '/' }) }) - test('name set and transact set to true', t => { + + t.test('Should not add hooks when `name` is set and `transact` is set to `true`', t => { t.plan(1) const fastify = Fastify() @@ -160,7 +199,8 @@ test('combinations of registrationOptions.name and routeOptions.pg.transact that fastify.inject({ url: '/' }) }) - test('name not set and transact set to string', t => { + + t.test('Should not add hooks when `name` is not set and `transact` is set and is a string', t => { t.plan(1) const fastify = Fastify() @@ -176,7 +216,8 @@ test('combinations of registrationOptions.name and routeOptions.pg.transact that fastify.inject({ url: '/' }) }) - test('name and transact set to different strings', t => { + + t.test('Should not add hooks when `name` and `transact` are set to different strings', t => { t.plan(1) const fastify = Fastify() @@ -193,11 +234,12 @@ test('combinations of registrationOptions.name and routeOptions.pg.transact that fastify.inject({ url: '/' }) }) + t.end() }) -test('incorrect combinations of registrationOptions.name and routeOptions.pg.transact should throw errors', t => { - t.test('name set as reserved keyword', t => { +test('Should throw errors with incorrect combinations of registration `options.name` and route options `pg.transact`', t => { + t.test('Should throw an error when `name` is set as reserved keyword', t => { t.plan(2) const fastify = Fastify() @@ -222,7 +264,7 @@ test('incorrect combinations of registrationOptions.name and routeOptions.pg.tra }) }) - t.test('named pg client has already registered', t => { + t.test('Should throw an error when pg client has already been registered with the same name', t => { t.plan(2) const fastify = Fastify() @@ -249,7 +291,7 @@ test('incorrect combinations of registrationOptions.name and routeOptions.pg.tra }) }) - t.test('pg client has already registered', t => { + t.test('Should throw an error when pg client has already been registered', t => { t.plan(2) const fastify = Fastify() @@ -272,5 +314,6 @@ test('incorrect combinations of registrationOptions.name and routeOptions.pg.tra }) }) }) + t.end() }) From b6f12f0dbf79fd8ad7fad524a4f66d3eee75c474 Mon Sep 17 00:00:00 2001 From: darkgl0w <31093081+darkgl0w@users.noreply.github.com> Date: Mon, 20 Sep 2021 20:00:06 +0200 Subject: [PATCH 4/7] refactor: improve code readability --- index.js | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/index.js b/index.js index 9299108..d5cab2b 100644 --- a/index.js +++ b/index.js @@ -102,19 +102,21 @@ function fastifyPostgres (fastify, options, next) { if (db[name]) { return next(new Error(`fastify-postgres '${name}' is a reserved keyword`)) } else if (!fastify.pg) { - fastify.decorate('pg', {}) + fastify.decorate('pg', Object.create(null)) } else if (fastify.pg[name]) { return next(new Error(`fastify-postgres '${name}' instance name has already been registered`)) } fastify.pg[name] = db } else { - if (!fastify.pg) { - fastify.decorate('pg', db) - } else if (fastify.pg.pool) { - return next(new Error('fastify-postgres has already been registered')) - } else { + if (fastify.pg) { + if (fastify.pg.pool) { + return next(new Error('fastify-postgres has already been registered')) + } + Object.assign(fastify.pg, db) + } else { + fastify.decorate('pg', db) } } @@ -125,13 +127,11 @@ function fastifyPostgres (fastify, options, next) { fastify.addHook('onRoute', routeOptions => { const transact = routeOptions && routeOptions.pg && routeOptions.pg.transact - if (!transact) { - return - } - if (typeof transact === 'string' && transact !== name) { - return - } - if (name && transact === true) { + if ( + !transact || + (typeof transact === 'string' && transact !== name) || + (name && transact === true) + ) { return } @@ -140,7 +140,7 @@ function fastifyPostgres (fastify, options, next) { if (name) { if (!req.pg) { - req.pg = {} + req.pg = Object.create(null) } if (client[name]) { From 4fce0c82e2eb1550cc61b774dd0da6a48dee538d Mon Sep 17 00:00:00 2001 From: darkgl0w <31093081+darkgl0w@users.noreply.github.com> Date: Thu, 23 Sep 2021 10:21:20 +0200 Subject: [PATCH 5/7] chore: upgrade package dependencies --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 5dddec2..6231ec1 100644 --- a/package.json +++ b/package.json @@ -39,11 +39,11 @@ "devDependencies": { "@tsconfig/node10": "^1.0.8", "@types/pg": "^8.6.1", - "fastify": "^3.21.3", + "fastify": "^3.21.5", "pg": "^8.7.1", "pg-native": "^3.0.0", "standard": "^16.0.3", - "tap": "^15.0.9", + "tap": "^15.0.10", "tsd": "^0.17.0", "typescript": "^4.4.3" }, From 28e391ec3b7a91d1b848cedecb84d4e4c9877ca6 Mon Sep 17 00:00:00 2001 From: darkgl0w <31093081+darkgl0w@users.noreply.github.com> Date: Thu, 23 Sep 2021 10:55:25 +0200 Subject: [PATCH 6/7] refactor (test): typo fix --- test/req-initialization.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/req-initialization.test.js b/test/req-initialization.test.js index 2c1fd01..08a2203 100644 --- a/test/req-initialization.test.js +++ b/test/req-initialization.test.js @@ -93,7 +93,7 @@ test('When we use the fastify-postgres transaction route option', t => { 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']) - // This one should fail (unknown_table does no exists) and trigger a rollback + // This one should fail (unknown_table does not exist) and trigger a rollback await req.pg.query('INSERT INTO unknown_table(username) VALUES($1) RETURNING id', ['fail-opt-in']) reply.send('complete') }) @@ -128,7 +128,7 @@ test('When we use the fastify-postgres transaction route option', t => { fastify.get('/fail', { pg: { transact: true } }, async (req, reply) => { await req.pg.test.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['fail-opt-in']) await req.pg.test.query('INSERT INTO users(username) VALUES($1) RETURNING id', ['fail-opt-in']) - // This one should fail (unknown_table does no exists) and trigger a rollback + // This one should fail (unknown_table does not exist) and trigger a rollback await req.pg.test.query('INSERT INTO unknown_table(username) VALUES($1) RETURNING id', ['fail-opt-in']) reply.send('complete') }) From 8b63c12de2b68b950b8b25c6ca322b5efa9738dc Mon Sep 17 00:00:00 2001 From: darkgl0w <31093081+darkgl0w@users.noreply.github.com> Date: Thu, 23 Sep 2021 11:58:41 +0200 Subject: [PATCH 7/7] refactor (test): apply @mcollina suggestion to use await fastify.ready() --- test/initialization.test.js | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/test/initialization.test.js b/test/initialization.test.js index ea9d847..0eaeec3 100644 --- a/test/initialization.test.js +++ b/test/initialization.test.js @@ -247,37 +247,35 @@ test('fastify.pg custom namespace should exist if a name is set', (t) => { }) }) -test('fastify.pg and a fastify.pg custom namespace should exist when registering a named instance before an unnamed instance)', (t) => { - t.plan(11) +test('fastify.pg and a fastify.pg custom namespace should exist when registering a named instance before an unnamed instance)', async (t) => { + t.plan(10) const fastify = Fastify() t.teardown(() => fastify.close()) - fastify.register(fastifyPostgres, { + await fastify.register(fastifyPostgres, { connectionString, name: 'one' }) - fastify.register(fastifyPostgres, { + await fastify.register(fastifyPostgres, { connectionString }) - fastify.ready(async (err) => { - t.error(err) + await fastify.ready().catch(err => t.error(err)) - t.ok(fastify.pg) - t.ok(fastify.pg.connect) - t.ok(fastify.pg.pool) - t.ok(fastify.pg.Client) + t.ok(fastify.pg) + t.ok(fastify.pg.connect) + t.ok(fastify.pg.pool) + t.ok(fastify.pg.Client) - t.ok(fastify.pg.one) - t.ok(fastify.pg.one.connect) - t.ok(fastify.pg.one.pool) - t.ok(fastify.pg.one.Client) + t.ok(fastify.pg.one) + t.ok(fastify.pg.one.connect) + t.ok(fastify.pg.one.pool) + t.ok(fastify.pg.one.Client) - const result = await fastify.pg.query('SELECT NOW()') - const resultOne = await fastify.pg.one.query('SELECT NOW()') - t.same(result.rowCount, 1) - t.same(resultOne.rowCount, 1) - }) + const result = await fastify.pg.query('SELECT NOW()') + const resultOne = await fastify.pg.one.query('SELECT NOW()') + t.same(result.rowCount, 1) + t.same(resultOne.rowCount, 1) })