Skip to content

Commit

Permalink
core/migrations: Use non-throwing remote methods (#2229)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
taratatach authored Apr 28, 2022
1 parent b670bc4 commit e6817b9
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 20 deletions.
4 changes: 2 additions & 2 deletions core/migrations/migrations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
)

Expand Down
20 changes: 15 additions & 5 deletions core/remote/cozy.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,17 @@ class RemoteCozy {
return this.toRemoteDoc(await this.client.files.statById(id))
}

async findMaybe(
id /*: string */
) /*: Promise<?(FullRemoteFile|RemoteDir)> */ {
try {
return await this.find(id)
} catch (err) {
if (err.status === 404) return null
else throw err
}
}

async findDir(id /*: string */) /*: Promise<RemoteDir> */ {
const remoteDir = await this.client.files.statById(id)
const doc = await this.toRemoteDoc(remoteDir)
Expand All @@ -368,13 +379,12 @@ class RemoteCozy {
return doc
}

async findMaybe(
id /*: string */
) /*: Promise<?(FullRemoteFile|RemoteDir)> */ {
async findDirMaybe(id /*: string */) /*: Promise<?RemoteDir> */ {
try {
return await this.find(id)
return await this.findDir(id)
} catch (err) {
return null
if (err.status === 404) return null
else throw err
}
}

Expand Down
74 changes: 61 additions & 13 deletions test/unit/remote/cozy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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 () {
Expand All @@ -531,25 +529,75 @@ 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
})
})
})

describe('findMaybe', 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)
})
})

Expand Down

0 comments on commit e6817b9

Please sign in to comment.