Merge pull request #1582 from ispyisail/fix/1579-changed-on-disk-network-share

Fix #1579: false "changed on disk" warning on network shares

sources/qetproject.cpp

@@ -504,7 +504,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();
+	rememberDiskState(&content);
 		//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"))
@@ -2804,16 +2804,37 @@ QETResult QETProject::writeCopyForOlderVersions(const QString &path)
 }
 
 /**
-	@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.
+	@brief diskHash
+	@return the SHA-1 of @p content, or of the file at @p path when
+	@p content is null; empty if the file cannot be read
 */
-void QETProject::rememberDiskState()
+static QByteArray diskHash(const QString &path, const QByteArray *content = nullptr)
+{
+	QCryptographicHash hash(QCryptographicHash::Sha1);
+	if (content) {
+		hash.addData(*content);
+		return hash.result();
+	}
+	QFile file(path);
+	if (!file.open(QIODevice::ReadOnly) || !hash.addData(&file))
+		return QByteArray();
+	return hash.result();
+}
+
+/**
+	@brief QETProject::rememberDiskState
+	Remember the size, modification time and content hash of the project
+	file, as this QET has just read or written it. changedOnDisk() compares
+	against them.
+	@param content the bytes just read from the file, if they are at hand
+*/
+void QETProject::rememberDiskState(const QByteArray *content)
 {
 	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();
+	m_disk_hash = info.exists() ? diskHash(m_file_path, content) : QByteArray();
 }
 
 /**
@@ -2823,6 +2844,10 @@ void QETProject::rememberDiskState()
 	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.
+	A different size or time alone is not enough: right after a save, a
+	network share (CIFS with its attribute cache, #1579) can report the size
+	and time from before the server settled them. The file is a change only
+	if its content differs from what this QET read or wrote.
 */
 bool QETProject::changedOnDisk(QDateTime *when) const
 {
@@ -2834,6 +2859,8 @@ bool QETProject::changedOnDisk(QDateTime *when) const
 		return false;
 	if (info.size() == m_disk_size && info.lastModified() == m_disk_mtime)
 		return false;
+	if (!m_disk_hash.isEmpty() && diskHash(target) == m_disk_hash)
+		return false;
 	if (when)
 		*when = info.lastModified();
 	return true;

sources/qetproject.h

@@ -337,7 +337,7 @@ class QETProject : public QObject
 		static QString spliceKeptFolios(QString text, const QHash<int, QByteArray> &kept_folios);
 		bool close();
 		QETResult write(bool overwrite_changed = false);
-		void rememberDiskState();
+		void rememberDiskState(const QByteArray *content = nullptr);
 		bool changedOnDisk(QDateTime *when = nullptr) const;
 		bool isReadOnly() const;
 			/// Opened from a .qetz of a later format (opened read-only)
@@ -558,6 +558,7 @@ class QETProject : public QObject
 		QString m_disk_path;
 		qint64 m_disk_size = -1;
 		QDateTime m_disk_mtime;
+		QByteArray m_disk_hash;
 			/// Saving a .qetz may copy the part of a folio not built
 			/// (toXml(), stage D4)
 		bool m_kept_parts_trusted = false;

tests/qttest/tst_changedondisk.cpp

@@ -12,7 +12,9 @@
 // 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.
+// else saved"; qet.save('') is the project's own save. Only a different content
+// counts: the same bytes with a new time are what a network share reports
+// right after our own save (#1579).
 class tst_changedondisk : public QObject
 {
 	Q_OBJECT
@@ -71,9 +73,21 @@ private slots:
 		QVERIFY2(out.contains(QStringLiteral("R2 true")), qPrintable(out));
 	}
 
+	void sameContentNewTimeSaves()
+	{
+		// the same bytes written behind the project (an unchanged project
+		// saves to an identical file, F124): only the time differs
+		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 true")), qPrintable(out));
+	}
+
 	void changedFileIsNotOverwritten()
 	{
 		const QString script = QStringLiteral(
+			"qet.moveElement(0, qet.elementUuids(0)[0], 10, 0);\n"
 			"qet.save(%1);\nqet.log('R ' + qet.save(''));\n")
 			.arg(QStringLiteral("'") + m_project + QStringLiteral("'"));
 		const QString out = run(script, m_project);
@@ -84,6 +98,7 @@ private slots:
 	void overrideOverwrites()
 	{
 		const QString script = QStringLiteral(
+			"qet.moveElement(0, qet.elementUuids(0)[0], 10, 0);\n"
 			"qet.save(%1);\nqet.log('R ' + qet.save(''));\n")
 			.arg(QStringLiteral("'") + m_project + QStringLiteral("'"));
 		const QString out = run(script, m_project, true);