From 1382f3049f15819c2c5d691600d3e95bbd3918a8 Mon Sep 17 00:00:00 2001 From: Hubert Argasinski Date: Mon, 12 Jun 2017 23:31:35 -0400 Subject: [PATCH] handle variadic and falsy results in concatLimit [fixes #1437] --- lib/concatLimit.js | 14 +- mocha_test/concat.js | 468 ++++++++++++++++++++++++++++++++++-------- mocha_test/groupBy.js | 2 +- mocha_test/slice.js | 32 +++ perf/suites.js | 18 ++ 5 files changed, 446 insertions(+), 88 deletions(-) create mode 100644 mocha_test/slice.js diff --git a/lib/concatLimit.js b/lib/concatLimit.js index 68dd8a39f..8b8f5d81b 100644 --- a/lib/concatLimit.js +++ b/lib/concatLimit.js @@ -1,4 +1,6 @@ import noop from 'lodash/noop'; +import wrapAsync from './internal/wrapAsync'; +import slice from './internal/slice'; import mapLimit from './mapLimit'; /** @@ -21,12 +23,18 @@ import mapLimit from './mapLimit'; */ export default function(coll, limit, iteratee, callback) { callback = callback || noop; - mapLimit(coll, limit, iteratee, function(err, mapResults) { + var _iteratee = wrapAsync(iteratee); + mapLimit(coll, limit, function(val, callback) { + _iteratee(val, function(err /*, ...args*/) { + if (err) return callback(err); + return callback(null, slice(arguments, 1)); + }); + }, function(err, mapResults) { var result = []; - + var _concat = Array.prototype.concat; for (var i = 0; i < mapResults.length; i++) { if (mapResults[i]) { - result = result.concat(mapResults[i]); + result = _concat.apply(result, mapResults[i]); } } diff --git a/mocha_test/concat.js b/mocha_test/concat.js index e0c8ba389..bec727efa 100644 --- a/mocha_test/concat.js +++ b/mocha_test/concat.js @@ -3,105 +3,405 @@ var expect = require('chai').expect; var assert = require('assert'); describe('concat', function() { - it('concat', function(done) { - var call_order = []; - var iteratee = function (x, cb) { - setTimeout(function(){ - call_order.push(x); - var r = []; - while (x > 0) { - r.push(x); - x--; + this.timeout(250); + + function concatIteratee(callOrder, val, next) { + setTimeout(function() { + callOrder.push(val); + next(null, [val, val+1]); + }, val * 25); + } + + context('concat', function() { + it('basics', function(done) { + var callOrder = []; + async.concat([1, 3, 2], concatIteratee.bind(this, callOrder), function(err, result) { + expect(err).to.eql(null); + expect(callOrder).to.eql([1, 2, 3]); + expect(result).to.eql([1, 2, 3, 4, 2, 3]); + done(); + }); + }); + + it('error', function(done) { + async.concat([1, 3, 2], function(val, next) { + if (val === 3) { + return next(new Error('fail')); } - cb(null, r); - }, x*25); - }; - async.concat([1,3,2], iteratee, function(err, results){ - expect(results).to.eql([1,3,2,1,2,1]); - expect(call_order).to.eql([1,2,3]); - assert(err === null, err + " passed instead of 'null'"); - done(); + next(null, [val, val+1]); + }, function(err, result) { + expect(err).to.not.eql(null); + expect(result).to.eql([1, 2]); + done(); + }); }); - }); - it('concat error', function(done) { - var iteratee = function (x, cb) { - cb(new Error('test error')); - }; - async.concat([1,2,3], iteratee, function(err){ - assert(err); - done(); + it('original untouched', function(done) { + var arr = ['foo', 'bar', 'baz']; + async.concat(arr, function(val, next) { + next(null, [val, val]); + }, function(err, result) { + expect(arr).to.eql(['foo', 'bar', 'baz']); + expect(result).to.eql(['foo', 'foo', 'bar', 'bar', 'baz', 'baz']); + done(); + }); }); - }); - it('concat preserves order', function(done) { - var arr = [30, 15]; - async.concat(arr, function(x, cb) { - setTimeout(function() { - cb(null, x); - }, x); - }, function(err, result) { - expect(err).to.eql(null); - expect(result).to.eql(arr); - done(); - }) + it('empty results', function(done) { + var arr = ['foo', 'bar', 'baz']; + async.concat(arr, function(val, next) { + next(null); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.be.an('array').that.is.empty; + done(); + }); + }); + + it('empty arrays', function(done) { + var arr = ['foo', 'bar', 'baz']; + async.concat(arr, function(val, next) { + next(null, []); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.be.an('array').that.is.empty; + done(); + }); + }); + + it('handles empty object', function(done) { + async.concat({}, function(val, next) { + assert(false, 'iteratee should not be called'); + next(); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.be.an('array').that.is.empty; + done(); + }); + }); + + it('variadic', function(done) { + var arr = ['foo', 'bar', 'baz']; + async.concat(arr, function(val, next) { + next(null, val, val); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.eql(['foo', 'foo', 'bar', 'bar', 'baz', 'baz']); + done(); + }); + }); + + it('flattens arrays', function(done) { + var arr = ['foo', 'bar']; + async.concat(arr, function(val, next) { + next(null, [val, [val]]); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.eql(['foo', ['foo'], 'bar', ['bar']]); + done(); + }); + }); + + it('handles fasly values', function(done) { + var falsy = [null, undefined, 0, '']; + async.concat(falsy, function(val, next) { + next(null, val); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.eql(falsy); + done(); + }); + }); + + it('handles objects', function(done) { + var obj = {a: 'foo', b: 'bar', c: 'baz'}; + async.concat(obj, function(val, next) { + next(null, val); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.eql(['foo', 'bar', 'baz']); + done(); + }); + }); + + it('main callback optional', function(done) { + var arr = [1, 2, 3]; + var runs = []; + async.concat(arr, function(val, next) { + runs.push(val); + var _done = (runs.length === arr.length); + async.setImmediate(function() { + next(null); + if (_done) { + expect(runs).to.eql(arr); + done(); + } + }); + }); + }); + + it('iteratee callback is only called once', function(done) { + async.concat([1, 2], function(val, next) { + try { + next(val); + } catch (exception) { + expect(function() { + next(exception); + }).to.throw(/already called/); + done(); + } + }, function() { + throw new Error(); + }); + }); + + it('preserves order', function(done) { + var arr = [30, 15]; + async.concat(arr, function(x, cb) { + setTimeout(function() { + cb(null, x); + }, x); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.eql(arr); + done(); + }); + }); + + it('handles Map', function(done) { + if (typeof Map !== 'function') { + return done(); + } + + var map = new Map([ + ['a', 'b'], + ['b', 'c'], + ['c', 'd'] + ]); + + async.concat(map, function(val, next) { + next(null, val); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.eql(['a', 'b', 'b', 'c', 'c', 'd']); + done(); + }); + }); + + it('handles sparse results', function(done) { + var arr = [1, 2, 3, 4]; + async.concat(arr, function(val, next) { + if (val === 1 || val === 3) { + return next(null, val+1); + } else if (val === 2) { + async.setImmediate(function() { + return next(null, val+1); + }); + } else { + return next('error'); + } + }, function(err, result) { + expect(err).to.not.eql(null); + expect(result).to.eql([2, 4]); + async.setImmediate(done); + }); + }); }); - it('concatSeries', function(done) { - var call_order = []; - var iteratee = function (x, cb) { - setTimeout(function(){ - call_order.push(x); - var r = []; - while (x > 0) { - r.push(x); - x--; + context('concatLimit', function() { + var arr = ['foo', 'bar', 'baz']; + it('basics', function(done) { + var running = 0; + var concurrency = {'foo': 2, 'bar': 2, 'baz': 1}; + async.concatLimit(arr, 2, function(val, next) { + running++; + async.setImmediate(function() { + expect(running).to.equal(concurrency[val]); + running--; + next(null, val, val); + }) + }, function(err, result) { + expect(running).to.equal(0); + expect(err).to.eql(null); + expect(result).to.eql(['foo', 'foo', 'bar', 'bar', 'baz', 'baz']); + done(); + }); + }); + + it('error', function(done) { + async.concatLimit(arr, 1, function(val, next) { + if (val === 'bar') { + return next(new Error('fail')); + } + next(null, val); + }, function(err, result) { + expect(err).to.not.eql(null); + expect(result).to.eql(['foo']); + done(); + }); + }); + + it('handles objects', function(done) { + async.concatLimit({'foo': 1, 'bar': 2, 'baz': 3}, 2, function(val, next) { + next(null, val+1); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.eql([2, 3, 4]); + done(); + }); + }); + + it('handles empty object', function(done) { + async.concatLimit({}, 2, function(val, next) { + assert(false, 'iteratee should not be called'); + next(); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.be.an('array').that.is.empty; + done(); + }); + }); + + it('handles undefined', function(done) { + async.concatLimit(undefined, 2, function(val, next) { + assert(false, 'iteratee should not be called'); + next(); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.be.an('array').that.is.empty; + done(); + }); + }); + + it('limit exceeds size', function(done) { + var callOrder = []; + async.concatLimit([3, 2, 2, 1], 10, concatIteratee.bind(this, callOrder), function(err, result) { + expect(err).to.eql(null); + expect(result).to.eql([3, 4, 2, 3, 2, 3, 1, 2]); + expect(callOrder).to.eql([1, 2, 2, 3]); + done(); + }); + }); + + it('limit equal size', function(done) { + var callOrder = []; + async.concatLimit([3, 2, 2, 1], 4, concatIteratee.bind(this, callOrder), function(err, result) { + expect(err).to.eql(null); + expect(result).to.eql([3, 4, 2, 3, 2, 3, 1, 2]); + expect(callOrder).to.eql([1, 2, 2, 3]); + done(); + }); + }); + + it('zero limit', function(done) { + async.concatLimit([3, 2, 2, 1], 0, function(val, next) { + assert(false, 'iteratee should not be called'); + next(); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.be.an('array').that.is.empty; + done(); + }); + }); + + it('does not continue replenishing after error', function(done) { + var started = 0; + var arr = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; + var delay = 10; + var limit = 3; + var maxTime = 10 * arr.length; + + async.concatLimit(arr, limit, function(val, next) { + started++; + if (started === 3) { + return next(new Error('fail')); } - cb(null, r); - }, x*25); - }; - async.concatSeries([1,3,2], iteratee, function(err, results){ - expect(results).to.eql([1,3,2,1,2,1]); - expect(call_order).to.eql([1,3,2]); - assert(err === null, err + " passed instead of 'null'"); - done(); + + setTimeout(function() { + next(); + }, delay); + }, function(err, result) { + expect(err).to.not.eql(null); + expect(result).to.be.an('array').that.is.empty; + }); + + setTimeout(function() { + expect(started).to.equal(3); + done(); + }, maxTime); }); }); - it('concatLimit basics', function(done) { - var running = 0; - var concurrency = { - 'foo': 2, - 'bar': 2, - 'baz': 1 - }; - - async.concatLimit(['foo', 'bar', 'baz'], 2, function(val, next) { - running++; - async.setImmediate(function() { - expect(running).to.equal(concurrency[val]); - running--; + context('concatSeries', function() { + it('basics', function() { + var callOrder = []; + var running = 0; + var iteratee = function (x, cb) { + running++; + setTimeout(function() { + expect(running).to.equal(1); + running--; + callOrder.push(x); + var r = []; + while (x > 0) { + r.push(x); + x--; + } + cb(null, r); + }, x*25); + }; + async.concatSeries([1,3,2], iteratee, function(err, results) { + expect(results).to.eql([1,3,2,1,2,1]); + expect(running).to.equal(0); + expect(call_order).to.eql([1,3,2]); + assert(err === null, err + " passed instead of 'null'"); + done(); + }); + }); + + it('error', function(done) { + async.concatSeries(['foo', 'bar', 'baz'], function(val, next) { + if (val === 'bar') { + return next(new Error('fail')); + } next(null, [val, val]); + }, function(err, result) { + expect(err).to.not.eql(null); + expect(result).to.eql(['foo', 'foo']); + done(); }); - }, function(err, result) { - expect(running).to.equal(0); - expect(err).to.eql(null); - expect(result).to.eql(['foo', 'foo', 'bar', 'bar', 'baz', 'baz']); - done(); }); - }); - it('concatLimit error', function(done) { - var arr = ['foo', 'bar', 'baz']; - async.concatLimit(arr, 2, function(val, next) { - if (val === 'bar') { - return next(new Error('fail')); - } - next(null, [val, val]); - }, function(err, result) { - expect(err).to.not.eql(null); - expect(result).to.eql(['foo', 'foo']); - done(); + it('handles objects', function(done) { + async.concatSeries({'foo': 1, 'bar': 2, 'baz': 3}, function(val, next) { + return next(null, [val, val+1]); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.eql([1, 2, 2, 3, 3, 4]); + done(); + }); + }); + + it('handles empty object', function(done) { + async.concatSeries({}, function(val, next) { + assert(false, 'iteratee should not be called'); + next(); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.be.an('array').that.is.empty; + done(); + }); + }); + + it('handles undefined', function(done) { + async.concatSeries(undefined, function(val, next) { + assert(false, 'iteratee should not be called'); + next(); + }, function(err, result) { + expect(err).to.eql(null); + expect(result).to.be.an('array').that.is.empty; + done(); + }); }); }); }); diff --git a/mocha_test/groupBy.js b/mocha_test/groupBy.js index afb612b0c..d20f385f4 100644 --- a/mocha_test/groupBy.js +++ b/mocha_test/groupBy.js @@ -330,7 +330,7 @@ describe('groupBy', function() { }); it('handles empty object', function(done) { - async.groupByLimit({}, 2, function(val, next) { + async.groupBySeries({}, function(val, next) { assert(false, 'iteratee should not be called'); next(); }, function(err, result) { diff --git a/mocha_test/slice.js b/mocha_test/slice.js new file mode 100644 index 000000000..70205264d --- /dev/null +++ b/mocha_test/slice.js @@ -0,0 +1,32 @@ +var slice = require('../lib/internal/slice').default; +var expect = require('chai').expect; + +describe('slice', function() { + it('should slice arrays', function() { + var arr = ['foo', 'bar', 'baz']; + var result = slice(arr, 2); + expect(arr).to.eql(['foo', 'bar', 'baz']); + expect(result).to.eql(['baz']); + }); + + it('should handle ArrayLike objects', function() { + var args = {0: 'foo', 1: 'bar', 2: 'baz', length: 3}; + var result = slice(args, 1); + expect(result).to.be.an('array'); + expect(result).to.eql(['bar', 'baz']); + }); + + it('should handle arguments', function() { + var foo = function() { + return slice(arguments, 1); + }; + var result = foo.apply(null, ['foo', 'bar', 'baz']); + expect(result).to.be.an('array'); + expect(result).to.eql(['bar', 'baz']); + }); + + it('should return an empty array on an invalid start', function() { + var result = slice(['foo', 'bar', 'baz'], 10); + expect(result).to.be.an('array').that.is.empty; + }); +}); diff --git a/perf/suites.js b/perf/suites.js index 677517ce9..77ab59beb 100644 --- a/perf/suites.js +++ b/perf/suites.js @@ -130,6 +130,24 @@ module.exports = [{ }); }, done); } +}, { + name: "concat", + // args lists are passed to the setup function + args: [ + [10], + [300], + [10000] + ], + setup: function setup(count) { + tasks = _.range(count); + }, + fn: function(async, done) { + async.concat(tasks, function(num, cb) { + async.setImmediate(function() { + cb(null, [num]); + }); + }, done); + } }, { name: "eachOf", // args lists are passed to the setup function