Merge pull request musescore#18376 from cbjeukendrup/message_open_error

Fix musescore#12028: Fix messaging when opening non-existing scores

src/engraving/infrastructure/mscreader.cpp

@@ -26,6 +26,7 @@
 #include "io/dir.h"
 #include "serialization/zipreader.h"
 #include "serialization/xmlstreamreader.h"
+#include "engraving/engravingerrors.h"
 
 #include "log.h"
 
@@ -65,7 +66,7 @@ const MscReader::Params& MscReader::params() const
     return m_params;
 }
 
-bool MscReader::open()
+Ret MscReader::open()
 {
     return reader()->open(m_params.device, m_params.filePath);
 }
@@ -253,18 +254,23 @@ MscReader::ZipFileReader::~ZipFileReader()
     }
 }
 
-bool MscReader::ZipFileReader::open(IODevice* device, const path_t& filePath)
+Ret MscReader::ZipFileReader::open(IODevice* device, const path_t& filePath)
 {
     m_device = device;
     if (!m_device) {
+        if (!FileInfo::exists(filePath)) {
+            LOGE() << "path does not exist: " << filePath;
+            return make_ret(Err::FileNotFound, filePath);
+        }
+
         m_device = new File(filePath);
         m_selfDeviceOwner = true;
     }
 
     if (!m_device->isOpen()) {
         if (!m_device->open(IODevice::ReadOnly)) {
-            LOGD() << "failed open file: " << filePath;
-            return false;
+            LOGE() << "failed open file: " << filePath;
+            return make_ret(Err::FileOpenError, filePath);
         }
     }
 
@@ -303,7 +309,7 @@ StringList MscReader::ZipFileReader::fileList() const
     StringList files;
     std::vector<ZipReader::FileInfo> fileInfoList = m_zip->fileInfoList();
     if (m_zip->hasError()) {
-        LOGD() << "failed read meta";
+        LOGE() << "failed read meta";
     }
 
     for (const ZipReader::FileInfo& fi : fileInfoList) {
@@ -332,27 +338,27 @@ ByteArray MscReader::ZipFileReader::fileData(const String& fileName) const
 
     ByteArray data = m_zip->fileData(fileName.toStdString());
     if (m_zip->hasError()) {
-        LOGD() << "failed read data";
+        LOGE() << "failed read data for filename " << fileName;
         return ByteArray();
     }
     return data;
 }
 
-bool MscReader::DirReader::open(IODevice* device, const path_t& filePath)
+Ret MscReader::DirReader::open(IODevice* device, const path_t& filePath)
 {
     if (device) {
         NOT_SUPPORTED;
         return false;
     }
 
     if (!FileInfo::exists(filePath)) {
-        LOGD() << "not exists path: " << filePath;
-        return false;
+        LOGE() << "path does not exist: " << filePath;
+        return make_ret(Err::FileNotFound, filePath);
     }
 
     m_rootPath = containerPath(filePath);
 
-    return true;
+    return make_ok();
 }
 
 void MscReader::DirReader::close()
@@ -400,29 +406,34 @@ ByteArray MscReader::DirReader::fileData(const String& fileName) const
     io::path_t filePath = m_rootPath + "/" + fileName;
     File file(filePath);
     if (!file.open(IODevice::ReadOnly)) {
-        LOGD() << "failed open file: " << filePath;
+        LOGE() << "failed open file: " << filePath;
         return ByteArray();
     }
 
     return file.readAll();
 }
 
-bool MscReader::XmlFileReader::open(IODevice* device, const path_t& filePath)
+Ret MscReader::XmlFileReader::open(IODevice* device, const path_t& filePath)
 {
     m_device = device;
     if (!m_device) {
+        if (!FileInfo::exists(filePath)) {
+            LOGE() << "path does not exist: " << filePath;
+            return make_ret(Err::FileNotFound, filePath);
+        }
+
         m_device = new File(filePath);
         m_selfDeviceOwner = true;
     }
 
     if (!m_device->isOpen()) {
         if (!m_device->open(IODevice::ReadOnly)) {
-            LOGD() << "failed open file: " << filePath;
-            return false;
+            LOGE() << "failed open file: " << filePath;
+            return make_ret(Err::FileOpenError, filePath);
         }
     }
 
-    return true;
+    return make_ok();
 }
 
 void MscReader::XmlFileReader::close()
