Skip to content

Commit

Permalink
#24908: Moved the first corruption check after the second one and onl…
Browse files Browse the repository at this point in the history
…y if unsuccessful to not degrade save times + code review changes
  • Loading branch information
krasko78 committed Sep 27, 2024
1 parent 84b06a5 commit 6a0c117
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 27 deletions.
3 changes: 1 addition & 2 deletions src/framework/global/io/devtools/allzerosfilecorruptor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,9 @@ AllZerosFileCorruptor::AllZerosFileCorruptor(const path_t& filePath)
{
}

size_t AllZerosFileCorruptor::writeData(const uint8_t* data, size_t len)
size_t AllZerosFileCorruptor::writeData(const uint8_t*, size_t len)
{
// Ignore the actual data and write all zeros so as to corrupt the file.
Q_UNUSED(data);
uint8_t* corruptData = new uint8_t[len];
memset(corruptData, 0, len);
return File::writeData(corruptData, len);
Expand Down
25 changes: 12 additions & 13 deletions src/project/internal/notationproject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -657,17 +657,7 @@ Ret NotationProject::doSave(const muse::io::path_t& path, engraving::MscIoMode i
} else {
Ret ret = muse::make_ok();

if (!isAutosave) {
ret = checkSavedFileForCorruption(ioMode, savePath, targetMainFileName.toQString());
if (!ret) {
if (ret.code() == (int)Err::CorruptionUponSavingError) {
ret.setData("corruptedAfterMove", false);
}
return ret;
}
}

ret = fileSystem()->move(savePath, targetContainerPath, true);
ret = fileSystem()->copy(savePath, targetContainerPath, true);
if (!ret) {
return ret;
}
Expand All @@ -676,11 +666,19 @@ Ret NotationProject::doSave(const muse::io::path_t& path, engraving::MscIoMode i
ret = checkSavedFileForCorruption(ioMode, targetContainerPath, targetMainFileName.toQString());
if (!ret) {
if (ret.code() == (int)Err::CorruptionUponSavingError) {
ret.setData("corruptedAfterMove", true);
// Validate the temporary "saving" file too.
Ret ret2 = checkSavedFileForCorruption(ioMode, savePath, targetMainFileName.toQString());
if (ret2.code() == (int)Err::CorruptionUponSavingError) {
ret2.setData("savingFileIsCorrupt", true);
return ret2;
}
}
return ret;
}
}

// Remove the temp save file (not problematic if fails)
fileSystem()->remove(savePath);
}
}

Expand Down Expand Up @@ -815,7 +813,8 @@ Ret NotationProject::saveSelectionOnScore(const muse::io::path_t& path)
return ret;
}

Ret NotationProject::checkSavedFileForCorruption(MscIoMode ioMode, const muse::io::path_t path, const muse::io::path_t scoreFileName)
Ret NotationProject::checkSavedFileForCorruption(MscIoMode ioMode, const muse::io::path_t& path,
const muse::io::path_t& scoreFileName)
{
TRACEFUNC;

Expand Down
6 changes: 3 additions & 3 deletions src/project/internal/notationproject.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,11 @@ class NotationProject : public INotationProject, public muse::Injectable, public
bool createThumbnail = true, bool isAutosave = false);
muse::Ret saveSelectionOnScore(const muse::io::path_t& path = muse::io::path_t());
muse::Ret exportProject(const muse::io::path_t& path, const std::string& suffix);
muse::Ret doSave(const muse::io::path_t& path, engraving::MscIoMode ioMode, bool generateBackup = true,
bool createThumbnail = true, bool isAutosave = false);
muse::Ret doSave(const muse::io::path_t& path, engraving::MscIoMode ioMode, bool generateBackup = true, bool createThumbnail = true,
bool isAutosave = false);
muse::Ret makeCurrentFileAsBackup();
muse::Ret writeProject(engraving::MscWriter& msczWriter, bool onlySelection, bool createThumbnail = true);
muse::Ret checkSavedFileForCorruption(engraving::MscIoMode ioMode, const muse::io::path_t path, const muse::io::path_t scoreFileName);
muse::Ret checkSavedFileForCorruption(engraving::MscIoMode ioMode, const muse::io::path_t& path, const muse::io::path_t& scoreFileName);

void listenIfNeedSaveChanges();
void markAsSaved(const muse::io::path_t& path);
Expand Down
18 changes: 9 additions & 9 deletions src/project/internal/projectactionscontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1585,19 +1585,19 @@ void ProjectActionsController::warnScoreCorruptAfterSave(const Ret& ret)
{
std::string errMessage = ret.toString();

std::any corruptedAfterMove = ret.data("corruptedAfterMove");
std::string originalFileCorruptedMessage = corruptedAfterMove.has_value() && std::any_cast<bool>(corruptedAfterMove)
? muse::trc("project/save", "Sadly, your file has become corrupt.\n\n")
: "";
std::any savingFileIsCorrupt = ret.data("savingFileIsCorrupt");
std::string savingFileIsCorruptMessage = savingFileIsCorrupt.has_value() && std::any_cast<bool>(savingFileIsCorrupt)
? ""
: muse::trc("project/save", " or from the _saving file");

std::string message = muse::qtrc("project/save",
"An error occurred while saving your file. This may be a one-off error. "
"Please retry saving the file in order not to lose your latest changes.\n\n"
"An error occurred while saving your file and sadly it has become corrupt. "
"This may be a one-off error. Please retry saving the file in order not to lose your latest changes.\n\n"
"If it keeps happening, try saving the file to a different location or to MuseScore.com. "
"Go to the “Support and bug reports” forum at https://musescore.org/en/forum for help.\n\n"
"%1"
"If all else fails, restore the file from the most recent backup created by MuseScore Studio%1.\n\n"
"If you need help, go to the “Support and bug reports” forum at https://musescore.org/en/forum\n\n"
"Error: %2")
.arg(originalFileCorruptedMessage.c_str(), errMessage.c_str())
.arg(savingFileIsCorruptMessage.c_str(), errMessage.c_str())
.toStdString();

interactive()->warning(muse::trc("project/save", "Your score could not be saved"), message);
Expand Down

0 comments on commit 6a0c117

Please sign in to comment.