From 2a1fcf6c9a43f75dbc1d788385ce12603c393c50 Mon Sep 17 00:00:00 2001
From: Erwan Guyader <erwan.guyader@cozycloud.cc>
Date: Mon, 23 Dec 2024 17:00:36 +0100
Subject: [PATCH] refactor: Add sync loop start method

  This clarifies what the sync loop does (especially the part about
  waiting for new changes to synchronize) and avoids duplicating code
  in the `start()` and `resume()` methods.
---
 core/sync/index.js            | 42 +++++++++++++++--------------------
 test/support/helpers/index.js |  2 +-
 test/unit/sync/index.js       |  6 ++---
 3 files changed, 22 insertions(+), 28 deletions(-)

diff --git a/core/sync/index.js b/core/sync/index.js
index b1010a81b..cf1428153 100644
--- a/core/sync/index.js
+++ b/core/sync/index.js
@@ -288,14 +288,7 @@ class Sync {
 
     this.lifecycle.end('start')
 
-    try {
-      while (!this.lifecycle.willStop()) {
-        await this.lifecycle.ready()
-        await this.sync()
-      }
-    } catch (err) {
-      await this.fatal(err)
-    }
+    await this.runSyncLoop()
   }
 
   async resume() {
@@ -314,14 +307,7 @@ class Sync {
     this.remote.resume()
     this.lifecycle.end('start')
 
-    try {
-      while (!this.lifecycle.willStop()) {
-        await this.lifecycle.ready()
-        await this.sync()
-      }
-    } catch (err) {
-      await this.fatal(err)
-    }
+    await this.runSyncLoop()
   }
 
   suspend() {
@@ -400,15 +386,23 @@ class Sync {
     return this.stop()
   }
 
-  async sync({
-    manualRun = false
-  } /*: { manualRun?: boolean } */ = {}) /*: Promise<*> */ {
-    let seq = await this.pouch.getLocalSeq()
+  async runSyncLoop() {
+    try {
+      while (!this.lifecycle.willStop()) {
+        await this.lifecycle.ready()
+
+        const seq = await this.pouch.getLocalSeq()
+        const change = await this.waitForChangeAfter(seq)
+        if (change == null) continue
 
-    if (!manualRun) {
-      const change = await this.waitForNewChanges(seq)
-      if (change == null) return
+        await this.sync()
+      }
+    } catch (err) {
+      await this.fatal(err)
     }
+  }
+
+  async sync() /*: Promise<*> */ {
     this.events.emit('sync-start')
     try {
       await this.syncBatch()
@@ -624,7 +618,7 @@ class Sync {
 
   // Wait until a change is emitted by PouchDB into its changesfeed (i.e. we've
   // merged some change on a document).
-  async waitForNewChanges(seq /*: number */) {
+  async waitForChangeAfter(seq /*: number */) {
     log.trace('Waiting for changes since seq', { seq })
     const opts = this.baseChangeOptions(seq)
     opts.live = true
diff --git a/test/support/helpers/index.js b/test/support/helpers/index.js
index 7c9cbfd0b..03e9e8cda 100644
--- a/test/support/helpers/index.js
+++ b/test/support/helpers/index.js
@@ -85,7 +85,7 @@ class TestHelpers {
 
   async syncAll() {
     this._sync.lifecycle.transitionTo('done-start')
-    await this._sync.sync({ manualRun: true })
+    await this._sync.sync()
     // Wait until all potentially blocking changes have been handled
     await this._sync.lifecycle.ready()
     this._sync.lifecycle.transitionTo('done-stop')
diff --git a/test/unit/sync/index.js b/test/unit/sync/index.js
index 11de5834b..b870aa7fa 100644
--- a/test/unit/sync/index.js
+++ b/test/unit/sync/index.js
@@ -98,7 +98,7 @@ describe('Sync', function() {
         events.emit('remote:fatal', err)
       })
       this.remote.stop = sinon.stub().resolves()
-      this.sync.sync = sinon.stub().resolves()
+      this.sync.runSyncLoop = sinon.stub().resolves()
       sinon.spy(this.sync, 'stop')
       sinon.spy(this.sync.events, 'emit')
     })
@@ -108,7 +108,7 @@ describe('Sync', function() {
       await this.sync.started()
       should(this.local.start).have.been.calledOnce()
       should(this.remote.start).have.been.calledOnce()
-      should(this.sync.sync).have.been.called()
+      should(this.sync.runSyncLoop).have.been.called()
     })
 
     context('if local watcher fails to start', () => {
@@ -118,7 +118,7 @@ describe('Sync', function() {
 
       it('does not start replication', async function() {
         await this.sync.start()
-        should(this.sync.sync).not.have.been.called()
+        should(this.sync.runSyncLoop).not.have.been.called()
       })
 
       it('does not start remote watcher', async function() {