Question 209121 in Javascript Question posted in
A very good W3school tutorial can be found here.

Javascript – Is this the right use of Promises

I have been exploring Promises for a while in JavaScript and thought of getting better at it.

I made a small blog app in Node. The scenario is whenever a user is deleted I want to delete all the blogs created by the user and also remove the user from the likes of the blog.

Below is the code for it.

        const user = req.user;

        console.log(user);

        // Finding the blogs created by the user and creating an array of promises
        const createdBlogs = await Blog.find({creator: user._id});
        const createdBlogsPromises = createdBlogs.map(async blog => await Blog.findByIdAndDelete(blog._id));
        
        // Finding the blogs liked by the user and creating an array of promises
        // removeUser is an instance method in blogSchema
        const likedBlogs = await Blog.find({likes: {$in: user._id}});
        const likedBlogsPromises = likedBlogs.map(async blog => await blog.removeUser(user));

        // Using all method for parallel execution of promises
        const promiseResponse = await Promise.all([...createdBlogsPromises, ...likedBlogsPromises]);

        console.log(promiseResponse);
        
        await User.deleteOne({_id: user._id});

        res.status(204).json({message: 'Successfully deleted!!'});

I am using Promise.all() for this. Is this the right way to use Promise.all() or is there an alternative and easy way to perform this ?

Tags:

3

Answers


  1. 0

    That’s about right. You can get more parallelism though if you wait to await. The way you did it, it doesn’t start fetching the liked blogs until after it gets the created blogs. You can start both at the same time.

        const user = req.user;

    console.log(user);

    // Finding the blogs created by the user and creating an array of promises
    const createdBlogsPromises = Blog.find({creator: user._id}).then(
        blogs => blogs.map(blog => Blog.findByIdAndDelete(blog._id)));

    // Finding the blogs liked by the user and creating an array of promises
    // removeUser is an instance method in blogSchema
    const likedBlogsPromises = Blog.find({likes: {$in: user._id}}).then(
        blogs => blogs.map(blog => blog.removeUser(user)));

    // Using all method for parallel execution of promises
    const promiseResponse = await Promise.all([...createdBlogsPromises, ...likedBlogsPromises]);

    console.log(promiseResponse);
    
    await User.deleteOne({_id: user._id});

    res.status(204).json({message: 'Successfully deleted!!'});
    Login or Signup to reply.
  2. 0

    I would suggest you to wrappe all your above code in an async function which should be triggered inside your delete function everytime a user is deleted.

    Example:

    //your async function goes here
const clearUserData = async (userId) =>{

    const user = req.body.user;

    console.log(user);

    // Finding the blogs created by the user and creating an array of promises
    const createdBlogsPromises = Blog.find({creator: user._id}).then(
        blogs => blogs.map(blog => Blog.findByIdAndDelete(blog._id)));

    // Finding the blogs liked by the user and creating an array of promises
    // removeUser is an instance method in blogSchema
    const likedBlogsPromises = Blog.find({likes: {$in: user._id}}).then(
        blogs => blogs.map(blog => blog.removeUser(user)));
....

};

//your delete function
const deleteUser = async ()=>{
   //Some code here..... 
   clearUserData();
};
    Login or Signup to reply.
  3. 0

    This is about right, but there are two problems:

    • Instead of calling Blog.findByIdAndDelete(blog._id) and blog.removeUser(user) (which I supposed actually does remove the likes?) many times, the blog repository service should ideally have methods like Blog.deleteAll(blogIds) where you can pass an array of elements to operate on. This would be significantly faster if supported by the underlying storage layer (database?) as a single command.

    • You are constructing the createdBlogsPromises, but then before waiting for them you first do another await Blog.find(…). If there’s an error in both of those, only the awaited promise rejection can be handled by the caller (e.g. with try/catch), but the other error will result in an unhandled promise rejection that crashes your server. See Waiting for more than one concurrent await operation and Any difference between await Promise.all() and multiple await? for details.
      Notice it’s not the Promsie.all that "executes" the promises (as indicated in your comment), the asynchronous code starts running when you call it. Promises are just a mechanism to wait for the results.

      You can fix this by immediately awaiting the array of promises:

      const createdBlogs = await Blog.find({creator: user._id});
await Promise.all(createdBlogs.map(blog => Blog.findByIdAndDelete(blog._id)));

const likedBlogs = await Blog.find({likes: {$in: user._id}});
await Promise.all(likedBlogs.map(blog => blog.removeUser(user)));

await User.deleteOne({_id: user._id});

      However, this means everything runs sequentially now, which you didn’t intend. (Also, even in your original code createdBlogs and likedBlogs were found sequentially, which is not necessary). You can refactor this for full concurrency as

      async function deleteCreatedBlogs(user) {
  const createdBlogs = await Blog.find({creator: user._id});
  await Promise.all(createdBlogs.map(blog => Blog.findByIdAndDelete(blog._id)));
}

async function removeLikes(user) {
  const likedBlogs = await Blog.find({likes: {$in: user._id}});
  await Promise.all(likedBlogs.map(blog => blog.removeUser(user)));
}

async function deleteUser(user) {
  await User.deleteOne({_id: user._id});
}

      then in your request handler do only

      await Promise.all([
  deleteCreatedBlogs(user),
  removeLikes(user),
  deleteUser(user),
]);

      or alternatively, using .then() instead of the helper functions,

      await Promise.all([
  Blog.find({creator: user._id}).then(createdBlogs => 
    Promise.all(createdBlogs.map(blog => Blog.findByIdAndDelete(blog._id)))
  ),
  Blog.find({likes: {$in: user._id}}).then(likedBlogs =>
    Promise.all(likedBlogs.map(blog => blog.removeUser(user)))
  ),
  User.deleteOne({_id: user._id}),
]);
    Login or Signup to reply.
Please signup or login to give your own answer.

Back To Top
Search