From e92a8d7d2ff2b00e899d97021f152e2abf2b4579 Mon Sep 17 00:00:00 2001 From: Erwan Guyader Date: Mon, 15 Mar 2021 11:47:13 +0100 Subject: [PATCH] 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 | 19 ++++++---- test/unit/remote/index.js | 75 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 9 deletions(-) diff --git a/core/remote/index.js b/core/remote/index.js index fe8c14d2c..059c3269f 100644 --- a/core/remote/index.js +++ b/core/remote/index.js @@ -406,11 +406,13 @@ class Remote /*:: implements Reader, Writer */ { if (results.length > 0) return results[0] } - async resolveRemoteConflict(newMetadata /*: SavedMetadata */) { + async resolveRemoteConflict( + newMetadata /*: SavedMetadata */ + ) /*: Promise */ { // 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) @@ -418,13 +420,16 @@ class Remote /*:: implements Reader, Writer */ { 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) } } diff --git a/test/unit/remote/index.js b/test/unit/remote/index.js index 99f9c6236..9ec8f9a74 100644 --- a/test/unit/remote/index.js +++ b/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() {