0

I'm currently making a web application with React front-end and Firebase back-end. It is an application for a local gym and consists of two parts:

  • A client application for people who train at the local gym
  • A trainer application for trainers of the local gym

The local gym offers programs for companies. So a company takes out a subscription, and employees from the company can train at the local gym and use the client application. It is important that the individual progress of the company employees is being tracked as well as the entire progress (total number of kilograms lost by all the employees of company x together).

In the Firestore collection 'users' every user document has the field bodyweight. Whenever a trainer fills in a progress form after a physical assessment for a specific client, the bodyweight field in the user document of the client gets updated to the new bodyweight.

In Firestore there is another collection 'companies' where every company has a document. My goal is to put the total amount of kilograms lost by the employees of the company in that specific document. So every time a trainer updates the weight of an employee, the company document needs to be updated. I've made a cloud function that listens to updates of a user's document. The function is listed down below:

exports.updateCompanyProgress = functions.firestore
  .document("users/{userID}")
  .onUpdate((change, context) => {

  const previousData = change.before.data();
  const data = change.after.data();

  if (previousData === data) {
    return null;
  }

  const companyRef = admin.firestore.doc(`/companies/${data.company}`);
  const newWeight = data.bodyweight;
  const oldWeight = previousData.bodyweight;
  const lostWeight = oldWeight > newWeight;
  const difference = diff(newWeight, oldWeight);
  const currentWeightLost = companyRef.data().weightLostByAllEmployees;

  if (!newWeight || difference === 0 || !oldWeight) {
    return null;
  } else {
    const newCompanyWeightLoss = calcNewCWL(
      currentWeightLost,
      difference,
      lostWeight
    );
    companyRef.update({ weightLostByAllEmployees: newCompanyWeightLoss });
  }
});

There are two simple functions in the cloud function above:

const diff = (a, b) => (a > b ? a - b : b - a);

const calcNewCWL = (currentWeightLost, difference, lostWeight) => {
  if (!lostWeight) {
    return currentWeightLost - difference;
  }
  return currentWeightLost + difference;
};

I've deployed the cloud function to Firebase to test it, but I can't get it to work. The function triggers whenever the user document is updated, but it doesn't update the company document with the new weightLostByAllEmployees value. It is the first time for me using Firebase cloud functions, so big change it is some sort of rookie mistake.

halfer
  • 19,824
  • 17
  • 99
  • 186

2 Answers2

1

Your current solution has some bugs in it that we can squash.

Always false equality check

You use the following equality check to determine if the data has not changed:

if (previousData === data) {
  return null;
}

This will always be false as the objects returned by change.before.data() and change.after.data() will always be different instances, even if they contain the same data.

Company changes are never handled

While this could be a rare, maybe impossible event, if a user's company was changed, you should remove their weight from the total of the original company and add it to the new company.

In a similar vein, when a employee leaves a company or deletes their account, you should remove their weight from the total in a onDelete handler.

Handling floating-point sums

In case you didn't know, floating point arithmetic has some minor quirks. Take for example the sum, 0.1 + 0.2, to a human, the answer is 0.3, but to JavaScript and many languages, the answer is 0.30000000000000004. See this question & thread for more information.

Rather than store your weight in the database as a floating point number, consider storing it as an integer. As weight is often not a whole number (e.g. 9.81kg), you should store this value multiplied by 100 (for 2 significant figures) and then round it to the nearest integer. Then when you display it, you either divide it by 100 or splice in the appropriate decimal symbol.

const v = 1201;
console.log(v/100); // -> 12.01

const vString = String(v);
console.log(vString.slice(0,-2) + "." + vString.slice(-2) + "kg"); // -> "12.01kg"

So for the sum, 0.1 + 0.2, you would scale it up to 10 + 20, with a result of 30.

console.log(0.1 + 0.2); // -> 0.30000000000000004
console.log((0.1*100 + 0.2*100)/100); // -> 0.3

But this strategy on its own isn't bullet proof because some multiplications still end up with these errors, like 0.14*100 = 14.000000000000002 and 0.29*100 = 28.999999999999996. To weed these out, we round the multiplied value.

console.log(0.01 + 0.14); // -> 0.15000000000000002
console.log((0.01*100 + 0.14*100)/100); // -> 0.15000000000000002
console.log((Math.round(0.01*100) + Math.round(0.14*100))/100) // -> 0.15

You can compare these using:

const arr = Array.from({length: 100}).map((_,i)=>i/100);

