delayed returning of array of collections from mongodb nodejs

Question

I need to retrieve a list of collections using express and mongodb module. First, ive retrieved a list of collection names which works, I then retrieve the data of those given collections in a loop. My problem is in getColAsync():

getColAsync()   {
    return new Promise((resolve, reject)    =>    {
        this.connectDB().then((db)  =>  {
            var allCols = [];
            let dbase = db.db(this.databaseName);
            dbase.listCollections().toArray((err, collectionNames)  =>  {
                if(err) {
                    console.log(err);
                    reject(err);
                }
                else    {
                    for(let i = 0; i < collectionNames.length; i++) {
                        dbase.collection(collectionNames[i].name.toString()).find({}).toArray((err, collectionData) =>  {
                            console.log("current collection data: " + collectionData);
                            allCols[i] = collectionData;
                        })
                    }
                    console.log("done getting all data");
                    resolve(allCols);
                }
            })
        })
    })
}

connectDB() {
    if(this.dbConnection)   {
        // if connection exists
        return this.dbConnection;
    }
    else    {
        this.dbConnection = new Promise((resolve, reject) =>    {
            mongoClient.connect(this.URL, (err, db) =>  {
                if(err) {
                    console.log("DB Access: Error on mongoClient.connect.");
                    console.log(err);
                    reject(err);
                }
                else    {
                    console.log("DB Access: resolved.");
                    resolve(db);
                }
            });
        });
        console.log("DB Access: db exists. Connected.");
        return this.dbConnection;
    }
}

In the forloop where i retrieve every collection, the console.log("done getting all data") gets called and the promise gets resolved before the forloop even begins. for example:

done getting all data
current collection data: something
current collection data: something2
current collection data: something3

Please help

Answer 1

The Problem

The problem in your code is this part:

for (let i = 0; i < collectionNames.length; i++) {
    dbase.collection(collectionNames[i].name.toString()).find({}).toArray((err, collectionData) => {
        console.log("current collection data: " + collectionData);
        allCols[i] = collectionData;
    })
}
console.log("done getting all data");
resolve(allCols);

You should notice that resolve(allCols); is called right after the for loop ends, but each iteration of the loop doesn't wait for the toArray callback to be called.

The line dbase.collection(collectionNames[i].name.toString()).find({}).toArray(callback) is asynchronous so the loop will end, you'll call resolve(allCols);, but the .find({}).toArray code won't have completed yet.

The Solution Concept

So, basically what you did was:

1. Initialize an array of results allCols = []
2. Start a series of async operations
3. Return the (still empty) array of results
4. As the async operations complete, fill the now useless results array.

What you should be doing instead is:

1. Start a series of async operations
2. Wait for all of them to complete
3. Get the results from each one
4. Return the list of results

The key to this is the Promise.all([/* array of promises */]) function which accepts an array of promises and returns a Promise itself passing downstream an array containing all the results, so what we need to obtain is something like this:

const dataPromises = []

for (let i = 0; i < collectionNames.length; i++) {
    dataPromises[i] = /* data fetch promise */;
}

return Promise.all(dataPromises);

As you can see, the last line is return Promise.all(dataPromises); instead of resolve(allCols) as in your code, so we can no longer execute this code inside of a new Promise(func) constructor.

Instead, we should chain Promises with .then() like this:

getColAsync() {
    return this.connectDB().then((db) => {
        let dbase = db.db(this.databaseName);
        const dataPromises = []
        dbase.listCollections().toArray((err, collectionNames) => {
            if (err) {
                console.log(err);
                return Promise.reject(err);
            } else {
                for (let i = 0; i < collectionNames.length; i++) {
                    dataPromises[i] = new Promise((res, rej) => {
                        dbase.collection(collectionNames[i].name.toString()).find({}).toArray((err, collectionData) => {
                            console.log("current collection data: " + collectionData);
                            if (err) {
                                console.log(err);
                                reject(err);
                            } else {
                                resolve(collectionData);
                            }
                        });
                    });
                }
                console.log("done getting all data");
                return Promise.all(dataPromises);
            }
        });
    })
}

Notice now we return a return this.connectDB().then(...), which in turn returns a Promise.all(dataPromises); this returning new Promises at each step lets us keep alive the Promise chain, thus getColAsync() will itself return a Promise you can then handle with .then() and .catch().

Cleaner Code

You can clean up your code a bit as fallows:

getColAsync() {
    return this.connectDB().then((db) => {
        let dbase = db.db(this.databaseName);
        const dataPromises = []
        // dbase.listCollections().toArray() returns a promise itself
        return dbase.listCollections().toArray()
            .then((collectionsInfo) => {
                // collectionsInfo.map converts an array of collection info into an array of selected
                // collections
                return collectionsInfo.map((info) => {
                    return dbase.collection(info.name);
                });
            })
    }).then((collections) => {
        // collections.map converts an array of selected collections into an array of Promises
        // to get each collection data.
        return Promise.all(collections.map((collection) => {
            return collection.find({}).toArray();
        }))
    })
}

