Skip to content

Commit

Permalink
Stop preventing modified folder deletions (#2186)
Browse files Browse the repository at this point in the history
While we were preventing the deletion of a folder whose content has
changed on the other side, this is getting in the way of the partial
synchronization feature.
Since this content can always be retrieved from the Trash either on the
local OS or the remote Cozy, we believe it is safe and easier to trash
the folder anyway.

We tried to merge logic handling trashed and deleted folders together
(thus all the refactoring done in this PR) but it still requires a lot
of work to be done, especially on the types side so we'll leave this for
another time while keeping the refactoring already done.
  • Loading branch information
taratatach authored Jan 5, 2022
2 parents 4c1d9cb + 35cfea2 commit fe6dcdb
Show file tree
Hide file tree
Showing 32 changed files with 731 additions and 616 deletions.
4 changes: 2 additions & 2 deletions core/local/atom/dispatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions core/local/atom/incomplete_fixer.js
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,15 @@ 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
// path in Pouch.
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
Expand Down
2 changes: 1 addition & 1 deletion core/local/atom/win_identical_renaming.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion core/local/chokidar/prepare_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const oldMetadata = async (
) /*: Promise<?Metadata> */ => {
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
}

/**
Expand Down
22 changes: 18 additions & 4 deletions core/local/chokidar/send_to_prep.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 }))
}

Expand All @@ -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 }))
}

Expand Down Expand Up @@ -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':
Expand Down
Loading

0 comments on commit fe6dcdb

Please sign in to comment.