Record project notification timestamp in Redis on applyUpdate (#29509)

* Record project notification timestamp in Redis on applyUpdate

* Refactor project notification timestamp recording in Redis

* Remove tests which are catching errors that are no longer used

GitOrigin-RevId: faa7a97fa929941cade5d62d2d680c6c3f34cdc8

services/document-updater/app/js/HistoryOTUpdateManager.js

@@ -118,6 +118,11 @@ async function tryApplyUpdate(projectId, docId, update, profiler) {
       // Just record the error here and acknowledge the write-op.
       Metrics.inc('history-queue-error')
     }
+    await RedisManager.promises.recordProjectNotificationTimestamp(
+      projectId,
+      update.meta.ts
+    )
+    profiler.log('recordProjectNotificationTimestamp')
   }
   RealTimeRedisManager.sendData({
     project_id: projectId,

services/document-updater/app/js/RedisManager.js

@@ -567,6 +567,18 @@ const RedisManager = {
     }
   },
 
+  /*
+   * Record the timestamp of the update operation (if it doesn't already exist).
+   * This is used for email notifications
+   */
+  async recordProjectNotificationTimestamp(projectId, timestamp) {
+    await rclient.set(
+      keys.projectNotificationTimestamp({ project_id: projectId }),
+      timestamp,
+      'NX'
+    )
+  },
+
   async blockProject(projectId) {
     // Make sure that this MULTI operation only operates on project
     // specific keys, i.e. keys that have the project id in curly braces.

services/document-updater/app/js/UpdateManager.js

@@ -195,6 +195,12 @@ const UpdateManager = {
           // Just record the error here and acknowledge the write-op.
           Metrics.inc('history-queue-error')
         }
+        const timestamp = update.meta?.ts || Date.now()
+        await RedisManager.promises.recordProjectNotificationTimestamp(
+          projectId,
+          timestamp
+        )
+        profile.log('recordProjectNotificationTimestamp')
       }
 
       if (rangesWereCollapsed) {

services/document-updater/test/acceptance/js/ApplyingUpdatesToADocTests.js

@@ -113,6 +113,15 @@ describe('Applying updates to a doc', function () {
       body.should.deep.equal({})
     })
 
+    it('should set the project notification timestamp in Redis', async function () {
+      const timestamp = await rclientDU.get(
+        Keys.projectNotificationTimestamp({ project_id: this.project_id })
+      )
+      expect(timestamp).to.exist
+      const timestampValue = parseInt(timestamp, 10)
+      timestampValue.should.be.within(this.startTime, Date.now())
+    })
+
     describe('when sending another update', function () {
       beforeEach(async function () {
         this.timeout(10000)
@@ -158,6 +167,16 @@ describe('Applying updates to a doc', function () {
           await DocUpdaterClient.getProjectLastUpdatedAt(this.project_id)
         lastUpdatedAt.should.be.within(this.secondStartTime, Date.now())
       })
+
+      it('should not change the project notification timestamp', async function () {
+        const timestamp = await rclientDU.get(
+          Keys.projectNotificationTimestamp({ project_id: this.project_id })
+        )
+        expect(timestamp).to.exist
+        const timestampValue = parseInt(timestamp, 10)
+        // Should still be within the first update time range, not the second
+        timestampValue.should.be.within(this.startTime, this.secondStartTime)
+      })
     })
 
     describe('when another client is sending a concurrent update', function () {
@@ -275,6 +294,15 @@ describe('Applying updates to a doc', function () {
       body.should.deep.equal({})
     })
 
+    it('should set the project notification timestamp in Redis', async function () {
+      const timestamp = await rclientDU.get(
+        Keys.projectNotificationTimestamp({ project_id: this.project_id })
+      )
+      expect(timestamp).to.exist
+      const timestampValue = parseInt(timestamp, 10)
+      timestampValue.should.be.within(this.startTime, Date.now())
+    })
+
     describe('when sending another update', function () {
       beforeEach(async function () {
         this.timeout(10000)
@@ -325,6 +353,16 @@ describe('Applying updates to a doc', function () {
           await DocUpdaterClient.getProjectLastUpdatedAt(this.project_id)
         lastUpdatedAt.should.be.within(this.secondStartTime, Date.now())
       })
+
+      it('should not change the project notification timestamp', async function () {
+        const timestamp = await rclientDU.get(
+          Keys.projectNotificationTimestamp({ project_id: this.project_id })
+        )
+        expect(timestamp).to.exist
+        const timestampValue = parseInt(timestamp, 10)
+        // Should still be within the first update time range, not the second
+        timestampValue.should.be.within(this.startTime, this.secondStartTime)
+      })
     })
 
     describe('when another client is sending a concurrent update', function () {

services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js

@@ -35,6 +35,7 @@ describe('UpdateManager', function () {
       promises: {
         setDocument: sinon.stub().resolves(),
         updateDocument: sinon.stub(),
+        recordProjectNotificationTimestamp: sinon.stub().resolves(),
       },
     }
 
@@ -416,6 +417,55 @@ describe('UpdateManager', function () {
           this.historyUpdates.length
         )
       })
+
+      it('should record the project notification timestamp', function () {
+        this.RedisManager.promises.recordProjectNotificationTimestamp.should.have.been.calledWith(
+          this.project_id,
+          sinon.match.number
+        )
+      })
+    })
+
+    describe('when update has a timestamp in meta', function () {
+      beforeEach(async function () {
+        this.timestamp = 1234567890
+        this.update = {
+          op: [{ p: 42, i: 'foo' }],
+          meta: { ...this.updateMeta, ts: this.timestamp },
+        }
+        await this.UpdateManager.promises.applyUpdate(
+          this.project_id,
+          this.doc_id,
+          this.update
+        )
+      })
+
+      it('should record the project notification timestamp with the provided timestamp', function () {
+        this.RedisManager.promises.recordProjectNotificationTimestamp.should.have.been.calledWith(
+          this.project_id,
+          this.timestamp
+        )
+      })
+    })
+
+    describe('when there are no history updates', function () {
+      beforeEach(async function () {
+        this.RangesManager.applyUpdate.returns({
+          newRanges: this.updated_ranges,
+          rangesWereCollapsed: false,
+          historyUpdates: [],
+        })
+        await this.UpdateManager.promises.applyUpdate(
+          this.project_id,
+          this.doc_id,
+          this.update
+        )
+      })
+
+      it('should not record the project notification timestamp', function () {
+        this.RedisManager.promises.recordProjectNotificationTimestamp.should.not
+          .have.been.called
+      })
     })
 
     describe('with UTF-16 surrogate pairs in the update', function () {