From d0712d007eeee5e6cf43dc931cc17593a01478b6 Mon Sep 17 00:00:00 2001 From: Calvin Montgomery Date: Wed, 31 May 2017 22:46:06 -0700 Subject: [PATCH] Work on refactoring global IP ban database calls --- package.json | 3 +- src/database.js | 49 ++++++++++++++++---------- src/db/globalban.js | 52 +++++++++++++++++++++++++++ test/db/globalban.js | 84 ++++++++++++++++++++++++++++++++++++++++++++ test/testutil/db.js | 24 +++++++++++++ 5 files changed, 192 insertions(+), 20 deletions(-) create mode 100644 src/db/globalban.js create mode 100644 test/db/globalban.js create mode 100644 test/testutil/db.js diff --git a/package.json b/package.json index 09737d3b..1b9a0fae 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,8 @@ "babel-plugin-transform-flow-strip-types": "^6.22.0", "coffee-script": "^1.9.2", "flow-bin": "^0.43.0", - "mocha": "^3.2.0" + "mocha": "^3.2.0", + "sinon": "^2.3.2" }, "babel": { "presets": [ diff --git a/src/database.js b/src/database.js index c42dbaef..63e160e7 100644 --- a/src/database.js +++ b/src/database.js @@ -14,29 +14,40 @@ var global_ipbans = {}; let db = null; class Database { - constructor() { - const config = { - client: 'mysql', - connection: { - host: Config.get('mysql.server'), - port: Config.get('mysql.port'), - user: Config.get('mysql.user'), - password: Config.get('mysql.password'), - database: Config.get('mysql.database'), - multipleStatements: true, // Legacy thing - charset: 'UTF8MB4_GENERAL_CI' - }, - pool: { - min: Config.get('mysql.pool-size'), - max: Config.get('mysql.pool-size') - }, - debug: !!process.env.KNEX_DEBUG - }; + constructor(knexConfig = null) { + if (knexConfig === null) { + knexConfig = { + client: 'mysql', + connection: { + host: Config.get('mysql.server'), + port: Config.get('mysql.port'), + user: Config.get('mysql.user'), + password: Config.get('mysql.password'), + database: Config.get('mysql.database'), + multipleStatements: true, // Legacy thing + charset: 'UTF8MB4_GENERAL_CI' + }, + pool: { + min: Config.get('mysql.pool-size'), + max: Config.get('mysql.pool-size') + }, + debug: !!process.env.KNEX_DEBUG + }; + } - this.knex = knex(config); + this.knex = knex(knexConfig); + } + + runTransaction(fn) { + const timer = Metrics.startTimer('db:queryTime'); + return this.knex.transaction(fn).finally(() => { + Metrics.stopTimer(timer); + }); } } +module.exports.Database = Database; + module.exports.init = function () { db = new Database(); db.knex.raw('select 1 from dual') diff --git a/src/db/globalban.js b/src/db/globalban.js new file mode 100644 index 00000000..1605ff95 --- /dev/null +++ b/src/db/globalban.js @@ -0,0 +1,52 @@ +import { LoggerFactory } from '@calzoneman/jsli'; +import util from '../utilities'; +import Promise from 'bluebird'; + +const LOGGER = LoggerFactory.getLogger('GlobalBanDB'); + +class GlobalBanDB { + constructor(db) { + this.db = db; + } + + listGlobalBans() { + return this.db.runTransaction(tx => { + return tx.table('global_bans').select(); + }).catch(error => { + LOGGER.error('Failed to list global IP bans: %s', error.stack); + throw error; + }); + } + + addGlobalIPBan(ip, reason) { + return this.db.runTransaction(tx => { + return tx.table('global_bans') + .insert({ ip, reason }) + .catch(error => { + if (error.code === 'ER_DUP_ENTRY') { + return tx.table('global_bans') + .where({ ip }) + .update({ reason }); + } else { + throw error; + } + }); + }).catch(error => { + LOGGER.error('Failed to add global IP ban for IP %s: %s', ip, error.stack); + throw error; + }); + } + + removeGlobalIPBan(ip) { + return this.db.runTransaction(tx => { + return tx.table('global_bans') + .where({ ip }) + .del(); + }).catch(error => { + LOGGER.error('Failed to remove global IP ban for IP %s: %s', ip, error.stack); + throw error; + }); + } +} + +export { GlobalBanDB }; diff --git a/test/db/globalban.js b/test/db/globalban.js new file mode 100644 index 00000000..eb991c86 --- /dev/null +++ b/test/db/globalban.js @@ -0,0 +1,84 @@ +const assert = require('assert'); +const sinon = require('sinon'); +const TestUtilDB = require('../testutil/db'); +const GlobalBanDB = require('../../lib/db/globalban').GlobalBanDB; + +describe('GlobalBanDB', () => { + let mockTx, mockDB, globalBanDB; + + beforeEach(() => { + mockTx = new TestUtilDB.MockTx(); + mockDB = new TestUtilDB.MockDB(mockTx); + globalBanDB = new GlobalBanDB(mockDB); + }); + + describe('#listGlobalBans', () => { + it('lists bans from the database', () => { + const expected = [ + { ip: '1.2.3.4', reason: 'spam' }, + { ip: '5.6', reason: 'ham' } + ]; + + sinon.stub(mockTx, 'table').withArgs('global_bans').returns(mockTx); + sinon.stub(mockTx, 'select').resolves(expected); + return globalBanDB.listGlobalBans().then(bans => { + assert.deepStrictEqual(bans, expected); + }); + }); + }); + + describe('#addGlobalIPBan', () => { + it('adds a new ban', () => { + const input = { ip: '1.2.3.4', reason: 'spam' }; + + sinon.stub(mockTx, 'table').withArgs('global_bans').returns(mockTx); + const insert = sinon.stub(mockTx, 'insert').withArgs(input).resolves(); + return globalBanDB.addGlobalIPBan(input.ip, input.reason).then(() => { + assert(insert.called, 'Expected insert to be called'); + }); + }); + + it('updates the ban reason for an existing ban', () => { + const input = { ip: '1.2.3.4', reason: 'spam' }; + + sinon.stub(mockTx, 'table').withArgs('global_bans').returns(mockTx); + const err = new Error(); + err.code = 'ER_DUP_ENTRY'; + const insert = sinon.stub(mockTx, 'insert').withArgs(input).rejects(err); + const where = sinon.stub(mockTx, 'where').withArgs({ ip: input.ip }).returns(mockTx); + const update = sinon.stub(mockTx, 'update').withArgs({ reason: input.reason }).resolves(); + return globalBanDB.addGlobalIPBan(input.ip, input.reason).then(() => { + assert(insert.called, 'Expected insert to be called'); + assert(where.called, 'Expected where({ ip }) to be called'); + assert(update.called, 'Expected update({ reason }) to be called'); + }); + }); + + it('doesn\'t call update for a non-ER_DUP_ENTRY error', () => { + const input = { ip: '1.2.3.4', reason: 'spam' }; + + sinon.stub(mockTx, 'table').withArgs('global_bans').returns(mockTx); + const err = new Error('explosions'); + const insert = sinon.stub(mockTx, 'insert').withArgs(input).rejects(err); + return globalBanDB.addGlobalIPBan(input.ip, input.reason).then(() => { + assert(false, 'Expected an error'); + }).catch(error => { + assert.strictEqual(error, err, 'Expected error to match'); + assert(insert.called, 'Expected insert to be called'); + }); + }); + }); + + describe('#removeGlobalIPBan', () => { + it('removes a global ban', () => { + const ip = '1.2.3.4'; + sinon.stub(mockTx, 'table').withArgs('global_bans').returns(mockTx); + const where = sinon.stub(mockTx, 'where').returns(mockTx); + const del = sinon.stub(mockTx, 'del').resolves(); + return globalBanDB.removeGlobalIPBan(ip).then(() => { + assert(where.called, 'Expected where({ ip }) to be called'); + assert(del.called, 'Expected del to be called'); + }); + }); + }); +}); diff --git a/test/testutil/db.js b/test/testutil/db.js new file mode 100644 index 00000000..da04648f --- /dev/null +++ b/test/testutil/db.js @@ -0,0 +1,24 @@ +const Promise = require('bluebird'); + +function MockDB(mockTx) { + this.mockTx = mockTx; +} + +MockDB.prototype = { + runTransaction: function runTransaction(fn) { + return fn(this.mockTx); + } +}; + +function MockTx() { + +} + +['insert', 'update', 'select', 'del', 'where', 'table'].forEach(method => { + MockTx.prototype[method] = function () { + return Promise.reject(new Error(`No stub defined for method "${method}"`)); + }; +}); + +exports.MockDB = MockDB; +exports.MockTx = MockTx;