Merge pull request #6025 from nextcloud/backport/6022/stable-3.10

[stable-3.10] Bugfix/lock state spam

src/gui/editlocallyjob.cpp

@@ -647,6 +647,8 @@ void EditLocallyJob::lockFile()
                                       this, &EditLocallyJob::fileLockError));
 
     _folderForFile->accountState()->account()->setLockFileState(_relPath,
+                                                                _folderForFile->remotePathTrailingSlash(),
+                                                                _folderForFile->path(),
                                                                 _folderForFile->journalDb(),
                                                                 SyncFileItem::LockStatus::LockedItem);
 }

src/gui/folder.cpp

@@ -666,7 +666,11 @@ void Folder::slotFilesLockReleased(const QSet<QString> &files)
             disconnect(_officeFileLockReleaseUnlockFailure);
             qCWarning(lcFolder) << "Failed to unlock a file:" << remoteFilePath << message;
         });
-        _accountState->account()->setLockFileState(remoteFilePath, journalDb(), SyncFileItem::LockStatus::UnlockedItem);
+        _accountState->account()->setLockFileState(remoteFilePath,
+                                                   remotePathTrailingSlash(),
+                                                   path(),
+                                                   journalDb(),
+                                                   SyncFileItem::LockStatus::UnlockedItem);
     }
 }
 
@@ -690,10 +694,7 @@ void Folder::slotLockedFilesFound(const QSet<QString> &files)
         const auto fileRecordPath = fileFromLocalPath(file);
         SyncJournalFileRecord rec;
 
-        const auto canLockFile = journalDb()->getFileRecord(fileRecordPath, &rec) && rec.isValid()
-            && (!rec._lockstate._locked
-                || (rec._lockstate._lockOwnerType == static_cast<qint64>(SyncFileItem::LockOwnerType::UserLock)
-                    && rec._lockstate._lockOwnerId == _accountState->account()->davUser()));
+        const auto canLockFile = journalDb()->getFileRecord(fileRecordPath, &rec) && rec.isValid() && !rec._lockstate._locked;
 
         if (!canLockFile) {
             qCDebug(lcFolder) << "Skipping locking file" << file << "with rec.isValid():" << rec.isValid()
@@ -712,7 +713,11 @@ void Folder::slotLockedFilesFound(const QSet<QString> &files)
             disconnect(_fileLockFailure);
             qCWarning(lcFolder) << "Failed to lock a file:" << remoteFilePath << message;
         });
-        _accountState->account()->setLockFileState(remoteFilePath, journalDb(), SyncFileItem::LockStatus::LockedItem);
+        _accountState->account()->setLockFileState(remoteFilePath,
+                                                   remotePathTrailingSlash(),
+                                                   path(),
+                                                   journalDb(),
+                                                   SyncFileItem::LockStatus::LockedItem);
     }
 }
 

src/gui/folderwatcher.cpp

@@ -35,7 +35,6 @@
 #include <QDir>
 #include <QMutexLocker>
 #include <QStringList>
-#include <QTimer>
 
 #include <array>
 #include <cstdint>
