Saving multiple documents with Mongoose and doing something when last one is saved

Question

I want to save 8 objects to a MongoDB database using Mongoose. When the last document is saved, I want to report (i.e. send an event) that all documents have been saved.

The way I'm doing it now is quite messy (especially for increasingly larger amounts of documents I want to save).

This is how I have it now (just for 4 people for this example). Is there a cleaner way you can recommend?

person1.save(function(err, result){
    if (err) console.log(err);
    else{
        person2.save(function(err, result){
            if (err) console.log(err);
            else{
                person3.save(function(err, result){
                    if (err) console.log(err);
                    else{
                        person4.save(function(err, result){
                            if (err) console.log(err);
                            else{
                                done();
                            }
                        });
                    }
                });
            }
        });
    }
});

Solution 1

A useful library to coordinate asynchronous operations is async. In your case, the code would look something like this:

var people = [ person1, person2, person3, person4, ... ];

async.eachSeries(people, function(person, asyncdone) {
  person.save(asyncdone);
}, function(err) {
  if (err) return console.log(err);
  done(); // or `done(err)` if you want the pass the error up
});

Solution 2

Using promises and Array.map()

const userPromises = persons.map(user => {
  return new Promise((resolve, reject) => {
    person.save((error, result) => {
      if (error) {
        reject(error)
      }
      resolve(result);
    })
  })
});

Promise.all(userPromises).then((results) => {
  //yay!
  //results = [first, second, etc...]
}, (error) => {
  //nay!
})

Solution 3

I would recommend to have an array and save with iteration. Will have same performance but code would be cleaner.

You can have

var Person = mongoose.model('Person');
var people = [];

people[0] = new Person({id: 'someid'});
people[0].set('name', 'Mario');
people[1] = new Person({id: 'someid'});
people[1].set('name', 'Mario');
people[2] = new Person({id: 'someid'});
people[2].set('name', 'Mario');

var errors = [];
for(person in people){
 people[person].save(function(err, done){
  if(err) errors.push(err);
  if (person === people.length){ yourCallbackFunction(errors){
    if (errors.length!=0) console.log(errors);
    //yourcode here
   };
  }
 });
}

Comments

[robertklep]

There's a slight difference though: your code saves the documents concurrently, whereas the original example saved the documents sequentially.

[robertklep]

This will call the callback when an X amount of save operations are started, but that doesn't necessarily mean that they have all finished. Also, why not just use Array#forEach?

[aaronmarruk]

Please see my updated answer regarding operations finishing.

[aaronmarruk]

To answer your question regarding Underscore, I prefer the convenience and simplicity / readability of the underscore.each method, compared to plain javascript loops.

[robertklep]

I don't see a whole lot of difference between _.each(array, fn) and array.forEach(fn) myself (I actually find the second more readable)

[Paweł Smołka]

This sollution needs additional library wich makes app more heavy. Still its good sollution. I give +

[Paweł Smołka]

In request description author did not mention this sequence is important but I update my ans to find if all are done well.

[CodyBugstein]

@PawełSmołka I suppose it doesn't matter if it is saved concurrently, I just want to know when the last one is saved

[Dan Sabin]

The post above does not guarantee that save() has worked for all cases as mentioned in the first comment.

[Dan Sabin]

This will fire when the last person is saved and finished, but does not take into account whether all the people have actually finished saving. Only async will properly count all the saves have finished.

[Dan Sabin]

The async library is not a major addition. Also since this is server side the value of async vs. it's small footprint does not really have an adverse affect on the code.

[Dan Sabin]

the save callback can happen later in time. You could go through and call save for all of them and think hasError = false and i === length, but then the save callback finally returns (say 100ms later) and you've already said nothing bad happened. I'm working on a code sample right now.

[aaronmarruk]

ok thanks for the response, I kind of came to the same conclusion myself. I will definitely use .done() or similar in the future.

[Dan Sabin]

In case anyone is curious: gist.github.com/sabind/91d8e13ff378767e2640 Consider using 'async' instead, like the leading answer recommends.

[Dan Sabin]

For something like this you actually want to use parallel.

[robertklep]

@DanSabin the initial question showed a sequential workflow, which I copied using async, but I would use parallel as well (or perhaps eachLimit)

[Paweł Smołka]

Thanks for that tip, removed else and should be fine now.

[Luzan Baral]

Where do I implement this code? I tried by pasting this on my controller. And it gave me person.save is not a function

[robertklep]

@LuzanBaral really depends on the rest of your setup (for instance, "controllers" suggests that you're using some sort of framework).

[Luzan Baral]

I am using NodeJS Express App

[robertklep]

@LuzanBaral Express doesn't have controllers per se, and it also doesn't come with Mongoose support out of the box (meaning that you need to add both yourself). This particular code is specific to Mongoose.

[Luzan Baral]

@robertklep Can you please checkout this gist gist.github.com/luzan/a760cc4649e09083fb5d72cc72705147 I hope this clears out my issue.

[robertklep]

@LuzanBaral looks like you mean point.save(asyncdone), not Point.save(asyncdone).

[robertklep]

@LuzanBaral I'll add my comments to the Gist instead of here.