Skip to content

Commit

Permalink
#24908: Validate saved files and alert user
Browse files Browse the repository at this point in the history
  • Loading branch information
krasko78 committed Sep 24, 2024
1 parent 090793d commit 13f2353
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 3 deletions.
24 changes: 23 additions & 1 deletion src/engraving/infrastructure/mscwriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,24 @@ using namespace muse;
using namespace muse::io;
using namespace mu::engraving;

// FileCorruptor
FileCorruptor::FileCorruptor(const path_t& filePath)
: File(filePath)
{
}

size_t FileCorruptor::writeData(const uint8_t* data, size_t len)
{
// Ignore the actual data and write only zeros
Q_UNUSED(data)
uint8_t* zerosData = new uint8_t[len];
for (size_t i = 0; i < len; i++) {
zerosData[i] = 0;
}
return File::writeData(zerosData, len);
}

// MscWriter
MscWriter::MscWriter(const Params& params)
: m_params(params)
{
Expand Down Expand Up @@ -266,7 +284,11 @@ Ret MscWriter::ZipFileWriter::open(io::IODevice* device, const path_t& filePath)
{
m_device = device;
if (!m_device) {
m_device = new File(filePath);
// Create a corrupted file if the filename contains " - CORRUPTED.mscz".
// This is only for DEV/QA testing and will not be included in a release.
m_device = filePath.toQString().contains(" - CORRUPTED.mscz")
? new FileCorruptor(filePath)
: new File(filePath);
m_selfDeviceOwner = true;
}

Expand Down
12 changes: 12 additions & 0 deletions src/engraving/infrastructure/mscwriter.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "types/ret.h"
#include "io/path.h"
#include "io/iodevice.h"
#include "io/file.h"
#include "mscio.h"

namespace muse {
Expand All @@ -34,6 +35,17 @@ class TextStream;
}

namespace mu::engraving {
// FileCorruptor: a class that writes only zeros to a file no matter what it is passed
class FileCorruptor : public muse::io::File
{
public:
FileCorruptor(const muse::io::path_t& filePath);

protected:
size_t writeData(const uint8_t* data, size_t len) override;
};

// MscWriter
class MscWriter
{
public:
Expand Down
55 changes: 54 additions & 1 deletion src/project/internal/notationproject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -641,8 +641,24 @@ Ret NotationProject::doSave(const muse::io::path_t& path, engraving::MscIoMode i
LOGW() << ret.toString();
}
} else {
Ret ret = fileSystem()->move(savePath, targetContainerPath, true);
Ret 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);
if (!ret) {
return ret;
}

ret = checkSavedFileForCorruption(ioMode, targetContainerPath, targetMainFileName.toQString());
if (!ret) {
if (ret.code() == (int)Err::CorruptionUponSavingError) {
ret.setData("corruptedAfterMove", true);
}
return ret;
}
}
Expand Down Expand Up @@ -779,6 +795,43 @@ Ret NotationProject::saveSelectionOnScore(const muse::io::path_t& path)
return ret;
}

Ret NotationProject::checkSavedFileForCorruption(engraving::MscIoMode ioMode, QString path, QString scoreFileName)
{
if (ioMode != MscIoMode::Zip) {
return muse::make_ok();
}

// TODO: Execute this on Windows only?

MscReader::Params params;
params.filePath = path;
params.mainFileName = scoreFileName;
params.mode = MscIoMode::Zip;

MscReader msczReader(params);
Ret ret = msczReader.open();
if (!ret) {
return Ret(static_cast<int>(Err::CorruptionUponSavingError),
muse::mtrc("project/save", "File “%1” could not be opened. %2")
.arg(path)
.arg(String(ret.toString().c_str()))
.toStdString());
}

// Try to extract the main score file.
ByteArray scoreFile = msczReader.readScoreFile();
msczReader.close();

if (scoreFile.empty()) {
return Ret(static_cast<int>(Err::CorruptionUponSavingError),
muse::mtrc("project/save", "File “%1” is corrupted or damaged: the score file could not be read.")
.arg(path)
.toStdString());
}

return muse::make_ok();
}

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

void listenIfNeedSaveChanges();
void markAsSaved(const muse::io::path_t& path);
Expand Down
29 changes: 28 additions & 1 deletion src/project/internal/projectactionscontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,11 @@ bool ProjectActionsController::saveProjectLocally(const muse::io::path_t& filePa

if (!ret) {
LOGE() << ret.toString();
warnScoreCouldnotBeSaved(ret);
if (ret.code() == (int)Err::CorruptionUponSavingError) {
warnScoreCorruptAfterSave(ret);
} else {
warnScoreCouldnotBeSaved(ret);
}
return false;
}

Expand Down Expand Up @@ -1577,6 +1581,29 @@ void ProjectActionsController::warnScoreCouldnotBeSaved(const std::string& error
interactive()->warning(muse::trc("project/save", "Your score could not be saved"), errorText);
}

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)
? "Unfortunately your original file has become corrupt.\n\n"
: "";

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"
"If it keeps happening, try saving the file to a different location or to MuseScore.com. "
" You could also try saving the file as an uncompressed folder or XML.\n\n"
"%1"
"For help please use the \"Support and bug reports\" forum at https://musescore.org/en/forum\n\n"
"Error: %2")
.arg(originalFileCorruptedMessage.c_str())
.arg(errMessage.c_str())
.toStdString();
interactive()->warning(muse::trc("project/save", "Your score could not be saved"), message);
}

void ProjectActionsController::revertCorruptedScoreToLastSaved()
{
TRACEFUNC;
Expand Down
1 change: 1 addition & 0 deletions src/project/internal/projectactionscontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ class ProjectActionsController : public IProjectFilesController, public muse::mi

void warnScoreCouldnotBeSaved(const muse::Ret& ret);
void warnScoreCouldnotBeSaved(const std::string& errorText);
void warnScoreCorruptAfterSave(const muse::Ret& ret);

void revertCorruptedScoreToLastSaved();

Expand Down
1 change: 1 addition & 0 deletions src/project/projecterrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ enum class Err {
NoPartsError,
CorruptionError,
CorruptionUponOpenningError,
CorruptionUponSavingError, // do we need this or can we use CorruptionError instead?

FileOpenError,
InvalidCloudScoreId,
Expand Down

0 comments on commit 13f2353

Please sign in to comment.