[project-history] fix callback signature when processing no updates (#26420)

* [project-history] fix tests and cover callback for processing updates

The before setup was hiding that some tests were not doing what the
assertions were expecting.

* [project-history] fix callback signature when processing no updates

GitOrigin-RevId: 4fa14d47b9a1afd998316b0c9024d49760785a47

services/project-history/app/js/UpdatesProcessor.js

@@ -546,7 +546,10 @@ export function _processUpdates(
       }
       if (filteredUpdates.length === 0) {
         // return early if there are no updates to apply
-        return SyncManager.setResyncState(projectId, newSyncState, callback)
+        return SyncManager.setResyncState(projectId, newSyncState, err => {
+          if (err) return callback(err)
+          callback(null, { resyncNeeded: false })
+        })
       }
       // only make request to history service if we have actual updates to process
       _getMostRecentVersionWithDebug(

services/project-history/test/unit/js/UpdatesManager/UpdatesProcessorTests.js

@@ -6,14 +6,14 @@ import * as Errors from '../../../../app/js/Errors.js'
 const MODULE_PATH = '../../../../app/js/UpdatesProcessor.js'
 
 describe('UpdatesProcessor', function () {
-  before(async function () {
+  beforeEach(async function () {
     this.extendLock = sinon.stub()
     this.BlobManager = {
       createBlobsForUpdates: sinon.stub(),
     }
     this.HistoryStoreManager = {
       getMostRecentVersion: sinon.stub(),
-      sendChanges: sinon.stub().yields(null, {}),
+      sendChanges: sinon.stub().yields(null, { resyncNeeded: true }),
     }
     this.LockManager = {
       runWithLock: sinon.spy((key, runner, callback) =>
@@ -318,8 +318,8 @@ describe('UpdatesProcessor', function () {
           this.ol_project_id,
           this.rawUpdates,
           this.extendLock,
-          err => {
-            this.callback(err)
+          (err, flushResponse) => {
+            this.callback(err, flushResponse)
             done()
           }
         )
@@ -385,8 +385,73 @@ describe('UpdatesProcessor', function () {
         )
       })
 
-      it('should call the callback with no error', function () {
-        this.callback.should.have.been.called
+      it('should call the callback with no error and flush response', function () {
+        this.callback.should.have.been.calledWith(null, { resyncNeeded: true })
+      })
+    })
+
+    describe('no updates', function () {
+      beforeEach(function (done) {
+        this.SyncManager.skipUpdatesDuringSync.yields(
+          null,
+          [],
+          this.newSyncState
+        )
+        this.UpdatesProcessor._processUpdates(
+          this.project_id,
+          this.ol_project_id,
+          this.rawUpdates,
+          this.extendLock,
+          (err, flushResponse) => {
+            this.callback(err, flushResponse)
+            done()
+          }
+        )
+      })
+
+      it('should not get the latest version id', function () {
+        this.HistoryStoreManager.getMostRecentVersion.should.not.have.been.calledWith(
+          this.project_id,
+          this.ol_project_id
+        )
+      })
+
+      it('should skip updates when resyncing', function () {
+        this.SyncManager.skipUpdatesDuringSync.should.have.been.calledWith(
+          this.project_id,
+          this.rawUpdates
+        )
+      })
+
+      it('should not expand sync updates', function () {
+        this.SyncManager.expandSyncUpdates.should.not.have.been.called
+      })
+
+      it('should not compress updates', function () {
+        this.UpdateCompressor.compressRawUpdates.should.not.have.been.called
+      })
+
+      it('should not create any blobs for the updates', function () {
+        this.BlobManager.createBlobsForUpdates.should.not.have.been.called
+      })
+
+      it('should not convert the updates into a change requests', function () {
+        this.UpdateTranslator.convertToChanges.should.not.have.been.called
+      })
+
+      it('should not send the change request to the history store', function () {
+        this.HistoryStoreManager.sendChanges.should.not.have.been.called
+      })
+
+      it('should set the sync state', function () {
+        this.SyncManager.setResyncState.should.have.been.calledWith(
+          this.project_id,
+          this.newSyncState
+        )
+      })
+
+      it('should call the callback with fake flush response', function () {
+        this.callback.should.have.been.calledWith(null, { resyncNeeded: false })
       })
     })
 
@@ -415,7 +480,7 @@ describe('UpdatesProcessor', function () {
   })
 
   describe('_skipAlreadyAppliedUpdates', function () {
-    before(function () {
+    beforeEach(function () {
       this.UpdateTranslator.isProjectStructureUpdate.callsFake(
         update => update.version != null
       )
@@ -423,7 +488,7 @@ describe('UpdatesProcessor', function () {
     })
 
     describe('with all doc ops in order', function () {
-      before(function () {
+      beforeEach(function () {
         this.updates = [
           { doc: 'id', v: 1 },
           { doc: 'id', v: 2 },
@@ -443,7 +508,7 @@ describe('UpdatesProcessor', function () {
     })
 
     describe('with all project ops in order', function () {
-      before(function () {
+      beforeEach(function () {
         this.updates = [
           { version: 1 },
           { version: 2 },
@@ -463,7 +528,7 @@ describe('UpdatesProcessor', function () {
     })
 
     describe('with all multiple doc and ops in order', function () {
-      before(function () {
+      beforeEach(function () {
         this.updates = [
           { doc: 'id1', v: 1 },
           { doc: 'id1', v: 2 },
@@ -491,64 +556,47 @@ describe('UpdatesProcessor', function () {
     })
 
     describe('with doc ops out of order', function () {
-      before(function () {
+      beforeEach(function () {
         this.updates = [
           { doc: 'id', v: 1 },
           { doc: 'id', v: 2 },
           { doc: 'id', v: 4 },
           { doc: 'id', v: 3 },
         ]
-        this.skipFn = sinon.spy(
-          this.UpdatesProcessor._mocks,
-          '_skipAlreadyAppliedUpdates'
-        )
-        try {
-          this.updatesToApply =
-            this.UpdatesProcessor._skipAlreadyAppliedUpdates(
-              this.project_id,
-              this.updates,
-              { docs: {} }
-            )
-        } catch (error) {}
-      })
-
-      after(function () {
-        this.skipFn.restore()
       })
 
       it('should throw an exception', function () {
-        this.skipFn.threw('OpsOutOfOrderError').should.equal(true)
+        expect(() => {
+          this.UpdatesProcessor._skipAlreadyAppliedUpdates(
+            this.project_id,
+            this.updates,
+            { docs: {} }
+          )
+        }).to.throw(Errors.OpsOutOfOrderError)
       })
     })
 
     describe('with project ops out of order', function () {
-      before(function () {
+      beforeEach(function () {
+        this.UpdateTranslator.isProjectStructureUpdate.callsFake(
+          update => update.version != null
+        )
         this.updates = [
           { version: 1 },
           { version: 2 },
           { version: 4 },
           { version: 3 },
         ]
-        this.skipFn = sinon.spy(
-          this.UpdatesProcessor._mocks,
-          '_skipAlreadyAppliedUpdates'
-        )
-        try {
-          this.updatesToApply =
-            this.UpdatesProcessor._skipAlreadyAppliedUpdates(
-              this.project_id,
-              this.updates,
-              { docs: {} }
-            )
-        } catch (error) {}
-      })
-
-      after(function () {
-        this.skipFn.restore()
       })
 
       it('should throw an exception', function () {
-        this.skipFn.threw('OpsOutOfOrderError').should.equal(true)
+        expect(() => {
+          this.UpdatesProcessor._skipAlreadyAppliedUpdates(
+            this.project_id,
+            this.updates,
+            { docs: {} }
+          )
+        }).to.throw(Errors.OpsOutOfOrderError)
       })
     })
   })