diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index 89c642692..ef9ba1efd 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -114,17 +114,17 @@ void FileSystem::setFolderMinimumPermissions(const QString &filename) #endif } - -void FileSystem::setFileReadOnlyWeak(const QString &filename, bool readonly) +bool FileSystem::setFileReadOnlyWeak(const QString &filename, bool readonly) { QFile file(filename); QFile::Permissions permissions = file.permissions(); if (!readonly && (permissions & QFile::WriteOwner)) { - return; // already writable enough + return false; // already writable enough } setFileReadOnly(filename, readonly); + return true; } bool FileSystem::rename(const QString &originFileName, diff --git a/src/common/filesystembase.h b/src/common/filesystembase.h index bc0b592c2..fd0572804 100644 --- a/src/common/filesystembase.h +++ b/src/common/filesystembase.h @@ -65,7 +65,7 @@ namespace FileSystem { * This means that it will preserve explicitly set rw-r--r-- permissions even * when the umask is 0002. (setFileReadOnly() would adjust to rw-rw-r--) */ - void OCSYNC_EXPORT setFileReadOnlyWeak(const QString &filename, bool readonly); + bool OCSYNC_EXPORT setFileReadOnlyWeak(const QString &filename, bool readonly); /** * @brief Try to set permissions so that other users on the local machine can not diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp index 52985fd44..656689a55 100644 --- a/src/libsync/syncengine.cpp +++ b/src/libsync/syncengine.cpp @@ -357,6 +357,7 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item) // mini-jobs later on, we just update metadata right now. if (item->_direction == SyncFileItem::Down) { + auto modificationHappened = false; // Decides whether or not we modify file metadata QString filePath = _localPath + item->_file; // If the 'W' remote permission changed, update the local filesystem @@ -365,15 +366,18 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item) && prev.isValid() && prev._remotePerm.hasPermission(RemotePermissions::CanWrite) != item->_remotePerm.hasPermission(RemotePermissions::CanWrite)) { const bool isReadOnly = !item->_remotePerm.isNull() && !item->_remotePerm.hasPermission(RemotePermissions::CanWrite); - FileSystem::setFileReadOnlyWeak(filePath, isReadOnly); + modificationHappened = FileSystem::setFileReadOnlyWeak(filePath, isReadOnly); } + + modificationHappened |= item->_size != prev._fileSize; + auto rec = item->toSyncJournalFileRecordWithInode(filePath); if (rec._checksumHeader.isEmpty()) rec._checksumHeader = prev._checksumHeader; rec._serverHasIgnoredFiles |= prev._serverHasIgnoredFiles; // Ensure it's a placeholder file on disk - if (item->_type == ItemTypeFile) { + if (item->_type == ItemTypeFile && _syncOptions._vfs->mode() != Vfs::Off) { const auto result = _syncOptions._vfs->convertToPlaceholder(filePath, *item); if (!result) { item->_status = SyncFileItem::Status::NormalError; @@ -382,10 +386,11 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item) emit itemCompleted(item, ErrorCategory::GenericError); return; } + modificationHappened = true; } // Update on-disk virtual file metadata - if (item->_type == ItemTypeVirtualFile) { + if (modificationHappened && item->_type == ItemTypeVirtualFile) { auto r = _syncOptions._vfs->updateMetadata(filePath, item->_modtime, item->_size, item->_fileId); if (!r) { item->_status = SyncFileItem::Status::NormalError; @@ -394,7 +399,7 @@ void OCC::SyncEngine::slotItemDiscovered(const OCC::SyncFileItemPtr &item) emit itemCompleted(item, ErrorCategory::GenericError); return; } - } else { + } else if (modificationHappened || prev._modtime != item->_modtime) { if (!FileSystem::setModTime(filePath, item->_modtime)) { item->_instruction = CSYNC_INSTRUCTION_ERROR; item->_errorString = tr("Could not update file metadata: %1").arg(filePath); diff --git a/test/syncenginetestutils.cpp b/test/syncenginetestutils.cpp index 82aa70166..88ece82c1 100644 --- a/test/syncenginetestutils.cpp +++ b/test/syncenginetestutils.cpp @@ -113,6 +113,12 @@ void DiskFileModifier::setE2EE([[maybe_unused]] const QString &relativePath, [[m { } +QFile DiskFileModifier::find(const QString &relativePath) const +{ + const auto path = _rootDir.filePath(relativePath); + return QFile(path); +} + FileInfo FileInfo::A12_B12_C12_S12() { FileInfo fi { QString {}, { diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index 4f9c552ba..3a299f3ac 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -108,6 +108,8 @@ public: void setModTime(const QString &relativePath, const QDateTime &modTime) override; void modifyLockState(const QString &relativePath, LockState lockState, int lockType, const QString &lockOwner, const QString &lockOwnerId, const QString &lockEditorId, quint64 lockTime, quint64 lockTimeout) override; void setE2EE(const QString &relativepath, const bool enabled) override; + + [[nodiscard]] QFile find(const QString &relativePath) const; }; class FileInfo : public FileModifier diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index 9516a0557..f4c4c0236 100644 --- a/test/testsyncengine.cpp +++ b/test/testsyncengine.cpp @@ -12,6 +12,7 @@ #include "propagatorjobs.h" #include "syncengine.h" +#include #include using namespace OCC; @@ -921,6 +922,29 @@ private slots: QCOMPARE(QFileInfo(fakeFolder.localPath() + "foo").lastModified(), datetime); } + // A local file should not be modified after upload to server if nothing has changed. + void testLocalFileInitialMtime() + { + constexpr auto fooFolder = "foo/"; + constexpr auto barFile = "foo/bar"; + + FakeFolder fakeFolder{FileInfo{}}; + fakeFolder.localModifier().mkdir(fooFolder); + fakeFolder.localModifier().insert(barFile); + + const auto localDiskFileModifier = dynamic_cast(fakeFolder.localModifier()); + + const auto localFile = localDiskFileModifier.find(barFile); + const auto localFileInfo = QFileInfo(localFile); + const auto expectedMtime = localFileInfo.metadataChangeTime(); + + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + const auto currentMtime = localFileInfo.metadataChangeTime(); + QCOMPARE(currentMtime, expectedMtime); + } + /** * Checks whether subsequent large uploads are skipped after a 507 error */