1

I am using Graphql with MongoDB. Used a subquery in resolver but while executing the subquery main query return data not wait till completion of the subquery. I want to operate the main query using subqueries parameters in resolver.

  return  await Articles.find({ Status: 1, isPublish : true  })
      .sort({TotalClapCount:-1})
      .sort({ViewCount:-1})
      .skip( offset )
      .limit(limit)
      .then( async ( lor ) => { await
       lor.forEach(async function(data, key){
          data["isBookmark"] =  
                  await ArticleBookmarks
                  .find({ ArticleID : data["ID"], UserID : ArgsUserID, Status : 1 })
                  .countDocuments()
                  .then( (hre) =>{return (hre == 1) ? true : false; });
      );
    });
    return  lor;
});

I want to display the list of the article with its bookmark in the single query but return lor is executed before the subquery operation. How async-await work for this?

Ratan Uday Kumar
  • 5,738
  • 6
  • 35
  • 54
ADDC
  • 103
  • 7

3 Answers3

1

You can just use regular promises, a regular for loop and async/await like this. There is no need for an external library function such as async.series() here:

let lor = await Articles.find({ Status: 1, isPublish: true})
    .sort({TotalClapCount: -1})
    .sort({ViewCount: -1})
    .skip(offset)
    .limit(limit);

for (let data of lor) {
    let hre = await ArticleBookmarks.find({
        ArticleID: data.ID,
        UserID: ArgsUserID,
        Status: 1
    }).countDocuments();
    data.isBookmark = (hre == 1);
}
return lor;

Changes:

  1. There's no use in return await Articles.find() as it does nothing different than return Articles.find() since both just return a promise since it's inside an async function and all async functions return a promise.

  2. Change .forEach() to a regular for loop because a for loop WILL pause execution of the function for await. forEach() will not pause the loop.

  3. await xxx.forEach() does not await anything because .forEach() has no return value and await only does something useful when you await a promise.

  4. Change (hre == 1) ? true : false; to just (hre == 1) because hre == 1 is already a boolean so there's no need for ? true : false. Personally, I would probably use (hre === 1) if you know the data is actually a number and not a string since === is nearly always more desirable than == (no implicit or accidental type conversions).

  5. If you're going to use await, then it's usually simpler to use it other places in that function instead of .then() as it leads to more sequential looking code that most would say looks simpler.

Other comments:

  1. await should only be used when you're awaiting a promise. That's the only time is does something useful.

  2. Wrapping new Promise() around other promise returning functions is considered a promise anti-pattern as it is not necessary and creates many opportunities for programming mistakes, particularly with error handling. You can just return and use the promises you already have without wrapping in a new one.

  3. async.eachSeries() offers nothing that regular promise sequencing using async/await can already do just fine unless you're trying to run in an environment that doesn't have async/await.

  4. When you want await to pause a loop, use a regular for loop. It does not pause the loop for array methods such as .map() or .forEach() because those iteration methods are not promise-aware. They don't pause on a promise returned from their callback (which is what happens when you declared the callback as async).


Another Possibility with Parallel Queries

Since the processing of one iteration of the loop is independent from the others, you could run all those database queries in parallel and use Promise.all() to know when they are done. That has one downside that if the array is large, you will be throwing a lot of queries at the database all at once. The upside is that the end-to-end processing time may be less if the database is not overwhelmed by the number of simultaneous queries. Here's how you could do that:

let lor = await Articles.find({ Status: 1, isPublish: true})
    .sort({TotalClapCount: -1})
    .sort({ViewCount: -1})
    .skip(offset)
    .limit(limit);

await Promise.all(lor.map(async data => {
    let hre =  await ArticleBookmarks.find({
        ArticleID: data.ID,
        UserID: ArgsUserID,
        Status: 1
    }).countDocuments();
    data.isBookmark = (hre == 1);
});
return lor;
jfriend00
  • 683,504
  • 96
  • 985
  • 979
0

forEach() will not wait for async operator to be finished. You can use for loop with await inside. However, the downside is other operators have to wait for previous one to be finished. The appropriate solution is to use Promise.all which returns a single Promise that resolves when all of the promises passed as an iterable have resolved. Hope this helps.

  • If the OP wants to sequence their loop (which is what they appear to want to do and might be useful to not overwhelm the database with too many parallel queries), then there is no need for `Promise.all()`. See my answer for how it can be done. – jfriend00 Nov 12 '19 at 07:32
  • I did read your answer before posting my answer. I don't see any reason in author's code to sequence his loop. "...might be useful to not overwhelm the database with too many parallel queries": I agree with you on this. However, author might want to make parallel queries to reduce response time as well. Good thing is that we propose both solution, LOL! – Long Tran thanh Nov 12 '19 at 07:47
  • Your points are good. I added code for a parallel version to my answer and explained the relative advantages and disadvantages vs. sequenced. – jfriend00 Nov 12 '19 at 08:29
-1

try like below

let MainFunction = () => {
    return new Promise(async (resolve, reject) => {
        try {
            let async = require("async");
            let query = {
                Status: 1,
                isPublish: true
            }
            let sortOptions = {
                TotalClapCount: -1,
                ViewCount: -1
            }
            let Data = await Articles.find(query).sort(sortOptions).skip(offset).limit(limit).lean();
            async.eachSeries(Data, async (data, callback) => {
                try {
                    let cquery = {
                        ArticleID: data["ID"],
                        UserID: ArgsUserID, Status: 1
                    };
                    let countedArticles = await ArticleBookmarks.countDocuments(cquery).lean();
                    data.isBookmark = (countedArticles >= 1) ? true : false;
                    callback();
                } catch (error) {
                    callback(error);
                }
            }, async (err) => {
                if (err) reject(err);
                resolve(Data);
            });
        } catch (error) {
            console.error(error);
            reject(error);
        }
    });
}


return await MainFunction();

Note: Please pass your necessary params inside the MainFunction based on necessity.

Ratan Uday Kumar
  • 5,738
  • 6
  • 35
  • 54
  • 1
    You shouldn't be wrapping a new promise around existing promises. That's a promise anti-pattern. As I advised the OP, just switch the `.forEach()` to a regular `for` loop and then `await` will stop the loop. Plus `await Data.forEach()` does nothing useful since `.forEach()` has NO return value. And the `await` inside the `.forEach()` loop doesn't cause the loop to stop. This code does not fix anything. – jfriend00 Nov 12 '19 at 05:36
  • I have updated my solution. it will damn sure will work. – Ratan Uday Kumar Nov 12 '19 at 05:49
  • 1
    Wow. Note to the OP, this is really not the best way to program with promises. When I get to a computer where I can write code, I'll show you a much better way to do it. As I said before, wrapping existing promises in a new manually constructed promise is considered an anti-pattern. And, with proper use of a `for` loop and `async/await`, the `async.series` library function is not necessary at all. – jfriend00 Nov 12 '19 at 06:09
  • Using the async library the questionnaire maybe get benefits from the async library and it will be useful for him in the future for complex functionality the questionnaire can use it smoothly. – Ratan Uday Kumar Nov 12 '19 at 06:13
  • @RatanUdayKumar Thank you, its working fine for the solution above. – ADDC Nov 12 '19 at 06:26
  • @ADDC please upvote my answer if you liked my efforts – Ratan Uday Kumar Nov 12 '19 at 06:29