Florent Pausé
Florent Pausé

Reputation: 11

My firebase function update document takes MINUTES to execute the first time

I am developping an app to order food online. As backend service I am using firestore to store the data and files. The user can order dishes and there are limited stocks. So every time a user order a dish and create a basket I update the stock of the corresponding ordered dishes. I am using a firebase function in order to perform this action. To be honest it is the first I am creating firebase function. Into the Basket object, there is a list of ordered Dishes with the corresponding database DishID. When the basket is created, I go through the DishID list and I update the Quantity in the firestore database. On my local emulator it works perfectly and very fast. But online it takes minutes to perform the first update. I can deal with some seconds. Even if it takes a few seconds (like for cold restart) it's okay. But sometimes it can take 3 minutes and someone else can order a dish during this time.

Here is my code:

//Update the dishes quantities when a basket is created
exports.updateDishesQuantity = functions.firestore.document('/Baskets/{documentId}').onCreate(async (snap, context) => {

      try{
        //Get the created basket
        const originalBasket = snap.data();

        originalBasket.OrderedDishes.forEach(async dish => {
          const doc = await db.collection('Dishes').doc(dish.DishID);
          console.log('Doc created');

          return docRef = doc.get()
          .then((result) =>{
            console.log('DocRef created');
            if(result.exists){
              console.log('Result exists');
              const dishAvailableOnDataBase = result.data().Available;
              console.log('Data created');
              const newQuantity = { Available: Math.max(dishAvailableOnDataBase - dish.Quantity, 0)};
              console.log('Online doc updated');
              return result.ref.set(newQuantity, { merge: true });
            }else{
              console.log("doc doesnt exist");
            }
            
          })
          .catch(error =>{
            console.log(error);
            return null;
          });       
        });

      }catch(error){
        console.log(error);
      }

});

I have a couple of logs output to debug the outputs on the server. It's the doc.get() function that takes 2 minutes to execute as you can see on the logger below: Firebase logger

Thanks for your help,

Upvotes: 0

Views: 510

Answers (2)

Florent Pausé
Florent Pausé

Reputation: 11

Thansk for your help. I just edited a little bit your code to make it work. I post my edited code. Thanks a lot, now it takes just 4 seconds to update the quantities. Kid regards

//Update the dishes quantities when a basket is created
exports.updateDishesQuantity = functions.firestore.document('/Baskets/{documentId}').onCreate(async (snap, context) => {

  try {
      //Get the created basket
      const originalBasket = snap.data();

      const promises = [];
      const quantities = [];
      originalBasket.OrderedDishes.forEach(dish => {
          promises.push(db.collection('Dishes').doc(dish.DishID).get());
          quantities.push(dish.Quantity);
      });
      const docSnapshotsArray = await Promise.all(promises);
      console.log("Promises", promises);

      const promises1 = [];
      var i = 0;
      docSnapshotsArray.forEach(result => {
          if (result.exists) {
              const dishAvailableOnDataBase = result.data().Available;
              const newQuantity = { Available: Math.max(dishAvailableOnDataBase - quantities[i], 0) };
              promises1.push(result.ref.set(newQuantity, { merge: true }));
          }
          i++;
      })

      return Promise.all(promises1)

  } catch (error) {
      console.log(error);
      return null;
  }

});

Upvotes: 1

Renaud Tarnec
Renaud Tarnec

Reputation: 83191

You should not use async/await within a forEach() loop, see "JavaScript: async/await with forEach()" and "Using async/await with a forEach loop".

And since your code executes, in parallel, a variable number of calls to the asynchronous Firebase get() and set() methods, you should use Promise.all().

You should refactor your Cloud Function along the following lines:

//Update the dishes quantities when a basket is created
exports.updateDishesQuantity = functions.firestore.document('/Baskets/{documentId}').onCreate(async (snap, context) => {

    try {
        //Get the created basket
        const originalBasket = snap.data();

        const promises = [];
        originalBasket.OrderedDishes.forEach(dish => {
            promises.push(db.collection('Dishes').doc(dish.DishID).get());
        });
        const docSnapshotsArray = await Promise.all(promises);

        const promises1 = [];
        docSnapshotsArray.forEach(snap => {
            if (result.exists) {
                const dishAvailableOnDataBase = result.data().Available;
                const newQuantity = { Available: Math.max(dishAvailableOnDataBase - dish.Quantity, 0) };
                promises1.push(result.ref.set(newQuantity, { merge: true }));
            }
        })

        return Promise.all(promises1)

    } catch (error) {
        console.log(error);
        return null;
    }

});

Note that instead of looping and calling push() you could use the map() method for a much concise code. However, for SO answers, I like the clarity brought by creating an empty array, populating it with a forEach() loop and passing it to Promise.all()...

Also note that since you are updating quantities in a basket you may need to use a Transaction.

Upvotes: 0

Related Questions