diff --git a/services/document-updater/app/js/HistoryOTUpdateManager.js b/services/document-updater/app/js/HistoryOTUpdateManager.js index 5a8b92099e..a3298b592c 100644 --- a/services/document-updater/app/js/HistoryOTUpdateManager.js +++ b/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, diff --git a/services/document-updater/app/js/RedisManager.js b/services/document-updater/app/js/RedisManager.js index 1dc20fdf38..3ff7b0fa64 100644 --- a/services/document-updater/app/js/RedisManager.js +++ b/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. diff --git a/services/document-updater/app/js/UpdateManager.js b/services/document-updater/app/js/UpdateManager.js index e5df48575e..5fa5e8dc23 100644 --- a/services/document-updater/app/js/UpdateManager.js +++ b/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) { diff --git a/services/document-updater/test/acceptance/js/ApplyingUpdatesToADocTests.js b/services/document-updater/test/acceptance/js/ApplyingUpdatesToADocTests.js index 855c4f937e..620b1f03a0 100644 --- a/services/document-updater/test/acceptance/js/ApplyingUpdatesToADocTests.js +++ b/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 () { diff --git a/services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js b/services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js index 912707e01d..ace490008a 100644 --- a/services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js +++ b/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 () {