diff --git a/core/local/atom/dispatch.js b/core/local/atom/dispatch.js index f38ffee55..7ab226662 100644 --- a/core/local/atom/dispatch.js +++ b/core/local/atom/dispatch.js @@ -248,7 +248,7 @@ actions = { deletedfile: async (event, { pouch, prep }) => { const was /*: ?Metadata */ = await pouch.byLocalPath(event.path) - if (!was || was.deleted) { + if (!was || was.trashed) { log.debug({ event }, 'Assuming file already removed') // The file was already marked as deleted in pouchdb // => we can ignore safely this event @@ -260,7 +260,7 @@ actions = { deleteddirectory: async (event, { pouch, prep }) => { const was /*: ?Metadata */ = await pouch.byLocalPath(event.path) - if (!was || was.deleted) { + if (!was || was.trashed) { log.debug({ event }, 'Assuming dir already removed') // The dir was already marked as deleted in pouchdb // => we can ignore safely this event diff --git a/core/local/atom/incomplete_fixer.js b/core/local/atom/incomplete_fixer.js index ad2ca79be..a7b7651b3 100644 --- a/core/local/atom/incomplete_fixer.js +++ b/core/local/atom/incomplete_fixer.js @@ -274,7 +274,7 @@ function step( ) if ( incompleteForExistingDoc && - !incompleteForExistingDoc.deleted && + !incompleteForExistingDoc.trashed && (item.event.action === 'created' || item.event.action === 'scan') ) { // Simply drop the incomplete event since we already have a document at this @@ -282,7 +282,7 @@ function step( keepEvent(event, { incompletes, batch }) } else if ( incompleteForExistingDoc && - !incompleteForExistingDoc.deleted && + !incompleteForExistingDoc.trashed && item.event.action === 'modified' ) { // Keep the completed modified event to make sure we don't miss any diff --git a/core/local/atom/win_identical_renaming.js b/core/local/atom/win_identical_renaming.js index 9c1cb221b..9c5608053 100644 --- a/core/local/atom/win_identical_renaming.js +++ b/core/local/atom/win_identical_renaming.js @@ -92,7 +92,7 @@ const fixIdenticalRenamed = async (event, { byLocalPath }) => { if (event.path === event.oldPath) { const doc /*: ?Metadata */ = await byLocalPath(event.path) - if (doc && !doc.deleted && doc.path !== event.oldPath) { + if (doc && !doc.trashed && doc.path !== event.oldPath) { _.set(event, [STEP_NAME, 'oldPathBeforeFix'], event.oldPath) event.oldPath = doc.path } diff --git a/core/local/chokidar/prepare_events.js b/core/local/chokidar/prepare_events.js index 926d1f55a..120039688 100644 --- a/core/local/chokidar/prepare_events.js +++ b/core/local/chokidar/prepare_events.js @@ -60,7 +60,7 @@ const oldMetadata = async ( ) /*: Promise */ => { if (e.old) return e.old const old /*: ?Metadata */ = await pouch.bySyncedPath(e.path) - if (old && !old.deleted) return old + if (old && !old.trashed) return old } /** diff --git a/core/local/chokidar/send_to_prep.js b/core/local/chokidar/send_to_prep.js index d658caff0..5abd800e6 100644 --- a/core/local/chokidar/send_to_prep.js +++ b/core/local/chokidar/send_to_prep.js @@ -91,12 +91,16 @@ const onAddDir = ( * same checksum is added and, if not, we declare this file as deleted. */ const onUnlinkFile = ( - { path: filePath } /*: LocalFileDeletion */, + { path: filePath, old } /*: LocalFileDeletion */, prep /*: Prep */ ) => { log.info({ path: filePath }, 'FileDeletion') + if (!old) { + log.debug({ path: filePath }, 'Assuming file already removed') + return + } return prep - .trashFileAsync(SIDE, { path: filePath }) + .trashFileAsync(SIDE, old) .catch(err => log.warn({ err, path: filePath })) } @@ -106,12 +110,16 @@ const onUnlinkFile = ( * after chokidar event to declare the folder as deleted. */ const onUnlinkDir = ( - { path: folderPath } /*: LocalDirDeletion */, + { path: folderPath, old } /*: LocalDirDeletion */, prep /*: Prep */ ) => { log.info({ path: folderPath }, 'DirDeletion') + if (!old) { + log.debug({ path: folderPath }, 'Assuming dir already removed') + return + } return prep - .trashFolderAsync(SIDE, { path: folderPath }) + .trashFolderAsync(SIDE, old) .catch(err => log.warn({ err, path: folderPath })) } @@ -144,9 +152,15 @@ const step = async ( switch (c.type) { // TODO: Inline old LocalWatcher methods case 'DirDeletion': + if (!c.old) { + c.old = await pouch.bySyncedPath(c.path) + } await onUnlinkDir(c, prep) break case 'FileDeletion': + if (!c.old) { + c.old = await pouch.bySyncedPath(c.path) + } await onUnlinkFile(c, prep) break case 'DirAddition': diff --git a/core/merge.js b/core/merge.js index 076d9b1e3..5dfd1d9f4 100644 --- a/core/merge.js +++ b/core/merge.js @@ -129,7 +129,7 @@ class Merge { const file /*: ?SavedMetadata */ = await this.pouch.bySyncedPath(doc.path) if (file) { - if (file.deleted) { + if (file.trashed) { return this.updateFileAsync(side, doc) } @@ -184,7 +184,7 @@ class Merge { } // Any local update call is an actual modification - if (file.deleted) { + if (file.trashed) { // If the existing record was marked for deletion, we only keep the // PouchDB attributes that will allow us to overwrite it. doc._id = file._id @@ -359,7 +359,7 @@ class Merge { } } - if (folder.deleted) { + if (folder.trashed) { // If the existing record was marked for deletion, we only keep the // PouchDB attributes that will allow us to overwrite it. doc._id = folder._id @@ -455,7 +455,7 @@ class Merge { ) /*: Promise<*> */ { log.debug({ path: doc.path, oldpath: was.path }, 'moveFileAsync') - if ((!metadata.wasSynced(was) && !was.moveFrom) || was.deleted) { + if ((!metadata.wasSynced(was) && !was.moveFrom) || was.trashed) { move.convertToDestinationAddition(side, was, doc) return this.pouch.put(doc) } else if (was.sides && was.sides[side]) { @@ -470,7 +470,7 @@ class Merge { const file /*: ?SavedMetadata */ = await this.pouch.bySyncedPath(doc.path) if (file) { - if (file.deleted) { + if (file.trashed) { doc.overwrite = file.overwrite || file } @@ -526,7 +526,7 @@ class Merge { const folder /*: ?SavedMetadata */ = await this.pouch.bySyncedPath(doc.path) if (folder) { - if (folder.deleted) { + if (folder.trashed) { doc.overwrite = folder.overwrite || folder } @@ -589,9 +589,9 @@ class Merge { for (let doc of docs) { // Don't move children marked for deletion as we can simply propagate the // deletion at their original path. - // Besides, as of today, `moveFrom` will have precedence over `deleted` in + // Besides, as of today, `moveFrom` will have precedence over `trashed` in // Sync and the deletion won't be propagated at all. - if (doc.deleted) continue + if (doc.trashed) continue // Update remote rev of documents which have been updated on the Cozy // after we've detected the move. @@ -690,16 +690,22 @@ class Merge { delete was.errors - if ( - was.deleted && - metadata.isAtLeastUpToDate(otherSide(side), was) && - !metadata.isAtLeastUpToDate(side, was) - ) { - log.debug( - { path: was.path, doc: was }, - 'Erasing doc already marked for deletion' - ) - return this.pouch.eraseDocument(was) + if (was.trashed) { + if (metadata.isAtLeastUpToDate(side, was)) { + // The document was already marked for deletion on the same side (e.g. + // when trashing a folder on the remote Cozy, we'll trash its content + // when merging the folder trashing itself and when merging the trashing + // of its children) so we can ignore this change. + // FIXME: we should at least save the updated side otherwise we'll end + // up with an out-of-date remote _rev. + return + } else if (metadata.isAtLeastUpToDate(otherSide(side), was)) { + log.debug( + { path: was.path, doc: was }, + 'Erasing doc already marked for deletion' + ) + return this.pouch.eraseDocument(was) + } } if (was.sides && was.sides[side]) { @@ -714,7 +720,7 @@ class Merge { delete was.overwrite if (overwrite) { - overwrite.deleted = true + overwrite.trashed = true metadata.markSide(side, overwrite, overwrite) delete overwrite._rev @@ -723,7 +729,7 @@ class Merge { } metadata.markSide(side, was, was) - was.deleted = true + was.trashed = true try { return await this.pouch.put(was) } catch (err) { @@ -737,9 +743,11 @@ class Merge { return this.pouch.put(was) } + // FIXME: we should save the new remote side when merging a remote trashing or + // deletion. async trashFileAsync( side /*: SideName */, - trashed /*: SavedMetadata|{path: string} */, + trashed /*: SavedMetadata */, doc /*: Metadata */ ) /*: Promise */ { const { path } = trashed @@ -807,21 +815,24 @@ class Merge { return this.doTrash(side, was) } + // Send a folder to the Trash + // + // When a folder is marked as deleted in PouchDB, we also mark the files and + // folders inside it to ensure consistency. + // The watchers often detect the deletion of a nested folder after the + // deletion of its parent. In this case, the call to trashFolderAsync for the + // child is considered as successful, even if the folder is already marked for + // deletion. + // FIXME: we should save the new remote side when merging a remote trashing or + // deletion. async trashFolderAsync( side /*: SideName */, - trashed /*: SavedMetadata|{path: string} */, + trashed /*: SavedMetadata */, doc /*: Metadata */ ) /*: Promise<*> */ { const { path } = trashed log.debug({ path }, 'trashFolderAsync') - let was /*: ?SavedMetadata */ - // $FlowFixMe _id exists in SavedMetadata - if (trashed._id != null) { - was = await this.pouch.byIdMaybe(trashed._id) - } else { - was = await this.pouch.bySyncedPath(trashed.path) - } - + const was /*: ?SavedMetadata */ = await this.pouch.byIdMaybe(trashed._id) if (!was) { log.debug({ path }, 'Nothing to trash') return @@ -837,13 +848,10 @@ class Merge { const children = await this.pouch.byRecursivePath(was.path, { descending: true }) - const hasOutOfDateChild = - Array.from(children).find( - child => !metadata.isUpToDate(side, child) && !child.deleted - ) != null - if (!hasOutOfDateChild) { - await this.doTrash(side, was) + for (const child of children) { + await this.doTrash(side, child) } + await this.doTrash(side, was) } // Remove a file from PouchDB @@ -876,7 +884,7 @@ class Merge { } if (file.sides && file.sides[side]) { metadata.markSide(side, file, file) - file.deleted = true + file.trashed = true delete file.errors return this.pouch.put(file) } else { @@ -887,76 +895,125 @@ class Merge { // Remove a folder // + // This method is only called for complete remote deletions (i.e. the document + // was trashed and then completely removed from the Trash). This means we + // don't have to keep remote metadata in the PouchDB record as the remote + // document is unrecoverable. + // It also means we can fetch the document from PouchDB via its _id. + // // When a folder is removed in PouchDB, we also remove the files and folders - // inside it to ensure consistency. The watchers often detects the deletion - // of a nested folder after the deletion of its parent. In this case, the - // call to deleteFolder for the child is considered as successful, even if - // the folder is missing in pouchdb (error 404). + // inside it to ensure consistency. The remote watcher will sort the deletion + // of a folder before the deletion of its content. async deleteFolderAsync(side /*: SideName */, doc /*: SavedMetadata */) { log.debug({ path: doc.path }, 'deleteFolderAsync') - const folder /*: ?SavedMetadata */ = await this.pouch.bySyncedPath(doc.path) + const folder /*: ?SavedMetadata */ = await this.pouch.byIdMaybe(doc._id) if (!folder) { - log.debug({ path }, 'Nothing to delete') - return - } - - if (folder.moveFrom) { - // We don't want Sync to pick up this move hint and try to synchronize a - // move so we delete it. - delete folder.moveFrom - } - - if (folder.sides && folder.sides[side]) { - return this.deleteFolderRecursivelyAsync(side, folder) - } else { - // It can happen after a conflict + log.debug({ path: doc.path }, 'Nothing to delete') return } - } - // Remove a folder and every thing inside it - async deleteFolderRecursivelyAsync( - side /*: SideName */, - folder /*: SavedMetadata */ - ) { // In the changes feed, nested subfolder must be deleted // before their parents, hence the reverse order. - const docs = await this.pouch.byRecursivePath(folder.path, { + const children = await this.pouch.byRecursivePath(folder.path, { descending: true }) - const toPreserve = new Set() - for (let doc of docs) { - if (doc.deleted) continue - + for (let child of children) { if ( - toPreserve.has(doc.path) || - (doc.sides && !metadata.isUpToDate(side, doc)) + child.trashed && + !metadata.isAtLeastUpToDate(otherSide(side), child) ) { - log.warn( - { - path: doc.path, - ancestorPath: folder.path, - otherSide: otherSide(side) - }, - 'Cannot be deleted with ancestor: document was modified on the other side.' - ) - log.info({ path: doc.path }, 'Dissociating from remote...') - metadata.dissociateRemote(doc) - toPreserve.add(path.dirname(doc.path)) + // No need to generate extra changes if the document was already marked + // for deletion on the remote side. + continue + } + + const { moveFrom } = child + + if (moveFrom && child.local) { + if (child.path.normalize() === child.local.path.normalize()) { + if ( + folder.moveFrom && + child.moveFrom.path.startsWith(folder.moveFrom.path + path.sep) + ) { + // The child was moved on the local side with the deleted folder thus + // we need to trash it on the local filesystem. + metadata.markSide(side, child, child) + } else { + // The child was moved on the local side into the deleted folder thus + // we need to trash it on the Cozy. + metadata.markSide(otherSide(side), child, child) + // XXX: the path displayed in logs will be the one after the child + // was moved (and became a child of folder) which can be a bit + // confusing but since we use the remote _id to apply actions on the + // remote Cozy we don't really care. + } + } else { + // The child was moved on the remote side into the deleted folder thus + // we need to trash its former location on the local filesystem. + // XXX: Since the Local module uses the main path instead of the local + // path, we have to "revert" the remote move by changing the main path + // back into its local value. + // TODO: use the local path in the Local module so we don't have to deal + // with such issues. + child.path = child.local.path + + metadata.markSide(side, child, child) + } + } else if (child.sides && !child.sides[side]) { + metadata.markSide(otherSide(side), child, child) + } else { + metadata.markSide(side, child, child) + } + + child.trashed = true + + // We don't want Sync to pick up other hints than the deletion and + // try to synchronize them. + delete child.moveFrom + delete child.overwrite + delete child.errors + } + + const { moveFrom, overwrite } = folder + + if (moveFrom && folder.local) { + if (folder.path.normalize() === folder.local.path.normalize()) { + if (overwrite) { + // TODO: Should we call deleteFolderAsync if overwrite is a folder to + // be sure we delete all its content as well? + overwrite.trashed = true + metadata.markSide(otherSide(side), overwrite, overwrite) + + delete overwrite.moveFrom + delete overwrite.overwrite + delete overwrite.errors + delete overwrite._id + delete overwrite._rev + + children.push(overwrite) + } } else { - metadata.markSide(side, doc, doc) - doc.deleted = true - delete doc.errors + // The folder was moved on the remote side before being deleted so we + // need to trash its former location on the local filesystem. + // XXX: Since the Local module uses the main path instead of the local + // path, we have to "revert" the remote move by changing the main path + // back into its local value. + // TODO: use the local path in the Local module so we don't have to deal + // with such issues. + folder.path = folder.local.path } } metadata.markSide(side, folder, folder) - folder.deleted = true + folder.trashed = true + // We don't want Sync to pick up other hints than the deletion and try to + // synchronize them. + delete folder.moveFrom + delete folder.overwrite delete folder.errors - docs.push(folder) - return this.pouch.bulkDocs(docs) + return this.pouch.bulkDocs(children.concat(folder)) } } diff --git a/core/metadata.js b/core/metadata.js index 0ea1f13cd..5527d27bb 100644 --- a/core/metadata.js +++ b/core/metadata.js @@ -149,7 +149,6 @@ export type Metadata = { class?: string, trashed?: true, - deleted?: true, errors?: number, overwrite?: SavedMetadata, childMove?: boolean, diff --git a/core/move.js b/core/move.js index 18960c83b..b64c94ac3 100644 --- a/core/move.js +++ b/core/move.js @@ -43,11 +43,9 @@ function move( } } - // TODO: Find out wether or not it would make sense to also delete the - // trashed property on the source, or explain why it doesn't. - delete dst.trashed - dst.moveFrom = src + // TODO: remove `_id` and `_rev` from the exception list above and stop + // assigning them manually. dst._id = src._id dst._rev = src._rev diff --git a/core/pouch/index.js b/core/pouch/index.js index 5c91a94a2..dbe2a9905 100644 --- a/core/pouch/index.js +++ b/core/pouch/index.js @@ -171,7 +171,7 @@ class Pouch { .filter( row => !row.key.startsWith('_') && // Filter out design docs - !row.doc.deleted && // Filter out docs already marked for deletion + !row.doc.trashed && // Filter out docs already marked for deletion // Keep only docs that have existed locally row.doc.sides && row.doc.sides.local && diff --git a/core/pouch/migrations.js b/core/pouch/migrations.js index 954a7207f..83fcd8cc9 100644 --- a/core/pouch/migrations.js +++ b/core/pouch/migrations.js @@ -324,6 +324,26 @@ const migrations /*: Migration[] */ = [ return doc }) } + }, + { + baseSchemaVersion: 12, + targetSchemaVersion: 13, + description: 'Merge trashed and deleted attributes into trashed', + affectedDocs: (docs /*: SavedMetadata[] */) /*: SavedMetadata[] */ => { + // $FlowFixMe `deleted` has been removed from Metadata thus this migration + return docs.filter(doc => doc.deleted != null) + }, + run: (docs /*: SavedMetadata[] */) /*: SavedMetadata[] */ => { + return docs.map(doc => { + // $FlowFixMe `deleted` has been removed from Metadata + if (doc.deleted) { + doc.trashed = true + } + // $FlowFixMe `deleted` has been removed from Metadata + delete doc.deleted + return doc + }) + } } ] diff --git a/core/prep.js b/core/prep.js index e14d32b42..f133ee704 100644 --- a/core/prep.js +++ b/core/prep.js @@ -4,11 +4,8 @@ */ const autoBind = require('auto-bind') -const { clone } = require('lodash') -const { join } = require('path') const metadata = require('./metadata') -const { TRASH_DIR_NAME } = require('./remote/constants') const logger = require('./utils/logger') /*:: @@ -127,7 +124,7 @@ class Prep { return } if (side === 'local' && docIgnored) { - return this.merge.deleteFileAsync(side, was) + return this.merge.trashFileAsync(side, was, was) } else if (side === 'local' && wasIgnored) { return this.merge.addFileAsync(side, doc) } else { @@ -167,7 +164,7 @@ class Prep { return } if (side === 'local' && docIgnored) { - return this.merge.deleteFolderAsync(side, was) + return this.merge.trashFolderAsync(side, was, was) } else if (side === 'local' && wasIgnored) { return this.merge.putFolderAsync(side, doc) } else { @@ -178,45 +175,45 @@ class Prep { // TODO add comments + tests async trashFileAsync( side /*: SideName */, - was /*: SavedMetadata|{path: string} */, + was /*: SavedMetadata */, doc /*: ?Metadata */ ) { log.debug({ path: doc && doc.path, oldpath: was.path }, 'trashFileAsync') metadata.ensureValidPath(was) if (!doc) { - doc = clone(was) - doc.path = join(TRASH_DIR_NAME, was.path) - } - - metadata.ensureValidPath(doc) - doc.trashed = true - doc.docType = 'file' + // XXX: Local deletions don't generate new records as we don't have any + // information about deleted files and folders (while we have new metadata + // for remote documents that were trashed). + return this.merge.trashFileAsync(side, was, was) + } else { + metadata.ensureValidPath(doc) + doc.docType = 'file' - // TODO metadata.shouldIgnore - return this.merge.trashFileAsync(side, was, doc) + return this.merge.trashFileAsync(side, was, doc) + } } // TODO add comments + tests async trashFolderAsync( side /*: SideName */, - was /*: SavedMetadata|{path: string} */, + was /*: SavedMetadata */, doc /*: ?Metadata */ ) { log.debug({ path: doc && doc.path, oldpath: was.path }, 'trashFolderAsync') metadata.ensureValidPath(was) if (!doc) { - doc = clone(was) - doc.path = join(TRASH_DIR_NAME, was.path) - } - - metadata.ensureValidPath(doc) - doc.trashed = true - doc.docType = 'folder' + // XXX: Local deletions don't generate new records as we don't have any + // information about deleted files and folders (while we have new metadata + // for remote documents that were trashed). + return this.merge.trashFolderAsync(side, was, was) + } else { + metadata.ensureValidPath(doc) + doc.docType = 'folder' - // TODO metadata.shouldIgnore - return this.merge.trashFolderAsync(side, was, doc) + return this.merge.trashFolderAsync(side, was, doc) + } } // Expectations: diff --git a/core/remote/cozy.js b/core/remote/cozy.js index b6beb340c..34c298573 100644 --- a/core/remote/cozy.js +++ b/core/remote/cozy.js @@ -322,6 +322,19 @@ class RemoteCozy { return { last_seq, docs } } + async fetchLastSeq() { + const client = await this.newClient() + const { last_seq } = await client + .collection(FILES_DOCTYPE) + .fetchChangesRaw({ + since: '0', + descending: true, + limit: 1, + includeDocs: false + }) + return last_seq + } + async completeRemoteDocs( rawDocs /*: Array */ ) /*: Promise> */ { diff --git a/core/remote/index.js b/core/remote/index.js index b5f036eb9..613ce9ae8 100644 --- a/core/remote/index.js +++ b/core/remote/index.js @@ -160,7 +160,7 @@ class Remote /*:: implements Reader, Writer */ { 'could not fetch conflicting directory' ) } - if (remoteDoc && (await this.isExcludedFromSync(remoteDoc))) { + if (remoteDoc && this.isExcludedFromSync(remoteDoc)) { throw new ExcludedDirError(path) } } @@ -182,7 +182,9 @@ class Remote /*:: implements Reader, Writer */ { } catch (err) { if (err.code === 'ENOENT') { log.warn({ path }, 'Local file does not exist anymore.') - doc.deleted = true // XXX: This prevents the doc to be saved with new revs + // FIXME: with this deletion marker, the record will be erased from + // PouchDB while the remote document will remain. + doc.trashed = true return doc } throw err @@ -210,7 +212,9 @@ class Remote /*:: implements Reader, Writer */ { } catch (err) { if (err.code === 'ENOENT') { log.warn({ path }, 'Local file does not exist anymore.') - doc.deleted = true // XXX: This prevents the doc to be saved with new revs + // FIXME: with this deletion marker, the record will be erased from + // PouchDB while the remote document will remain. + doc.trashed = true return doc } throw err @@ -448,9 +452,7 @@ class Remote /*:: implements Reader, Writer */ { return conflict } - async isExcludedFromSync( - doc /*: MetadataRemoteInfo */ - ) /*: Promise */ { + isExcludedFromSync(doc /*: MetadataRemoteInfo */) /*: boolean */ { const { client: { clientID } } = this.config diff --git a/core/sync/index.js b/core/sync/index.js index 8d1afa8d3..64583bf92 100644 --- a/core/sync/index.js +++ b/core/sync/index.js @@ -57,7 +57,7 @@ const isMarkedForDeletion = (doc /*: SavedMetadata */) => { // During a transition period, we'll need to consider both documents with the // deletion marker and documents which were deleted but not yet synced before // the application was updated and are thus completely _deleted from PouchDB. - return doc.trashed || doc.deleted || doc._deleted + return doc.trashed || doc._deleted } const shouldAttemptRetry = (change /*: PouchDBFeedData */) => { @@ -471,7 +471,7 @@ class Sync { if (shouldAttemptRetry(change)) { this.blockSyncFor({ err, change }) } else { - if (change.doc.deleted) { + if (isMarkedForDeletion(change.doc)) { await this.skipChange(change, err) } else if (sideName === 'remote') { delete change.doc.moveFrom @@ -731,7 +731,7 @@ class Sync { // Clean up documents so that we don't mistakenly take action based on // previous changes and keep our Pouch documents as small as possible // and especially avoid deep nesting levels. - if (doc.deleted) { + if (doc.trashed) { await this.pouch.eraseDocument(doc) if (doc.docType === 'file') { this.events.emit('delete-file', _.clone(doc)) @@ -1115,6 +1115,9 @@ class Sync { { path: doc.path }, `${doc.docType} will be trashed within its parent directory` ) + // XXX: doc will be erased from PouchDB as it is marked as `deleted`. + // This means that, until the parent deletion is synchronized, our local + // PouchDB won't reflect the reality. return } } diff --git a/test/integration/move.js b/test/integration/move.js index 2c42129ad..7e5561f35 100644 --- a/test/integration/move.js +++ b/test/integration/move.js @@ -215,7 +215,7 @@ describe('Move', () => { should( helpers.putDocs('path', 'deleted', 'trashed', 'moveFrom') - ).deepEqual([{ path: path.normalize('src/file'), deleted: true }]) + ).deepEqual([{ path: path.normalize('src/file'), trashed: true }]) await helpers.syncAll() @@ -264,7 +264,7 @@ describe('Move', () => { ).deepEqual([ { path: intermediaryPath, - deleted: true + trashed: true } ]) @@ -756,17 +756,17 @@ describe('Move', () => { ).deepEqual([ { path: path.normalize('parent/src/dir/subdir/file'), - deleted: true + trashed: true }, { path: path.normalize('parent/src/dir/subdir'), - deleted: true + trashed: true }, { path: path.normalize('parent/src/dir/empty-subdir'), - deleted: true + trashed: true }, - { path: path.normalize('parent/src/dir'), deleted: true } + { path: path.normalize('parent/src/dir'), trashed: true } ]) await helpers.syncAll() @@ -811,17 +811,17 @@ describe('Move', () => { ).deepEqual([ { path: path.normalize('parent/dst/dir/subdir/file'), - deleted: true + trashed: true }, { path: path.normalize('parent/dst/dir/subdir'), - deleted: true + trashed: true }, { path: path.normalize('parent/dst/dir/empty-subdir'), - deleted: true + trashed: true }, - { path: path.normalize('parent/dst/dir'), deleted: true } + { path: path.normalize('parent/dst/dir'), trashed: true } ]) await helpers.syncAll() diff --git a/test/integration/permanent_deletion.js b/test/integration/permanent_deletion.js index a37863a5b..5183360d4 100644 --- a/test/integration/permanent_deletion.js +++ b/test/integration/permanent_deletion.js @@ -22,13 +22,13 @@ describe('Permanent deletion remote', () => { afterEach(pouchHelpers.cleanDatabase) after(configHelpers.cleanConfig) - beforeEach(function() { + beforeEach(async function() { helpers = TestHelpers.init(this) - helpers.local.setupTrash() + await helpers.local.setupTrash() + await helpers.remote.ignorePreviousChanges() }) it('file', async () => { - await helpers.remote.ignorePreviousChanges() const file = await cozy.files.create('File content', { name: 'file' }) await helpers.remote.pullChanges() await helpers.syncAll() @@ -39,7 +39,7 @@ describe('Permanent deletion remote', () => { await helpers.remote.pullChanges() should(helpers.putDocs('path', 'deleted', 'trashed')).deepEqual([ - { path: 'file', deleted: true } + { path: 'file', trashed: true } ]) await helpers.syncAll() diff --git a/test/integration/sync_state.js b/test/integration/sync_state.js index 147160dd2..45b5bec0c 100644 --- a/test/integration/sync_state.js +++ b/test/integration/sync_state.js @@ -24,12 +24,12 @@ describe('Sync state', () => { let events, helpers - beforeEach(function() { + beforeEach(async function() { helpers = TestHelpers.init(this) events = helpers.events sinon.spy(events, 'emit') - // await helpers.local.setupTrash() - // await helpers.remote.ignorePreviousChanges() + + await helpers.remote.ignorePreviousChanges() }) it('1 sync error (missing remote file)', async () => { diff --git a/test/integration/trash.js b/test/integration/trash.js index 47b8c1726..d26a25b4f 100644 --- a/test/integration/trash.js +++ b/test/integration/trash.js @@ -56,10 +56,13 @@ describe('Trash', () => { context('on the local filesystem', () => { it('trashes the file on the remote Cozy', async () => { - await prep.trashFileAsync('local', { path: 'parent/file' }) + const doc = await helpers.pouch.bySyncedPath( + path.normalize('parent/file') + ) + await prep.trashFileAsync('local', doc) - should(helpers.putDocs('path', 'deleted', 'trashed')).deepEqual([ - { path: path.normalize('parent/file'), deleted: true } + should(helpers.putDocs('path', 'trashed')).deepEqual([ + { path: path.normalize('parent/file'), trashed: true } ]) await should(pouch.db.get(file._id)).be.rejectedWith({ status: 404 }) @@ -174,8 +177,8 @@ describe('Trash', () => { await helpers.remote.pullChanges() - should(helpers.putDocs('path', 'deleted', 'trashed')).deepEqual([ - { path: path.normalize('parent/file'), deleted: true } + should(helpers.putDocs('path', 'trashed')).deepEqual([ + { path: path.normalize('parent/file'), trashed: true } ]) await should(pouch.db.get(file._id)).be.rejectedWith({ status: 404 }) @@ -285,12 +288,17 @@ describe('Trash', () => { context('on the local filesystem', () => { it('trashes the directory on the remote Cozy', async () => { - await prep.trashFolderAsync('local', { - path: path.normalize('parent/dir') - }) - - should(helpers.putDocs('path', 'deleted', 'trashed')).deepEqual([ - { path: path.normalize('parent/dir'), deleted: true } + const doc = await helpers.pouch.bySyncedPath( + path.normalize('parent/dir') + ) + await prep.trashFolderAsync('local', doc) + + should(helpers.putDocs('path', 'trashed')).deepEqual([ + // Recursively trash parent/dir; children are trashed first + { path: path.normalize('parent/dir/subdir/file'), trashed: true }, + { path: path.normalize('parent/dir/subdir'), trashed: true }, + { path: path.normalize('parent/dir/empty-subdir'), trashed: true }, + { path: path.normalize('parent/dir'), trashed: true } ]) await helpers.syncAll() @@ -311,11 +319,12 @@ describe('Trash', () => { await cozy.files.trashById(dir._id) await helpers.remote.pullChanges() - should(helpers.putDocs('path', 'deleted', 'trashed')).deepEqual([ - { path: path.normalize('parent/dir'), deleted: true }, - { path: path.normalize('parent/dir/empty-subdir'), deleted: true }, - { path: path.normalize('parent/dir/subdir'), deleted: true }, - { path: path.normalize('parent/dir/subdir/file'), deleted: true } + should(helpers.putDocs('path', 'trashed')).deepEqual([ + // Recursively trash parent/dir; children are trashed first + { path: path.normalize('parent/dir/subdir/file'), trashed: true }, + { path: path.normalize('parent/dir/subdir'), trashed: true }, + { path: path.normalize('parent/dir/empty-subdir'), trashed: true }, + { path: path.normalize('parent/dir'), trashed: true } ]) await helpers.syncAll() diff --git a/test/integration/update.js b/test/integration/update.js index 4d094db41..c4aa59c94 100644 --- a/test/integration/update.js +++ b/test/integration/update.js @@ -154,16 +154,16 @@ describe('Update file', () => { beforeEach(async () => { await helpers.remote.ignorePreviousChanges() await helpers.local.syncDir.outputFile(existingPath, 'existing content') - await helpers.flushLocalAndSyncAll() - await helpers.local.syncDir.remove(existingPath) - await helpers.local.syncDir.removeParentDir(existingPath) - await helpers.flushLocalAndSyncAll() - await helpers.local.syncDir.outputFile( path.normalize('src/file'), 'initial content' ) await helpers.flushLocalAndSyncAll() + + await helpers.local.syncDir.remove(existingPath) + await helpers.local.syncDir.removeParentDir(existingPath) + await helpers.flushLocalAndSyncAll() + await helpers.local.syncDir.move('src', 'dst') await helpers.local.scan() }) diff --git a/test/scenarios/move_overwriting_dir/not_trashed/scenario.js b/test/scenarios/move_overwriting_dir/not_trashed/scenario.js index f9605ed40..b00fd8433 100644 --- a/test/scenarios/move_overwriting_dir/not_trashed/scenario.js +++ b/test/scenarios/move_overwriting_dir/not_trashed/scenario.js @@ -62,7 +62,7 @@ module.exports = ({ 'dir/deletedFile', 'dir/subdir/', 'file', // XXX: content is trashed before on disk - 'file (__cozy__: ...)' // XXX: content is trashed before on disk + 'file (...)' // XXX: content is trashed before on disk ], contents: { 'dst/dir/deletedFile': 'should be kept', diff --git a/test/scenarios/update_delete_then_replace_file/scenario.js b/test/scenarios/update_delete_then_replace_file/scenario.js index 9e7cf40ef..a75ba454a 100644 --- a/test/scenarios/update_delete_then_replace_file/scenario.js +++ b/test/scenarios/update_delete_then_replace_file/scenario.js @@ -40,7 +40,7 @@ module.exports = ({ tree: ['dst/', 'final/', 'final/file2', 'src/', 'src/file1'], remoteTrash: [ 'file2', // src/file2 - 'file2 (__cozy__: ...)' // final/file2 + 'file2 (...)' // final/file2 ], localTrash: [ 'file2' // src/file2 diff --git a/test/support/builders/metadata/base.js b/test/support/builders/metadata/base.js index 50dc9aa46..912960322 100644 --- a/test/support/builders/metadata/base.js +++ b/test/support/builders/metadata/base.js @@ -70,12 +70,16 @@ module.exports = class BaseMetadataBuilder { return this } - moveFrom(was /*: Metadata */) /*: this */ { + moveFrom( + was /*: Metadata */, + { childMove = false } /*: { childMove?: boolean } */ = {} + ) /*: this */ { this.doc = { ..._.cloneDeep(was), ...this.doc } this.doc.moveFrom = was + if (childMove) this.doc.moveFrom.childMove = true return this } @@ -171,11 +175,6 @@ module.exports = class BaseMetadataBuilder { return this } - deleted() /*: this */ { - this.doc.deleted = true - return this - } - erased() /*: this */ { this.doc._deleted = true return this @@ -377,6 +376,10 @@ module.exports = class BaseMetadataBuilder { .size(String(this.doc.size)) } + if (this.doc.trashed) { + builder = builder.trashed() + } + this.doc.remote = builder.build() this.doc.remote.path = pathUtils.localToRemote(this.doc.path) } diff --git a/test/support/doubles/side.js b/test/support/doubles/side.js index 2db0b6516..43c1e51ff 100644 --- a/test/support/doubles/side.js +++ b/test/support/doubles/side.js @@ -29,5 +29,10 @@ module.exports = function stubSide(name /*: SideName */) /*: Writer */ { double.name = name double.watcher = {} double.watcher.running = new Promise(() => {}) + + if (name === 'remote') { + double.isExcludedFromSync = sinon.stub.returns() + } + return double } diff --git a/test/support/helpers/remote.js b/test/support/helpers/remote.js index d4ec332f7..9a147694e 100644 --- a/test/support/helpers/remote.js +++ b/test/support/helpers/remote.js @@ -44,7 +44,7 @@ class RemoteTestHelpers { } async ignorePreviousChanges() { - const { last_seq } = await this.side.remoteCozy.changes() + const last_seq = await this.side.remoteCozy.fetchLastSeq() await this.pouch.setRemoteSeq(last_seq) } @@ -151,10 +151,14 @@ class RemoteTestHelpers { const ellipsizeDate = opts.ellipsize ? conflictHelpers.ellipsizeDate : _.identity - return relPaths - .sort() - .map(ellipsizeDate) - .map(p => p.replace(/\(__cozy__: \d+\)/, '(__cozy__: ...)')) + return ( + relPaths + .sort() + // XXX: replace Desktop conflict dates with ... + .map(ellipsizeDate) + // XXX: replace random conflit suffix for trashed files with same name + .map(p => p.replace(/\(\d+\)/, '(...)')) + ) } async treeWithoutTrash( diff --git a/test/unit/local/chokidar/initial_scan.js b/test/unit/local/chokidar/initial_scan.js index 9ec1acf1d..c361ddd56 100644 --- a/test/unit/local/chokidar/initial_scan.js +++ b/test/unit/local/chokidar/initial_scan.js @@ -65,7 +65,7 @@ onPlatform('darwin', () => { await builders .metadir() .path('.cozy_trash/folder5') - .deleted() + .trashed() .changedSide('remote') .create() // File still exists @@ -103,7 +103,7 @@ onPlatform('darwin', () => { builders .metafile() .path('folder1/file5') - .deleted() + .trashed() .changedSide('local') .create() const initialScan = { paths: ['folder1', 'file1'].map(metadata.id) } diff --git a/test/unit/merge.js b/test/unit/merge.js index 9b9c177aa..7715bb4bb 100644 --- a/test/unit/merge.js +++ b/test/unit/merge.js @@ -125,6 +125,7 @@ describe('Merge', function() { }) return conflict }) + this.merge.remote.isExcludedFromSync = sinon.stub().returns(false) builders = new Builders({ cozy: cozyHelpers.cozy, pouch: this.pouch }) }) afterEach('clean pouch', pouchHelpers.cleanDatabase) @@ -290,7 +291,7 @@ describe('Merge', function() { .create() deleted = await builders .metafile(synced) - .deleted() + .trashed() .changedSide('local') .create() }) @@ -504,7 +505,7 @@ describe('Merge', function() { .create() await builders .metafile(synced) - .deleted() + .trashed() .changedSide('remote') .create() }) @@ -547,12 +548,12 @@ describe('Merge', function() { .create() deleted = await builders .metafile(synced) - .deleted() + .trashed() .changedSide('remote') .create() }) - it('updates the existing record', async function() { + it('updates the existing record as a new local file', async function() { const doc = await builders .metafile(synced) .data('local content') @@ -569,12 +570,11 @@ describe('Merge', function() { _.defaults( { _id: deleted._id, - // Local side is increased by 2 to overcome the unsynced - // remote deletion. - sides: increasedSides(deleted.sides, 'local', 2), - remote: deleted.remote + sides: { target: 1, local: 1 } }, - _.omit(doc, ['_rev']) + // XXX: doc has no remote side but this makes sure the + // resulting record does not either. + _.omit(doc, ['_rev', 'remote']) ) ], resolvedConflicts: [] @@ -2548,7 +2548,7 @@ describe('Merge', function() { await builders .metafile() .path('OLD/child') - .deleted() + .trashed() .changedSide('local') .create() const doc = builders @@ -3935,39 +3935,6 @@ describe('Merge', function() { }) }) - describe('trashFolderAsync', () => { - it('does not trash a folder if the other side has added a new file in it', async function() { - const was = await builders - .metadir() - .path('trashed-folder') - .upToDate() - .create() - await builders - .metafile() - .path('trashed-folder/file') - .sides({ [otherSide(this.side)]: 1 }) - .create() - const doc = await builders - .metadir(was) - .path(`.cozy_trash/${was.path}`) - .trashed() - .build() - - const sideEffects = await mergeSideEffects(this, () => - this.merge.trashFolderAsync( - this.side, - _.cloneDeep(was), - _.cloneDeep(doc) - ) - ) - - should(sideEffects).deepEqual({ - savedDocs: [], - resolvedConflicts: [] - }) - }) - }) - describe('deleteFileAsync', () => { context('when a record is found in Pouch', () => { it('deletes a file', async function() { @@ -3987,7 +3954,7 @@ describe('Merge', function() { _.defaults( { sides: increasedSides(doc.sides, this.side, 1), - deleted: true + trashed: true }, _.omit(doc, ['_rev']) ) @@ -4019,7 +3986,7 @@ describe('Merge', function() { _.defaults( { sides: increasedSides(doc.sides, this.side, 1), - deleted: true + trashed: true }, _.omit(doc, ['_rev', 'moveFrom']) ) @@ -4029,47 +3996,12 @@ describe('Merge', function() { }) }) - context('when a trashed record is found in Pouch', () => { - it('marks it for deletion and updates sides info', async function() { - const was = await builders - .metafile() - .path('FILE') - .trashed() - .changedSide(otherSide(this.side)) - .create() - const doc = builders - .metafile(was) - .unmerged(this.side) - .build() - - const sideEffects = await mergeSideEffects(this, () => - this.merge.deleteFileAsync(this.side, _.cloneDeep(doc)) - ) - - should(sideEffects).deepEqual({ - savedDocs: [ - _.defaults( - { - // We increase the side by 2 since the other side was increased - // when `was` was trashed - sides: increasedSides(was.sides, this.side, 2), - [this.side]: doc[this.side], - deleted: true - }, - _.omit(was, ['_rev']) - ) - ], - resolvedConflicts: [] - }) - }) - }) - context('when a record marked for deletion is found in Pouch', () => { it('keeps the deletion marker and updates sides info', async function() { const was = await builders .metafile() .path('FILE') - .deleted() + .trashed() .changedSide(otherSide(this.side)) .create() const doc = builders @@ -4089,7 +4021,7 @@ describe('Merge', function() { // when `was` was marked for deletion sides: increasedSides(was.sides, this.side, 2), [this.side]: doc[this.side], - deleted: true + trashed: true }, _.omit(was, ['_rev']) ) @@ -4101,8 +4033,17 @@ describe('Merge', function() { }) describe('deleteFolderAsync', function() { + before(function() { + // XXX: deleteFolderAsync is only used for remote deletions + this.side = 'remote' + }) + after(function() { + // XXX: 'local' is the current side value but it could change + this.side = 'local' + }) + context('when a record is found in Pouch', () => { - it('deletes a folder', async function() { + it('marks the folder for deletion on the local filesystem', async function() { const doc = await builders .metadir() .path('FOLDER') @@ -4118,7 +4059,7 @@ describe('Merge', function() { _.defaults( { sides: increasedSides(doc.sides, this.side, 1), - deleted: true + trashed: true }, _.omit(doc, ['_rev']) ) @@ -4127,135 +4068,323 @@ describe('Merge', function() { }) }) - it('removes nested content', async function() { - const doc = await builders - .metadir() - .path('FOLDER') - .sides({ [this.side]: 1 }) - .create() - const subdir = await builders - .metafile() - .path('FOLDER/DIR') - .sides({ [this.side]: 1 }) - .create() - const subsubdir = await builders - .metafile() - .path('FOLDER/DIR/DIR') - .sides({ [this.side]: 1 }) - .create() - const subsubsubfile = await builders - .metafile() - .path('FOLDER/DIR/DIR/FILE') - .sides({ [this.side]: 1 }) - .data('content') - .create() + context('and it has children', () => { + it('marks children for deletion on the local filesystem', async function() { + const doc = await builders + .metadir() + .path('FOLDER') + .upToDate() + .create() + const subdir = await builders + .metadir() + .path('FOLDER/DIR') + .upToDate() + .create() + const subsubdir = await builders + .metadir() + .path('FOLDER/DIR/DIR') + .upToDate() + .create() + const subsubsubfile = await builders + .metafile() + .path('FOLDER/DIR/DIR/FILE') + .data('content') + .upToDate() + .create() - const sideEffects = await mergeSideEffects(this, () => - this.merge.deleteFolderAsync(this.side, _.cloneDeep(doc)) - ) + const sideEffects = await mergeSideEffects(this, () => + this.merge.deleteFolderAsync(this.side, _.cloneDeep(doc)) + ) - should(sideEffects).deepEqual({ - savedDocs: [ - _.defaults( - { - sides: increasedSides(subsubsubfile.sides, this.side, 1), - deleted: true - }, - _.omit(subsubsubfile, ['_rev']) - ), - _.defaults( - { - sides: increasedSides(subsubdir.sides, this.side, 1), - deleted: true - }, - _.omit(subsubdir, ['_rev']) - ), - _.defaults( - { - sides: increasedSides(subdir.sides, this.side, 1), - deleted: true - }, - _.omit(subdir, ['_rev']) - ), - _.defaults( - { - sides: increasedSides(doc.sides, this.side, 1), - deleted: true - }, - _.omit(doc, ['_rev']) - ) - ], - resolvedConflicts: [] + should(sideEffects).deepEqual({ + savedDocs: [ + _.defaults( + { + sides: increasedSides(subsubsubfile.sides, this.side, 1), + trashed: true + }, + _.omit(subsubsubfile, ['_rev']) + ), + _.defaults( + { + sides: increasedSides(subsubdir.sides, this.side, 1), + trashed: true + }, + _.omit(subsubdir, ['_rev']) + ), + _.defaults( + { + sides: increasedSides(subdir.sides, this.side, 1), + trashed: true + }, + _.omit(subdir, ['_rev']) + ), + _.defaults( + { + sides: increasedSides(doc.sides, this.side, 1), + trashed: true + }, + _.omit(doc, ['_rev']) + ) + ], + resolvedConflicts: [] + }) }) - }) - it('removes move hints', async function() { - // TODO: make sure we remove the move hints on children. This will be part - // of a larger refactoring to make sure folders are trashed with their - // hierarchy. - const old = await builders - .metadir() - .path('FOLDER') - .sides({ [this.side]: 1 }) - .create() - const doc = await builders - .metadir() - .moveFrom(old) - .path('MOVED') - .sides({ [this.side]: 1 }) - .create() + context( + 'and child was moved into the folder on the local filesystem', + () => { + it('marks it for deletion on the remote Cozy', async function() { + const doc = await builders + .metadir() + .path('folder') + .upToDate() + .create() - const sideEffects = await mergeSideEffects(this, () => - this.merge.deleteFolderAsync(this.side, _.cloneDeep(doc)) + const dir = await builders + .metadir() + .path('dir') + .upToDate() + .create() + const movedDir = await builders + .metadir() + .moveFrom(dir) + .path('folder/dir') + .changedSide(otherSide(this.side)) + .create() + + const sideEffects = await mergeSideEffects(this, () => + this.merge.deleteFolderAsync(this.side, _.cloneDeep(doc)) + ) + + should(sideEffects).deepEqual({ + savedDocs: [ + _.defaults( + { + [this.side]: dir[this.side], + sides: increasedSides( + movedDir.sides, + otherSide(this.side), + 1 + ), + trashed: true + }, + _.omit(movedDir, ['_rev', 'moveFrom']) + ), + _.defaults( + { + sides: increasedSides(doc.sides, this.side, 1), + trashed: true + }, + _.omit(doc, ['_rev']) + ) + ], + resolvedConflicts: [] + }) + }) + } ) + }) - should(sideEffects).deepEqual({ - savedDocs: [ - _.defaults( - { - sides: increasedSides(doc.sides, this.side, 1), - deleted: true - }, - _.omit(doc, ['_rev', 'moveFrom']) + context('and it was moved on the local filesystem', () => { + it('removes move hints', async function() { + const old = await builders + .metadir() + .path('FOLDER') + .upToDate() + .create() + const doc = await builders + .metadir() + .moveFrom(old) + .path('MOVED') + .changedSide(otherSide(this.side)) + .create() + + const sideEffects = await mergeSideEffects(this, () => + this.merge.deleteFolderAsync(this.side, _.cloneDeep(doc)) + ) + + should(sideEffects).deepEqual({ + savedDocs: [ + _.defaults( + { + // XXX: folder was moved on local side so the remote side is + // increased by 2 to compensate. + sides: increasedSides(doc.sides, this.side, 2), + trashed: true + }, + _.omit(doc, ['_rev', 'moveFrom']) + ) + ], + resolvedConflicts: [] + }) + }) + + context('with children', () => { + it('marks the children for deletion on the local filesystem', async function() { + const dir = await builders + .metadir() + .path('folder') + .upToDate() + .create() + const subdir = await builders + .metadir() + .path('folder/subdir') + .upToDate() + .create() + const file = await builders + .metafile() + .path('folder/subdir/file') + .data('content') + .upToDate() + .create() + + const movedDir = await builders + .metadir() + .moveFrom(dir) + .path('dir') + .changedSide(otherSide(this.side)) + .create() + const movedSubdir = await builders + .metadir() + .moveFrom(subdir, { childMove: true }) + .path('dir/subdir') + .changedSide(otherSide(this.side)) + .create() + const movedFile = await builders + .metadir() + .moveFrom(file, { childMove: true }) + .path('dir/subdir/file') + .changedSide(otherSide(this.side)) + .create() + + const sideEffects = await mergeSideEffects(this, () => + this.merge.deleteFolderAsync(this.side, _.cloneDeep(movedDir)) ) - ], - resolvedConflicts: [] + + should(sideEffects).deepEqual({ + savedDocs: [ + _.defaultsDeep( + { + [this.side]: movedFile[this.side], + // XXX: child was moved on local side so the remote side is + // increased by 2 to compensate. + sides: increasedSides(movedFile.sides, this.side, 2), + trashed: true + }, + _.omit(movedFile, ['_rev', 'moveFrom', 'childMove']) + ), + _.defaults( + { + [this.side]: movedSubdir[this.side], + // XXX: child was moved on local side so the remote side is + // increased by 2 to compensate. + sides: increasedSides(movedSubdir.sides, this.side, 2), + trashed: true + }, + _.omit(movedSubdir, ['_rev', 'moveFrom', 'childMove']) + ), + _.defaults( + { + [this.side]: movedDir[this.side], + sides: increasedSides(movedDir.sides, this.side, 2), + trashed: true + }, + _.omit(movedDir, ['_rev', 'moveFrom']) + ) + ], + resolvedConflicts: [] + }) + }) + }) + + context('overwriting another document', () => { + it('marks the overwritten document for deletion on the remote Cozy', async function() { + const overwritten = await builders + .metafile() + .path('MOVED') + .upToDate() + .create() + const old = await builders + .metadir() + .path('FOLDER') + .upToDate() + .create() + const doc = await builders + .metadir() + .moveFrom(old) + .overwrite(overwritten) + .path('MOVED') + .changedSide(otherSide(this.side)) + .create() + + const sideEffects = await mergeSideEffects(this, () => + this.merge.deleteFolderAsync(this.side, _.cloneDeep(doc)) + ) + + should(sideEffects).deepEqual({ + savedDocs: [ + _.defaults( + { + sides: increasedSides( + overwritten.sides, + otherSide(this.side), + 1 + ), + trashed: true + }, + // XXX: overwritten is recreated so we don't keep its _id + _.omit(overwritten, ['_id', '_rev']) + ), + _.defaults( + { + // XXX: folder was moved on local side so the remote side is + // increased by 2 to compensate. + sides: increasedSides(doc.sides, this.side, 2), + trashed: true + }, + _.omit(doc, ['_rev', 'moveFrom', 'overwrite']) + ) + ], + resolvedConflicts: [] + }) + }) }) }) - }) - context('when a trashed record is found in Pouch', () => { - it('marks it for deletion and updates sides info', async function() { - const was = await builders - .metadir() - .path('FOLDER') - .trashed() - .changedSide(otherSide(this.side)) - .create() - const doc = builders - .metadir(was) - .trashed() - .unmerged(this.side) - .build() + context('and it was moved on the remote Cozy', () => { + it('marks the previous location for deletion on the local filesystem', async function() { + const old = await builders + .metadir() + .path('FOLDER') + .upToDate() + .create() + const doc = await builders + .metadir() + .moveFrom(old) + .path('MOVED') + .changedSide(this.side) + .create() - const sideEffects = await mergeSideEffects(this, () => - this.merge.deleteFolderAsync(this.side, _.cloneDeep(doc)) - ) + const sideEffects = await mergeSideEffects(this, () => + this.merge.deleteFolderAsync(this.side, _.cloneDeep(doc)) + ) - should(sideEffects).deepEqual({ - savedDocs: [ - _.defaults( - { - // We increase the side by 2 since the other side was increased - // when `was` was trashed - sides: increasedSides(was.sides, this.side, 2), - [this.side]: doc[this.side], - deleted: true - }, - _.omit(was, ['_rev']) - ) - ], - resolvedConflicts: [] + should(sideEffects).deepEqual({ + savedDocs: [ + _.defaults( + { + // XXX: folder was moved on local side so the remote side is + // increased by 2 to compensate. + sides: increasedSides(doc.sides, this.side, 1), + path: doc.local.path, + trashed: true + }, + _.omit(doc, ['_rev', 'moveFrom']) + ) + ], + resolvedConflicts: [] + }) }) }) }) @@ -4265,14 +4394,10 @@ describe('Merge', function() { const was = await builders .metadir() .path('FOLDER') - .deleted() + .trashed() .changedSide(otherSide(this.side)) .create() - const doc = builders - .metadir(was) - .trashed() - .unmerged(this.side) - .build() + const doc = builders.metadir(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.deleteFolderAsync(this.side, _.cloneDeep(doc)) @@ -4286,7 +4411,7 @@ describe('Merge', function() { // when `was` was marked for deletion sides: increasedSides(was.sides, this.side, 2), [this.side]: doc[this.side], - deleted: true + trashed: true }, _.omit(was, ['_rev']) ) @@ -4304,10 +4429,7 @@ describe('Merge', function() { .metafile() .upToDate() .create() - const doc = builders - .metafile(was) - .trashed() - .build() + const doc = builders.metafile(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFileAsync( @@ -4322,45 +4444,7 @@ describe('Merge', function() { _.defaults( { sides: increasedSides(was.sides, this.side, 1), - deleted: true - }, - _.omit(was, ['_rev']) - ) - ], - resolvedConflicts: [] - }) - }) - }) - - context('when a trashed record is found in Pouch', () => { - it('marks it for deletion and updates sides info', async function() { - const was = await builders - .metafile() - .trashed() - .changedSide(otherSide(this.side)) - .create() - const doc = builders - .metafile(was) - .trashed() - .build() - - const sideEffects = await mergeSideEffects(this, () => - this.merge.trashFileAsync( - this.side, - _.cloneDeep(was), - _.cloneDeep(doc) - ) - ) - - should(sideEffects).deepEqual({ - savedDocs: [ - _.defaults( - { - // We increase the side by 2 since the other side was increased - // when `was` was trashed - sides: increasedSides(was.sides, this.side, 2), - [this.side]: doc[this.side], - deleted: true + trashed: true }, _.omit(was, ['_rev']) ) @@ -4375,13 +4459,10 @@ describe('Merge', function() { it('completely erases the document from PouchDB', async function() { const was = await builders .metafile() - .deleted() + .trashed() .changedSide(otherSide(this.side)) .create() - const doc = builders - .metafile(was) - .trashed() - .build() + const doc = builders.metafile(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFileAsync( @@ -4405,16 +4486,13 @@ describe('Merge', function() { }) context('and the record was modified on the same side', () => { - it('keeps the deletion marker and updates sides info', async function() { + it('does nothing', async function() { const was = await builders .metafile() - .deleted() + .trashed() .changedSide(this.side) .create() - const doc = builders - .metafile(was) - .trashed() - .build() + const doc = builders.metafile(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFileAsync( @@ -4425,16 +4503,7 @@ describe('Merge', function() { ) should(sideEffects).deepEqual({ - savedDocs: [ - _.defaults( - { - sides: increasedSides(was.sides, this.side, 1), - [this.side]: doc[this.side], - deleted: true - }, - _.omit(was, ['_rev']) - ) - ], + savedDocs: [], resolvedConflicts: [] }) }) @@ -4443,11 +4512,14 @@ describe('Merge', function() { context('when no records are found in Pouch', () => { it('does nothing', async function() { - const was = builders.metafile().build() - const doc = builders - .metafile(was) - .trashed() - .build() + const was = await builders + .metafile() + .upToDate() + .create() + const doc = builders.metafile(was).build() + + // Erase record from PouchDB + await this.pouch.eraseDocument(was) const sideEffects = await mergeSideEffects(this, () => this.merge.trashFileAsync( @@ -4473,7 +4545,6 @@ describe('Merge', function() { const doc = builders .metadir() .path(was.path) - .trashed() .build() const sideEffects = await mergeSideEffects(this, () => @@ -4497,10 +4568,7 @@ describe('Merge', function() { .metafile() .sides({ [this.side]: 1 }) .create() - const doc = builders - .metafile(was) - .trashed() - .build() + const doc = builders.metafile(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFileAsync( @@ -4516,7 +4584,7 @@ describe('Merge', function() { { sides: increasedSides(was.sides, this.side, 1), [this.side]: doc[this.side], - deleted: true + trashed: true }, _.omit(was, ['_rev']) ) @@ -4544,10 +4612,7 @@ describe('Merge', function() { .moveFrom(src) .changedSide(this.side) .create() - const doc = builders - .metafile(was) - .trashed() - .build() + const doc = builders.metafile(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFileAsync( @@ -4561,7 +4626,7 @@ describe('Merge', function() { savedDocs: [ _.defaults( { - deleted: true, + trashed: true, sides: increasedSides(was.sides, this.side, 1) }, _.omit(was, ['_rev', 'moveFrom']) @@ -4589,10 +4654,7 @@ describe('Merge', function() { .overwrite(existing) .changedSide(this.side) .create() - const doc = builders - .metafile(was) - .trashed() - .build() + const doc = builders.metafile(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFileAsync( @@ -4606,14 +4668,14 @@ describe('Merge', function() { savedDocs: [ _.defaults( { - deleted: true, + trashed: true, sides: increasedSides(existing.sides, this.side, 1) }, _.omit(existing, ['_rev']) ), _.defaults( { - deleted: true, + trashed: true, sides: increasedSides(was.sides, this.side, 1) }, _.omit(was, ['_rev', 'moveFrom', 'overwrite']) @@ -4637,10 +4699,7 @@ describe('Merge', function() { .data('updated') .changedSide(this.side) .create() - const doc = builders - .metafile(was) - .trashed() - .build() + const doc = builders.metafile(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFileAsync( @@ -4656,7 +4715,7 @@ describe('Merge', function() { { sides: increasedSides(was.sides, this.side, 1), [this.side]: doc[this.side], - deleted: true + trashed: true }, _.omit(was, ['_rev']) ) @@ -4678,10 +4737,7 @@ describe('Merge', function() { .data('updated') .changedSide(otherSide(this.side)) .create() - const doc = builders - .metafile(was) - .trashed() - .build() + const doc = builders.metafile(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFileAsync( @@ -4719,10 +4775,7 @@ describe('Merge', function() { .moveFrom(src) .changedSide('remote') .create() - const doc = builders - .metafile(was) - .trashed() - .build() + const doc = builders.metafile(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFileAsync( @@ -4811,10 +4864,7 @@ describe('Merge', function() { .metadir() .upToDate() .create() - const doc = builders - .metadir(was) - .trashed() - .build() + const doc = builders.metadir(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFolderAsync( @@ -4829,45 +4879,7 @@ describe('Merge', function() { _.defaults( { sides: increasedSides(was.sides, this.side, 1), - deleted: true - }, - _.omit(was, ['_rev']) - ) - ], - resolvedConflicts: [] - }) - }) - }) - - context('when a trashed record is found in Pouch', () => { - it('marks it for deletion and updates sides info', async function() { - const was = await builders - .metadir() - .trashed() - .changedSide(otherSide(this.side)) - .create() - const doc = builders - .metadir(was) - .trashed() - .build() - - const sideEffects = await mergeSideEffects(this, () => - this.merge.trashFolderAsync( - this.side, - _.cloneDeep(was), - _.cloneDeep(doc) - ) - ) - - should(sideEffects).deepEqual({ - savedDocs: [ - _.defaults( - { - // We increase the side by 2 since the other side was increased - // when `was` was trashed - sides: increasedSides(was.sides, this.side, 2), - [this.side]: doc[this.side], - deleted: true + trashed: true }, _.omit(was, ['_rev']) ) @@ -4882,13 +4894,10 @@ describe('Merge', function() { it('completely erases the record from PouchDB', async function() { const was = await builders .metadir() - .deleted() + .trashed() .changedSide(otherSide(this.side)) .create() - const doc = builders - .metadir(was) - .trashed() - .build() + const doc = builders.metadir(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFolderAsync( @@ -4912,16 +4921,13 @@ describe('Merge', function() { }) context('and the record was modified on the same side', () => { - it('keeps the deletion marker and updates sides info', async function() { + it('does nothing', async function() { const was = await builders .metadir() - .deleted() + .trashed() .changedSide(this.side) .create() - const doc = builders - .metadir(was) - .trashed() - .build() + const doc = builders.metadir(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFolderAsync( @@ -4932,16 +4938,7 @@ describe('Merge', function() { ) should(sideEffects).deepEqual({ - savedDocs: [ - _.defaults( - { - sides: increasedSides(was.sides, this.side, 1), - [this.side]: doc[this.side], - deleted: true - }, - _.omit(was, ['_rev']) - ) - ], + savedDocs: [], resolvedConflicts: [] }) }) @@ -4950,11 +4947,14 @@ describe('Merge', function() { context('when no records are found in Pouch', () => { it('does nothing', async function() { - const was = builders.metadir().build() - const doc = builders - .metadir(was) - .trashed() - .build() + const was = await builders + .metadir() + .upToDate() + .create() + const doc = builders.metadir(was).build() + + // Erase record from PouchDB + await this.pouch.eraseDocument(was) const sideEffects = await mergeSideEffects(this, () => this.merge.trashFolderAsync( @@ -4980,7 +4980,6 @@ describe('Merge', function() { const doc = builders .metafile() .path(was.path) - .trashed() .build() const sideEffects = await mergeSideEffects(this, () => @@ -5004,10 +5003,7 @@ describe('Merge', function() { .metadir() .sides({ [this.side]: 1 }) .create() - const doc = builders - .metadir(was) - .trashed() - .build() + const doc = builders.metadir(was).build() const sideEffects = await mergeSideEffects(this, () => this.merge.trashFolderAsync( @@ -5023,7 +5019,7 @@ describe('Merge', function() { { sides: increasedSides(was.sides, this.side, 1), [this.side]: doc[this.side], - deleted: true + trashed: true }, _.omit(was, ['_rev']) ) diff --git a/test/unit/pouch/index.js b/test/unit/pouch/index.js index c0e9da35f..cd2bbeca3 100644 --- a/test/unit/pouch/index.js +++ b/test/unit/pouch/index.js @@ -609,7 +609,7 @@ describe('Pouch', function() { await builders .metafile() .path('deleted') - .deleted() + .trashed() .changedSide('local') .create() diff --git a/test/unit/prep.js b/test/unit/prep.js index 1afe7e5c9..d2b3dc32b 100644 --- a/test/unit/prep.js +++ b/test/unit/prep.js @@ -2,11 +2,9 @@ const sinon = require('sinon') const should = require('should') -const path = require('path') const { Ignore } = require('../../core/ignore') const Prep = require('../../core/prep') -const { TRASH_DIR_NAME } = require('../../core/remote/constants') describe('Prep', function() { beforeEach('instanciate prep', function() { @@ -367,7 +365,7 @@ describe('Prep', function() { .then(() => should.fail(), err => err.should.match(/Invalid path/)) }) - it('generates a doc when none is passed', async function() { + it('calls Merge with the trashed record when none is passed', async function() { const was = { path: 'file-to-be-trashed', md5sum: 'rcg7GeeTSRscbqD9i0bNnw==' @@ -375,13 +373,9 @@ describe('Prep', function() { await this.prep.trashFileAsync(this.side, was) - should(this.merge.trashFileAsync).be.calledOnce() - should(this.merge.trashFileAsync).be.calledWith(this.side, was, { - ...was, - path: path.join(TRASH_DIR_NAME, was.path), - trashed: true, - docType: 'file' - }) + should(this.merge.trashFileAsync) + .be.calledOnce() + .and.be.calledWith(this.side, was, was) }) // FIXME @@ -403,18 +397,14 @@ describe('Prep', function() { .then(() => should.fail(), err => err.should.match(/Invalid path/)) }) - it('generates a doc when none is passed', async function() { + it('calls Merge with the trashed record when none is passed', async function() { const was = { path: 'folder-to-be-trashed' } await this.prep.trashFolderAsync(this.side, was) - should(this.merge.trashFolderAsync).be.calledOnce() - should(this.merge.trashFolderAsync).be.calledWith(this.side, was, { - ...was, - path: path.join(TRASH_DIR_NAME, was.path), - trashed: true, - docType: 'folder' - }) + should(this.merge.trashFolderAsync) + .be.calledOnce() + .and.be.calledWith(this.side, was, was) }) // FIXME diff --git a/test/unit/remote/cozy.js b/test/unit/remote/cozy.js index 7db2dd77b..8234113cd 100644 --- a/test/unit/remote/cozy.js +++ b/test/unit/remote/cozy.js @@ -387,22 +387,10 @@ describe('RemoteCozy', function() { }) describe('changes', function() { - it('resolves with changes since then given seq', async function() { - const { last_seq } = await remoteCozy.changes() - - const dir = await builders.remoteDir().create() - const file = await builders - .remoteFile() - .inDir(dir) - .create() - - const { docs } = await remoteCozy.changes(last_seq) - const ids = docs.map(doc => doc._id) - - should(ids.sort()).eql([file._id, dir._id].sort()) - }) - context('when no seq given', function() { + // XXX: This test might timeout if a lot of changes were made on the + // remote Cozy as we're doing an initial fetch here and thus cannot speed + // it up by ignoring the previous changes. it('resolves only with non trashed, non deleted docs', async function() { const dir = await builders.remoteDir().create() const file = await builders @@ -432,7 +420,24 @@ describe('RemoteCozy', function() { }) }) + it('resolves with changes since the given seq', async function() { + const last_seq = await remoteCozy.fetchLastSeq() + + const dir = await builders.remoteDir().create() + const file = await builders + .remoteFile() + .inDir(dir) + .create() + + const { docs } = await remoteCozy.changes(last_seq) + const ids = docs.map(doc => doc._id) + + should(ids.sort()).eql([file._id, dir._id].sort()) + }) + it('resolves with docs ordered by path asc', async function() { + const last_seq = await remoteCozy.fetchLastSeq() + const dirB = await builders .remoteDir() .inRootDir() @@ -454,7 +459,7 @@ describe('RemoteCozy', function() { .name('fileA') .create() - const { docs } = await remoteCozy.changes() + const { docs } = await remoteCozy.changes(last_seq) should(docs).containDeepOrdered([dirA, fileA, dirB, fileB]) }) diff --git a/test/unit/remote/index.js b/test/unit/remote/index.js index 74bdac734..5a78c3569 100644 --- a/test/unit/remote/index.js +++ b/test/unit/remote/index.js @@ -173,7 +173,7 @@ describe('remote.Remote', function() { } await this.remote.addFileAsync(doc) should(doc) - .have.property('deleted') + .have.property('trashed') .and.not.have.property('remote') }) @@ -381,7 +381,7 @@ describe('remote.Remote', function() { await this.remote.overwriteFileAsync(doc) should(doc) - .have.property('deleted') + .have.property('trashed') .and.not.have.propertyByPath('remote') }) diff --git a/test/unit/remote/watcher.js b/test/unit/remote/watcher.js index 572202211..6d2489a2b 100644 --- a/test/unit/remote/watcher.js +++ b/test/unit/remote/watcher.js @@ -2455,7 +2455,7 @@ describe('RemoteWatcher', function() { const trashedFile = builders .metafile() .fromRemote(origFile) - .deleted() + .trashed() .build() const newFile = builders .remoteFile(origFile) @@ -2484,7 +2484,7 @@ describe('RemoteWatcher', function() { const trashedFile = builders .metafile() .fromRemote(origFile) - .deleted() + .trashed() .changedSide('local') .build() const movedFile = builders diff --git a/test/unit/sync/index.js b/test/unit/sync/index.js index 4f295a551..bf44ffa4f 100644 --- a/test/unit/sync/index.js +++ b/test/unit/sync/index.js @@ -361,9 +361,9 @@ describe('Sync', function() { this.remote ) should(this.remote.trashAsync).not.have.been.called() - should( - await this.pouch.bySyncedPath(deletedChild.path) - ).have.properties(Object.keys(deletedChild)) + // XXX: the child change is erased after we've skipped it since it is + // marked as `deleted`. + should(await this.pouch.bySyncedPath(deletedChild.path)).be.undefined() } finally { this.sync.trashWithParentOrByItself.restore() } @@ -599,7 +599,7 @@ describe('Sync', function() { const doc = await builders .metafile() .path('foo') - .deleted() + .trashed() .changedSide('remote') .create() await this.sync.applyDoc(doc, this.local, 'local') @@ -610,7 +610,7 @@ describe('Sync', function() { const doc = await builders .metafile() .path('tmp/fooz') - .deleted() + .trashed() .sides({ local: 2 }) .create() await this.sync.applyDoc(doc, this.remote, 'remote') @@ -668,7 +668,7 @@ describe('Sync', function() { const doc = await builders .metadir() .path('baz') - .deleted() + .trashed() .changedSide('remote') .create() await this.sync.applyDoc(doc, this.local, 'local') @@ -679,7 +679,7 @@ describe('Sync', function() { const doc = await builders .metadir() .path('tmp/foobaz') - .deleted() + .trashed() .sides({ local: 2 }) .create() await this.sync.applyDoc(doc, this.remote, 'remote')