From e6817b90973f011497c4efbe32eea3bdde454e2c Mon Sep 17 00:00:00 2001 From: Erwan Guyader Date: Thu, 28 Apr 2022 13:56:18 +0200 Subject: [PATCH] core/migrations: Use non-throwing remote methods (#2229) In the latest migration, we make requests to the remote Cozy to fetch the latest version of known directories from their _id. However, these known directories can have been deleted on the Cozy. This was expected and the migration can deal with missing directories but the method we were using the fetch the directories would throw on missing documents instead of returning the expected `null` value. This leads to failing migrations. To avoid this, we created another method returning `null` instead of throwing when the expected remote directory is missing and we'll use it in the migration. --- core/migrations/migrations.js | 4 +- core/remote/cozy.js | 20 +++++++--- test/unit/remote/cozy.js | 74 +++++++++++++++++++++++++++++------ 3 files changed, 78 insertions(+), 20 deletions(-) diff --git a/core/migrations/migrations.js b/core/migrations/migrations.js index b41f40da7..df88d7370 100644 --- a/core/migrations/migrations.js +++ b/core/migrations/migrations.js @@ -356,7 +356,7 @@ module.exports = ([ return Promise.map( docs, async doc => { - const remoteDir = await remote.remoteCozy.findDir(doc.remote._id) + const remoteDir = await remote.remoteCozy.findDirMaybe(doc.remote._id) if (remoteDir != null) { if ( @@ -385,7 +385,7 @@ module.exports = ([ const { overwrite } = doc if (overwrite != null && overwrite.remote != null) { - const remoteOverwritten = await remote.remoteCozy.findDir( + const remoteOverwritten = await remote.remoteCozy.findDirMaybe( overwrite.remote._id ) diff --git a/core/remote/cozy.js b/core/remote/cozy.js index b107a51fb..9a988da91 100644 --- a/core/remote/cozy.js +++ b/core/remote/cozy.js @@ -359,6 +359,17 @@ class RemoteCozy { return this.toRemoteDoc(await this.client.files.statById(id)) } + async findMaybe( + id /*: string */ + ) /*: Promise */ { + try { + return await this.find(id) + } catch (err) { + if (err.status === 404) return null + else throw err + } + } + async findDir(id /*: string */) /*: Promise */ { const remoteDir = await this.client.files.statById(id) const doc = await this.toRemoteDoc(remoteDir) @@ -368,13 +379,12 @@ class RemoteCozy { return doc } - async findMaybe( - id /*: string */ - ) /*: Promise */ { + async findDirMaybe(id /*: string */) /*: Promise */ { try { - return await this.find(id) + return await this.findDir(id) } catch (err) { - return null + if (err.status === 404) return null + else throw err } } diff --git a/test/unit/remote/cozy.js b/test/unit/remote/cozy.js index 9fda91031..25eea787b 100644 --- a/test/unit/remote/cozy.js +++ b/test/unit/remote/cozy.js @@ -502,9 +502,7 @@ describe('RemoteCozy', function () { it('fetches a remote directory matching the given id', async function () { const remoteDir = await builders.remoteDir().create() - const foundDir = await remoteCozy.find(remoteDir._id) - - foundDir.should.be.deepEqual(remoteDir) + await should(remoteCozy.find(remoteDir._id)).be.fulfilledWith(remoteDir) }) it('fetches a remote root file including its path', async function () { @@ -514,9 +512,9 @@ describe('RemoteCozy', function () { .name('foo') .create() - const foundFile = await remoteCozy.find(remoteFile._id) - - foundFile.should.deepEqual(_.defaults({ path: '/foo' }, remoteFile)) + await should(remoteCozy.find(remoteFile._id)).be.fulfilledWith( + _.defaults({ path: '/foo' }, remoteFile) + ) }) it('fetches a remote non-root file including its path', async function () { @@ -531,9 +529,15 @@ describe('RemoteCozy', function () { .inDir(remoteDir) .create() - const foundFile = await remoteCozy.find(remoteFile._id) + await should(remoteCozy.find(remoteFile._id)).be.fulfilledWith( + _.defaults({ path: '/foo/bar' }, remoteFile) + ) + }) - foundFile.should.deepEqual(_.defaults({ path: '/foo/bar' }, remoteFile)) + it('throws an error when directory is not found', async function () { + await should(remoteCozy.find('missing')).be.rejectedWith({ + status: 404 + }) }) }) @@ -541,15 +545,59 @@ describe('RemoteCozy', function () { it('does the same as find() when file or directory exists', async function () { const remoteDir = await builders.remoteDir().create() - const foundDir = await remoteCozy.findMaybe(remoteDir._id) - - foundDir.should.deepEqual(remoteDir) + await should(remoteCozy.findMaybe(remoteDir._id)).be.fulfilledWith( + remoteDir + ) }) it('returns null when file or directory is not found', async function () { - const found = await remoteCozy.findMaybe('missing') + await should(remoteCozy.findMaybe('missing')).be.fulfilledWith(null) + }) + }) + + describe('findDir', function () { + it('fetches a remote directory matching the given id', async function () { + const remoteDir = await builders.remoteDir().create() + + await should(remoteCozy.findDir(remoteDir._id)).be.fulfilledWith( + remoteDir + ) + }) + + it('throws an error if a remote file matches the given id', async function () { + const remoteFile = await builders.remoteFile().create() + + await should(remoteCozy.findDir(remoteFile._id)).be.rejectedWith( + /Unexpected file/ + ) + }) + + it('throws an error when directory is not found', async function () { + await should(remoteCozy.findDir('missing')).be.rejectedWith({ + status: 404 + }) + }) + }) + + describe('findDirMaybe', function () { + it('does the same as findDir() when directory exists', async function () { + const remoteDir = await builders.remoteDir().create() + + await should(remoteCozy.findDirMaybe(remoteDir._id)).be.fulfilledWith( + remoteDir + ) + }) + + it('does the same as findDir() when file exists', async function () { + const remoteFile = await builders.remoteFile().create() + + await should(remoteCozy.findDirMaybe(remoteFile._id)).be.rejectedWith( + /Unexpected file/ + ) + }) - should.not.exist(found) + it('returns null when directory is not found', async function () { + await should(remoteCozy.findDirMaybe('missing')).be.fulfilledWith(null) }) })