core/remote: Conflicts use most recent update date (#2053)

When a remote document is renamed with a remote conflict by Sync
instead of Merge, we use a dedicated method, `Remote.resolveRemoteConflict()`.

We were assuming in this method that the current date would always be
the most recent one compared to the last modification date on the
remote Cozy for the document with the given name.

However, there are multiple situations in which this would not be the
case and `cozy-stack` would thus reject the renaming:
- the computer's time is off and late
- the modification date on the remote Cozy was set to a date in the
  future

To make sure the Cozy will not reject the renaming of the remote
document, we fetch the remote modification date with the remote _id
and revision and use the most recent date between the current date and
the remote one.

core/remote/index.js

@@ -406,25 +406,30 @@ class Remote /*:: implements Reader, Writer */ {
     if (results.length > 0) return results[0]
   }
 
-  async resolveRemoteConflict(newMetadata /*: SavedMetadata */) {
+  async resolveRemoteConflict(
+    newMetadata /*: SavedMetadata */
+  ) /*: Promise<void> */ {
     // Find conflicting document on remote Cozy
-    const { _id: remoteId, _rev: remoteRev } = await this.findDocByPath(
-      newMetadata.path
-    )
+    const remoteDoc = await this.findDocByPath(newMetadata.path)
+    if (!remoteDoc) return
+
     // Generate a new name with a conflict suffix for the remote document
     const newName = path.basename(
       conflicts.generateConflictPath(newMetadata.path)
     )
 
     const attrs = {
       name: newName,
-      updated_at: new Date().toISOString()
+      updated_at: timestamp.maxDate(
+        new Date().toISOString(),
+        remoteDoc.updated_at
+      )
     }
     const opts = {
-      ifMatch: remoteRev
+      ifMatch: remoteDoc._rev
     }
 
-    await this.remoteCozy.updateAttributesById(remoteId, attrs, opts)
+    await this.remoteCozy.updateAttributesById(remoteDoc._id, attrs, opts)
   }
 }
 

test/unit/remote/index.js

@@ -19,6 +19,7 @@ const { DirectoryNotFound } = require('../../../core/remote/cozy')
 const { ROOT_DIR_ID, TRASH_DIR_ID } = require('../../../core/remote/constants')
 const { FetchError } = require('../../../core/remote/cozy')
 const timestamp = require('../../../core/utils/timestamp')
+const { CONFLICT_REGEXP } = require('../../../core/utils/conflicts')
 
 const configHelpers = require('../../support/helpers/config')
 const pouchHelpers = require('../../support/helpers/pouch')
@@ -1226,14 +1227,20 @@ describe('remote.Remote', function() {
     })
 
     it('does not swallow destroy errors', async function() {
+      sinon.stub(this.remote.remoteCozy, 'destroyById').rejects('whatever')
+
       const dir = await builders.remoteDir().create()
       const doc = builders
         .metadir()
         .fromRemote(dir)
         .changedSide('local')
         .build()
-      sinon.stub(this.remote.remoteCozy, 'destroyById').rejects('whatever')
-      await should(this.remote.deleteFolderAsync(doc)).be.rejected()
+
+      try {
+        await should(this.remote.deleteFolderAsync(doc)).be.rejected()
+      } finally {
+        this.remote.remoteCozy.destroyById.restore()
+      }
     })
   })
 
@@ -1305,6 +1312,70 @@ describe('remote.Remote', function() {
       await should(this.remote.ping()).be.fulfilledWith(false)
     })
   })
+
+  describe('resolveRemoteConflict', () => {
+    let remoteFile, file
+    beforeEach(async function() {
+      remoteFile = await builders
+        .remoteFile()
+        .name('file.txt')
+        .create()
+      file = await builders
+        .metafile()
+        .fromRemote(remoteFile)
+        .create()
+    })
+
+    it('does not fail if there are no remote documents with the given path', async function() {
+      await this.remote.remoteCozy.destroyById(remoteFile._id)
+
+      await should(this.remote.resolveRemoteConflict(file)).be.fulfilled()
+    })
+
+    it('renames the remote document with a conflict suffix', async function() {
+      await this.remote.resolveRemoteConflict(file)
+      should(await this.remote.remoteCozy.find(remoteFile._id))
+        .have.property('name')
+        .match(CONFLICT_REGEXP)
+    })
+
+    it('fails with a 412 error if file changes on remote Cozy during the call', async function() {
+      // Stub findDocByPath which is called by resolveRemoteConflict so that it
+      // returns the remote document and fakes an update closely following it.
+      sinon.stub(this.remote, 'findDocByPath').callsFake(async () => {
+        await builders
+          .remoteFile(remoteFile)
+          .data('update')
+          .update()
+        return remoteFile
+      })
+
+      try {
+        await should(this.remote.resolveRemoteConflict(file)).be.rejectedWith({
+          name: 'FetchError',
+          status: 412
+        })
+      } finally {
+        this.remote.findDocByPath.restore()
+      }
+    })
+
+    it('uses the most recent modification date between now and the remote date', async function() {
+      // Make sure remote file has modification date more recent than `now`.
+      const oneHour = 3600 * 1000 // milliseconds
+      const moreRecentDate = new Date(Date.now() + oneHour).toISOString()
+      const updatedRemoteFile = await this.remote.remoteCozy.updateAttributesById(
+        remoteFile._id,
+        { updated_at: moreRecentDate }
+      )
+
+      await this.remote.resolveRemoteConflict(file)
+      should(await this.remote.remoteCozy.find(remoteFile._id)).have.property(
+        'updated_at',
+        updatedRemoteFile.updated_at
+      )
+    })
+  })
 })
 
 describe('remote', function() {