handle variadic and falsy results in concatLimit [fixes #1437]

5 files changed, 446 insertions, 88 deletions

diff --git a/lib/concatLimit.js b/lib/concatLimit.js
index 68dd8a3..8b8f5d8 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 e0c8ba3..bec727e 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 afb612b..d20f385 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 0000000..7020526
--- /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 677517c..77ab59b 100644
--- a/perf/suites.js
+++ b/perf/suites.js
@@ -131,6 +131,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
     args: [