From 2754e11feeeb3c936bc25abc608fd46b0a846e8d Mon Sep 17 00:00:00 2001 From: starnayuta <58112571+starnayuta@users.noreply.github.com> Date: Wed, 5 Apr 2023 03:38:23 +0900 Subject: [PATCH 1/2] fix: closeClient does not work --- README.md | 6 ++++ index.js | 14 ++++++-- test/index.test.js | 87 ++++++++++++++++++++++++++++++++++++++++++++++ types/index.d.ts | 3 ++ 4 files changed, 107 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 3ec0c38..8b1b19f 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,12 @@ fastify.register(require('@fastify/redis'), { client }) Note: by default, *@fastify/redis* will **not** automatically close the client connection when the Fastify server shuts down. +To automatically close client connections, set clientClose to true. + +```js +fastify.register(require('@fastify/redis'), { client, closeClient: true }) +``` + ## Registering multiple Redis client instances By using the `namespace` option you can register multiple Redis client instances. diff --git a/index.js b/index.js index f8e2a32..25299d4 100644 --- a/index.js +++ b/index.js @@ -4,7 +4,7 @@ const fp = require('fastify-plugin') const Redis = require('ioredis') function fastifyRedis (fastify, options, next) { - const { namespace, url, ...redisOptions } = options + const { namespace, url, closeClient = false, ...redisOptions } = options let client = options.client || null @@ -21,7 +21,11 @@ function fastifyRedis (fastify, options, next) { return fastify.redis[namespace].quit() } - if (!client) { + if (client) { + if (closeClient === true) { + fastify.addHook('onClose', closeNamedInstance) + } + } else { try { if (url) { client = new Redis(url, redisOptions) @@ -40,7 +44,11 @@ function fastifyRedis (fastify, options, next) { if (fastify.redis) { return next(new Error('@fastify/redis has already been registered')) } else { - if (!client) { + if (client) { + if (closeClient === true) { + fastify.addHook('onClose', close) + } + } else { try { if (url) { client = new Redis(url, redisOptions) diff --git a/test/index.test.js b/test/index.test.js index c81872f..75d966f 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -242,6 +242,93 @@ test('custom ioredis client that is already connected', (t) => { }) }) +test('If closeClient is enabled, close the client.', (t) => { + t.plan(10) + const fastify = Fastify() + const Redis = require('ioredis') + const redis = new Redis({ host: 'localhost', port: 6379 }) + + redis.set('key', 'value', (err) => { + t.error(err) + redis.get('key', (err, val) => { + t.error(err) + t.equal(val, 'value') + + fastify.register(fastifyRedis, { + client: redis, + closeClient: true + }) + + fastify.ready((err) => { + t.error(err) + t.equal(fastify.redis, redis) + + fastify.redis.set('key2', 'value2', (err) => { + t.error(err) + fastify.redis.get('key2', (err, val) => { + t.error(err) + t.equal(val, 'value2') + + const originalQuit = fastify.redis.quit + fastify.redis.quit = (callback) => { + t.pass('redis client closed') + originalQuit.call(fastify.redis, callback) + } + + fastify.close(function (err) { + t.error(err) + }) + }) + }) + }) + }) + }) +}) + +test('If closeClient is enabled, close the client namespace.', (t) => { + t.plan(10) + const fastify = Fastify() + const Redis = require('ioredis') + const redis = new Redis({ host: 'localhost', port: 6379 }) + + redis.set('key', 'value', (err) => { + t.error(err) + redis.get('key', (err, val) => { + t.error(err) + t.equal(val, 'value') + + fastify.register(fastifyRedis, { + client: redis, + namespace: 'foo', + closeClient: true + }) + + fastify.ready((err) => { + t.error(err) + t.equal(fastify.redis.foo, redis) + + fastify.redis.foo.set('key2', 'value2', (err) => { + t.error(err) + fastify.redis.foo.get('key2', (err, val) => { + t.error(err) + t.equal(val, 'value2') + + const originalQuit = fastify.redis.foo.quit + fastify.redis.foo.quit = (callback) => { + t.pass('redis client closed') + originalQuit.call(fastify.redis.foo, callback) + } + + fastify.close(function (err) { + t.error(err) + }) + }) + }) + }) + }) + }) +}) + test('fastify.redis.test should throw with duplicate connection namespaces', (t) => { t.plan(1) diff --git a/types/index.d.ts b/types/index.d.ts index 4372db8..f218904 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -24,6 +24,9 @@ declare namespace fastifyRedis { }) | { client: Redis | Cluster; namespace?: string; + /** + * @default false + */ closeClient?: boolean; } /* From dfd9777f37dfaa840435d5738b591ac5160bbd94 Mon Sep 17 00:00:00 2001 From: starnayuta <58112571+starnayuta@users.noreply.github.com> Date: Tue, 11 Apr 2023 00:20:19 +0900 Subject: [PATCH 2/2] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8b1b19f..d374c15 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ fastify.register(require('@fastify/redis'), { client }) Note: by default, *@fastify/redis* will **not** automatically close the client connection when the Fastify server shuts down. -To automatically close client connections, set clientClose to true. +To automatically close the client connection, set clientClose to true. ```js fastify.register(require('@fastify/redis'), { client, closeClient: true })