Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Handle deletion/move after move on macOS #2336

Merged

Conversation

taratatach
Copy link
Member

@taratatach taratatach commented Jul 4, 2024

Please make sure the following boxes are checked:

  • PR is not too big
  • it improves UX & DX in some way
  • it includes unit tests matching the implementation changes
  • it includes scenarios matching a new behaviour or has been manually tested
  • it includes relevant documentation

@taratatach taratatach self-assigned this Jul 4, 2024
@taratatach taratatach force-pushed the fix/chokidar-handles-deletion-and-move-after-complete-move branch 2 times, most recently from 8e97658 to dbf3b75 Compare July 8, 2024 16:24
  When a file or directory is moved on the local filesystem on macOS and
  then deleted (or moved again), then the second deletion event will
  find an existing document in PouchDB as the deleted document still has
  the same inode but this document has an outdated path as the first
  move has not yet been merged.
  This would lead to the incorrect detection of the deletion of the file
  or directory at its initial path which would overwrite the first move
  and lead to unexpected results.

  We were already handling this situation when the second move was very
  close to the first one (i.e. the second move would occur before we had
  the time to compute the checksum of the moved file) but we were not
  handling it properly when the second move was occurring slightly later
  (i.e. a few seconds, less than 10 or the first move would be flushed).

  We will now make sure to remove the second deletion event's `old`
  Pouch document in this situation as it is outdated and rely on the
  path of events to match them.
  When processing Chokidar events, we can either:
  - detect a new change
  - update an existing change (e.g. turn a deletion into a move)
  - do nothing

  Each time we detect a new change, we store it in a map using its
  `path` as key so we can find it later and group it with another event.
  In this case, we update the change in place and store it again in the
  map using its new `path` as key.

  However, when the new and old `path` differ (e.g. when turning a
  deletion into a move), we would leave an entry with the old `path` in
  the map.
  This could lead to unexpected behavior if later events target that
  path altough the existing change it maps to is about a document that
  does not exist there anymore.

  Therefore we'll now look for entries mapping to the updated change and
  remove them before adding the new one.
@taratatach taratatach force-pushed the fix/chokidar-handles-deletion-and-move-after-complete-move branch from dbf3b75 to 296c175 Compare July 31, 2024 14:49
@taratatach taratatach changed the title WIP: fix: Handle deletion/move after complete local move in chokidar fix: Handle deletion/move after move on macOS Jul 31, 2024
@taratatach taratatach marked this pull request as ready for review July 31, 2024 14:55
@taratatach taratatach force-pushed the fix/chokidar-handles-deletion-and-move-after-complete-move branch 2 times, most recently from 8489018 to 296c175 Compare August 2, 2024 12:42
@taratatach taratatach merged commit 1933830 into master Aug 2, 2024
19 of 32 checks passed
@taratatach taratatach deleted the fix/chokidar-handles-deletion-and-move-after-complete-move branch August 2, 2024 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant