musecz
musecz

Reputation: 805

Issue with promises in a for loop

I'm confronted to a situation which drives me a bit mad.

So The situation is as below :

module.exports = {
  
  generation: function (req, res) {

    // Let's firstly fetch all the products from the productTmp Table
    function fetchProductsTmp (){
      ProductsTmp.find().then(function (products) {
        return Promise.all(products.map (function (row){
           Service.importProcess(row);
        }));
      });
    }
    fetchProductsTmp();
  }

Here I simply call my model ProductsTmp to fetch my datas and iterate through my rows calling importProcess.

importProcess :

importProcess: function (product) {

    async.series([
      function (callback) {
        return SousFamille.findOne({name: product.sous_famille}).then(function (sf) {
          console.log('1');
          if (!sf) {
            return SousFamille.create({name: product.sous_famille}).then(function (_sf)             {
              console.log('2');
              callback(null, _sf.sf_id);
            });
          } else {
              callback(null, sf.sf_id);
          }
        });
      },
      function (callback){
        console.log('3');
      },
    ], function(err, results){
      if(err) return res.send({message: "Error"});

    });
      
}

So I got with my console log : 1 1 1 2 3 2 3 2 3

What I want to Obtain is 1 2 3 1 2 3 1 2 3 So that each function wait for the promise to finish before calling the next one.

Upvotes: 2

Views: 78

Answers (2)

mido
mido

Reputation: 25034

Sorry, can't help the urge to do it in ES6, basically things can be reduced to single line, like Bergi said, async is redundant( using Bluebird Promise Library):

importProcess: product => 
                  SousFamille.findOne({name: product.sous_famille})
                    .then(sf =>  sf? sf.sf_id : SousFamille.create({name: product.sous_famille}).then(_sf => _sf.sf_id))

// the other module
module.exports = {

  generation: (req, res) => ProductsTmp.find()
                              .then(products => Promise.mapSeries(products, Service.importProcess.bind(Service)) )
                              .then(ids => res.send({ids}))
                              .catch(error => res.send({message: 'Error'}))
  }

also like noppa said, your problem is the missing return in Service.importProcess(row), same code in ES5:

module.exports = {

  generation: function (req, res) {

      ProductsTmp.find()
        .then(function (products) {
          return Promise.mapSeries(products, Service.importProcess.bind(Service)) );
      }).then(function(ids){
        res.send({ids: ids});
      }).catch(function(error){
        res.send({message: 'Error'});
      })
}


importProcess: function (product) {

    return SousFamille.findOne({name: product.sous_famille})
      .then(function (sf) {
          if (sf) return sf.sf_id;
          return SousFamille.create({name: product.sous_famille})
                    .then(function (_sf){ return _sf.sf_id});
    });      
}

Upvotes: 0

noppa
noppa

Reputation: 4066

In the generation function of the first section, replace

return Promise.all(products.map (function (row){
    Service.importProcess(row);
}));

with

var results = [],
    pushResult = id => results.push(id);
return products.reduce(function(prev, row){//Go through all the products
    //Take the previous promise, and schedule next call to Service.importProcess to be
    //made after the previous promise has been resolved
    return prev.then(function(){
        return Service.importProcess(row).then(pushResult);
    });
}, Promise.resolve())
.then(() => results);

You also need to return a promise from importProcess for this to work. Just ditch the whole async.series thingy and do something like

return new Promise(function(resolve, reject){
     ...
     resolve(sf.sf_id); //instead of the callback(null, sf.sf_id)
     ...
});

Update: This forces the calls to Service.importProcess to be sequential instead of concurrent, which does affect the overall performance of calls to generation. But I guess you have more solid reasons to do so than sequential console.logs.

Upvotes: 1

Related Questions