From b91e06d07a017ca66f3efa25ba3c47cd3f5257dd Mon Sep 17 00:00:00 2001 From: Evan Shortiss Date: Tue, 25 Oct 2016 23:04:30 -0400 Subject: [PATCH] add fail fast check to ensure sync handlers are functions. add bdd tests --- lib/sync-DataSetModel.js | 68 ++++------ lib/sync-srv.js | 233 ++++++++++----------------------- lib/sync-util.js | 9 ++ test/fixtures/syncHandler.js | 4 +- test/test_sync.js | 2 +- test/test_sync_bdd.js | 134 +++++++++++++++++++ test/test_sync_datasetmodel.js | 62 +++++++++ test/test_sync_utils.js | 33 +++++ 8 files changed, 332 insertions(+), 213 deletions(-) create mode 100644 test/test_sync_bdd.js create mode 100644 test/test_sync_datasetmodel.js create mode 100644 test/test_sync_utils.js diff --git a/lib/sync-DataSetModel.js b/lib/sync-DataSetModel.js index 76efe5f..6b7233c 100644 --- a/lib/sync-DataSetModel.js +++ b/lib/sync-DataSetModel.js @@ -3,6 +3,20 @@ var async = require('async'); var defaultDataHandler = require('./sync-datahandler'); var syncUtil = require('./sync-util'); +/** + * Removes code duplication and adds safety by ensuring global handlers passed + * in are functions and not some other type. + * @param {String} handlerName The name of the handler, e.g "globalHandleRead" + * @param {Object} inst The instance to set it on if valid + * @return {Function} + */ +function generateGlobalSetter (handlerName, inst) { + return function _setGlobalHandler (handlerFn) { + syncUtil.ensureHandlerIsFunction(handlerName, handlerFn); + inst[handlerName] = handlerFn; + }; +} + var self = { globalListHandler: undefined, @@ -485,42 +499,6 @@ var self = { } }, - setGlobalListHandler: function (globalListHandler) { - self.globalListHandler = globalListHandler; - }, - - setGlobalCreateHandler: function (globalCreateHandler) { - self.globalCreateHandler = globalCreateHandler; - }, - - setGlobalReadHandler: function (globalReadHandler) { - self.globalReadHandler = globalReadHandler; - }, - - setGlobalUpdateHandler: function (globalUpdateHandler) { - self.globalUpdateHandler = globalUpdateHandler; - }, - - setGlobalDeleteHandler: function (globalDeleteHandler) { - self.globalDeleteHandler = globalDeleteHandler; - }, - - setGlobalCollisionHandler: function (globalCollisionHandler) { - self.globalCollisionHandler = globalCollisionHandler; - }, - - setGlobalCollisionLister: function (globalCollisionLister) { - self.globalCollisionLister = globalCollisionLister; - }, - - setGlobalCollisionRemover: function (globalCollisionRemover) { - self.globalCollisionRemover = globalCollisionRemover; - }, - - setGlobalRequestInterceptor: function (globalRequestInterceptor) { - self.globalRequestInterceptor = globalRequestInterceptor; - }, - setDefaultHandlers: function () { self.globalListHandler = defaultDataHandler.doList; self.globalCreateHandler = defaultDataHandler.doCreate; @@ -595,15 +573,15 @@ var init = function () { module.exports = { forceSyncList: self.forceSyncList, - setGlobalListHandler: self.setGlobalListHandler, - setGlobalCreateHandler: self.setGlobalCreateHandler, - setGlobalReadHandler: self.setGlobalReadHandler, - setGlobalUpdateHandler: self.setGlobalUpdateHandler, - setGlobalDeleteHandler: self.setGlobalDeleteHandler, - setGlobalCollisionHandler: self.setGlobalCollisionHandler, - setGlobalCollisionLister: self.setGlobalCollisionLister, - setGlobalCollisionRemover: self.setGlobalCollisionRemover, - setGlobalRequestInterceptor: self.setGlobalRequestInterceptor, + setGlobalListHandler: generateGlobalSetter('globalListHandler', self), + setGlobalCreateHandler: generateGlobalSetter('globalCreateHandler', self), + setGlobalReadHandler: generateGlobalSetter('globalReadHandler', self), + setGlobalUpdateHandler: generateGlobalSetter('globalUpdateHandler', self), + setGlobalDeleteHandler: generateGlobalSetter('globalDeleteHandler', self), + setGlobalCollisionHandler: generateGlobalSetter('globalCollisionHandler', self), + setGlobalCollisionLister: generateGlobalSetter('globalCollisionLister', self), + setGlobalCollisionRemover: generateGlobalSetter('globalCollisionRemover', self), + setGlobalRequestInterceptor: generateGlobalSetter('globalRequestInterceptor', self), doListHandler: self.doListHandler, doCreateHandler: self.doCreateHandler, doReadHandler: self.doReadHandler, diff --git a/lib/sync-srv.js b/lib/sync-srv.js index 7ef5f93..f42958d 100644 --- a/lib/sync-srv.js +++ b/lib/sync-srv.js @@ -17,7 +17,54 @@ module.exports = function (cfg) { var syncInited = false; var sync = function () { - + + /** + * Generic setter that can be used to override a default sync handler. + * Ensures passed in handler is a function and throws an AssertionError if not + * + * @param {String} target The handler to override + * @return {Function} + */ + function generateSetHandlerFn (target) { + return function _doSetHandler (dataset_id, fn) { + syncUtil.ensureHandlerIsFunction(target, fn); + + DataSetModel.getDataset(dataset_id, function (err, dataset) { + if (!err) { + dataset[target] = fn; + } + }); + }; + } + + + /** + * Each handler override that is supported needs a setter function, this + * generates them so they can be attached to the sync instance. + * + * For example this will bind and $fh.sync.handleList function, which allows + * one to set a custom "listHandler" + * + * @return {Object} + */ + function bindHandlerSetters (instance) { + var handlerMap = { + handleList: 'listHandler', + handleCreate: 'createHandler', + handleRead: 'readHandler', + handleUpdate: 'updateHandler', + handleDelete: 'deleteHandler', + handleCollision: 'collisionHandler', + listCollisions: 'collisionLister', + removeCollision: 'collisionRemover', + interceptRequest: 'requestInterceptor' + }; + + Object.keys(handlerMap).forEach(function (key) { + instance[key] = generateSetHandlerFn(handlerMap[key]); + }); + } + var globalInit = function () { if (!syncInited) { syncInited = true; @@ -27,180 +74,36 @@ var sync = function () { } } - var init = function (dataset_id, options, cb) { - initDataset(dataset_id, options, cb); - }; - - var invoke = function (dataset_id, params, callback) { - return doInvoke(dataset_id, params, callback); - }; - - var stop = function (dataset_id, callback) { - return stopDatasetSync(dataset_id, callback); - }; - - var stopAll = function (callback) { - return stopAllDatasetSync(callback); - }; - - var toJSON = function (dataset_id, returnData, cb) { - return DataSetModel.toJSON(dataset_id, returnData, cb); - }; - - var globalHandleList = function (fn) { - DataSetModel.setGlobalListHandler(fn); - }; - - var globalHandleCreate = function (fn) { - DataSetModel.setGlobalCreateHandler(fn); - }; - - var globalHandleRead = function (fn) { - DataSetModel.setGlobalReadHandler(fn); - }; - - var globalHandleUpdate = function (fn) { - DataSetModel.setGlobalUpdateHandler(fn); - }; - - var globalHandleDelete = function (fn) { - DataSetModel.setGlobalDeleteHandler(fn); - }; - - var globalHandleCollision = function (fn) { - DataSetModel.setGlobalCollisionHandler(fn); - }; - - var globalListCollisions = function (fn) { - DataSetModel.setGlobalCollisionLister(fn); - }; - - var globalRemoveCollision = function (fn) { - DataSetModel.setGlobalCollisionRemover(fn); - }; - - var globalInterceptRequest = function (fn) { - DataSetModel.setGlobalRequestInterceptor(fn); - }; - - var handleList = function (dataset_id, fn) { - DataSetModel.getDataset(dataset_id, function (err, dataset) { - if (!err) { - dataset.listHandler = fn; - } - }); - }; - - var handleCreate = function (dataset_id, fn) { - DataSetModel.getDataset(dataset_id, function (err, dataset) { - if (!err) { - dataset.createHandler = fn; - } - }); - }; - - var handleRead = function (dataset_id, fn) { - DataSetModel.getDataset(dataset_id, function (err, dataset) { - if (!err) { - dataset.readHandler = fn; - } - }); - }; - - var handleUpdate = function (dataset_id, fn) { - DataSetModel.getDataset(dataset_id, function (err, dataset) { - if (!err) { - dataset.updateHandler = fn; - } - }); - }; - - var handleDelete = function (dataset_id, fn) { - DataSetModel.getDataset(dataset_id, function (err, dataset) { - if (!err) { - dataset.deleteHandler = fn; - } - }); - }; - - var handleCollision = function (dataset_id, fn) { - DataSetModel.getDataset(dataset_id, function (err, dataset) { - if (!err) { - dataset.collisionHandler = fn; - } - }); - }; - - var listCollisions = function (dataset_id, fn) { - DataSetModel.getDataset(dataset_id, function (err, dataset) { - if (!err) { - dataset.collisionLister = fn; - } - }); - }; - - var removeCollision = function (dataset_id, fn) { - DataSetModel.getDataset(dataset_id, function (err, dataset) { - if (!err) { - dataset.collisionRemover = fn; - } - }); - }; + globalInit(); - var interceptRequest = function (dataset_id, fn) { - DataSetModel.getDataset(dataset_id, function (err, dataset) { - if (!err) { - dataset.requestInterceptor = fn; - } - }); + var instance = { + init: DataSetModel.createDataset.bind(null), + invoke: doInvoke.bind(null), + stop: DataSetModel.stopDatasetSync.bind(null), + stopAll: DataSetModel.stopAllDatasetSync.bind(null), + toJSON: DataSetModel.toJSON.bind(null), + setLogLevel: doSetLogLevel, + globalHandleList: DataSetModel.setGlobalListHandler.bind(null), + globalHandleCreate: DataSetModel.setGlobalCreateHandler.bind(null), + globalHandleRead: DataSetModel.setGlobalReadHandler.bind(null), + globalHandleUpdate: DataSetModel.setGlobalUpdateHandler.bind(null), + globalHandleDelete: DataSetModel.setGlobalDeleteHandler.bind(null), + globalHandleCollision: DataSetModel.setGlobalCollisionHandler.bind(null), + globalListCollisions: DataSetModel.setGlobalCollisionLister.bind(null), + globalRemoveCollision: DataSetModel.setGlobalCollisionRemover.bind(null), + globalInterceptRequest: DataSetModel.setGlobalRequestInterceptor.bind(null) }; - globalInit(); + // Make sure to bind custom handler setters + bindHandlerSetters(instance); - return { - init: init, - invoke: invoke, - stop: stop, - stopAll: stopAll, - toJSON: toJSON, - setLogLevel: doSetLogLevel, - globalHandleList: globalHandleList, - globalHandleCreate: globalHandleCreate, - globalHandleRead: globalHandleRead, - globalHandleUpdate: globalHandleUpdate, - globalHandleDelete: globalHandleDelete, - globalHandleCollision: globalHandleCollision, - globalListCollisions: globalListCollisions, - globalRemoveCollision: globalRemoveCollision, - globalInterceptRequest: globalInterceptRequest, - handleList: handleList, - handleCreate: handleCreate, - handleRead: handleRead, - handleUpdate: handleUpdate, - handleDelete: handleDelete, - handleCollision: handleCollision, - listCollisions: listCollisions, - removeCollision: removeCollision, - interceptRequest: interceptRequest - } + return instance; } /* ======================================================= */ /* ================== PRIVATE FUNCTIONS ================== */ /* ======================================================= */ -function initDataset(dataset_id, options, cb) { - DataSetModel.createDataset(dataset_id, options, cb); -} - -function stopDatasetSync(dataset_id, cb) { - DataSetModel.stopDatasetSync(dataset_id, cb); -} - -function stopAllDatasetSync(cb) { - DataSetModel.stopAllDatasetSync(cb); -} - /* jshint ignore:start */ function toJSON(dataset_id, returnData, cb) { DataSetModel.toJSON(dataset_id, returnData, cb); diff --git a/lib/sync-util.js b/lib/sync-util.js index 342e801..263c867 100644 --- a/lib/sync-util.js +++ b/lib/sync-util.js @@ -1,6 +1,7 @@ var crypto = require('crypto'); var winston = require('winston'); var moment = require('moment'); +var assert = require('assert'); var SYNC_LOGGER = 'SYNC'; var loggers = {}; @@ -81,6 +82,14 @@ var getCuid = function (params) { return cuid; } +exports.ensureHandlerIsFunction = function (target, fn) { + assert.equal( + typeof fn, + 'function', + 'sync handler (' + target + ') must be a function' + ); +}; + module.exports.generateHash = generateHash; module.exports.sortObject = sortObject; module.exports.sortedStringify = sortedStringify; diff --git a/test/fixtures/syncHandler.js b/test/fixtures/syncHandler.js index 5dc3ded..06e85f7 100644 --- a/test/fixtures/syncHandler.js +++ b/test/fixtures/syncHandler.js @@ -14,7 +14,7 @@ exports.doRead = function(dataset_id, uid, cb) { return cb(null, {}); }; -exports.listCollissions = function(dataset_id, uid, cb) { +exports.listCollisions = function(dataset_id, uid, cb) { console.log("listCollisions : ", dataset_id, " :: ", uid); return cb(null, {}); }; @@ -36,4 +36,4 @@ exports.doCollision = function(dataset_id, hash, uid, pre, post) { exports.removeCollision = function(dataset_id, hash, cb) { return cb(null, {}); -} \ No newline at end of file +} diff --git a/test/test_sync.js b/test/test_sync.js index 6f04731..88ffb37 100644 --- a/test/test_sync.js +++ b/test/test_sync.js @@ -63,6 +63,6 @@ module.exports = { ditchMock.done(); finish(); }); - + } }; diff --git a/test/test_sync_bdd.js b/test/test_sync_bdd.js new file mode 100644 index 0000000..d674119 --- /dev/null +++ b/test/test_sync_bdd.js @@ -0,0 +1,134 @@ +'use strict'; + +describe('$fh.sync', function () { + + var sinon = require('sinon'); + var expect = require('chai').expect; + var proxyquire = require('proxyquire').noCallThru(); + + var STUB_NAMES = { + DB: './db', + SYNC_UTIL: './sync-util', + DATA_MODEL: './sync-DataSetModel' + }; + + var sync, db, stubs; + + beforeEach(function () { + // Clear all require cache to ensure old datasets etc. aren't hanging about + require('clear-require').all(); + + db = sinon.stub(); + + // Once we drop node 0.10 we'll be able to use the new object literals here! + stubs = {}; + stubs[STUB_NAMES.DB] = sinon.stub().returns(db); + stubs[STUB_NAMES.SYNC_UTIL] = { + ensureHandlerIsFunction: sinon.stub(), + doLog: sinon.stub(), + setLogger: sinon.spy() + }; + stubs[STUB_NAMES.DATA_MODEL] = { + getDataset: sinon.stub(), + setFHDB: sinon.spy(), + init: sinon.spy(), + createDataset: sinon.spy(), + stopDatasetSync: sinon.spy(), + stopAllDatasetSync: sinon.spy(), + toJSON: sinon.spy(), + setGlobalListHandler: sinon.spy(), + setGlobalCreateHandler: sinon.spy(), + setGlobalReadHandler: sinon.spy(), + setGlobalUpdateHandler: sinon.spy(), + setGlobalDeleteHandler: sinon.spy(), + setGlobalCollisionHandler: sinon.spy(), + setGlobalCollisionLister: sinon.spy(), + setGlobalCollisionRemover: sinon.spy(), + setGlobalRequestInterceptor: sinon.spy() + }; + + // Creates a new sync instance + sync = proxyquire('lib/sync-srv', stubs)({}); + }); + + it('should have initialised the db connection DataSetModel', function () { + expect(stubs[STUB_NAMES.DATA_MODEL].setFHDB.called).to.be.true; + expect(stubs[STUB_NAMES.DATA_MODEL].init.called).to.be.true; + }); + + describe('setters for custom handlers', function () { + it('should throw an AssertionError if not passed a function', function () { + stubs[STUB_NAMES.SYNC_UTIL].ensureHandlerIsFunction.throwsException( + new Error('AssertionError') + ); + + expect(function () { + sync.handleList('my-dataset', {}); + }).to.throw('AssertionError'); + }); + + it('should not throw an AssertionError if passed a function', function () { + stubs[STUB_NAMES.SYNC_UTIL].ensureHandlerIsFunction.returns(null); + + expect(function () { + sync.handleList('my-dataset', sinon.spy()); + }).to.not.throw(); + + expect(stubs[STUB_NAMES.DATA_MODEL].getDataset.called).to.be.true; + }); + }); + + describe('#invoke', function () { + it('should return error if params is not provided', function (done) { + sync.invoke('my-dataset', null, function (err) { + expect(err).to.equal('no_fn'); + done(); + }); + }); + + it('should return error if params.fn is not provided', function (done) { + sync.invoke('my-dataset', {}, function (err) { + expect(err).to.equal('no_fn'); + done(); + }); + }); + + it('should return error if invalid params.fn is provided', function (done) { + sync.invoke('my-dataset', { + fn: '!blah!' + }, function (err) { + expect(err).to.equal('unknown_fn : !blah!'); + done(); + }); + }); + }); + + describe('#setLogLevel', function () { + it('should return error if params.logLevel is undefined', function (done) { + sync.setLogLevel('my-dataset', {}, function (err) { + expect(err).to.equal('logLevel parameter required'); + done(); + }); + }); + + it('should set the logLevel', function (done) { + sync.setLogLevel('my-dataset', { + logLevel: 10 + }, function (err) { + var args = stubs[STUB_NAMES.SYNC_UTIL].setLogger.getCall(0).args; + + expect(err).to.be.null; + expect(args).to.deep.equal([ + 'my-dataset', + { + logLevel: 10 + } + ]); + done(); + }); + }); + }); + + + +}); diff --git a/test/test_sync_datasetmodel.js b/test/test_sync_datasetmodel.js new file mode 100644 index 0000000..da7227e --- /dev/null +++ b/test/test_sync_datasetmodel.js @@ -0,0 +1,62 @@ +'use strict'; + +describe('sync dataset model', function () { + + var sinon = require('sinon'); + var expect = require('chai').expect; + + var mod; + + beforeEach(function () { + // Clear all require cache to ensure old datasets etc. aren't hanging about + require('clear-require').all(); + + mod = require('lib/sync-DataSetModel'); + }); + + describe('global setters', function () { + var map = { + 'setGlobalListHandler': 'globalListHandler', + 'setGlobalCreateHandler': 'globalCreateHandler', + 'setGlobalReadHandler': 'globalReadHandler', + 'setGlobalUpdateHandler': 'globalUpdateHandler', + 'setGlobalDeleteHandler': 'globalDeleteHandler', + 'setGlobalCollisionHandler': 'globalCollisionHandler', + 'setGlobalCollisionLister': 'globalCollisionLister', + 'setGlobalCollisionRemover': 'globalCollisionRemover', + 'setGlobalRequestInterceptor': 'globalRequestInterceptor' + }; + + Object.keys(map).forEach(function (fnName) { + describe('#' + fnName, function () { + it('should throw an AssertionError due invalid argument', function () { + expect(function () { + mod[fnName](null) + }).to.throw( + 'AssertionError: sync handler (' + map[fnName] + ') must be a function' + ); + }); + + it('should set the ' + map[fnName] + ' property', function () { + var spy = sinon.spy(); + expect(function () { + mod[fnName](spy) + }).to.not.throw(); + }); + }); + }); + + it('should use a custom "globalRequestInterceptor"', function (done) { + var customInterceptor = sinon.stub().yields(null); + + mod.setGlobalRequestInterceptor(customInterceptor); + + mod.doRequestInterceptor('a', {}, function (err) { + expect(err).to.be.null; + expect(customInterceptor.called).to.be.true; + done(); + }); + }); + }); + +}); diff --git a/test/test_sync_utils.js b/test/test_sync_utils.js new file mode 100644 index 0000000..9e4f47b --- /dev/null +++ b/test/test_sync_utils.js @@ -0,0 +1,33 @@ +'use strict'; + +describe('sync utils', function () { + + var sinon = require('sinon'); + var expect = require('chai').expect; + var mod; + + beforeEach(function () { + // Clear all require cache to ensure old datasets etc. aren't hanging about + require('clear-require').all(); + + // Creates a new sync instance + mod = require('lib/sync-util'); + }); + + describe('#ensureHandlerIsFunction', function () { + it('should throw an AssertionError if not passed a function', function () { + expect(function () { + mod.ensureHandlerIsFunction('listHandler', {}); + }).to.throw( + 'AssertionError: sync handler (listHandler) must be a function' + ); + }); + + it('should not throw an AssertionError if passed a function', function () { + expect(function () { + mod.ensureHandlerIsFunction('listHandler', sinon.spy()); + }).to.not.throw(); + }); + }); + +});