diff --git a/sources/projectview.cpp b/sources/projectview.cpp index 158cc2ae5..b6e85aef2 100644 --- a/sources/projectview.cpp +++ b/sources/projectview.cpp @@ -268,7 +268,9 @@ bool ProjectView::tryClosing() QString filepath = askUserForFilePath(); if (filepath.isEmpty()) return(false); // users may cancel the closing } - QETResult result = m_project -> write(); + bool cancelled = false; + QETResult result = writeProject(&cancelled); + if (cancelled) return(false); updateWindowTitle(); if (!result.isOk()) emit(errorEncountered(result.errorMessage())); return(result.isOk()); @@ -753,6 +755,50 @@ QETResult ProjectView::saveCopyForOlderVersions() return(m_project->writeCopyForOlderVersions(path)); } +/** + @brief ProjectView::writeProject + Write the project file. If the file was changed on disk since this QET + read or wrote it, ask what to do first: a plain save would replace + the other change. + @param cancelled set to true if the user gave up, or chose to save a + copy and did not complete it: nothing was saved to the project's file + @return the result of the write +*/ +QETResult ProjectView::writeProject(bool *cancelled) +{ + *cancelled = false; + QDateTime when; + if (!m_project -> changedOnDisk(&when)) { + return(m_project -> write()); + } + + const QString name = QFileInfo(m_project -> writeTarget()).fileName(); + QMessageBox box(QMessageBox::Warning, + tr("File changed on disk", "message box title"), + tr("Someone else changed %1 on disk at %2 since you opened it.\n\n" + "Saving over it would lose their changes.") + .arg(name, QLocale::system().toString(when, QLocale::ShortFormat)), + QMessageBox::NoButton, this); + QPushButton *copy_button = box.addButton(tr("Save as a copy"), QMessageBox::AcceptRole); + QPushButton *overwrite_button = box.addButton(tr("Overwrite"), QMessageBox::DestructiveRole); + box.addButton(QMessageBox::Cancel); + box.setDefaultButton(copy_button); + box.setEscapeButton(QMessageBox::Cancel); + box.exec(); + + if (box.clickedButton() == overwrite_button) { + return(m_project -> write(true)); + } + if (box.clickedButton() == copy_button) { + QETResult result = saveAs(); + //The Save As dialog was cancelled: the project is still modified + if (m_project -> projectWasModified()) *cancelled = true; + return(result); + } + *cancelled = true; + return(QETResult()); +} + /** Save project content, then write the project file. May call saveAs if no filepath was provided before. @@ -795,7 +841,9 @@ QETResult ProjectView::doSave() } // write to file - QETResult result = m_project -> write(); + bool cancelled = false; + QETResult result = writeProject(&cancelled); + if (cancelled) return(QETResult()); updateWindowTitle(); // This marks the stack's current index as the new "saved" point (so isClean()/cleanChanged() correctly // resume tracking unsaved changes from here) without discarding the undo history. Edits made before this save diff --git a/sources/projectview.h b/sources/projectview.h index eec92dcc7..ec0c0feeb 100644 --- a/sources/projectview.h +++ b/sources/projectview.h @@ -131,6 +131,7 @@ class ProjectView : public QWidget QETResult save(); QETResult saveAs(); QETResult doSave(); + QETResult writeProject(bool *cancelled); QETResult saveCopyForOlderVersions(); int cleanProject(); void updateWindowTitle(); diff --git a/sources/qetproject.cpp b/sources/qetproject.cpp index 33f164bca..70061cbed 100644 --- a/sources/qetproject.cpp +++ b/sources/qetproject.cpp @@ -387,7 +387,13 @@ void QETProject::init() connect(&m_autosave_timer, &QTimer::timeout, this, [this]() { if(!this->m_file_path.isEmpty()) - this->write(); + { + //No one is there to ask: a file changed behind this + //QET is left alone + const QETResult result = this->write(); + if (!result.isOk()) + qWarning() << "autosave:" << result.errorMessage(); + } }); m_autosave_timer.start(); } @@ -472,6 +478,7 @@ QETProject::ProjectState QETProject::openFile(QFile *file) //Extract the content of the xml. The raw bytes are kept : a project //file without a persisted uuid derives its uuid from them. const QByteArray content = file->readAll(); + rememberDiskState(); //Each folio's text is kept while it is not built (readDiagramsXml()): //from a .qet only, a .qetz is a zip of parts if (!content.startsWith("PK\x03\x04")) @@ -2735,7 +2742,64 @@ QETResult QETProject::writeCopyForOlderVersions(const QString &path) return(QETResult()); } -QETResult QETProject::write() +/** + @brief QETProject::rememberDiskState + Remember the size and modification time of the project file, as this QET + has just read or written it. changedOnDisk() compares against them. +*/ +void QETProject::rememberDiskState() +{ + const QFileInfo info(m_file_path); + m_disk_path = m_file_path; + m_disk_size = info.exists() ? info.size() : -1; + m_disk_mtime = info.exists() ? info.lastModified() : QDateTime(); +} + +/** + @brief QETProject::writeTarget + @return the file write() writes to: the .qetz beside a project read + from a .qet, else the project's own file +*/ +QString QETProject::writeTarget() const +{ + return m_opened_from_qet ? besideContainerPath() : m_file_path; +} + +/** + @brief QETProject::changedOnDisk + @param when set to the modification time of the file now on disk + @return true if the file write() would replace is not the one this QET + last read or wrote: another user, another program or another QET saved + it since. A file that does not exist yet, or one this QET never read + or wrote (Save As to another name), is not a change. +*/ +bool QETProject::changedOnDisk(QDateTime *when) const +{ + const QString target = writeTarget(); + if (target.isEmpty() || target != m_disk_path || m_disk_size < 0) + return false; + const QFileInfo info(target); + if (!info.exists()) + return false; + if (info.size() == m_disk_size && info.lastModified() == m_disk_mtime) + return false; + if (when) + *when = info.lastModified(); + return true; +} + +/** + @brief QETProject::write + Save the project in a file. + If the file was changed on disk since this QET read or wrote it, nothing + is written and the result says so: the interactive save asks the user + first (ProjectView) and calls again with @p overwrite_changed; scripts, + the command line, autosave and MCP have no one to ask and keep the + refusal. Setting QET_OVERWRITE_CHANGED=1 in the environment overwrites + without asking, for a script that means to. + @param overwrite_changed write even if the file changed on disk +*/ +QETResult QETProject::write(bool overwrite_changed) { // this operation requires a filepath if (m_file_path.isEmpty()) @@ -2771,6 +2835,15 @@ QETResult QETProject::write() return(QString("the file %1 was opened read-only and thus will not be written").arg(m_file_path)); } + if (!overwrite_changed && qEnvironmentVariableIntValue("QET_OVERWRITE_CHANGED") == 0) { + QDateTime when; + if (changedOnDisk(&when)) + return(failed(tr("someone else changed the file on disk at %1 since it was opened, " + "and saving would overwrite that. Save to another name, or " + "set QET_OVERWRITE_CHANGED=1 to overwrite it.") + .arg(QLocale::system().toString(when, QLocale::ShortFormat)))); + } + //The file as it was goes among its previous versions first //(Settings > General > Projects); a copy that fails does not stop //the save @@ -2797,6 +2870,8 @@ QETResult QETProject::write() return(failed(error_message)); } + rememberDiskState(); + // The project has just been written to a writable file (e.g. saved to // a new location with "Save As"), so it is no longer read-only. if (isReadOnly()) diff --git a/sources/qetproject.h b/sources/qetproject.h index 05603ca42..42aef41fb 100644 --- a/sources/qetproject.h +++ b/sources/qetproject.h @@ -39,6 +39,7 @@ # include #endif +#include #include #include #include @@ -333,7 +334,10 @@ class QETProject : public QObject QStringList *kept_parts = nullptr); static QString spliceKeptFolios(QString text, const QHash &kept_folios); bool close(); - QETResult write(); + QETResult write(bool overwrite_changed = false); + QString writeTarget() const; + void rememberDiskState(); + bool changedOnDisk(QDateTime *when = nullptr) const; bool isReadOnly() const; /// Opened from a .qetz of a later format (opened read-only) bool isFromLaterVersion() const {return m_from_later_version;} @@ -555,6 +559,11 @@ class QETProject : public QObject bool m_kept_folios_trusted = false; /// Read from a .qet whose path nobody chose since: saved beside it bool m_opened_from_qet = false; + /// The project file as this QET last read or wrote it, to see + /// whether something else changed it since (changedOnDisk()) + QString m_disk_path; + qint64 m_disk_size = -1; + QDateTime m_disk_mtime; /// Saving a .qetz may copy the part of a folio not built /// (toXml(), stage D4) bool m_kept_parts_trusted = false; diff --git a/tests/qttest/CMakeLists.txt b/tests/qttest/CMakeLists.txt index 9b589d403..fdf34309e 100644 --- a/tests/qttest/CMakeLists.txt +++ b/tests/qttest/CMakeLists.txt @@ -955,6 +955,18 @@ if(QET_HAS_SCRIPTING) "QET_TEST_BINARY_PATH=\"$\"" "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") + # Saving refuses a project file another user or program changed since it was + # opened (QETProject::changedOnDisk()); QET_OVERWRITE_CHANGED=1 overrides. + add_executable( + tst_changedondisk + tst_changedondisk.cpp) + add_test(NAME tst_changedondisk COMMAND tst_changedondisk) + add_dependencies(tst_changedondisk qelectrotech) + target_link_libraries(tst_changedondisk PRIVATE Qt::Test) + target_compile_definitions(tst_changedondisk PRIVATE + "QET_TEST_BINARY_PATH=\"$\"" + "QET_EXAMPLES_DIR=\"${QET_DIR}/examples\"") + # qet.elementTerminals() lists each terminal's uuid and qet.terminalIndex() # turns element uuid + terminal uuid back into the index the conductor # calls take, on examples/perceuse.qet (two terminals at one point). diff --git a/tests/qttest/tst_changedondisk.cpp b/tests/qttest/tst_changedondisk.cpp new file mode 100644 index 000000000..7b18aac68 --- /dev/null +++ b/tests/qttest/tst_changedondisk.cpp @@ -0,0 +1,107 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +#include + +#include +#include +#include +#include + +// Saving a project replaced the file whatever had happened to it since it was +// opened: another user's save, or another program's, was lost without a word. +// QETProject now remembers the file as it read or wrote it and write() refuses +// when the file on disk is no longer that one. Driven through the real +// binary's --run: qet.save() writes the file behind +// the open project without updating what it remembers, which is exactly "someone +// else saved"; qet.save('') is the project's own save. +class tst_changedondisk : public QObject +{ + Q_OBJECT + + QTemporaryDir m_dir; + QString m_project; // a .qetz made from examples/741.qet + + QString run(const QString &script, const QString &project, bool overwrite = false) + { + const QString path = m_dir.filePath(QStringLiteral("probe.js")); + const QString home = m_dir.filePath(QStringLiteral("home")); + QDir().mkpath(home); + QFile f(path); + if (!f.open(QIODevice::WriteOnly)) return QString(); + f.write(script.toUtf8()); + f.close(); + + QProcessEnvironment env = QProcessEnvironment::systemEnvironment(); + env.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("offscreen")); + env.insert(QStringLiteral("QET_ENABLE_SCRIPTING"), QStringLiteral("1")); + env.insert(QStringLiteral("HOME"), home); + env.insert(QStringLiteral("XDG_CONFIG_HOME"), home + QStringLiteral("/config")); + env.insert(QStringLiteral("XDG_DATA_HOME"), home + QStringLiteral("/data")); + env.insert(QStringLiteral("TMPDIR"), m_dir.path()); + env.remove(QStringLiteral("QET_OVERWRITE_CHANGED")); + if (overwrite) + env.insert(QStringLiteral("QET_OVERWRITE_CHANGED"), QStringLiteral("1")); + QProcess proc; + proc.setProcessEnvironment(env); + proc.start(QStringLiteral(QET_TEST_BINARY_PATH), + {QStringLiteral("--run"), path, project}); + if (!proc.waitForFinished(120000)) return QString(); + return QString::fromUtf8(proc.readAllStandardOutput() + proc.readAllStandardError()); + } + +private slots: + void initTestCase() + { + QVERIFY(m_dir.isValid()); + QVERIFY(QFile::exists(QStringLiteral(QET_TEST_BINARY_PATH))); + const QString qet = m_dir.filePath(QStringLiteral("p.qet")); + QVERIFY(QFile::copy(QStringLiteral(QET_EXAMPLES_DIR "/741.qet"), qet)); + // a .qet is saved as a .qetz beside it; that .qetz is the project below + const QString out = run(QStringLiteral("qet.log('R ' + qet.save(''));\n"), qet); + QVERIFY2(out.contains(QStringLiteral("R true")), qPrintable(out)); + m_project = m_dir.filePath(QStringLiteral("p.qetz")); + QVERIFY(QFile::exists(m_project)); + } + + void unchangedFileSaves() + { + const QString out = run(QStringLiteral( + "qet.log('R1 ' + qet.save(''));\nqet.log('R2 ' + qet.save(''));\n"), m_project); + QVERIFY2(out.contains(QStringLiteral("R1 true")), qPrintable(out)); + // the project's own save updates what it remembers: a second one is no change + QVERIFY2(out.contains(QStringLiteral("R2 true")), qPrintable(out)); + } + + void changedFileIsNotOverwritten() + { + const QString script = QStringLiteral( + "qet.save(%1);\nqet.log('R ' + qet.save(''));\n") + .arg(QStringLiteral("'") + m_project + QStringLiteral("'")); + const QString out = run(script, m_project); + QVERIFY2(out.contains(QStringLiteral("R false")), qPrintable(out)); + QVERIFY2(out.contains(QStringLiteral("changed the file on disk")), qPrintable(out)); + } + + void overrideOverwrites() + { + const QString script = QStringLiteral( + "qet.save(%1);\nqet.log('R ' + qet.save(''));\n") + .arg(QStringLiteral("'") + m_project + QStringLiteral("'")); + const QString out = run(script, m_project, true); + QVERIFY2(out.contains(QStringLiteral("R true")), qPrintable(out)); + } + + void saveAsTargetIsNotAConflict() + { + const QString other = m_dir.filePath(QStringLiteral("other.qetz")); + const QString script = QStringLiteral( + "qet.log('R1 ' + qet.save(%1));\nqet.log('R2 ' + qet.save(%1));\n") + .arg(QStringLiteral("'") + other + QStringLiteral("'")); + const QString out = run(script, m_project); + QVERIFY2(out.contains(QStringLiteral("R1 true")), qPrintable(out)); + QVERIFY2(out.contains(QStringLiteral("R2 true")), qPrintable(out)); + } +}; + +QTEST_APPLESS_MAIN(tst_changedondisk) + +#include "tst_changedondisk.moc"