24908: Retried saves won't create backups and backups will be created for the target file

[krasko78]

Resolves: #24908 (continuation)

Key points:

• Retried saves upon corruption won't create backups. On the first save, the target file will be corrupted already so backing it up won't help and will overwrite the healthy backup file created on the first save attempt. We want the healthy backup file to remain intact.

• I've changed the code to create a backup of the file being saved to. Currently MSS always backs up the original file even if you are doing a "Save as" and saving to another file. On the Corrupt dialog we offer a Save as button and don't want to overwrite the healhy backup with the corrupted original file when doing a "Save as". This would defeat the whole idea of this PR (which is to make sure a healthy backup file will be available if MSS corrupts a file).

• I signed the CLA

• The title of the PR describes the problem it addresses

• Each commit's message describes its purpose and effects, and references the issue it resolves

• If changes are extensive, there is a sequence of easily reviewable commits

• The code in the PR follows the coding rules

• There are no unnecessary changes

• The code compiles and runs on my machine, preferably after each commit individually

• I created a unit test or vtest to verify the changes I made (if applicable)

[cbjeukendrup]

While this is okay, it seems not ideal that whether a backup is created now depends on the forceNoCreateBackup argument and on the configuration()->createBackupBeforeSaving(); call inside makeBackup.

It seems even questionable to me whether that configuration()->createBackupBeforeSaving(); call belongs there at all.

I would propose to pass always a createBackup argument to INotationProject::save, perhaps with a convenient default value.
And, in ProjectActionsController, create two overloads of saveProjectLocally like the following:

bool ProjectActionsController::saveProjectLocally(const muse::io::path_t& filePath, SaveMode saveMode)
{
    bool createBackup = configuration()->createBackupBeforeSaving();
    return saveProjectLocally(filePath, saveMode, createBackup);
}

bool ProjectActionsController::saveProjectLocally(const muse::io::path_t& filePath, SaveMode saveMode, bool createBackup)
{
    // …

If you have time to make this change now, that would be great. Otherwise we can merge this as is, and do it later.

[krasko78]

How about now? I couldn't do exactly what you are proposing since INotationProject::save is called from ProjectActionsController::uploadProject as well. So I think we need configuration()->createBackupBeforeSaving() to be inside INotationProject::save.

[cbjeukendrup]

Ah I see. Then this seems a sensible compromise!

[krasko78]

QA notes: When MSS shows the corruption dialog upon save, it has already corrupted the score file (because we check the target file first rather than the temporary .saving file). At that moment there will be a healthy backup in the backup folder. if you click the Try again button on the dialog and the save fails again, MSS will show the dialog again but the healthy backup file in the backups folder will be all zeros, i.e. lost. With this PR we want to prevent that and ensure the healthy backup file will remain intact no matter how many times the user presses the Try again and Save as buttons on the dialog.

Caveat: once the user dismisses the corruption dialog and tries to save the score again, if the save fails, they will lose the healthy backup. But this is outside of the scope of this PR.

I think the easiest way to test this PR is do it the way I've done it:

1. Start out with a healthy score but rename it to whatever - ALL_ZEROS_CORRUPTED.mscz.
2. Open it in MSS. It opens fine albeit the scary name. :)
3. Make a change and click Save.
4. Since the name of the file is the special one that corrupts the score on save, MSS will corrupt the file and display the corruption dialog.
5. Make sure there is a healthy backup of whatever - ALL_ZEROS_CORRUPTED.mscz in your backups folder.
6. On the corruption dialog click Try again as many times as you want. The dialog will reappear again and again.
7. Click Save as... and save the file under a different name not ending in - ALL_ZEROS_CORRUPTED.mscz so the file saves successfully.
8. Ensure the healthy backup of whatever - ALL_ZEROS_CORRUPTED.mscz in your backups folder is still there and intact.

One more scenario to test: currently MSS creates a backup of the original score file even if you are saving under a new filename. For example, you open A.mscz and save it as B.mscz. B.mscz will be created and a backup of A.mscz will be created in the backups folder. Not only that, but if B.mszc already existed, it would be lost (no backup). So we want to test out this and make sure that with this PR a backup of B.mscz will be created (if it exists) and not of A.mscz. We are not saving to A.mscz so it makes no sense to create a backup for it.

Hope this makes sense. Any questions let me know! Thanks!