@@ -453,13 +464,13 @@ StringList MscReader::XmlFileReader::fileList() const
     m_device->seek(0);
     XmlStreamReader xml(m_device);
     while (xml.readNextStartElement()) {
-        if ("files" != xml.name()) {
+        if (xml.name() != "files") {
             xml.skipCurrentElement();
             continue;
         }
 
         while (xml.readNextStartElement()) {
-            if ("file" != xml.name()) {
+            if (xml.name() != "file") {
                 xml.skipCurrentElement();
                 continue;
             }
@@ -511,13 +522,13 @@ ByteArray MscReader::XmlFileReader::fileData(const String& fileName) const
     m_device->seek(0);
     XmlStreamReader xml(m_device);
     while (xml.readNextStartElement()) {
-        if ("files" != xml.name()) {
+        if (xml.name() != "files") {
             xml.skipCurrentElement();
             continue;
         }
 
         while (xml.readNextStartElement()) {
-            if ("file" != xml.name()) {
+            if (xml.name() != "file") {
                 xml.skipCurrentElement();
                 continue;
             }

src/engraving/infrastructure/mscreader.h

@@ -22,6 +22,7 @@
 #ifndef MU_ENGRAVING_MSCREADER_H
 #define MU_ENGRAVING_MSCREADER_H
 
+#include "types/ret.h"
 #include "types/string.h"
 #include "io/path.h"
 #include "io/iodevice.h"
@@ -51,7 +52,7 @@ class MscReader
     void setParams(const Params& params);
     const Params& params() const;
 
-    bool open();
+    Ret open();
     void close();
     bool isOpened() const;
 
@@ -77,7 +78,7 @@ class MscReader
     struct IReader {
         virtual ~IReader() = default;
 
-        virtual bool open(io::IODevice* device, const io::path_t& filePath) = 0;
+        virtual Ret open(io::IODevice* device, const io::path_t& filePath) = 0;
         virtual void close() = 0;
         virtual bool isOpened() const = 0;
         //! NOTE In the case of reading from a directory,
@@ -92,7 +93,7 @@ class MscReader
     struct ZipFileReader : public IReader
     {
         ~ZipFileReader() override;
-        bool open(io::IODevice* device, const io::path_t& filePath) override;
+        Ret open(io::IODevice* device, const io::path_t& filePath) override;
         void close() override;
         bool isOpened() const override;
         bool isContainer() const override;
@@ -107,7 +108,7 @@ class MscReader
 
     struct DirReader : public IReader
     {
-        bool open(io::IODevice* device, const io::path_t& filePath) override;
+        Ret open(io::IODevice* device, const io::path_t& filePath) override;
         void close() override;
         bool isOpened() const override;
         bool isContainer() const override;
@@ -120,7 +121,7 @@ class MscReader
 
     struct XmlFileReader : public IReader
     {
-        bool open(io::IODevice* device, const io::path_t& filePath) override;
+        Ret open(io::IODevice* device, const io::path_t& filePath) override;
         void close() override;
         bool isOpened() const override;
         bool isContainer() const override;

src/project/internal/notationproject.cpp

@@ -184,15 +184,16 @@ mu::Ret NotationProject::doLoad(const io::path_t& path, const io::path_t& styleP
     }
 
     MscReader reader(params);
-    if (!reader.open()) {
-        return make_ret(engraving::Err::FileOpenError);
+    Ret ret = reader.open();
+    if (!ret) {
+        return ret;
     }
 
     // Load engraving project
     m_engravingProject->setFileInfoProvider(std::make_shared<ProjectFileInfoProvider>(this));
 
     SettingsCompat settingsCompat;
-    Ret ret = m_engravingProject->loadMscz(reader, settingsCompat, forceMode);
+    ret = m_engravingProject->loadMscz(reader, settingsCompat, forceMode);
     if (!ret) {
         return ret;
     }

src/project/internal/projectactionscontroller.cpp

@@ -1390,10 +1390,15 @@ bool ProjectActionsController::checkCanIgnoreError(const Ret& ret, const io::pat
         return false;
     case engraving::Err::FileCorrupted:
         return askIfUserAgreesToOpenCorruptedProject(io::filename(filepath).toString(), ret.text());
-    default:
-        warnProjectCannotBeOpened(io::filename(filepath).toString(), ret.text());
+    case engraving::Err::FileCriticallyCorrupted:
+        warnProjectCriticallyCorrupted(io::filename(filepath).toString(), ret.text());
         return false;
+    default:
+        break;
     }
+
+    warnProjectCannotBeOpened(ret, filepath);
+    return false;
 }
 
 bool ProjectActionsController::askIfUserAgreesToOpenProjectWithIncompatibleVersion(const std::string& errorText)
@@ -1430,7 +1435,7 @@ bool ProjectActionsController::askIfUserAgreesToOpenCorruptedProject(const Strin
     return btn == openAnywayBtn.btn;
 }
 
-void ProjectActionsController::warnProjectCannotBeOpened(const String& projectName, const std::string& errorText)
+void ProjectActionsController::warnProjectCriticallyCorrupted(const String& projectName, const std::string& errorText)
 {
     std::string title = mtrc("project", "File “%1” is corrupted and cannot be opened").arg(projectName).toStdString();
     std::string body = trc("project", "Get help for this issue on musescore.org.");
@@ -1447,6 +1452,29 @@ void ProjectActionsController::warnProjectCannotBeOpened(const String& projectNa
     }
 }
 
+void ProjectActionsController::warnProjectCannotBeOpened(const Ret& ret, const io::path_t& filepath)
+{
+    std::string title = mtrc("project", "Cannot read file %1").arg(io::toNativeSeparators(filepath).toString()).toStdString();
+    std::string body;
+
+    switch (ret.code()) {
+    case int(engraving::Err::FileNotFound):
+        body = trc("project", "This file does not exist or cannot be accessed at the moment.");
+        break;
+    case int(engraving::Err::FileOpenError):
+        body = trc("project", "This file could not be opened. Please make sure that MuseScore has permission to read this file.");
+        break;
+    default:
+        if (!ret.text().empty()) {
+            body = ret.text();
+        } else {
+            body = trc("project", "An error occurred while reading this file.");
+        }
+    }
+
+    interactive()->error(title, body);
+}
+
 void ProjectActionsController::importPdf()
 {
     interactive()->openUrl("https://musescore.com/import");

src/project/internal/projectactionscontroller.h

@@ -107,7 +107,8 @@ class ProjectActionsController : public IProjectFilesController, public QObject,
     bool askIfUserAgreesToOpenProjectWithIncompatibleVersion(const std::string& errorText);
     void warnFileTooNew(const io::path_t& filepath);
     bool askIfUserAgreesToOpenCorruptedProject(const String& projectName, const std::string& errorText);
-    void warnProjectCannotBeOpened(const String& projectName, const std::string& errorText);
+    void warnProjectCriticallyCorrupted(const String& projectName, const std::string& errorText);
+    void warnProjectCannotBeOpened(const Ret& ret, const io::path_t& filepath);
 
     framework::IInteractive::Button askAboutSavingScore(INotationProjectPtr project);