perepm
perepm

Reputation: 990

Is this the expected behavior of firestore increments inside transactions?

EDIT: Turns out there is a bug in my code and nothing wrong with the SDK. There is no problem in using two or more atomic increment operations on the same document in the same transaction. For more context read the accepted answer and its comments.


I'm using firestore to count how many users are there in my users collection depending on their childrenSituation field value. This field can be HAS_KIDS, DOESNT_HAVE_KIDS or PERIODICALLY_HAS_KIDS.

To do so, I have a data document in a collection called aggregated. This document has three numeric fields which I increase or decrease whenever I create or delete a user by using an atomic increase. But when it comes to updating a user I have to update two numeric values in the same document in the same transaction, which results in setting one of the fields to -1 instead of reducing it in one unit, and not setting the other to any value.

I do this with the increment FieldValue like so:

async updateUser(user: User, userChildrenSituationBeforeUpdate: ChildrenSituationEnum): Promise<void> {
    return await this._firestore.runTransaction(async (t) => {
      const userDocSnap = await t.get(this._usersCollection.doc(user.id));
      const aggregatedDataDocSnap = await t.get(this._agregateDataCollection.doc('data'));

      t.set(userDocSnap.ref, this._serializer.serializeUser(user), {merge: true});
      t.update(aggregatedDataDocSnap.ref, {
        [user.childrenSituation]: this._admin.firestore.FieldValue.increment(1),
        [userChildrenSituationBeforeUpdate]: this._admin.firestore.FieldValue.increment(-1),
      });
    });
  }

I pass the updated user and the previous childrenSituation to this method [I did it to save a read, but I see now that it's unnecessary as I'm fetching the user doc from the DB]. To update the user, I "serialize" it because Firestore can't take class instances directly and update the document. For the aggregated document, I update both fields using an atomic increase: one is positive and the other is negative.

The expected behavior

In my units tests, I create a user in the pretest and so this is the content of the aggregated/data document before calling the updateUser method:

{
    DOESNT_HAVE_KIDS: 1,
}

Then updateUser is called and I expected the contents of aggregated/data to be:

{
    HAS_KIDS: 1,
    DOESNT_HAVE_KIDS: 0
}

What actually happened

With the same pretest conditions I mentioned, the contents of aggregated/data after executing updateUser are:

{
    DOESNT_HAVE_KIDS: -1
}

Is this the expected behavior for Firestore, or should I open an issue in github? If Firestore is not compatible with having more than one atomic increase in the same write operation, at least it should set it to 0. Right?

Upvotes: 1

Views: 682

Answers (1)

Renaud Tarnec
Renaud Tarnec

Reputation: 83093

It's difficult to exactly understand what happens with your code because we cannot reproduce the exact same situation (we only see one function).

However, I've done a test with a simple HTML page, JavaScript and the JS SDK (not TypeScript and not the Admin SDK) and it seems to works correctly:

  • Firestore Transaction can execute several writes in one atomic operation, as explained in the doc;
  • If the field does not exists in the aggregated/data document it is correctly created with the correct value (i.e. 1 in case of increment on a non-existing field or -1 in case of decrement on a non-existing field).

You can open the following HTML page locally in a browser and test it and find what to potentially change in your code. I've just wrote dummy date to the user doc as I don't really understand what you do with this._serializer.serializeUser(user). You need to adapt the config object to your Firebase project.

Note that you don't need to fetch the docs in order to get their DocumentReference: So no need to do const userDocSnap = await t.get(...) and then userDocSnap.ref. Just do t.set(this._usersCollection.doc(user.id), this._serializer.serializeUser(user), {merge: true}); since you already know the DocumentReference of this doc. Same for aggregatedDataDocSnap: no need to fetch the doc.

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Test SO</title>

    <script src="https://www.gstatic.com/firebasejs/8.2.5/firebase-app.js"></script>
    <script src="https://www.gstatic.com/firebasejs/8.2.5/firebase-firestore.js"></script>

    <script>
      // Initialize Firebase
      var config = {
        projectId: '......',
      };

      firebase.initializeApp(config);
      const db = firebase.firestore();

      async function updateUser(user, userChildrenSituationBeforeUpdate) {
        return await db.runTransaction(async (t) => {
          t.set(
            db.collection('users').doc(user.id),
            { foo: 'bar' },
            { merge: true }
          );
          t.update(db.collection('aggregated').doc('data'), {
            [user.childrenSituation]: firebase.firestore.FieldValue.increment(
              1
            ),
            [userChildrenSituationBeforeUpdate]: firebase.firestore.FieldValue.increment(
              -1
            ),
          });
        });
      }

      updateUser({ id: '2', childrenSituation: 'HAS_KIDS' }, 'DOESNT_HAVE_KIDS')
        .then(() => {
          console.log('OK');
        })
        .catch((error) => {
          console.log(error);
        });
    </script>
  </head>
  <body></body>
</html>

Upvotes: 4

Related Questions