0

I'm new to both node.js and Promise functionality so please forgive me if this question is really foolish.

I'm trying to make a child Promise call forEach child within a Parent call (if that makes sense).

This is my code:

        return new Promise(function(resolve, reject) {
            var authorMapArray = new Array
            db.sequelize.query(authorQuery, {
                replacements: queryParams
            }).spread(function(authorSitemap) {
                authorSitemap.forEach(function(obj) {
                    /*
                    return new Promise (function(resolve, reject){
                        var thisQuery = photoQuery + ' AND author = ' + obj.id.toString();
                        db.sequelize.query(thisQuery, {
                            queryParams
                        }).spread(function(authorImages) {
                            var authorImageArray = new Array;
                            authorImages.forEach(function(obj) {
                                var imgLink = { url: imgHost + obj.img_id + '.jpg', title : img_tags }
                                authorImageArray.push(imgLink);
                            })
                        });
                        resolve(authorImageArray);
                    });
                    */
                    var authorLink = { url: 'author/' + obj.id, /*img: authorImageArray,*/ changefreq: 'weekly', priority: 0.6, lastmodrealtime: true }
                    siteMapArray.push(authorLink);
                });
                resolve(siteMapArray);

                //and finally create it
                createSiteMap(siteMapArray);
            });
        })

You'll note, the section in the middle is commented out. When I run the code like this I get the results I expect, that is the authorLink added to the sitemap. When I uncomment the code (in order to include the images associated with the author in the sitemap), not even the authorlinks are added.

How do I get the images for the author included within their record?

EDIT

This is the more complete code:

function createSiteMap(myURLs) {
    var rows = 10000;
    var totalMaps = Math.trunc(myURLs.length/rows)+1;
    var today = new Date();
    var mySitemaps = new Array;
    for (var i=1; i<totalMaps+1; i++) {
        var filename = "public/sitemap-" + i.toString() + ".xml";
        var sitemap = sm.createSitemap({
            hostname: hostname,
            cacheTime: 600000,  //600 sec (10 min) cache purge period 
            urls: myURLs.slice((i-1)*rows,i*rows)
            });
        fs.writeFileSync(filename, sitemap.toString());
        mySitemaps.push(filename);
    }

    // this needs to create sitemap tags not url tags
    var smi = sm.buildSitemapIndex({
        urls: mySitemaps
        });
    fs.writeFileSync("public/sitemap.xml", smi.toString());

    process.exit();
}

function uniq(a) {
    var seen = {};
    return a.filter(function(item) {
        return seen.hasOwnProperty(item) ? false : (seen[item] = true);
    });
}

function getPhotos() {
    return new Promise(function(resolve, reject) {
        var siteMapArray = new Array()        
        var tags = new Array()
        siteMapArray.push ({ url: '/' , changefreq: 'weekly', priority: 0.8, lastmodrealtime: true, lastmodfile: 'views/home.hbs' },)
        db.sequelize.query(photoQuery, {
            replacements: queryParams
        }).spread(function(makeSiteMap) {
            makeSiteMap.forEach(function(obj) {
                // images for sitemap
                var img_tags = obj.tags.replace(/,/g , " ");
                var imgLink = { url: imgHost + obj.img_id + '.jpg', title : img_tags }
                var siteLink = { url: 'photo/' + obj.img_id, img: imgLink, changefreq: 'weekly', priority: 0.6, lastmodrealtime: true }
                siteMapArray.push(siteLink);
                obj.tags = obj.tags.split(',').map(function(e) {
                    return e.trim().split(' ').join('+');
                });
                for (var tag in obj.tags) {
                    tags.push(obj.tags[tag])
                }
            });

            resolve (siteMapArray);

            //tags for sitemap
            var uniqueTags = uniq(tags);
            for (var tag in uniqueTags) {
                var siteLink = { url: '/search/' + uniqueTags[tag], changefreq: 'weekly', priority: 0.8, lastmodrealtime: true }
                siteMapArray.push (siteLink);
            }

            //now author tags
            return new Promise(function(resolve, reject) {
                var authorMapArray = new Array
                db.sequelize.query(authorQuery, {
                    replacements: queryParams
                }).spread(function(authorSitemap) {
                    authorSitemap.forEach(function(obj) {
                        /*
                        return new Promise (function(resolve, reject){
                            var thisQuery = photoQuery + ' AND author = ' + obj.id.toString();
                            db.sequelize.query(thisQuery, {
                                queryParams
                            }).spread(function(authorImages) {
                                var authorImageArray = new Array;
                                authorImages.forEach(function(obj) {
                                    var imgLink = { url: imgHost + obj.img_id + '.jpg', title : img_tags }
                                    authorImageArray.push(imgLink);
                                })
                            });
                            resolve(authorImageArray);
                        });
                        */
                        var authorLink = { url: 'author/' + obj.id, /*img: authorImageArray,*/ changefreq: 'weekly', priority: 0.6, lastmodrealtime: true }
                        siteMapArray.push(authorLink);
                    });
                    resolve(siteMapArray);

                    //and finally create it
                    createSiteMap(siteMapArray);
                });
            })

        });
    });
};

