Work on refactoring global IP ban database calls

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": [

src/database.js

@@ -14,8 +14,9 @@ var global_ipbans = {};
 let db = null;
 
 class Database {
-    constructor() {
+    constructor(knexConfig = null) {
-        const config = {
+        if (knexConfig === null) {
+            knexConfig = {
                 client: 'mysql',
                 connection: {
                     host: Config.get('mysql.server'),
@@ -32,11 +33,21 @@ class Database {
                 },
                 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')

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 };

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');
+            });
+        });
+    });
+});

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;