Merge pull request #1506 from ispyisail/feat/changed-on-disk-check

Ask before saving over a project someone else changed

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 

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();

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())

sources/qetproject.h

@@ -39,6 +39,7 @@
 #	include <KAutoSaveFile>
 #endif
 
+#include <QDateTime>
 #include <QHash>
 #include <QSet>
 #include <QUuid>
@@ -333,7 +334,10 @@ class QETProject : public QObject
 						   QStringList *kept_parts = nullptr);
 		static QString spliceKeptFolios(QString text, const QHash<int, QByteArray> &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;

tests/qttest/CMakeLists.txt

@@ -955,6 +955,18 @@ if(QET_HAS_SCRIPTING)
     "QET_TEST_BINARY_PATH=\"$<TARGET_FILE:qelectrotech>\""
     "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=\"$<TARGET_FILE:qelectrotech>\""
+    "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).

tests/qttest/tst_changedondisk.cpp

@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <QtTest>
+
+#include <QFile>
+#include <QProcess>
+#include <QProcessEnvironment>
+#include <QTemporaryDir>
+
+// 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(<the project's own path>) 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"