getPhotos();
HenryM
  • 5,557
  • 7
  • 49
  • 105
  • 1
    Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Apr 06 '18 at 12:10

2 Answers2

3

Okay, let's assume that you want something like this:

function getSiteMapArray() {
  // return a promise that resolves to the siteMapArray
}

First step would be to rewrite it without using new Promise() - you shouldn't be needing this one very often, since most of work with promises is just chaining .then() calls which is much more readable.

Note that .spread() is just a .then() with sugar on top. The sugar is not standard Promise syntax, but rather an addon from bluebird that sequelize recommends to use. These are equivalent for a promise that resolves with array with 2 values:

something.then(resultArray => ...);
something.spread((resultItem1, resultItem2) => ...);

(I'm going to use arrow functions, is that OK?)


So the first step is to get rid of new Promise() as promised, before we start incorporating the code from your comments:

function getSiteMapArray() {
  var authorMapArray = new Array();
  return db.sequelize
    .query(authorQuery, {
      replacements: queryParams
    })
    .spread(authorSitemap => {
      authorSitemap.forEach(function(obj) {
        var authorLink = {
          url: "author/" + obj.id,
          /*img: authorImageArray,*/
          changefreq: "weekly",
          priority: 0.6,
          lastmodrealtime: true
        };
        siteMapArray.push(authorLink);
      });
      return siteMapArray;
    });
}

Simple enough?

  • We use .query() to get a promise of results,
  • then we use .then() or .spread() passing a callback that processes the results,
  • spread() returns a new promise that resolves when we're done with everything, and this promise is the result of getSiteMapArray(). It's going to resolve with the value from return siteMapArray.

We could simplify one step further with map() instead of forEach which is recommended whenever you want to transform each element in an array:

function getSiteMapArray() {
  return db.sequelize
    .query(authorQuery, {
      replacements: queryParams
    })
    .spread(authorSitemap => {
      return authorSitemap.map(obj => ({
        url: "author/" + obj.id,
        /*img: authorImageArray,*/
        changefreq: "weekly",
        priority: 0.6,
        lastmodrealtime: true
      }));
    });
}

So that was the easy part, now how do we incorporate the authorImage query here?

Let me extract a helper first:

function getSiteMapArray() {
  return db.sequelize
    .query(authorQuery, {
      replacements: queryParams
    })
    .spread(authorSitemap => {
      return authorSitemap.map(getAuthorDescription);
    });
}

function getAuthorDescription(obj) {
  return {
    url: "author/" + obj.id,
    /*img: authorImageArray,*/
    changefreq: "weekly",
    priority: 0.6,
    lastmodrealtime: true
  };
}

Now getAuthorDescription is synchronous, but we want it to do a query on its own, so let's rewrite it to async, so that it returns a promise too!

function getAuthorDescription(obj) {
  var thisQuery = photoQuery + " AND author = " + obj.id.toString();
  return db.sequelize
    .query(thisQuery, {
      queryParams
    })
    .spread(function(authorImages) {
      var authorImageArray = new Array();
      authorImages.forEach(function(obj) {
        var imgLink = { url: imgHost + obj.img_id + ".jpg", title: img_tags };
        authorImageArray.push(imgLink);
      });
      return {
        url: "author/" + obj.id,
        img: authorImageArray,
        changefreq: "weekly",
        priority: 0.6,
        lastmodrealtime: true
      };
    });
}

Another great case to use .map() but I'll leave that one for you.

Back to the original code:

function getSiteMapArray() {
  return db.sequelize
    .query(authorQuery, {
      replacements: queryParams
    })
    .spread(authorSitemap => {
      return authorSitemap.map(getAuthorDescription); // !!!
    });
}

Wow, now we're in trouble - getAuthorDescription returns a promise, so we're resolving getSiteMapArray with a list of promises, instead of list of values!

We need a way to wait for each of the promises returned from getAuthorDescription to finish, and obtain an array of collected results of all these promises. This way is called Promise.all:

So the code becomes:

function getSiteMapArray() {
  return db.sequelize
    .query(authorQuery, {
      replacements: queryParams
    })
    .spread(authorSitemap => {
      return Promise.all(authorSitemap.map(getAuthorDescription));
    });
}

Let me know if this helps!

Kos
  • 70,399
  • 25
  • 169
  • 233
  • Wow! OK, so I've tried implementing this and it still isn't working. I think part of the issue is I didn't show all my code because I've actually got another Promise above which I think is causing an issue. I've added it in an edit, but I'm going to need some time to read through and understand this post. - Thank you – HenryM Apr 06 '18 at 12:45
  • Careful - I only covered extracting `getSiteMapArray`, after calling and resolving it you'll need to call `createSiteMap()` somewhere in there – Kos Apr 06 '18 at 12:47
  • ok - I did add `createSiteMap()` to be called directly after `getSiteMapArray()` but I now realise, I don't think we're adding to `siteMapArray` anywhere in the new code so I need to add that but – HenryM Apr 06 '18 at 12:59
  • That would be `getSiteMapArray().then(siteMapArray => doSomethingWith(siteMapArray))` – Kos Apr 06 '18 at 13:48
  • Brilliant - thank you. I'm not sure I understand it really well yet but I have it working – HenryM Apr 06 '18 at 17:45
2

There are couple of issues in your implementation. I will suggest not to use [].foreach for child promise. make separate method for child promise and called it for each authorSitemap using promise.all.

Below is sample implementation with update.

return new Promise(function(resolve, reject) {
            var authorMapArray = new Array
            db.sequelize.query(authorQuery, {
                replacements: queryParams
            }).spread(function(authorSitemap) {

                return Promise.all(authorSitemap.map(GetAuthor))
                .then(function(authorImageArray){
                    var authorLink = { url: 'author/' + obj.id, img: authorImageArray, changefreq: 'weekly', priority: 0.6, lastmodrealtime: true }
                    siteMapArray.push(authorLink);
                    createSiteMap(siteMapArray);
                    resolve(siteMapArray);
                })
                .catch(function(error){
                    reject(error);
                })
            });
        })

function GetAuthor(obj) {
    return new Promise(function(reject,resolve){
        var thisQuery = photoQuery + ' AND author = ' + obj.id.toString();
        db.sequelize.query(thisQuery, { queryParams})
                    .spread(function(authorImages) {
                            var authorImageArray = new Array;
                            authorImages.forEach(function(obj) {
                                var imgLink = { url: imgHost + obj.img_id + '.jpg', title : img_tags }
                                authorImageArray.push(imgLink);
                            })
                            resolve(authorImageArray);
                        });
    })
}   
Jaydip Jadhav
  • 12,179
  • 6
  • 24
  • 40
  • This is giving me an error: `Unhandled rejection (<[{"url":"https://www.example.com/img-...>, no stack trace)` without any traceback which I need to try and understand – HenryM Apr 06 '18 at 13:02