diff --git a/cmake/qet_compilation_vars.cmake b/cmake/qet_compilation_vars.cmake index d57817384..eab3ba0a7 100644 --- a/cmake/qet_compilation_vars.cmake +++ b/cmake/qet_compilation_vars.cmake @@ -752,6 +752,8 @@ set(QET_SRC_FILES ${QET_DIR}/sources/ui/diagrampropertieseditordockwidget.h ${QET_DIR}/sources/ui/backupdialog.cpp ${QET_DIR}/sources/ui/backupdialog.h + ${QET_DIR}/sources/ui/backuprestoredialog.cpp + ${QET_DIR}/sources/ui/backuprestoredialog.h ${QET_DIR}/sources/ui/dialogwaiting.cpp ${QET_DIR}/sources/ui/dialogwaiting.h ${QET_DIR}/sources/ui/duplicateoffsetdialog.cpp diff --git a/sources/qetapp.cpp b/sources/qetapp.cpp index 829e3a74b..615428f07 100644 --- a/sources/qetapp.cpp +++ b/sources/qetapp.cpp @@ -57,6 +57,7 @@ #define QUOTE(x) STRINGIFY(x) #define STRINGIFY(x) #x #include +#include #include #include #include @@ -68,6 +69,9 @@ #else # include #endif +#include "ui/backuprestoredialog.h" + +#include #ifdef QET_ALLOW_OVERRIDE_CED_OPTION QString QETApp::m_overrided_common_elements_dir = QString(); @@ -2945,48 +2949,47 @@ void QETApp::checkBackupFiles() /** @brief QETApp::offerBackupFiles Ask whether to reopen the recovery files left by a previous run, and - open or discard them accordingly. + open or discard them accordingly. A project can leave several recovery + files, one per snapshot (@see QETProject::writeBackup): they are grouped + by project, and the user picks which one to reopen, the newest by default. @param stale_files : the recovery files to offer */ void QETApp::offerBackupFiles(const QList &stale_files) { - QString text; - if(stale_files.size() == 1) { - text.append(tr("Le fichier de restauration suivant a été trouvé,
" - "Voulez-vous l'ouvrir ?
")); - } else { - text.append(tr("Les fichiers de restauration suivant on été trouvé,
" - "Voulez-vous les ouvrir ?
")); + //Group the snapshots by the project they recover, newest first. + QHash> groups; + for (KAutoSaveFile *kasf : stale_files) { + groups[kasf->managedFile().path()].append(kasf); } - for(const KAutoSaveFile *kasf : stale_files) - { -# ifdef Q_OS_WIN - //Remove the first character '/' before the name of the drive - text.append("
" + kasf->managedFile().path().remove(0,1)); -# else - text.append("
" + kasf->managedFile().path()); -# endif + for (auto &snapshots : groups) { + std::sort(snapshots.begin(), snapshots.end(), + [](KAutoSaveFile *a, KAutoSaveFile *b) { + return QFileInfo(*a).lastModified() + > QFileInfo(*b).lastModified(); + }); } - //Open backup file - if (QET::QetMessageBox::question(nullptr, - tr("Fichier de restauration"), - text, - QMessageBox::Ok - |QMessageBox::Cancel - ) - == QMessageBox::Ok) + BackupRestoreDialog dialog(groups, nullptr); + if (dialog.exec() == QDialog::Accepted) { + //The snapshots not picked are no longer needed. + for (KAutoSaveFile *discarded : dialog.discardedFiles()) + { + discarded->open(QIODevice::ReadWrite); + delete discarded; + } + + const QList to_open = dialog.selectedFiles(); //If there are open editors, find those that are visible if (diagramEditors().count()) { diagramEditors().first()->setVisible(true); - diagramEditors().first()->openBackupFiles(stale_files); + diagramEditors().first()->openBackupFiles(to_open); } else { QETDiagramEditor *editor = new QETDiagramEditor(); - editor->openBackupFiles(stale_files); + editor->openBackupFiles(to_open); } } else //Clear backup file diff --git a/sources/qetproject.cpp b/sources/qetproject.cpp index a44e127ef..d126b45f9 100644 --- a/sources/qetproject.cpp +++ b/sources/qetproject.cpp @@ -205,7 +205,7 @@ QETProject::QETProject(KAutoSaveFile *backup, QObject *parent) : QETProject::~QETProject() { //Wait for any in-flight async crash-recovery backup to finish: the worker - //writes through &m_backup_file, a member that would otherwise be destroyed + //writes through m_backup_files, a member that would otherwise be destroyed //under it (issue #492). m_backup_future.waitForFinished(); @@ -562,12 +562,16 @@ void QETProject::setFilePath(const QString &filepath) if (filepath == m_file_path) { return; } - //Don't close/re-point the backup file while a backup is still writing it. + //Don't close/re-point the backup files while a backup is still writing one. m_backup_future.waitForFinished(); - if (m_backup_file.isOpen()) { - m_backup_file.close(); + const QUrl managed_file = QUrl::fromLocalFile(filepath); + for (auto &backup_file : m_backup_files) { + if (backup_file.isOpen()) { + backup_file.close(); + } + backup_file.setManagedFile(managed_file); } - m_backup_file.setManagedFile(QUrl::fromLocalFile(filepath)); + m_next_backup_slot = 0; m_file_path = filepath; QFileInfo fi(m_file_path); @@ -2337,14 +2341,17 @@ void QETProject::detachDiagram(Diagram *diagram) /** @brief QETProject::writeBackup - Write a backup file of this project, in the case that QET crash + Write a backup file of this project, in the case that QET crash. + The snapshots are written in turn to m_backup_files, so a write made + while the project is already in a bad state only replaces the oldest + snapshot, and the earlier ones are still there to recover from. */ void QETProject::writeBackup() { if (!m_backup_enabled) return; //Don't launch a new backup while the previous one is still writing: - //both would write through &m_backup_file on different threads. + //both could write through the same m_backup_files slot on different threads. if (m_backup_future.isRunning()) return; //toXml() walks the whole project on the GUI thread, which freezes @@ -2357,8 +2364,10 @@ void QETProject::writeBackup() //Qt5-style QtConcurrent::run(function, reference-args) call did not //survive the Qt6 API change, a lambda behaves identically on both. QDomDocument xml_project(toXml()); - m_backup_future = QtConcurrent::run([this, xml_project]() mutable { - return QET::writeToFile(xml_project, &m_backup_file, nullptr); + KAutoSaveFile *target = &m_backup_files[m_next_backup_slot]; + m_next_backup_slot = (m_next_backup_slot + 1) % BackupGenerations; + m_backup_future = QtConcurrent::run([target, xml_project]() mutable { + return QET::writeToFile(xml_project, target, nullptr); }); } diff --git a/sources/qetproject.h b/sources/qetproject.h index b30b07075..9e260bcc4 100644 --- a/sources/qetproject.h +++ b/sources/qetproject.h @@ -39,6 +39,8 @@ #include #include +#include + class Diagram; class ElementsLocation; class QETResult; @@ -128,6 +130,10 @@ class QETProject : public QObject /// process can destroy the project before the write finishes (crash). static void setBackupEnabled(bool enabled); + /// Number of crash-recovery snapshots kept per project, written in + /// turn by writeBackup(), so one bad write cannot replace the only copy + static constexpr int BackupGenerations = 3; + ///DEFAULT PROPERTIES BorderProperties defaultBorderProperties() const; void setDefaultBorderProperties(const BorderProperties &); @@ -366,7 +372,9 @@ class QETProject : public QObject QTimer m_save_backup_timer, m_autosave_timer; QFuture m_backup_future; - KAutoSaveFile m_backup_file; + /// Crash-recovery snapshots, written in turn by writeBackup() + std::array m_backup_files; + int m_next_backup_slot = 0; QUuid m_uuid = QUuid::createUuid(); QHash m_derived_uuid_keys; QSet m_saved_item_uuids; //symbol and wire uuids the file carries, see derivedItemUuid() diff --git a/sources/ui/backuprestoredialog.cpp b/sources/ui/backuprestoredialog.cpp new file mode 100644 index 000000000..72e46712b --- /dev/null +++ b/sources/ui/backuprestoredialog.cpp @@ -0,0 +1,142 @@ +/* + Copyright 2006-2026 The QElectroTech Team + This file is part of QElectroTech. + + QElectroTech is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 2 of the License, or + (at your option) any later version. + + QElectroTech is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with QElectroTech. If not, see . +*/ + +#include "backuprestoredialog.h" + +#ifdef BUILD_WITHOUT_KF +# include "nokde/kautosavefile.h" +#else +# include +#endif + +#include +#include +#include +#include +#include +#include +#include +#include + +/** + @brief BackupRestoreDialog::BackupRestoreDialog + @param groups : managed project path -> its recovery generations + (newest-first) + @param parent : parent widget +*/ +BackupRestoreDialog::BackupRestoreDialog( + const QHash> &groups, + QWidget *parent) : + QDialog(parent), + m_groups(groups) +{ + setWindowTitle(tr("Fichiers de restauration", "window title")); + + auto main_layout = new QVBoxLayout(this); + + auto intro = new QLabel( + tr("Des fichiers de restauration ont été trouvés,
" + "voulez-vous les ouvrir ?
" + "Pour un projet ayant plusieurs versions de restauration, " + "la plus récente est sélectionnée par défaut.", + "dialog message")); + intro->setWordWrap(true); + main_layout->addWidget(intro); + + auto grid = new QGridLayout(); + int row = 0; + for (auto it = m_groups.constBegin(); it != m_groups.constEnd(); ++it) + { + const QString &path = it.key(); + const QList &generations = it.value(); + +# ifdef Q_OS_WIN + QString display_path = path; + display_path.remove(0, 1); +# else + const QString &display_path = path; +# endif + grid->addWidget(new QLabel(display_path), row, 0); + + auto combo = new QComboBox(this); + for (int i = 0; i < generations.size(); ++i) + { + const QDateTime modified = + QFileInfo(*generations.at(i)).lastModified(); + QString label = QLocale::system().toString( + modified, QLocale::ShortFormat); + if (i == 0) { + label = tr("%1 (la plus récente)", "recovery generation label") + .arg(label); + } + combo->addItem(label); + } + combo->setEnabled(generations.size() > 1); + grid->addWidget(combo, row, 1); + + m_combo_for_path.insert(path, combo); + ++row; + } + main_layout->addLayout(grid); + + auto buttons = new QDialogButtonBox( + QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this); + connect(buttons, &QDialogButtonBox::accepted, this, &QDialog::accept); + connect(buttons, &QDialogButtonBox::rejected, this, &QDialog::reject); + main_layout->addWidget(buttons); +} + +/** + @brief BackupRestoreDialog::~BackupRestoreDialog +*/ +BackupRestoreDialog::~BackupRestoreDialog() = default; + +/** + @return one recovery generation per project, matching the combo box + selection (the newest generation, unless the user picked another). +*/ +QList BackupRestoreDialog::selectedFiles() const +{ + QList selected; + for (auto it = m_groups.constBegin(); it != m_groups.constEnd(); ++it) + { + const int index = m_combo_for_path.value(it.key())->currentIndex(); + selected << it.value().at(index); + } + return selected; +} + +/** + @return every recovery generation the user did not pick; the caller + should release and delete these. +*/ +QList BackupRestoreDialog::discardedFiles() const +{ + QList discarded; + for (auto it = m_groups.constBegin(); it != m_groups.constEnd(); ++it) + { + const int index = m_combo_for_path.value(it.key())->currentIndex(); + const QList &generations = it.value(); + for (int i = 0; i < generations.size(); ++i) { + if (i != index) { + discarded << generations.at(i); + } + } + } + return discarded; +} diff --git a/sources/ui/backuprestoredialog.h b/sources/ui/backuprestoredialog.h new file mode 100644 index 000000000..30eed692d --- /dev/null +++ b/sources/ui/backuprestoredialog.h @@ -0,0 +1,60 @@ +/* + Copyright 2006-2026 The QElectroTech Team + This file is part of QElectroTech. + + QElectroTech is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 2 of the License, or + (at your option) any later version. + + QElectroTech is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with QElectroTech. If not, see . +*/ + +#ifndef BACKUPRESTOREDIALOG_H +#define BACKUPRESTOREDIALOG_H + +#include +#include +#include +#include + +class KAutoSaveFile; +class QComboBox; + +/** + @brief Lets the user pick, per crashed project, which recovery + generation to reopen when the crash-recovery rotation + (@see QETProject::writeBackup) left more than one snapshot behind. +*/ +class BackupRestoreDialog : public QDialog +{ + Q_OBJECT + + public: + /// @param groups : managed project file path -> its stale recovery + /// generations, each list already sorted newest-first. Ownership of + /// the KAutoSaveFile objects stays with the caller. + explicit BackupRestoreDialog( + const QHash> &groups, + QWidget *parent = nullptr); + ~BackupRestoreDialog() override; + + /// Valid once accepted: one entry per project, the chosen generation + /// (defaults to the most recent one). + QList selectedFiles() const; + /// The generations the user did not pick; the caller should discard + /// (release + delete) these. + QList discardedFiles() const; + + private: + QHash> m_groups; + QHash m_combo_for_path; +}; + +#endif // BACKUPRESTOREDIALOG_H diff --git a/tests/catch/src/kautosavefile_test.cpp b/tests/catch/src/kautosavefile_test.cpp index 64258c117..6e1e1c829 100644 --- a/tests/catch/src/kautosavefile_test.cpp +++ b/tests/catch/src/kautosavefile_test.cpp @@ -3,14 +3,18 @@ #include #include +#include #include #include #include #include +#include #include +#include #ifdef Q_OS_UNIX +#include #include #include #include @@ -106,3 +110,104 @@ TEST_CASE("Qt-only KAutoSaveFile recovers stale files", "[nokde][autosave]") CHECK_FALSE(QFile::exists(lock_file_name)); #endif } + +TEST_CASE("Multiple KAutoSaveFile generations for one managed file are all " + "found stale after a crash", "[nokde][autosave]") +{ +#ifndef Q_OS_UNIX + SUCCEED("crash-style stale lock test is Unix-only"); +#else + //Simulates QETProject's rotating crash-recovery generations + //(BackupGenerations snapshots written round-robin): several + //KAutoSaveFile instances sharing one managed file, alive at once. + QTemporaryDir data_home; + REQUIRE(data_home.isValid()); + + qputenv("XDG_DATA_HOME", QFile::encodeName(data_home.path())); + QCoreApplication::setOrganizationName(QStringLiteral("QElectroTech")); + QCoreApplication::setApplicationName( + QStringLiteral("KAutoSaveFileGenerationsTest")); + + const auto managed_path = data_home.filePath(QStringLiteral("project.qet")); + QFile managed_file(managed_path); + REQUIRE(managed_file.open(QIODevice::WriteOnly | QIODevice::Text)); + REQUIRE(managed_file.write("\n") > 0); + managed_file.close(); + + constexpr int generations = 3; + + int ready_pipe[2] = {-1, -1}; + REQUIRE(pipe(ready_pipe) == 0); + + const auto child_pid = fork(); + REQUIRE(child_pid >= 0); + + if (child_pid == 0) { + close(ready_pipe[0]); + + std::vector> backups; + for (int i = 0; i < generations; ++i) { + auto backup = std::make_unique( + QUrl::fromLocalFile(managed_path)); + if (!backup->open(QIODevice::WriteOnly + | QIODevice::Truncate + | QIODevice::Text)) { + _exit(2); + } + const QByteArray payload = + "\n"; + if (backup->write(payload) != payload.size()) { + _exit(3); + } + if (!backup->flush()) { + _exit(4); + } + //Give each generation a distinct, increasing mtime. + struct timespec pause{0, 20 * 1000 * 1000}; + nanosleep(&pause, nullptr); + backups.push_back(std::move(backup)); + } + + const char ready = '1'; + if (write(ready_pipe[1], &ready, 1) != 1) { + _exit(5); + } + close(ready_pipe[1]); + + for (;;) { + pause(); + } + } + + close(ready_pipe[1]); + char ready = 0; + REQUIRE(read(ready_pipe[0], &ready, 1) == 1); + close(ready_pipe[0]); + REQUIRE(ready == '1'); + + REQUIRE(kill(child_pid, SIGKILL) == 0); + int status = 0; + REQUIRE(waitpid(child_pid, &status, 0) == child_pid); + REQUIRE(WIFSIGNALED(status)); + REQUIRE(WTERMSIG(status) == SIGKILL); + + auto stale_files = KAutoSaveFile::allStaleFiles(); + REQUIRE(stale_files.size() == generations); + + std::sort(stale_files.begin(), stale_files.end(), + [](KAutoSaveFile *a, KAutoSaveFile *b) { + return QFileInfo(*a).lastModified() < QFileInfo(*b).lastModified(); + }); + + for (int i = 0; i < generations; ++i) { + std::unique_ptr stale_file(stale_files.at(i)); + CHECK(stale_file->managedFile().path() + == QFileInfo(managed_path).absoluteFilePath()); + REQUIRE(stale_file->open(QIODevice::ReadOnly | QIODevice::Text)); + const QByteArray content = stale_file->readAll(); + CHECK(content.contains( + "generation n=\"" + QByteArray::number(i) + "\"")); + } +#endif +}