From a1a5c54d7a476625cfd11822040dd500036eb507 Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Tue, 29 Mar 2022 22:00:52 -0700 Subject: [PATCH 1/4] dep: redis 3 -> 4, API change to promises --- .codeclimate.yml | 4 +- .eslintrc.yaml | 3 + .github/workflows/ci-test.yml | 6 +- .github/workflows/lint.yml | 6 +- .github/workflows/master.yml | 1 + .npmignore | 5 +- Changes.md | 6 ++ README.md | 4 +- config/redis.ini | 5 +- index.js | 160 +++++++++++++++++----------------- package.json | 10 +-- test/config/redis.ini | 5 +- test/redis.js | 90 ++++++++++--------- 13 files changed, 158 insertions(+), 147 deletions(-) diff --git a/.codeclimate.yml b/.codeclimate.yml index 461d96c..0e443ca 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -1,9 +1,9 @@ engines: eslint: enabled: true - channel: "eslint-3" + channel: "eslint-8" config: - config: ".eslintrc" + config: ".eslintrc.yaml" ratings: paths: diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 8fc24f4..f7ecf96 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -9,6 +9,9 @@ extends: [ eslint:recommended, plugin:haraka/recommended ] root: true +parserOptions: + ecmaVersion: 2020 + globals: OK: true CONT: true diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index f4bf903..f68916f 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -2,6 +2,9 @@ name: Tests on: [ push ] +env: + CI: true + jobs: ci-test: @@ -20,9 +23,6 @@ jobs: node-version: [ 12, 14, 16] fail-fast: false - env: - CI: true - steps: - uses: actions/checkout@v2 name: Checkout Code diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 66f1323..ffb4b16 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -2,6 +2,9 @@ name: Lint on: [ push ] +env: + CI: true + jobs: lint: @@ -28,6 +31,3 @@ jobs: - name: Lint run: npm run lint - - env: - CI: true diff --git a/.github/workflows/master.yml b/.github/workflows/master.yml index c4db8ea..7f682e5 100644 --- a/.github/workflows/master.yml +++ b/.github/workflows/master.yml @@ -7,6 +7,7 @@ on: - master env: + CI: true node_version: 14 jobs: diff --git a/.npmignore b/.npmignore index 119814b..22ce579 100644 --- a/.npmignore +++ b/.npmignore @@ -4,8 +4,11 @@ .gitignore .gitmodules .lgtm.yml -appveyor.yml +http/bower_components +http/node_modules .travis.yml +appveyor.yml .eslintrc.yaml .eslintrc.json codecov.yml +.codeclimate.yml \ No newline at end of file diff --git a/Changes.md b/Changes.md index e80c879..9a8709b 100644 --- a/Changes.md +++ b/Changes.md @@ -1,4 +1,10 @@ +### 1.0.14 - 2022-03-29 + +- bump redis major version 3 -> 4 +- API change, callbacks replaced by promises + + ### 1.0.13 - 2021-10-14 - switch CI from Travis to GitHub Actions diff --git a/README.md b/README.md index cb7f96d..77a8625 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ attached to another database. The `redis.ini` file has the following sections (defaults shown): -### [server] +### [socket] ```ini ; host=127.0.0.1 @@ -25,7 +25,7 @@ The `redis.ini` file has the following sections (defaults shown): ; port=6379 ``` -Publish & Subscribe are DB agnostic and thus have no db setting. If host and port and not defined, they default to the same as [server] settings. +Publish & Subscribe are DB agnostic and thus have no db setting. If host and port and not defined, they default to the same as [socket] settings. ### [opts] diff --git a/config/redis.ini b/config/redis.ini index b86fc2b..e134be1 100644 --- a/config/redis.ini +++ b/config/redis.ini @@ -1,13 +1,12 @@ -[server] +[socket] ; host=127.0.0.1 ; port=6379 -; db=0 [pubsub] ; host=127.0.0.1 ; port=6379 [opts] -; db=0 +; database=0 ; password=dontUseThisOne diff --git a/index.js b/index.js index 5e8c375..8fb0ece 100644 --- a/index.js +++ b/index.js @@ -16,7 +16,7 @@ exports.register = function () { plugin.register_hook('init_child', 'init_redis_shared'); } -const defaultOpts = { host: '127.0.0.1', port: '6379' }; +const defaultOpts = { socket: { host: '127.0.0.1', port: '6379' } } exports.load_redis_ini = function () { const plugin = this; @@ -28,10 +28,27 @@ exports.load_redis_ini = function () { }); const rc = plugin.redisCfg; - plugin.redisCfg.server = Object.assign({}, defaultOpts, rc.opts, rc.server); - if (rc.server.ip && !rc.server.host) rc.server.host = rc.server.ip; // backwards compat + plugin.redisCfg.server = Object.assign({}, defaultOpts, rc.opts, rc.socket); - plugin.redisCfg.pubsub = Object.assign({}, defaultOpts, rc.opts, rc.server, rc.pubsub); + // backwards compat + if (rc.server.ip && !rc.server.host) { + rc.server.host = rc.server.ip + delete rc.server.ip + } + + // backwards compat with node-redis < 4 + if (rc.server && !rc.socket) { + rc.socket = rc.server + delete rc.server + } + + // same as above + if (rc.db && !rc.database) { + rc.database = rc.db + delete rc.db + } + + plugin.redisCfg.pubsub = Object.assign({}, defaultOpts, rc.opts, rc.socket, rc.pubsub); } exports.merge_redis_ini = function () { @@ -87,7 +104,7 @@ exports.init_redis_plugin = function (next, server) { if (!plugin.cfg) plugin.cfg = { redis: {} }; if (!server) server = { notes: {} }; - const pidb = plugin.cfg.redis.db; + const pidb = plugin.cfg.redis.database; if (server.notes.redis) { // server-wide redis is available // and the DB not specified or is the same as server-wide if (pidb === undefined || pidb === plugin.redisCfg.db) { @@ -102,62 +119,70 @@ exports.init_redis_plugin = function (next, server) { } exports.shutdown = function () { - if (this.db) this.db.quit(); + if (this.db) this.db.disconnect(); if (server && server.notes && server.notes.redis) { - server.notes.redis.quit(); + server.notes.redis.disconnect(); } } -exports.redis_ping = function (done) { +exports.redis_ping = async function () { const plugin = this; - function nope (err) { - if (err) plugin.logerror(err.message); - plugin.redis_pings=false; - done(err); - } + plugin.redis_pings=false; if (!plugin.db) { - return nope(new Error('redis not initialized')); + return new Error('redis not initialized'); } - plugin.db.ping((err, res) => { - if (err) return nope(err); - if (res !== 'PONG') return nope(new Error('not PONG')); - plugin.redis_pings=true; - done(err, true); - }); + try { + const r = await plugin.db.ping() + if (r !== 'PONG') return new Error('not PONG'); + plugin.redis_pings=true + } + catch (e) { + plugin.logerror(e.message) + } } function getUriStr (client, opts) { - let msg = `redis://${opts.host}:${opts.port}`; - if (opts.db) msg += `/${opts.db}`; - if (client && client.server_info && client.server_info.redis_version) { - msg += `\tv${client.server_info.redis_version}`; + let msg = `redis://${opts?.socket?.host}:${opts?.socket?.port}`; + if (opts.database) msg += `/${opts.database}`; + if (client?.server_info?.redis_version) { + msg += `\tv${client?.server_info?.redis_version}`; } return msg; } -exports.get_redis_client = function (opts, next) { +exports.get_redis_client = async function (opts) { + + const client = redis.createClient(opts) - const client = redis.createClient(opts); - const urlStr = getUriStr(client, opts); + let urlStr client .on('error', (err) => { this.logerror(err.message); - next(err); - }) - .on('ready', () => { - this.loginfo(`connected to ${urlStr}`); - next(); }) .on('end', () => { this.loginfo(`Disconnected from ${urlStr}`); - }); + }) + + + try { + await client.connect() + + if (opts.database) client.dbid = opts.database + + client.server_info = await client.info() + urlStr = getUriStr(client, opts) + this.loginfo(`connected to ${urlStr}`); - return client; + return client + } + catch (e) { + console.error(e) + } } exports.get_redis_pub_channel = function (conn) { @@ -168,71 +193,46 @@ exports.get_redis_sub_channel = function (conn) { return `result-${conn.uuid}*`; } -exports.redis_subscribe_pattern = function (pattern, next) { +exports.redis_subscribe_pattern = async function (pattern) { const plugin = this; - if (plugin.redis) return next(); // already subscribed? + if (plugin.redis) return // already subscribed? - plugin.redis = require('redis').createClient(plugin.redisCfg.pubsub) - .on('error', function (err) { - next(err.message); - }) - .on('psubscribe', function (pattern2, count) { - plugin.logdebug(plugin, `psubscribed to ${pattern2}`); - next(); - }) - .on('punsubscribe', function (pattern3, count) { - plugin.logdebug(plugin, `unsubsubscribed from ${pattern3}`); - connection.notes.redis.quit(); - }); + plugin.redis = await redis.createClient(plugin.redisCfg.pubsub) - plugin.redis.psubscribe(pattern); + await plugin.redis.psubscribe(pattern); + plugin.logdebug(plugin, `psubscribed to ${pattern}`); } -exports.redis_subscribe = function (connection, next) { - const plugin = this; +exports.redis_subscribe = async function (connection) { if (connection.notes.redis) { - connection.logdebug(plugin, `redis already subscribed`); - // another plugin has already called this. Do nothing - return next(); - } - - let calledNext = false; - function nextOnce (errMsg) { - if (calledNext) return; - calledNext = true; - if (errMsg && connection) connection.logerror(plugin, errMsg); - next(); + connection.logdebug(this, `redis already subscribed`); + return; // another plugin has already called this. } const timer = setTimeout(() => { - nextOnce('redis psubscribe timed out'); + connection.logerror('redis psubscribe timed out'); }, 3 * 1000); - connection.notes.redis = require('redis').createClient(plugin.redisCfg.pubsub) - .on('error', (err) => { - clearTimeout(timer); - nextOnce(err.message); - }) - .on('psubscribe', function (pattern, count) { - clearTimeout(timer); - connection.logdebug(plugin, `psubscribed to ${pattern}`); - nextOnce(); - }) - .on('punsubscribe', function (pattern, count) { - connection.logdebug(plugin, `unsubsubscribed from ${pattern}`); - connection.notes.redis.quit(); - }); + connection.notes.redis = await redis.createClient(this.redisCfg.pubsub) - connection.notes.redis.psubscribe(plugin.get_redis_sub_channel(connection)); + clearTimeout(timer); + + const pattern = this.get_redis_sub_channel(connection) + connection.notes.redis.psubscribe(pattern); + connection.logdebug(this, `psubscribed to ${pattern}`); } -exports.redis_unsubscribe = function (connection) { +exports.redis_unsubscribe = async function (connection) { if (!connection.notes.redis) { connection.logerror(this, `redis_unsubscribe called when no redis`) return; } - connection.notes.redis.punsubscribe(this.get_redis_sub_channel(connection)); + + const pattern = this.get_redis_sub_channel(connection) + await connection.notes.redis.punsubscribe(pattern); + connection.logdebug(this, `unsubsubscribed from ${pattern}`); + connection.notes.redis.disconnect(); } diff --git a/package.json b/package.json index 660f2ba..e4ee81c 100644 --- a/package.json +++ b/package.json @@ -1,25 +1,25 @@ { "name": "haraka-plugin-redis", - "version": "1.0.13", + "version": "2.0.0", "description": "Redis plugin for Haraka & other plugins to inherit from", "main": "index.js", "directories": { "test": "test" }, "dependencies": { - "redis": "^3.1.2" + "redis": "^4.0.0" }, "devDependencies": { - "eslint": ">=7", + "eslint": ">=8", "eslint-plugin-haraka": "*", "haraka-test-fixtures": "*", - "mocha": "*" + "mocha": "^9.0.0" }, "scripts": { "lint": "npx eslint *.js test/*.js", "lintfix": "npx eslint --fix *.js test/*.js", "cover": "NODE_ENV=cov npx nyc --reporter=lcovonly npm run test", - "test": "npx mocha --exit" + "test": "npx mocha" }, "repository": { "type": "git", diff --git a/test/config/redis.ini b/test/config/redis.ini index f190a84..d8e0fa3 100644 --- a/test/config/redis.ini +++ b/test/config/redis.ini @@ -1,12 +1,13 @@ +[socket] ; host=127.0.0.1 ; port=6379 -; db=0 +; path=/var/db/redis.sock [pubsub] ; host=127.0.0.1 ; port=6379 [opts] -db=5 +database=5 password=dontUseThisOne \ No newline at end of file diff --git a/test/redis.js b/test/redis.js index 2658add..d807472 100644 --- a/test/redis.js +++ b/test/redis.js @@ -13,102 +13,100 @@ function retry (options) { } describe('config', function () { - before(function (done) { + before(async function () { this.plugin = new fixtures.plugin('index') this.plugin.register() - done() }) - it('loads', function (done) { + it('loads', async function () { assert.equal(this.plugin.name, 'index'); - done() }) - it('config defaults', function (done) { - assert.equal(this.plugin.redisCfg.server.host, '127.0.0.1') - assert.equal(this.plugin.redisCfg.server.port, 6379) - done() + it('config defaults', async function () { + assert.equal(this.plugin.redisCfg.server.socket.host, '127.0.0.1') + assert.equal(this.plugin.redisCfg.server.socket.port, 6379) }) - it('merges [opts] into server config', function (done) { + it('merges [opts] into server config', async function () { this.plugin.config = this.plugin.config.module_config(path.resolve('test')); this.plugin.load_redis_ini(); assert.deepEqual(this.plugin.redisCfg, { main: {}, + socket: {}, pubsub: { - host: '127.0.0.1', - port: '6379', - db: 5, + socket: { + host: '127.0.0.1', + port: '6379', + }, + database: 5, password: 'dontUseThisOne' }, - opts: { db: 5, password: 'dontUseThisOne' }, + opts: { database: 5, password: 'dontUseThisOne' }, server: { - host: '127.0.0.1', - port: '6379', - db: 5, + socket: { + host: '127.0.0.1', + port: '6379', + }, + database: 5, password: 'dontUseThisOne' } }); - done(); }) - it('merges redis.ini [opts] into plugin config', function (done) { + it('merges redis.ini [opts] into plugin config', async function () { this.plugin.config = this.plugin.config.module_config(path.resolve('test')); this.plugin.load_redis_ini(); this.plugin.cfg = {}; this.plugin.merge_redis_ini(); assert.deepEqual(this.plugin.cfg, { redis: { - host: '127.0.0.1', - port: '6379', - db: 5, + socket: { + host: '127.0.0.1', + port: '6379', + }, + database: 5, password: 'dontUseThisOne' } }) - done() }) }) describe('connects', function () { - before(function (done) { + before(async function () { this.plugin = new fixtures.plugin('index') this.plugin.register() - done() }) - it('loads', function (done) { + it('loads', async function () { assert.equal(this.plugin.name, 'index'); - done(); }) - it('connects', function (done) { - const redis = this.plugin.get_redis_client({ - host: this.plugin.redisCfg.server.host, - port: this.plugin.redisCfg.server.port, + it('connects', async function () { + const redis = await this.plugin.get_redis_client({ + socket: { + host: this.plugin.redisCfg.server.host, + port: this.plugin.redisCfg.server.port, + }, retry_strategy: retry, - }, - function () { - assert.ok(redis.connected); - done(); - }); + }) + assert.ok(redis); + redis.disconnect() }) - it('populates plugin.cfg.redis when asked', function (done) { + it('populates plugin.cfg.redis when asked', async function () { assert.equal(this.plugin.cfg, undefined); this.plugin.merge_redis_ini(); - assert.deepEqual(this.plugin.cfg.redis, { host: '127.0.0.1', port: '6379' }); - done(); + assert.deepEqual(this.plugin.cfg.redis, { socket: { host: '127.0.0.1', port: '6379' } }); }) - it('connects to a different redis db', function (done) { + it('connects to a different redis db', async function () { this.plugin.merge_redis_ini(); - this.plugin.cfg.redis.db = 2; + this.plugin.cfg.redis.database = 2; this.plugin.cfg.redis.retry_strategy = retry; - const client = this.plugin.get_redis_client(this.plugin.cfg.redis, function () { - // console.log(client); - assert.equal(client.connected, true) - assert.equal(client.selected_db, 2) - done() - }) + const client = await this.plugin.get_redis_client(this.plugin.cfg.redis) + const res = await client.ping() + assert.equal(res, 'PONG') + assert.ok(client) + await client.disconnect() }) }) From 678551f7f459bb2f85379bcf4493b743e084b822 Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Tue, 29 Mar 2022 22:13:41 -0700 Subject: [PATCH 2/4] - shed some usage of 'plugin = this' --- .github/workflows/ci-test.yml | 2 +- Changes.md | 3 ++- README.md | 20 ++++++++---------- index.js | 39 +++++++++++++++-------------------- 4 files changed, 29 insertions(+), 35 deletions(-) diff --git a/.github/workflows/ci-test.yml b/.github/workflows/ci-test.yml index f68916f..9222100 100644 --- a/.github/workflows/ci-test.yml +++ b/.github/workflows/ci-test.yml @@ -20,7 +20,7 @@ jobs: strategy: matrix: os: [ ubuntu-latest ] - node-version: [ 12, 14, 16] + node-version: [ 14, 16 ] fail-fast: false steps: diff --git a/Changes.md b/Changes.md index 9a8709b..06eed55 100644 --- a/Changes.md +++ b/Changes.md @@ -1,5 +1,6 @@ -### 1.0.14 - 2022-03-29 + +### 2.0.0 - 2022-03-29 - bump redis major version 3 -> 4 - API change, callbacks replaced by promises diff --git a/README.md b/README.md index 77a8625..f53609b 100644 --- a/README.md +++ b/README.md @@ -30,8 +30,8 @@ Publish & Subscribe are DB agnostic and thus have no db setting. If host and por ### [opts] ```ini -; see https://www.npmjs.com/package/redis#options-object-properties -; db=0 +; see https://github.com/redis/node-redis/blob/HEAD/docs/client-configuration.md +; database=0 ; password=battery-horse-staple ``` @@ -74,17 +74,16 @@ optionally with a redis db ID. All redis config options must be listed in your p ```js exports.register = function () { - const plugin = this; - plugin.inherits('redis'); + this.inherits('redis'); - plugin.cfg = plugin.config.get('my-plugin.ini'); + this.cfg = this.config.get('my-plugin.ini'); // populate plugin.cfg.redis with defaults from redis.ini - plugin.merge_redis_ini(); + this.merge_redis_ini(); // cluster aware redis connection(s) - plugin.register_hook('init_master', 'init_redis_plugin'); - plugin.register_hook('init_child', 'init_redis_plugin'); + this.register_hook('init_master', 'init_redis_plugin'); + this.register_hook('init_child', 'init_redis_plugin'); } ``` @@ -101,8 +100,7 @@ Notice the database ID numbers appended to each plugins redis connection message. - -[ci-img]: https://github.com/haraka/haraka-plugin-redis/workflows/Tests/badge.svg -[ci-url]: https://github.com/haraka/haraka-plugin-redis/actions?query=workflow%3ATests +[ci-img]: https://github.com/haraka/haraka-plugin-redis/actions/workflows/ci-test.yml/badge.svg +[ci-url]: https://github.com/haraka/haraka-plugin-redis/actions/workflows/ci-test.yml [clim-img]: https://codeclimate.com/github/haraka/haraka-plugin-redis/badges/gpa.svg [clim-url]: https://codeclimate.com/github/haraka/haraka-plugin-redis diff --git a/index.js b/index.js index 8fb0ece..065a6e2 100644 --- a/index.js +++ b/index.js @@ -4,16 +4,14 @@ const redis = require('redis'); exports.register = function () { - const plugin = this; - - plugin.load_redis_ini(); + this.load_redis_ini(); // another plugin has called us with: inherits('haraka-plugin-redis') - if (plugin.name !== 'redis') return; + if (this.name !== 'redis') return; // register when 'redis' is declared in config/plugins - plugin.register_hook('init_master', 'init_redis_shared'); - plugin.register_hook('init_child', 'init_redis_shared'); + this.register_hook('init_master', 'init_redis_shared'); + this.register_hook('init_child', 'init_redis_shared'); } const defaultOpts = { socket: { host: '127.0.0.1', port: '6379' } } @@ -52,13 +50,12 @@ exports.load_redis_ini = function () { } exports.merge_redis_ini = function () { - const plugin = this; - if (!plugin.cfg) plugin.cfg = {}; // no .ini loaded? - if (!plugin.cfg.redis) plugin.cfg.redis = {}; // no [redis] in .ini file - if (!plugin.redisCfg) plugin.load_redis_ini(); + if (!this.cfg) this.cfg = {}; // no .ini loaded? + if (!this.cfg.redis) this.cfg.redis = {}; // no [redis] in .ini file + if (!this.redisCfg) this.load_redis_ini(); - plugin.cfg.redis = Object.assign({}, plugin.redisCfg.server, plugin.cfg.redis); + this.cfg.redis = Object.assign({}, this.redisCfg.server, this.cfg.redis); } exports.init_redis_shared = function (next, server) { @@ -127,21 +124,20 @@ exports.shutdown = function () { } exports.redis_ping = async function () { - const plugin = this; - plugin.redis_pings=false; + this.redis_pings=false; - if (!plugin.db) { + if (!this.db) { return new Error('redis not initialized'); } try { - const r = await plugin.db.ping() + const r = await this.db.ping() if (r !== 'PONG') return new Error('not PONG'); - plugin.redis_pings=true + this.redis_pings=true } catch (e) { - plugin.logerror(e.message) + this.logerror(e.message) } } @@ -194,14 +190,13 @@ exports.get_redis_sub_channel = function (conn) { } exports.redis_subscribe_pattern = async function (pattern) { - const plugin = this; - if (plugin.redis) return // already subscribed? + if (this.redis) return // already subscribed? - plugin.redis = await redis.createClient(plugin.redisCfg.pubsub) + this.redis = await redis.createClient(this.redisCfg.pubsub) - await plugin.redis.psubscribe(pattern); - plugin.logdebug(plugin, `psubscribed to ${pattern}`); + await this.redis.psubscribe(pattern); + this.logdebug(this, `psubscribed to ${pattern}`); } exports.redis_subscribe = async function (connection) { From 4b41f807c6a868bf69d149de0a04aff412ba6c95 Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Tue, 29 Mar 2022 22:19:58 -0700 Subject: [PATCH 3/4] bump dev dep versions --- package.json | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index e4ee81c..6888a9f 100644 --- a/package.json +++ b/package.json @@ -10,15 +10,16 @@ "redis": "^4.0.0" }, "devDependencies": { - "eslint": ">=8", + "eslint": "^8.12.0", "eslint-plugin-haraka": "*", "haraka-test-fixtures": "*", - "mocha": "^9.0.0" + "mocha": "^9.2.0" }, "scripts": { "lint": "npx eslint *.js test/*.js", "lintfix": "npx eslint --fix *.js test/*.js", "cover": "NODE_ENV=cov npx nyc --reporter=lcovonly npm run test", + "versions": "npx dependency-version-checker check", "test": "npx mocha" }, "repository": { From ed2b504ae401beb336feefa04086f189be1b5b86 Mon Sep 17 00:00:00 2001 From: Matt Simerson Date: Tue, 29 Mar 2022 22:23:25 -0700 Subject: [PATCH 4/4] update changes --- Changes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Changes.md b/Changes.md index 06eed55..61090ec 100644 --- a/Changes.md +++ b/Changes.md @@ -4,6 +4,9 @@ - bump redis major version 3 -> 4 - API change, callbacks replaced by promises +- config.ini + - [server] -> [socket] + - opts.db -> opts.database (to match upstream) ### 1.0.13 - 2021-10-14