From 58602693586bd3366d15a05659856e6acca2d2b7 Mon Sep 17 00:00:00 2001 From: Christian Kaiser Date: Thu, 19 Sep 2024 15:50:02 -0300 Subject: [PATCH] Use new list_files, parse dates to check if sessions are old (#4660) --- src/app/crash/data_recovery.cpp | 38 ++++++------- src/app/crash/read_document.cpp | 2 +- src/app/crash/session.cpp | 70 +++++++++++++++++++----- src/app/crash/session.h | 1 + src/app/extensions.cpp | 5 +- src/app/font_path_unix.cpp | 5 +- src/app/i18n/strings.cpp | 6 +- src/app/res/palettes_loader_delegate.cpp | 5 +- src/app/script/app_fs_object.cpp | 2 +- src/app/sentry_wrapper.cpp | 13 ++--- src/app/ui/font_popup.cpp | 6 +- 11 files changed, 89 insertions(+), 64 deletions(-) diff --git a/src/app/crash/data_recovery.cpp b/src/app/crash/data_recovery.cpp index b9348930c..ee2c64777 100644 --- a/src/app/crash/data_recovery.cpp +++ b/src/app/crash/data_recovery.cpp @@ -143,29 +143,27 @@ void DataRecovery::searchForSessions() // Existent sessions RECO_TRACE("RECO: Listing sessions from '%s'\n", m_sessionsDir.c_str()); - for (auto& itemname : base::list_files(m_sessionsDir)) { - std::string itempath = base::join_path(m_sessionsDir, itemname); - if (base::is_directory(itempath)) { - RECO_TRACE("RECO: Session '%s'\n", itempath.c_str()); + for (const auto& itemname : base::list_files(m_sessionsDir, base::ItemType::Directories)) { + const auto& itempath = base::join_path(m_sessionsDir, itemname); + RECO_TRACE("RECO: Session '%s' ", itempath.c_str()); - SessionPtr session(new Session(&m_config, itempath)); - if (!session->isRunning()) { - if ((session->isEmpty()) || - (!session->isCrashedSession() && session->isOldSession())) { - RECO_TRACE("RECO: - to be deleted (%s)\n", - session->isEmpty() ? "is empty": - (session->isOldSession() ? "is old": - "unknown reason")); - session->removeFromDisk(); - } - else { - RECO_TRACE("RECO: - to be loaded\n"); - sessions.push_back(session); - } + SessionPtr session(new Session(&m_config, itempath)); + if (!session->isRunning()) { + if ((session->isEmpty()) || + (!session->isCrashedSession() && session->isOldSession())) { + RECO_TRACE("to be deleted (%s)\n", + session->isEmpty() ? "is empty": + (session->isOldSession() ? "is old": + "unknown reason")); + session->removeFromDisk(); + } + else { + RECO_TRACE("to be loaded\n"); + sessions.push_back(session); } - else - RECO_TRACE("is running\n"); } + else + RECO_TRACE("is running\n"); } // Sort sessions from the most recent one to the oldest one diff --git a/src/app/crash/read_document.cpp b/src/app/crash/read_document.cpp index e6c712a98..6dfbd957d 100644 --- a/src/app/crash/read_document.cpp +++ b/src/app/crash/read_document.cpp @@ -77,7 +77,7 @@ public: , m_docVersions(nullptr) , m_loadInfo(nullptr) , m_taskToken(t) { - for (const auto& fn : base::list_files(dir)) { + for (const auto& fn : base::list_files(dir, base::ItemType::Files)) { auto i = fn.find('-'); if (i == std::string::npos) continue; // Has no ID diff --git a/src/app/crash/session.cpp b/src/app/crash/session.cpp index 7bd47d4b4..99c42bcee 100644 --- a/src/app/crash/session.cpp +++ b/src/app/crash/session.cpp @@ -30,6 +30,7 @@ #include "base/thread.h" #include "base/time.h" #include "doc/cancel_io.h" +#include "ui/app_state.h" #include "fmt/format.h" #include "ver/info.h" @@ -114,11 +115,12 @@ std::string Session::version() const Session::Backups& Session::backups() { if (m_backups.empty()) { - for (auto& item : base::list_files(m_path)) { + for (const auto& item : base::list_files(m_path, base::ItemType::Directories)) { + if (ui::is_app_state_closing()) + continue; + std::string docDir = base::join_path(m_path, item); - if (base::is_directory(docDir)) { - m_backups.push_back(std::make_shared(docDir)); - } + m_backups.push_back(std::make_shared(docDir)); } } return m_backups; @@ -147,18 +149,40 @@ bool Session::isOldSession() return true; int lifespanDays = m_config->keepEditedSpriteDataFor; - base::Time sessionTime = base::get_modification_time(verfile); + base::Time sessionTime; + + // Get the session time from the name if possible, to avoid re-scanning when transferring files + std::vector parts; + base::split_string(base::get_file_title(m_path), parts, "-"); + + if (parts.size() == 3 && parts[0].size() == 8) { + try { + sessionTime = base::Time(filenamePartToInt(parts[0].substr(0, 4)), + filenamePartToInt(parts[0].substr(4, 2)), + filenamePartToInt(parts[0].substr(6, 2)), + 0, + 0, + 0); + } + catch (const std::exception& ex) { + LOG(ERROR, + "Failed to parse a date from '%s', error: %s", + parts[0].c_str(), + ex.what()); + } + } + + if (!sessionTime.valid()) { + // Get modification time as a fallback + sessionTime = base::get_modification_time(verfile); + } return (sessionTime.addDays(lifespanDays) < base::current_time()); } bool Session::isEmpty() { - for (auto& item : base::list_files(m_path)) { - if (base::is_directory(base::join_path(m_path, item))) - return false; - } - return true; + return base::list_files(m_path, base::ItemType::Directories).empty(); } void Session::create(base::pid pid) @@ -385,12 +409,13 @@ void Session::deleteDirectory(const std::string& dir) if (dir.empty()) return; - for (auto& item : base::list_files(dir)) { + for (const auto& item : base::list_files(dir, base::ItemType::Files)) { + if (ui::is_app_state_closing()) + return; + std::string objfn = base::join_path(dir, item); - if (base::is_file(objfn)) { - RECO_TRACE("RECO: Deleting file '%s'\n", objfn.c_str()); - base::delete_file(objfn); - } + RECO_TRACE("RECO: Deleting file '%s'\n", objfn.c_str()); + base::delete_file(objfn); } base::remove_directory(dir); } @@ -411,5 +436,20 @@ void Session::fixFilename(Doc* doc) base::get_file_title(fn) + "-Recovered" + ext)); } +int Session::filenamePartToInt(const std::string& part) const +{ + if (part.empty()) + throw base::Exception("Invalid part"); + + int result = std::strtol(part.c_str(), NULL, 10); + if (errno == ERANGE) + throw base::Exception("Number out of range"); + + if (result < 0) + throw base::Exception("Negative value"); + + return result; +} + } // namespace crash } // namespace app diff --git a/src/app/crash/session.h b/src/app/crash/session.h index 30b7f67e1..4ae1280d9 100644 --- a/src/app/crash/session.h +++ b/src/app/crash/session.h @@ -78,6 +78,7 @@ namespace crash { void markDocumentAsCorrectlyClosed(Doc* doc); void deleteDirectory(const std::string& dir); void fixFilename(Doc* doc); + int filenamePartToInt(const std::string& part) const; base::pid m_pid; std::string m_path; diff --git a/src/app/extensions.cpp b/src/app/extensions.cpp index 2eb537f10..413300373 100644 --- a/src/app/extensions.cpp +++ b/src/app/extensions.cpp @@ -816,11 +816,8 @@ Extensions::Extensions() if (!base::is_directory(extensionsDir)) continue; - for (auto& fn : base::list_files(extensionsDir)) { + for (const auto& fn : base::list_files(extensionsDir, base::ItemType::Directories)) { const auto dir = base::join_path(extensionsDir, fn); - if (!base::is_directory(dir)) - continue; - const bool isBuiltinExtension = (m_userExtensionsPath != base::get_file_path(dir)); diff --git a/src/app/font_path_unix.cpp b/src/app/font_path_unix.cpp index 7f3d94a6f..57e0b3c1c 100644 --- a/src/app/font_path_unix.cpp +++ b/src/app/font_path_unix.cpp @@ -36,10 +36,9 @@ void get_font_dirs(base::paths& fontDirs) fontDirs.push_back(fontDir); - for (const auto& file : base::list_files(fontDir)) { + for (const auto& file : base::list_files(fontDir, base::ItemType::Directories)) { std::string fullpath = base::join_path(fontDir, file); - if (base::is_directory(fullpath)) - q.push(fullpath); // Add subdirectory in the queue + q.push(fullpath); // Add subdirectory in the queue } } diff --git a/src/app/i18n/strings.cpp b/src/app/i18n/strings.cpp index 31820f8f6..3f22ca916 100644 --- a/src/app/i18n/strings.cpp +++ b/src/app/i18n/strings.cpp @@ -64,11 +64,7 @@ std::set Strings::availableLanguages() const if (!base::is_directory(stringsPath)) continue; - for (const auto& fn : base::list_files(stringsPath)) { - // Ignore README/LICENSE files. - if (base::get_file_extension(fn) != "ini") - continue; - + for (const auto& fn : base::list_files(stringsPath, base::ItemType::Files, "*.ini")) { const std::string langId = base::get_file_title(fn); std::string path = base::join_path(stringsPath, fn); std::string displayName = langId; diff --git a/src/app/res/palettes_loader_delegate.cpp b/src/app/res/palettes_loader_delegate.cpp index 3f7424c9a..25a75fb20 100644 --- a/src/app/res/palettes_loader_delegate.cpp +++ b/src/app/res/palettes_loader_delegate.cpp @@ -45,7 +45,7 @@ void PalettesLoaderDelegate::getResourcesPaths(std::map