diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp index ac6d0e8f8356f..57426dd8a175b 100644 --- a/src/libsync/discovery.cpp +++ b/src/libsync/discovery.cpp @@ -345,7 +345,7 @@ void ProcessDirectoryJob::processFile(PathTuple path, { const char *hasServer = serverEntry.isValid() ? "true" : _queryServer == ParentNotChanged ? "db" : "false"; const char *hasLocal = localEntry.isValid() ? "true" : _queryLocal == ParentNotChanged ? "db" : "false"; - const auto serverFileIsLocked = serverEntry.locked == SyncFileItem::LockStatus::LockedItem ? "locked" : "not locked"; + const auto serverFileIsLocked = (serverEntry.isValid() ? (serverEntry.locked == SyncFileItem::LockStatus::LockedItem ? "locked" : "not locked") : ""); const auto localFileIsLocked = dbEntry._lockstate._locked ? "locked" : "not locked"; qCInfo(lcDisco).nospace() << "Processing " << path._original << " | (db/local/remote)" @@ -394,29 +394,6 @@ void ProcessDirectoryJob::processFile(PathTuple path, if (item->_type == ItemTypeVirtualFileDehydration) item->_type = ItemTypeFile; - // We want to check the lock state of this file after the lock time has expired - if(serverEntry.locked == SyncFileItem::LockStatus::LockedItem) { - const auto lockExpirationTime = serverEntry.lockTime + serverEntry.lockTimeout; - const auto timeRemaining = QDateTime::currentDateTime().secsTo(QDateTime::fromSecsSinceEpoch(lockExpirationTime)); - // Add on a second as a precaution, sometimes we catch the server before it has had a chance to update - const auto lockExpirationTimeout = qMax(5LL, timeRemaining + 1); - - qCInfo(lcDisco) << "File:" << path._original << "is locked." - << "Lock expires in:" << lockExpirationTimeout << "seconds." - << "A sync run will be scheduled for around that time."; - - _discoveryData->_anotherSyncNeeded = true; - _discoveryData->_filesNeedingScheduledSync.insert(path._original, lockExpirationTimeout); - - } else if (serverEntry.locked == SyncFileItem::LockStatus::UnlockedItem && dbEntry._lockstate._locked) { - // We have received data that this file has been unlocked remotely, so let's notify the sync engine - // that we no longer need a scheduled sync run for this file - qCInfo(lcDisco) << "File:" << path._original << "is unlocked and a scheduled sync is no longer needed." - << "Will remove scheduled sync if there is one."; - - _discoveryData->_filesUnscheduleSync.append(path._original); - } - // VFS suffixed files on the server are ignored if (isVfsWithSuffix()) { if (hasVirtualFileSuffix(serverEntry.name) @@ -523,14 +500,28 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( } } - // We need to make sure that we update the info in the database if the lockstate has changed - const auto checkFileLockState = [&item, &dbEntry, &serverEntry] { - const bool isServerEntryLocked = serverEntry.locked == SyncFileItem::LockStatus::LockedItem; + // We want to check the lock state of this file after the lock time has expired + if(serverEntry.locked == SyncFileItem::LockStatus::LockedItem) { + const auto lockExpirationTime = serverEntry.lockTime + serverEntry.lockTimeout; + const auto timeRemaining = QDateTime::currentDateTime().secsTo(QDateTime::fromSecsSinceEpoch(lockExpirationTime)); + // Add on a second as a precaution, sometimes we catch the server before it has had a chance to update + const auto lockExpirationTimeout = qMax(5LL, timeRemaining + 1); - if(isServerEntryLocked != dbEntry._lockstate._locked) { - item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA; - } - }; + qCInfo(lcDisco) << "File:" << path._original << "is locked." + << "Lock expires in:" << lockExpirationTimeout << "seconds." + << "A sync run will be scheduled for around that time."; + + _discoveryData->_anotherSyncNeeded = true; + _discoveryData->_filesNeedingScheduledSync.insert(path._original, lockExpirationTimeout); + + } else if (serverEntry.locked == SyncFileItem::LockStatus::UnlockedItem && dbEntry._lockstate._locked) { + // We have received data that this file has been unlocked remotely, so let's notify the sync engine + // that we no longer need a scheduled sync run for this file + qCInfo(lcDisco) << "File:" << path._original << "is unlocked and a scheduled sync is no longer needed." + << "Will remove scheduled sync if there is one."; + + _discoveryData->_filesUnscheduleSync.append(path._original); + } // The file is known in the db already if (dbEntry.isValid()) { @@ -613,12 +604,10 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo( return ParentNotChanged; }(); - checkFileLockState(); processFileAnalyzeLocalInfo(item, path, localEntry, serverEntry, dbEntry, serverQueryMode); return; } - checkFileLockState(); processFileAnalyzeLocalInfo(item, path, localEntry, serverEntry, dbEntry, _queryServer); return; } diff --git a/test/syncenginetestutils.cpp b/test/syncenginetestutils.cpp index 70b87bb5a9d6f..f7e1faf5abb61 100644 --- a/test/syncenginetestutils.cpp +++ b/test/syncenginetestutils.cpp @@ -103,6 +103,10 @@ void DiskFileModifier::setModTime(const QString &relativePath, const QDateTime & OCC::FileSystem::setModTime(_rootDir.filePath(relativePath), OCC::Utility::qDateTimeToTime_t(modTime)); } +void DiskFileModifier::modifyLockState([[maybe_unused]] const QString &relativePath, [[maybe_unused]] LockState lockState, [[maybe_unused]] int lockType, [[maybe_unused]] const QString &lockOwner, [[maybe_unused]] const QString &lockOwnerId, [[maybe_unused]] const QString &lockEditorId, [[maybe_unused]] quint64 lockTime, [[maybe_unused]] quint64 lockTimeout) +{ +} + FileInfo FileInfo::A12_B12_C12_S12() { FileInfo fi { QString {}, { @@ -195,6 +199,19 @@ void FileInfo::setModTimeKeepEtag(const QString &relativePath, const QDateTime & file->lastModified = modTime; } +void FileInfo::modifyLockState(const QString &relativePath, LockState lockState, int lockType, const QString &lockOwner, const QString &lockOwnerId, const QString &lockEditorId, quint64 lockTime, quint64 lockTimeout) +{ + FileInfo *file = findInvalidatingEtags(relativePath); + Q_ASSERT(file); + file->lockState = lockState; + file->lockType = lockType; + file->lockOwner = lockOwner; + file->lockOwnerId = lockOwnerId; + file->lockEditorId = lockEditorId; + file->lockTime = lockTime; + file->lockTimeout = lockTimeout; +} + FileInfo *FileInfo::find(PathComponents pathComponents, const bool invalidateEtags) { if (pathComponents.isEmpty()) { @@ -334,6 +351,13 @@ FakePropfindReply::FakePropfindReply(FileInfo &remoteRootFileInfo, QNetworkAcces xml.writeTextElement(ocUri, QStringLiteral("permissions"), !fileInfo.permissions.isNull() ? QString(fileInfo.permissions.toString()) : fileInfo.isShared ? QStringLiteral("SRDNVCKW") : QStringLiteral("RDNVCKW")); xml.writeTextElement(ocUri, QStringLiteral("id"), QString::fromUtf8(fileInfo.fileId)); xml.writeTextElement(ocUri, QStringLiteral("checksums"), QString::fromUtf8(fileInfo.checksums)); + xml.writeTextElement(ncUri, QStringLiteral("lock-owner"), fileInfo.lockOwnerId); + xml.writeTextElement(ncUri, QStringLiteral("lock"), fileInfo.lockState == FileInfo::LockState::FileLocked ? QStringLiteral("1") : QStringLiteral("0")); + xml.writeTextElement(ncUri, QStringLiteral("lock-owner-type"), fileInfo.lockOwnerId); + xml.writeTextElement(ncUri, QStringLiteral("lock-owner-displayname"), fileInfo.lockOwnerId); + xml.writeTextElement(ncUri, QStringLiteral("lock-owner-editor"), fileInfo.lockOwnerId); + xml.writeTextElement(ncUri, QStringLiteral("lock-time"), QString::number(fileInfo.lockTime)); + xml.writeTextElement(ncUri, QStringLiteral("lock-timeout"), QString::number(fileInfo.lockTimeout)); buffer.write(fileInfo.extraDavProperties); xml.writeEndElement(); // prop xml.writeTextElement(davUri, QStringLiteral("status"), QStringLiteral("HTTP/1.1 200 OK")); diff --git a/test/syncenginetestutils.h b/test/syncenginetestutils.h index ddd50273bbb7e..ab4074df6155c 100644 --- a/test/syncenginetestutils.h +++ b/test/syncenginetestutils.h @@ -76,6 +76,11 @@ class PathComponents : public QStringList { class FileModifier { public: + enum class LockState { + FileLocked, + FileUnlocked, + }; + virtual ~FileModifier() = default; virtual void remove(const QString &relativePath) = 0; virtual void insert(const QString &relativePath, qint64 size = 64, char contentChar = 'W') = 0; @@ -84,6 +89,7 @@ class FileModifier virtual void mkdir(const QString &relativePath) = 0; virtual void rename(const QString &relativePath, const QString &relativeDestinationDirectory) = 0; virtual void setModTime(const QString &relativePath, const QDateTime &modTime) = 0; + virtual void modifyLockState(const QString &relativePath, LockState lockState, int lockType, const QString &lockOwner, const QString &lockOwnerId, const QString &lockEditorId, quint64 lockTime, quint64 lockTimeout) = 0; }; class DiskFileModifier : public FileModifier @@ -99,6 +105,7 @@ class DiskFileModifier : public FileModifier void mkdir(const QString &relativePath) override; void rename(const QString &from, const QString &to) override; 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; }; class FileInfo : public FileModifier @@ -130,6 +137,8 @@ class FileInfo : public FileModifier void setModTimeKeepEtag(const QString &relativePath, const QDateTime &modTime); + void modifyLockState(const QString &relativePath, LockState lockState, int lockType, const QString &lockOwner, const QString &lockOwnerId, const QString &lockEditorId, quint64 lockTime, quint64 lockTimeout) override; + FileInfo *find(PathComponents pathComponents, const bool invalidateEtags = false); FileInfo *createDir(const QString &relativePath); @@ -163,6 +172,13 @@ class FileInfo : public FileModifier QByteArray extraDavProperties; qint64 size = 0; char contentChar = 'W'; + LockState lockState = LockState::FileUnlocked; + int lockType = 0; + QString lockOwner; + QString lockOwnerId; + QString lockEditorId; + quint64 lockTime = 0; + quint64 lockTimeout = 0; // Sorted by name to be able to compare trees QMap children; diff --git a/test/testlocaldiscovery.cpp b/test/testlocaldiscovery.cpp index 80350bf907827..9ed191c8e8cb3 100644 --- a/test/testlocaldiscovery.cpp +++ b/test/testlocaldiscovery.cpp @@ -621,15 +621,7 @@ private slots: fakeFolder.remoteModifier().insert(fooFileRootFolder); fakeFolder.remoteModifier().insert(barFileRootFolder); - - const auto lockedFileDavProps = QByteArray("1" - "0" - "user1" - "user1" - "user1" - "1648046707"); - - fakeFolder.remoteModifier().find("bar")->extraDavProperties = lockedFileDavProps; + fakeFolder.remoteModifier().modifyLockState(QStringLiteral("bar"), FileInfo::LockState::FileLocked, 0, QStringLiteral("user1"), {}, QStringLiteral("user1"), 1648046707, 0); fakeFolder.remoteModifier().mkdir(QStringLiteral("subfolder")); fakeFolder.remoteModifier().insert(fooFileSubFolder); @@ -648,8 +640,7 @@ private slots: QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("bar"), &fileRecordBefore)); QVERIFY(fileRecordBefore._lockstate._locked); - const auto unlockedFileDavProps = QByteArray("0"); - fakeFolder.remoteModifier().find("bar")->extraDavProperties = unlockedFileDavProps; + fakeFolder.remoteModifier().modifyLockState(QStringLiteral("bar"), FileInfo::LockState::FileUnlocked, {}, {}, {}, {}, {}, {}); fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::DatabaseAndFilesystem); diff --git a/test/testlockfile.cpp b/test/testlockfile.cpp index 97538295e5b4f..d0dcc2ae523f8 100644 --- a/test/testlockfile.cpp +++ b/test/testlockfile.cpp @@ -5,6 +5,7 @@ #include "common/syncjournaldb.h" #include "common/syncjournalfilerecord.h" #include "syncenginetestutils.h" +#include "localdiscoverytracker.h" #include #include @@ -482,6 +483,154 @@ private slots: QVERIFY(jobFailure.wait()); QCOMPARE(jobSuccess.count(), 0); } + + void testSyncLockedFilesAlmostExpired() + { + FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + QSignalSpy spySyncCompleted(&fakeFolder.syncEngine(), &OCC::SyncEngine::finished); + + ItemCompletedSpy completeSpy(fakeFolder); + + completeSpy.clear(); + QVERIFY(fakeFolder.syncOnce()); + + QCOMPARE(completeSpy.findItem(QStringLiteral("A/a1"))->_locked, OCC::SyncFileItem::LockStatus::UnlockedItem); + OCC::SyncJournalFileRecord fileRecordBefore; + QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("A/a1"), &fileRecordBefore)); + QVERIFY(!fileRecordBefore._lockstate._locked); + + fakeFolder.remoteModifier().modifyLockState(QStringLiteral("A/a1"), FileModifier::LockState::FileLocked, 1, QStringLiteral("Nextcloud Office"), {}, QStringLiteral("richdocuments"), QDateTime::currentDateTime().toSecsSinceEpoch() - 1220, 1226); + + completeSpy.clear(); + QVERIFY(fakeFolder.syncOnce()); + + QCOMPARE(completeSpy.findItem(QStringLiteral("A/a1"))->_locked, OCC::SyncFileItem::LockStatus::LockedItem); + OCC::SyncJournalFileRecord fileRecordLocked; + QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("A/a1"), &fileRecordLocked)); + QVERIFY(fileRecordLocked._lockstate._locked); + + spySyncCompleted.clear(); + + QTest::qWait(5000); + + fakeFolder.remoteModifier().modifyLockState(QStringLiteral("A/a1"), FileModifier::LockState::FileUnlocked, {}, {}, {}, {}, {}, {}); + + QCOMPARE(spySyncCompleted.count(), 0); + QVERIFY(spySyncCompleted.wait(3000)); + QCOMPARE(spySyncCompleted.count(), 1); + + OCC::SyncJournalFileRecord fileRecordUnlocked; + QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("A/a1"), &fileRecordUnlocked)); + QVERIFY(!fileRecordUnlocked._lockstate._locked); + } + + void testSyncLockedFilesNoExpiredLockedFiles() + { + FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + QSignalSpy spySyncCompleted(&fakeFolder.syncEngine(), &OCC::SyncEngine::finished); + + ItemCompletedSpy completeSpy(fakeFolder); + + completeSpy.clear(); + QVERIFY(fakeFolder.syncOnce()); + + QCOMPARE(completeSpy.findItem(QStringLiteral("A/a1"))->_locked, OCC::SyncFileItem::LockStatus::UnlockedItem); + OCC::SyncJournalFileRecord fileRecordBefore; + QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("A/a1"), &fileRecordBefore)); + QVERIFY(!fileRecordBefore._lockstate._locked); + + fakeFolder.remoteModifier().modifyLockState(QStringLiteral("A/a1"), FileModifier::LockState::FileLocked, 1, QStringLiteral("Nextcloud Office"), {}, QStringLiteral("richdocuments"), QDateTime::currentDateTime().toSecsSinceEpoch() - 1220, 1226); + + completeSpy.clear(); + QVERIFY(fakeFolder.syncOnce()); + + QCOMPARE(completeSpy.findItem(QStringLiteral("A/a1"))->_locked, OCC::SyncFileItem::LockStatus::LockedItem); + OCC::SyncJournalFileRecord fileRecordLocked; + QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("A/a1"), &fileRecordLocked)); + QVERIFY(fileRecordLocked._lockstate._locked); + + completeSpy.clear(); + QVERIFY(fakeFolder.syncOnce()); + + spySyncCompleted.clear(); + + QTest::qWait(1000); + + fakeFolder.remoteModifier().modifyLockState(QStringLiteral("A/a1"), FileModifier::LockState::FileUnlocked, {}, {}, {}, {}, {}, {}); + + QCOMPARE(spySyncCompleted.count(), 0); + QVERIFY(!spySyncCompleted.wait(3000)); + QCOMPARE(spySyncCompleted.count(), 0); + + OCC::SyncJournalFileRecord fileRecordUnlocked; + QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("A/a1"), &fileRecordUnlocked)); + QVERIFY(fileRecordUnlocked._lockstate._locked); + } + + void testSyncLockedFiles() + { + FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() }; + QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState()); + + int nGET = 0, nPUT = 0; + QObject parent; + fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *outgoingData) -> QNetworkReply * { + Q_UNUSED(outgoingData) + Q_UNUSED(request) + + if (op == QNetworkAccessManager::PutOperation) { + ++nPUT; + } else if (op == QNetworkAccessManager::GetOperation) { + ++nGET; + } + + return nullptr; + }); + + ItemCompletedSpy completeSpy(fakeFolder); + + completeSpy.clear(); + QVERIFY(fakeFolder.syncOnce()); + QCOMPARE(nGET, 0); + QCOMPARE(nPUT, 0); + + QCOMPARE(completeSpy.findItem(QStringLiteral("A/a1"))->_locked, OCC::SyncFileItem::LockStatus::UnlockedItem); + OCC::SyncJournalFileRecord fileRecordBefore; + QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("A/a1"), &fileRecordBefore)); + QVERIFY(!fileRecordBefore._lockstate._locked); + + fakeFolder.remoteModifier().modifyLockState(QStringLiteral("A/a1"), FileModifier::LockState::FileLocked, 1, QStringLiteral("Nextcloud Office"), {}, QStringLiteral("richdocuments"), QDateTime::currentDateTime().toSecsSinceEpoch(), 1226); + fakeFolder.remoteModifier().setModTimeKeepEtag(QStringLiteral("A/a1"), QDateTime::currentDateTime()); + fakeFolder.remoteModifier().appendByte(QStringLiteral("A/a1")); + + completeSpy.clear(); + QVERIFY(fakeFolder.syncOnce()); + + QCOMPARE(nGET, 1); + QCOMPARE(nPUT, 0); + + QCOMPARE(completeSpy.findItem(QStringLiteral("A/a1"))->_locked, OCC::SyncFileItem::LockStatus::LockedItem); + OCC::SyncJournalFileRecord fileRecordLocked; + QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("A/a1"), &fileRecordLocked)); + QVERIFY(fileRecordLocked._lockstate._locked); + + completeSpy.clear(); + QVERIFY(fakeFolder.syncOnce()); + + QCOMPARE(nGET, 1); + QCOMPARE(nPUT, 0); + + OCC::SyncJournalFileRecord fileRecordAfter; + QVERIFY(fakeFolder.syncJournal().getFileRecord(QStringLiteral("A/a1"), &fileRecordAfter)); + QVERIFY(fileRecordAfter._lockstate._locked); + + auto expectedState = fakeFolder.currentLocalState(); + QCOMPARE(fakeFolder.currentRemoteState(), expectedState); + } }; QTEST_GUILESS_MAIN(TestLockFile)