@@ -44,6 +43,8 @@ namespace
 {
 const std::array<const char *, 2> lockFilePatterns = {{".~lock.", "~$"}};
 
+constexpr auto lockChangeDebouncingTimerIntervalMs = 500;
+
 QString filePathLockFilePatternMatch(const QString &path)
 {
     qCDebug(OCC::lcFolderWatcher) << "Checking if it is a lock file:" << path;
@@ -78,6 +79,8 @@ FolderWatcher::FolderWatcher(Folder *folder)
     : QObject(folder)
     , _folder(folder)
 {
+    _lockChangeDebouncingTimer.setInterval(lockChangeDebouncingTimerIntervalMs);
+
     if (_folder && _folder->accountState() && _folder->accountState()->account()) {
         connect(_folder->accountState()->account().data(), &Account::capabilitiesChanged, this, &FolderWatcher::folderAccountCapabilitiesChanged);
         folderAccountCapabilitiesChanged();
@@ -157,6 +160,20 @@ void FolderWatcher::startNotificationTestWhenReady()
     });
 }
 
+void FolderWatcher::lockChangeDebouncingTimerTimedOut()
+{
+    if (!_unlockedFiles.isEmpty()) {
+        const auto unlockedFilesCopy = _unlockedFiles;
+        emit filesLockReleased(unlockedFilesCopy);
+        _unlockedFiles.clear();
+    }
+    if (!_lockedFiles.isEmpty()) {
+        const auto lockedFilesCopy = _lockedFiles;
+        emit filesLockImposed(lockedFilesCopy);
+        emit lockedFilesFound(lockedFilesCopy);
+        _lockedFiles.clear();
+    }
+}
 
 int FolderWatcher::testLinuxWatchCount() const
 {
@@ -195,8 +212,6 @@ void FolderWatcher::changeDetected(const QStringList &paths)
     _timer.restart();
 
     QSet<QString> changedPaths;
-    QSet<QString> unlockedFiles;
-    QSet<QString> lockedFiles;
 
     for (const auto &path : paths) {
         if (!_testNotificationPath.isEmpty()
@@ -208,18 +223,18 @@ void FolderWatcher::changeDetected(const QStringList &paths)
         const auto checkResult = lockFileTargetFilePath(path,lockFileNamePattern);
         if (_shouldWatchForFileUnlocking) {
             // Lock file has been deleted, file now unlocked
-            if (checkResult.type == FileLockingInfo::Type::Unlocked && !checkResult.path.isEmpty() && !QFile::exists(path)) {
-                unlockedFiles.insert(checkResult.path);
-            } else if (!checkResult.path.isEmpty() && QFile::exists(path)) { // Lock file found
-                lockedFiles.insert(checkResult.path);
+            if (checkResult.type == FileLockingInfo::Type::Unlocked && !checkResult.path.isEmpty()) {
+                _lockedFiles.remove(checkResult.path);
+                _unlockedFiles.insert(checkResult.path);
             }
         }
 
         if (checkResult.type == FileLockingInfo::Type::Locked && !checkResult.path.isEmpty()) {
-            lockedFiles.insert(checkResult.path);
+            _unlockedFiles.remove(checkResult.path);
+            _lockedFiles.insert(checkResult.path);
         }
 
-        qCDebug(lcFolderWatcher) << "Locked files:" << lockedFiles.values();
+        qCDebug(lcFolderWatcher) << "Locked files:" << _lockedFiles.values();
 
         // ------- handle ignores:
         if (pathIsIgnored(path)) {
@@ -229,19 +244,16 @@ void FolderWatcher::changeDetected(const QStringList &paths)
         changedPaths.insert(path);
     }
 
-    qCDebug(lcFolderWatcher) << "Unlocked files:" << unlockedFiles.values();
-    qCDebug(lcFolderWatcher) << "Locked files:" << lockedFiles;
+    qCDebug(lcFolderWatcher) << "Unlocked files:" << _unlockedFiles.values();
+    qCDebug(lcFolderWatcher) << "Locked files:" << _lockedFiles;
 
-    if (!unlockedFiles.isEmpty()) {
-        emit filesLockReleased(unlockedFiles);
-    }
-
-    if (!lockedFiles.isEmpty()) {
-        emit filesLockImposed(lockedFiles);
-    }
-
-    if (!lockedFiles.isEmpty()) {
-        emit lockedFilesFound(lockedFiles);
+    if (!_lockedFiles.isEmpty() || !_unlockedFiles.isEmpty()) {
+        if (_lockChangeDebouncingTimer.isActive()) {
+            _lockChangeDebouncingTimer.stop();
+        }
+        _lockChangeDebouncingTimer.setSingleShot(true);
+        _lockChangeDebouncingTimer.start();
+        _lockChangeDebouncingTimer.connect(&_lockChangeDebouncingTimer, &QTimer::timeout, this, &FolderWatcher::lockChangeDebouncingTimerTimedOut, Qt::UniqueConnection);
     }
 
     if (changedPaths.isEmpty()) {

src/gui/folderwatcher.h

@@ -27,8 +27,7 @@
 #include <QScopedPointer>
 #include <QSet>
 #include <QDir>
-
-class QTimer;
+#include <QTimer>
 
 namespace OCC {
 
@@ -130,6 +129,7 @@ protected slots:
 
 private slots:
     void startNotificationTestWhenReady();
+    void lockChangeDebouncingTimerTimedOut();
 
 protected:
     QHash<QString, int> _pendingPathes;
@@ -156,6 +156,11 @@ private slots:
     /** Path of the expected test notification */
     QString _testNotificationPath;
 
+    QSet<QString> _unlockedFiles;
+    QSet<QString> _lockedFiles;
+
+    QTimer _lockChangeDebouncingTimer;
+
     friend class FolderWatcherPrivate;
 };
 }

src/gui/socketapi/socketapi.cpp

@@ -1091,7 +1091,11 @@ void SocketApi::setFileLock(const QString &localFile, const SyncFileItem::LockSt
         return;
     }
 
-    shareFolder->accountState()->account()->setLockFileState(fileData.serverRelativePath, shareFolder->journalDb(), lockState);
+    shareFolder->accountState()->account()->setLockFileState(fileData.serverRelativePath,
+                                                             shareFolder->remotePathTrailingSlash(),
+                                                             shareFolder->path(),
+                                                             shareFolder->journalDb(),
+                                                             lockState);
 
     shareFolder->journalDb()->schedulePathForRemoteDiscovery(fileData.serverRelativePath);
     shareFolder->scheduleThisFolderSoon();

src/libsync/account.cpp

@@ -913,6 +913,17 @@ void Account::slotDirectEditingRecieved(const QJsonDocument &json)
     }
 }
 
+void Account::removeLockStatusChangeInprogress(const QString &serverRelativePath, const SyncFileItem::LockStatus lockStatus)
+{
+    const auto foundLockStatusJobInProgress = _lockStatusChangeInprogress.find(serverRelativePath);
+    if (foundLockStatusJobInProgress != _lockStatusChangeInprogress.end()) {
+        foundLockStatusJobInProgress.value().removeAll(lockStatus);
+        if (foundLockStatusJobInProgress.value().isEmpty()) {
+            _lockStatusChangeInprogress.erase(foundLockStatusJobInProgress);
+        }
+    }
+}
+
 PushNotifications *Account::pushNotifications() const
 {
     return _pushNotifications;
@@ -924,14 +935,24 @@ std::shared_ptr<UserStatusConnector> Account::userStatusConnector() const
 }
 
 void Account::setLockFileState(const QString &serverRelativePath,
+                               const QString &remoteSyncPathWithTrailingSlash,
+                               const QString &localSyncPath,
                                SyncJournalDb * const journal,
                                const SyncFileItem::LockStatus lockStatus)
 {
-    auto job = std::make_unique<LockFileJob>(sharedFromThis(), journal, serverRelativePath, lockStatus);
-    connect(job.get(), &LockFileJob::finishedWithoutError, this, [this]() {
+    auto& lockStatusJobInProgress = _lockStatusChangeInprogress[serverRelativePath];
+    if (lockStatusJobInProgress.contains(lockStatus)) {
+        qCWarning(lcAccount) << "Already running a job with lockStatus:" << lockStatus << " for: " << serverRelativePath;
+        return;
+    }
+    lockStatusJobInProgress.push_back(lockStatus);
+    auto job = std::make_unique<LockFileJob>(sharedFromThis(), journal, serverRelativePath, remoteSyncPathWithTrailingSlash, localSyncPath, lockStatus);
+    connect(job.get(), &LockFileJob::finishedWithoutError, this, [this, serverRelativePath, lockStatus]() {
+        removeLockStatusChangeInprogress(serverRelativePath, lockStatus);
         Q_EMIT lockFileSuccess();
     });
     connect(job.get(), &LockFileJob::finishedWithError, this, [lockStatus, serverRelativePath, this](const int httpErrorCode, const QString &errorString, const QString &lockOwnerName) {
+        removeLockStatusChangeInprogress(serverRelativePath, lockStatus);
         auto errorMessage = QString{};
         const auto filePath = serverRelativePath.mid(1);
 

src/libsync/account.h

@@ -308,6 +308,8 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject
     [[nodiscard]] std::shared_ptr<UserStatusConnector> userStatusConnector() const;
 
     void setLockFileState(const QString &serverRelativePath,
+                          const QString &remoteSyncPathWithTrailingSlash,
+                          const QString &localSyncPath,
                           SyncJournalDb * const journal,
                           const SyncFileItem::LockStatus lockStatus);
 
@@ -373,6 +375,9 @@ protected Q_SLOTS:
     void slotCredentialsAsked();
     void slotDirectEditingRecieved(const QJsonDocument &json);
 
+private slots:
+    void removeLockStatusChangeInprogress(const QString &serverRelativePath, const SyncFileItem::LockStatus lockStatus);
+
 private:
     Account(QObject *parent = nullptr);
     void setSharedThis(AccountPtr sharedThis);
@@ -436,6 +441,8 @@ protected Q_SLOTS:
 
     std::shared_ptr<UserStatusConnector> _userStatusConnector;
 
+    QHash<QString, QVector<SyncFileItem::LockStatus>> _lockStatusChangeInprogress;
+
     /* IMPORTANT - remove later - FIXME MS@2019-12-07 -->
      * TODO: For "Log out" & "Remove account": Remove client CA certs and KEY!
      *

src/libsync/lockfilejobs.cpp

@@ -28,12 +28,19 @@ Q_LOGGING_CATEGORY(lcLockFileJob, "nextcloud.sync.networkjob.lockfile", QtInfoMs
 LockFileJob::LockFileJob(const AccountPtr account,
                          SyncJournalDb* const journal,
                          const QString &path,
+                         const QString &remoteSyncPathWithTrailingSlash,
+                         const QString &localSyncPath,
                          const SyncFileItem::LockStatus requestedLockState,
                          QObject *parent)
     : AbstractNetworkJob(account, path, parent)
     , _journal(journal)
     , _requestedLockState(requestedLockState)
+    , _remoteSyncPathWithTrailingSlash(remoteSyncPathWithTrailingSlash)
+    , _localSyncPath(localSyncPath)
 {
+    if (!_localSyncPath.endsWith(QLatin1Char('/'))) {
+        _localSyncPath.append(QLatin1Char('/'));
+    }
 }
 
 void LockFileJob::start()
@@ -164,12 +171,12 @@ SyncJournalFileRecord LockFileJob::handleReply()
         }
     }
 
-    const auto relativePath = path().mid(1);
-    if (_journal->getFileRecord(relativePath, &record) && record.isValid()) {
+    const auto relativePathInDb = path().mid(_remoteSyncPathWithTrailingSlash.size());
+    if (_journal->getFileRecord(relativePathInDb, &record) && record.isValid()) {
         setFileRecordLocked(record);
-        if (_lockOwnerType != SyncFileItem::LockOwnerType::UserLock ||
-                _userId != account()->davUser()) {
-            FileSystem::setFileReadOnly(relativePath, true);
+        if ((_lockStatus == SyncFileItem::LockStatus::LockedItem)
+            && (_lockOwnerType != SyncFileItem::LockOwnerType::UserLock || _userId != account()->davUser())) {
+            FileSystem::setFileReadOnly(_localSyncPath + relativePathInDb, true);
         }
         const auto result = _journal->setFileRecord(record);
         if (!result) {

src/libsync/lockfilejobs.h

@@ -22,6 +22,8 @@ class OWNCLOUDSYNC_EXPORT LockFileJob : public AbstractNetworkJob
     explicit LockFileJob(const AccountPtr account,
                          SyncJournalDb* const journal,
                          const QString &path,
+                         const QString &remoteSyncPathWithTrailingSlash,
+                         const QString &localSyncPath,
                          const SyncFileItem::LockStatus requestedLockState,
                          QObject *parent = nullptr);
     void start() override;
@@ -54,6 +56,8 @@ class OWNCLOUDSYNC_EXPORT LockFileJob : public AbstractNetworkJob
     QString _userId;
     qint64 _lockTime = 0;
     qint64 _lockTimeout = 0;
+    QString _remoteSyncPathWithTrailingSlash;
+    QString _localSyncPath;
 };
 
 }