console.table(arr.map((a) => arr.map((b) => a + b)));
console.table(arr.map((a) => arr.map((b) => (a*100 + b*100)/100)));
console.table(arr.map((a) => arr.map((b) => (Math.round(a*100) + Math.round(b*100))/100)));

Therefore we can end up with these helper functions:

function sumFloats(a,b) {
  return (Math.round(a * 100) + Math.round(b * 100)) / 100;
}

function sumFloatsForStorage(a,b) {
  return (Math.round(a * 100) + Math.round(b * 100));
}

The main benefit of handling the weights this way is that you can now use FieldValue#increment() instead of a full blown transaction to shortcut updating the value. In the rare case that two users from the same company have an update collision, you can either retry the increment or fall back to the full transaction.

Inefficient data parsing

In your current code, you make use of .data() on the before and after states to get the data you need for your function. However, because you are pulling the user's entire document, you end up parsing all the fields in the document instead of just what you need - the bodyweight and company fields. You can do this using DocumentSnapshot#get(fieldName).

const afterData = change.after.data(); // parses everything - username, email, etc.
const { bodyweight, company } = afterData;

in comparison to:

const bodyweight = change.after.get("bodyweight"); // parses only "bodyweight"
const company = change.after.get("company"); // parses only "company"

Redundant math

For some reason you are calculating an absolute value of the difference between the weights, storing the sign of difference as a boolean and then using them together to apply the change back to the total weight lost.

The following lines:

const previousData = change.before.data();
const data = change.after.data();

const newWeight = data.bodyweight;
const oldWeight = previousData.bodyweight;
const lostWeight = oldWeight > newWeight;
const difference = diff(newWeight, oldWeight);
const currentWeightLost = companyRef.data().weightLostByAllEmployees;

const calcNewCWL = (currentWeightLost, difference, lostWeight) => {
  if (!lostWeight) {
    return currentWeightLost - difference;
  }
  return currentWeightLost + difference;
};

const newWeightLost = calcNewCWL(currentWeightLost, difference, lostWeight);

could be replaced with just:

const newWeight = change.after.get("bodyweight");
const oldWeight = change.before.get("bodyweight");
const deltaWeight = newWeight - oldWeight;
const currentWeightLost = companyRef.get("weightLostByAllEmployees") || 0;

const newWeightLost = currentWeightLost + deltaWeight;

Rolling it all together

exports.updateCompanyProgress = functions.firestore
  .document("users/{userID}")
  .onUpdate(async (change, context) => {

  // "bodyweight" is the weight scaled up by 100
  // i.e. "9.81kg" is stored as 981
  const oldHundWeight = change.before.get("bodyweight") || 0;
  const newHundWeight = change.after.get("bodyweight") || 0;
  
  const oldCompany = change.before.get("company");
  const newCompany = change.after.get("company");
  
  const db = admin.firestore();
  
  if (oldCompany === newCompany) {
    // company unchanged
    const deltaHundWeight = newHundWeight - oldHundWeight;
    
    if (deltaHundWeight === 0) {
      return null; // no action needed
    }
    
    const companyRef = db.doc(`/companies/${newCompany}`);
    
    await companyRef.update({
      weightLostByAllEmployees: admin.firestore.FieldValue.increment(deltaHundWeight)
    });
  } else {
    // company was changed
    
    const batch = db.batch();
    
    const oldCompanyRef = db.doc(`/companies/${oldCompany}`);
    const newCompanyRef = db.doc(`/companies/${newCompany}`);
    
    // remove weight from old company
    batch.update(oldCompanyRef, {
      weightLostByAllEmployees: admin.firestore.FieldValue.increment(-oldHundWeight)
    });
    
    // add weight to new company
    batch.update(newCompanyRef, {
      weightLostByAllEmployees: admin.firestore.FieldValue.increment(newHundWeight)
    });
    
    // apply changes
    await db.batch();
  }
});

With transaction fallbacks

In the rare case where you get a write collision, this variant falls back to a traditional transaction to reattempt the change.

/**
 * Increments weightLostByAllEmployees in all documents atomically
 * using a transaction.
 *
 * `arrayOfCompanyRefToDeltaWeightPairs` is an array of company-increment pairs.
 */
function transactionIncrementWeightLostByAllEmployees(db, arrayOfCompanyRefToDeltaWeightPairs) {
  return db.runTransaction((transaction) => {
    // get all needed documents, then add the update for each to the transaction
    return Promise
      .all( 
        arrayOfCompanyRefToDeltaWeightPairs
          .map(([companyRef, deltaWeight]) => {
            return transaction.get(companyRef)
              .then((companyDocSnapshot) => [companyRef, deltaWeight, companyDocSnapshot])
          })
      )
      .then((arrayOfRefWeightSnapshotGroups) => {
        arrayOfRefWeightSnapshotGroups.forEach(([companyRef, deltaWeight, companyDocSnapshot]) => {
          const currentValue = companyDocSnapshot.get("weightLostByAllEmployees") || 0;
          transaction.update(companyRef, {
            weightLostByAllEmployees: currentValue + deltaWeight
          })
        });
      });
  });
}