As you see the main changes are:

1. Using mondodb functions in their promise form
2. Using Array.map to easily convert an array of data into a new array

Below I also present a variant of your code using functions with callbacks and a module I'm working on.

Promise-Mix

I'm recently working on this npm module to help get a cleaner and more readable composition of Promises.

In your case I'd use the fCombine function to handle the first steps where you select the db and fetch the list of collection info:

Promise.fCombine({
    dbase: (dbURL, done) => mongoClient.connect(dbURL, done),
    collInfos: ({ dbase }, done) => getCollectionsInfo(dbase, done),
}, { dbURL: this.URL })

This results in a promise, passing downstream an object {dbase: /* the db instance */, collInfos: [/* the list of collections info */]}. Where getCollectionNames(dbase, done) is a function with callback pattern like this:

getCollectionsInfo = (db, done) => {
    let dbase = db.db(this.databaseName);
    dbase.listCollections().toArray(done);
}

Now you can chain the previous Promise and convert the list of collections info into selected db collections, like this:

Promise.fCombine({
    dbase: ({ dbURL }, done) => mongoClient.connect(dbURL, done),
    collInfos: ({ dbase }, done) => getCollectionsInfo(dbase, done),
}, { dbURL: this.URL }).then(({ dbase, collInfos }) => {
    return Promise.resolve(collInfos.map((info) => {
        return dbase.collection(info.name);
    }));
})

Now downstream we have a the list of selected collections from our db and we should fetch data from each one, then merge the results in an array with collection data. In my module I have a _mux option which creates a PromiseMux which mimics the behaviour and composition patterns of a regular Promise, but it's actually working on several Promises at the same time. Each Promise gets in input one item from the downstream collections array, so you can write the code to fetch data from a generic collection and it will be executed for each collection in the array:

Promise.fCombine({
    dbase: ({ dbURL }, done) => mongoClient.connect(dbURL, done),
    collInfos: ({ dbase }, done) => getCollectionsInfo(dbase, done),
}, { dbURL: this.URL }).then(({ dbase, collInfos }) => {
    return Promise.resolve(collInfos.map((info) => {
        return dbase.collection(info.name);
    }));
})._mux((mux) => {
    return mux._fReduce([
        (collection, done) => collection.find({}).toArray(done)
    ]).deMux((allCollectionsData) => {
        return Promise.resolve(allCollectionsData);
    })
});

In the code above, _fReduce behaves like _fCombine, but it accepts an array of functions with callbacks instead of an object and it passes downstream only the result of the last function (not a structured object with all the results). Finally deMux executes a Promise.all on each simultaneous Promise of the mux, putting together their results.

Thus the whole code would look like this:

getCollectionsInfo = (db, done) => {
    let dbase = db.db(this.databaseName);
    dbase.listCollections().toArray(done);
}

getCollAsync = () => {
    return Promise.fCombine({
        /**
         * fCombine uses an object whose fields are functions with callback pattern to
         * build consecutive Promises. Each subsequent functions gets as input the results
         * from previous functions.
         * The second parameter of the fCombine is the initial value, which in our case is
         * the db url.
         */
        dbase: ({ dbURL }, done) => mongoClient.connect(dbURL, done), // connect to DB, return the connected dbase
        collInfos: ({ dbase }, done) => getCollectionsInfo(dbase, done), // fetch collection info from dbase, return the info objects
    }, { dbURL: this.URL }).then(({ dbase, collInfos }) => {
        return Promise.resolve(collInfos.map((info) => {
            /**
             *  we use Array.map to convert collection info into 
             *  a list of selected db collections
             */
            return dbase.collection(info.name);
        }));
    })._mux((mux) => {
        /**
         * _mux splits the list of collections returned before into a series of "simultaneous promises"
         * which you can manipulate as if they were a single Promise.
         */
        return mux._fReduce([ // this fReduce here gets as input a single collection from the retrieved list
            (collection, done) => collection.find({}).toArray(done)
        ]).deMux((allCollectionsData) => {
            // finally we can put back together all the results.
            return Promise.resolve(allCollectionsData);
        })
    });
}

In my module I tried to avoid most common anti-pattern thought there still is some Ghost Promise which I'll be working on.

Using the promises from mongodb this would get even cleaner:

getCollAsync = () => {
    return Promise.combine({
        dbase: ({ dbURL }) => { return mongoClient.connect(dbURL); },
        collInfos: ({ dbase }) => {
            return dbase.db(this.databaseName)
                .listCollections().toArray();
        },
    }, { dbURL: this.URL }).then(({ dbase, collInfos }) => {
        return Promise.resolve(collInfos.map((info) => {
            return dbase.collection(info.name);
        }));
    }).then((collections) => {
        return Promise.all(collections.map((collection) => {
            return collection.find({}).toArray();
        }))
    });
}