fix(welcome): remove the sample database journal files on reset - #2985
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reset Sample Database replaced
Chinook.sqliteand left its-journal,-waland-shmfiles next to the fresh copy. SQLite treats a leftover hot journal as an unfinished transaction and rolls it back into whatever file now has that name, and it replays a leftover-walthe same way. So a reset could hand back a sample that was neither the original nor the user's edited copy. #2983 fixed the iOS installer. This is the macOS side of the same bug, plus the same bug in the remote-file fetch it shares a rule with.Root cause
Two places replace a SQLite file and decide which of its sidecars to delete:
SampleDatabaseService.resetToBundledremoved only the main file.installIfNeededremoved nothing, so an install over a missing database adopted journal files that a failed reset had left behind.RemoteDatabaseFileTransfer.clearStaleSidecarskept every sidecar the fetch plan listed, not the ones the fetch actually downloaded. A-walthe server checkpointed away, or a-journalwhose transaction ended between planning and fetching, was never downloaded, so the local file with that name still belonged to the previous copy and was kept.DatabaseFileLayout.staleAfterReplaceSuffixesalso left out-journalfor the SQLite family.Changes
DatabaseFileLayout.sqliteFamily.staleAfterReplaceSuffixesnow includes-journal.SampleDatabaseServiceremoves the main file and every sidecar before it copies, on both reset and install. When the installed database is still there, install never touches its journal files.fetchSidecarsreturns the suffixes it downloaded, andclearStaleSidecarskeeps only those. ARemoteFileSourceprotocol overLibSSH2SFTPSessionlets the tests drive a server whose sidecar disappears between plan and fetch.Tests
SampleDatabaseServiceTests,RemoteDatabaseFileTests,RemoteDatabaseFileCorrectnessTests: 37 cases, all passing on the rebased tree. The new cases open real SQLite files with a hot journal and a live-waland check that a reset never rolls back or replays them over the fresh copy.connectionInUseand leaves the files alone.No UI automation: the change is inside a file operation. The user flow (Reset Sample Database in the welcome window) is unchanged.