0

I'm stuck in a function I'm working with ( I can be doing this all wrong ). So a quick explanation, I want to add bulk data in a collection, the collection is called "Sites" the format of the CSV is site,country,type. I'm trying to use promises for this (Bluebird). So consider the code: 

 Promise.each(sites, sites => new Promise((resolve, reject) => {
    //console.log(sites);
    let name = tools.extractDomain(req, res, sites[0]);
    let country = sites[1];
    let group = sites[2];
    if (name != "" && country != "" && group != "") {

        Site.findOne({ name: name }, "_id", function(err, duplicate) {
            if (false) {
                console.log("Duplicate site: " + duplicate);

            } else { 
                   //console.log("Adding " + name)
                let site = new Site()
                site.name = name
                site.meta = {}
                site.group = group
                site.country = country
                site.geomix = []
                site.addedBy = req.user._id
                site.addedAt = Date.now()
                site.saveAsync().then(function(response){
                    tools.saveHistory(req, res, response._id, response.name, "Website Meta fetched.");
                    tools.saveHistory(req, res, response._id, response.name, "Link added for the first time."); //Save in history 
                    resolve(site);
                }).catch(function (e){
                    console.log(name);
                    reject();
                });  
            }
         });

    }else{
        console.log('Wrong Format');
    }
}).then((data) => {
      console.log('All websites processed!');
      addedSites.push(data);
}).catch(err => {
      //console.error('Failed');
}));

    res.send({ status: 'ok', message: ''});

I'm making ajax calls so I return a res.send({ status: 'ok', message: ''}), I know that its in the incorrect place and I want to send some data along the res.send. Currently it sends the headers before the code actually finishes. I want to send the headers after all the data is added in Mongo but for every each in this case he resolve() so if I send the headers inside the ".then" of the ".each" I will get headers already sent error.

This might be a bit confusing. I feel I'm not doing this right. I'm going a bit crazy as well as I can't find a proper example that I can understand and implement.

But in the end my main question is: using an Ajax call what's the proper way to add let's say 1000 records in a collection using promises and actually control properly those who fail to add and those who don't?

Right now my code actually works but the logic is wrong for sure.

Thanks.

DanielPanic
  • 184
  • 1
  • 11
  • 1
    Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! If you need to promisify `Site.findOne`, do that in a separate function (or just let Bluebird do that for you). – Bergi May 31 '17 at 18:13

2 Answers2

0

You can use bulkWrite on your model.

Ref: http://mongoosejs.com/docs/api.html#model_Model.bulkWrite

EDIT:

Sorry I misunderstood you. You need to move res.send({ status: 'ok', message: ''}); to then() and catch() blocks, so you will get something like this:

Promise.each(sites, sites => new Promise((resolve, reject) => {
    // stuff you did before
}).then((data) => {
    console.log('All websites processed!');
    addedSites.push(data);
    res.send({ status: 'ok', message: ''});
}).catch(err => {
    res.send({ status: 'failed', message: err.message});
}));
ponury-kostek
  • 7,824
  • 4
  • 23
  • 31
  • Not sure if that's helpful. I'm aware of that function and I could use it I'm still confused and my point is more to understand the proper way to bulk with promises and handling the errors. Even if I implement bulkWrite I will still have the same problem. – DanielPanic May 31 '17 at 11:01
  • That's better but I had still reached that conclusion. But check this everytime I save I have a resolve() so he sends that header each time and before the code actually work (In my case in the Front End I the status.ok and I send a warning to the user as well I fill the dom with the data) if I put that res.send there. I fixed it by counting the size of the records to add and have an increment when they both match I send te header. Not sure if that the most "elegant" way to do – DanielPanic May 31 '17 at 17:07
0

This is what I came too, if someone can tell me if this is a good arch.

    exports.addBulkSite = function(req, res, next) {
    let siteArray = csv.parse((req.body.sites).trim()),
        addedSites = [],
        failedSites = [],
        duplicated = [],
        sites = siteArray,
        size = sites.length,
        processed = 0,
        meta;
    Promise.each(sites, sites => new Promise((resolve, reject) => {
        let name = tools.extractDomain(req, res, sites[0]),
            country = sites[1],
            group = sites[2];
        if (name != "" && country != "" && group != "") {
            Site.findOneAsync({ name: name }, "_id").then(function(duplicate) {
                duplicated.push(duplicate);
                reject({name:name, message: 'Duplicated', critical:false});         
             }).catch(function(notDuplicated){
                let site = new Site()
                site = { 
                            name: name, 
                            meta: {}, 
                            group: group, 
                            country: country, geomix:{}, 
                            addedBy: req.user._id,
                            addedAt:Date.now()
                        }
                site.saveAsync().then(function(response){
                    tools.saveHistory(req, res, response._id, response.name, "Website Meta fetched.");
                    tools.saveHistory(req, res, response._id, response.name, "Link added for the first time."); //Save in history 
                    resolve(site);
                }).catch(function (e){
                    console.log(e);
                    reject({name:name, message: 'Error saving in the database. Please contact the administrator.', critical: true});
                });  
            });
        }else{
            reject({name:name, message: 'Paramaters are missing', critical:false});
        }
    }).then((data) => {
          processed++;
          addedSites.push(data);
          if(processed==size){
            console.log('out');
            res.send({ status: 'ok', addedSites: addedSites, failedSites: failedSites, duplicated: duplicated});
          }
    }).catch((err) => {
          processed++;
          console.log(err);
          failedSites.push(err);
          if(processed==size){
            console.log('out');
            res.send({ status: 'ok', addedSites: addedSites, failedSites: failedSites, duplicated: duplicated});
          }
    })); 
}
DanielPanic
  • 184
  • 1
  • 11
  • 1
    When you use `findOneAsync` anyway, avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi May 31 '17 at 18:16
  • I'm reading some stuff about that but as a being new to promises Im finding hard to understand and put that in my code, would love if you could explain it better using my code that would put me way more in perspective. Still doing some research tought and your tip is already really helpfull since it means it the right direction. Thanks. – DanielPanic May 31 '17 at 18:18
  • Just drop the `new Promise((resolve, reject) => {` part and instead add `return` to *every single function*, returning a value or promise (or `throw` for rejection). – Bergi May 31 '17 at 18:25
  • Will try to understand, have been killing myself all day and Im a bit confused. Not sure about how to implement that right away but that tip plus the link you provide will help and just need to sleep on it :P Thanks mate. – DanielPanic May 31 '17 at 18:30