diff --git a/lib/executor.js b/lib/executor.js index a555bd3..979ca7d 100755 --- a/lib/executor.js +++ b/lib/executor.js @@ -11,17 +11,16 @@ function Executor () { // This queue will execute all commands, one-by-one in order this.queue = async.queue(function (task, cb) { - var callback - , lastArg = task.arguments[task.arguments.length - 1] - , i, newArguments = [] - ; + var newArguments = []; // task.arguments is an array-like object on which adding a new field doesn't work, so we transform it into a real array - for (i = 0; i < task.arguments.length; i += 1) { newArguments.push(task.arguments[i]); } + for (var i = 0; i < task.arguments.length; i += 1) { newArguments.push(task.arguments[i]); } + var lastArg = task.arguments[task.arguments.length - 1]; // Always tell the queue task is complete. Execute callback if any was given. if (typeof lastArg === 'function') { - callback = function () { + // Callback was supplied + newArguments[newArguments.length - 1] = function () { if (typeof setImmediate === 'function') { setImmediate(cb); } else { @@ -29,11 +28,12 @@ function Executor () { } lastArg.apply(null, arguments); }; - - newArguments[newArguments.length - 1] = callback; + } else if (!lastArg && task.arguments.length !== 0) { + // false/undefined/null supplied as callbback + newArguments[newArguments.length - 1] = function () { cb(); }; } else { - callback = function () { cb(); }; - newArguments.push(callback); + // Nothing supplied as callback + newArguments.push(function () { cb(); }); } @@ -48,7 +48,8 @@ function Executor () { * @param {Object} task * task.this - Object to use as this * task.fn - Function to execute - * task.arguments - Array of arguments + * task.arguments - Array of arguments, IMPORTANT: only the last argument may be a function (the callback) + * and the last argument cannot be false/undefined/null * @param {Boolean} forceQueuing Optional (defaults to false) force executor to queue task even if it is not ready */ Executor.prototype.push = function (task, forceQueuing) { diff --git a/test/executor.test.js b/test/executor.test.js index 6bd1d23..73c2060 100755 --- a/test/executor.test.js +++ b/test/executor.test.js @@ -38,12 +38,31 @@ function testThrowInCallback (d, done) { }); } +// Test that if the callback is falsy, the next DB operations will still be executed +function testFalsyCallback (d, done) { + d.insert({ a: 1 }, null); + process.nextTick(function () { + d.update({ a: 1 }, { a: 2 }, {}, null); + process.nextTick(function () { + d.update({ a: 2 }, { a: 1 }, null); + process.nextTick(function () { + d.remove({ a: 2 }, {}, null); + process.nextTick(function () { + d.remove({ a: 2 }, null); + process.nextTick(function () { + d.find({}, done); + }); + }); + }); + }); + }); +} // Test that operations are executed in the right order // We prevent Mocha from catching the exception we throw on purpose by remembering all current handlers, remove them and register them back after test ends function testRightOrder (d, done) { var currentUncaughtExceptionHandlers = process.listeners('uncaughtException'); - + process.removeAllListeners('uncaughtException'); process.on('uncaughtException', function (err) { @@ -80,8 +99,6 @@ function testRightOrder (d, done) { }); } - - // Note: The following test does not have any assertion because it // is meant to address the deprecation warning: // (node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral. @@ -95,9 +112,20 @@ var testEventLoopStarvation = function(d, done){ }); } done(); - }; +}; + +// Test that operations are executed in the right order even with no callback +function testExecutorWorksWithoutCallback (d, done) { + d.insert({ a: 1 }); + d.insert({ a: 2 }, false); + d.find({}, function (err, docs) { + docs.length.should.equal(2); + done(); + }); +} + -describe('Executor', function () { +describe.only('Executor', function () { describe('With persistent database', function () { var d; @@ -130,7 +158,11 @@ describe('Executor', function () { it('A throw in a callback doesnt prevent execution of next operations', function(done) { testThrowInCallback(d, done); }); - + + it('A falsy callback doesnt prevent execution of next operations', function(done) { + testFalsyCallback(d, done); + }); + it('Operations are executed in the right order', function(done) { testRightOrder(d, done); }); @@ -138,7 +170,11 @@ describe('Executor', function () { it('Does not starve event loop and raise warning when more than 1000 callbacks are in queue', function(done){ testEventLoopStarvation(d, done); }); - + + it('Works in the right order even with no supplied callback', function(done){ + testExecutorWorksWithoutCallback(d, done); + }); + }); // ==== End of 'With persistent database' ==== @@ -159,11 +195,19 @@ describe('Executor', function () { it('A throw in a callback doesnt prevent execution of next operations', function(done) { testThrowInCallback(d, done); }); - + + it('A falsy callback doesnt prevent execution of next operations', function(done) { + testFalsyCallback(d, done); + }); + it('Operations are executed in the right order', function(done) { testRightOrder(d, done); }); - + + it('Works in the right order even with no supplied callback', function(done){ + testExecutorWorksWithoutCallback(d, done); + }); + }); // ==== End of 'With non persistent database' ==== });