exports.updateCompanyProgress = functions.firestore
  .document("users/{userID}")
  .onUpdate(async (change, context) => {

  // "bodyweight" is the weight scaled up by 100
  // i.e. "9.81kg" is stored as 981
  const oldHundWeight = change.before.get("bodyweight") || 0;
  const newHundWeight = change.after.get("bodyweight") || 0;
  
  const oldCompany = change.before.get("company");
  const newCompany = change.after.get("company");
  
  const db = admin.firestore();
  
  if (oldCompany === newCompany) {
    // company unchanged
    const deltaHundWeight = newHundWeight - oldHundWeight;
    
    if (deltaHundWeight === 0) {
      return null; // no action needed
    }
    
    const companyRef = db.doc(`/companies/${newCompany}`);
    
    await companyRef
      .update({
        weightLostByAllEmployees: admin.firestore.FieldValue.increment(deltaHundWeight)
      })
      .catch((error) => {
        // if an unexpected error, just rethrow it
        if (error.code !== "resource-exhausted")
          throw error;
      
        // encountered write conflict, fall back to transaction
        return transactionIncrementWeightLostByAllEmployees(db, [
          [companyRef, deltaHundWeight]
        ]);
      });
  } else {
    // company was changed
    
    const batch = db.batch();
    
    const oldCompanyRef = db.doc(`/companies/${oldCompany}`);
    const newCompanyRef = db.doc(`/companies/${newCompany}`);
    
    // remove weight from old company
    batch.update(oldCompanyRef, {
      weightLostByAllEmployees: admin.firestore.FieldValue.increment(-oldHundWeight)
    });
    
    // add weight to new company
    batch.update(newCompanyRef, {
      weightLostByAllEmployees: admin.firestore.FieldValue.increment(newHundWeight)
    });
    
    // apply changes
    await db.batch()
      .catch((error) => {
        // if an unexpected error, just rethrow it
        if (error.code !== "resource-exhausted")
          throw error;
      
        // encountered write conflict, fall back to transaction
        return transactionIncrementWeightLostByAllEmployees(db, [
          [oldCompanyRef, -oldHundWeight],
          [newCompanyRef, newHundWeight]
        ]);
      });
  }
});
samthecodingman
  • 23,122
  • 4
  • 30
  • 54
  • Thanks for the most detailed answer I've seen yet. I really learned something from this, and the cloud function now works flawlessly. – Sjoerd Vink May 29 '21 at 09:17
0

There are several points to adapt in your Cloud Function:

  • Do admin.firestore() instead of admin.firestore
  • You cannot get the data of the Company document by doing companyRef.data(). You must call the asynchronous get() method.
  • Use a Transaction when updating the Company document and return the promise returned by this transaction (see here for more details on this key aspect).

So the following code should do the trick.

Note that since we use a Transaction, we actually don't implement the recommendation of the second bullet point above. We use transaction.get(companyRef) instead.

exports.updateCompanyProgress = functions.firestore
    .document("users/{userID}")
    .onUpdate((change, context) => {

        const previousData = change.before.data();
        const data = change.after.data();

        if (previousData === data) {
            return null;
        }

        // You should do admin.firestore() instead of admin.firestore
        const companyRef = admin.firestore().doc(`/companies/${data.company}`);


        const newWeight = data.bodyweight;
        const oldWeight = previousData.bodyweight;
        const lostWeight = oldWeight > newWeight;
        const difference = diff(newWeight, oldWeight);

        if (!newWeight || difference === 0 || !oldWeight) {
            return null;
        } else {
            return admin.firestore().runTransaction((transaction) => {

                return transaction.get(companyRef).then((compDoc) => {
                    if (!compDoc.exists) {
                        throw "Document does not exist!";
                    }
                    const currentWeightLost = compDoc.data().weightLostByAllEmployees;
                    const newCompanyWeightLoss = calcNewCWL(
                        currentWeightLost,
                        difference,
                        lostWeight
                    );

                    transaction.update(companyRef, { weightLostByAllEmployees: newCompanyWeightLoss });
                });
            })
        }
    });
Renaud Tarnec
  • 79,263
  • 10
  • 95
  • 121