Skip to content

Commit

Permalink
File locking. Manual lock should never be modified without user inter…
Browse files Browse the repository at this point in the history 
…vention. Set token lock instead of user lock when locking office files automatically.

Signed-off-by: alex-z <[email protected]>
  • Loading branch information
@allexzander @backportbot-nextcloud
allexzander authored and backportbot-nextcloud[bot] committed Jan 18, 2024
1 parent dfe8780 commit c959fe7
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 24 deletions.
3 changes: 2 additions & 1 deletion src/gui/editlocallyjob.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,8 @@ void EditLocallyJob::lockFile()
_folderForFile->remotePathTrailingSlash(),
_folderForFile->path(),
_folderForFile->journalDb(),
SyncFileItem::LockStatus::LockedItem);
SyncFileItem::LockStatus::LockedItem,
SyncFileItem::LockOwnerType::TokenLock);
}

void EditLocallyJob::disconnectFolderSignals()
Expand Down
9 changes: 6 additions & 3 deletions src/gui/folder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -649,7 +649,7 @@ void Folder::slotFilesLockReleased(const QSet<QString> &files)
}
const auto canUnlockFile = isFileRecordValid
&& rec._lockstate._locked
&& rec._lockstate._lockOwnerType == static_cast<qint64>(SyncFileItem::LockOwnerType::UserLock)
&& rec._lockstate._lockOwnerType == static_cast<qint64>(SyncFileItem::LockOwnerType::TokenLock)
&& rec._lockstate._lockOwnerId == _accountState->account()->davUser();

if (!canUnlockFile) {
Expand All @@ -668,11 +668,13 @@ void Folder::slotFilesLockReleased(const QSet<QString> &files)
disconnect(_officeFileLockReleaseUnlockFailure);
qCWarning(lcFolder) << "Failed to unlock a file:" << remoteFilePath << message;
});
const auto lockOwnerType = static_cast<SyncFileItem::LockOwnerType>(rec._lockstate._lockOwnerType);
_accountState->account()->setLockFileState(remoteFilePath,
remotePathTrailingSlash(),
path(),
journalDb(),
SyncFileItem::LockStatus::UnlockedItem);
SyncFileItem::LockStatus::UnlockedItem,
lockOwnerType);
}
}

Expand Down Expand Up @@ -719,7 +721,8 @@ void Folder::slotLockedFilesFound(const QSet<QString> &files)
remotePathTrailingSlash(),
path(),
journalDb(),
SyncFileItem::LockStatus::LockedItem);
SyncFileItem::LockStatus::LockedItem,
SyncFileItem::LockOwnerType::TokenLock);
}
}

Expand Down
9 changes: 8 additions & 1 deletion src/gui/socketapi/socketapi.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1091,11 +1091,18 @@ void SocketApi::setFileLock(const QString &localFile, const SyncFileItem::LockSt
return;
}

const auto record = fileData.journalRecord();
if (static_cast<SyncFileItem::LockOwnerType>(record._lockstate._lockOwnerType) != SyncFileItem::LockOwnerType::UserLock) {
qCDebug(lcSocketApi) << "Only user lock state or non-locked files can be affected manually!";
return;
}

shareFolder->accountState()->account()->setLockFileState(fileData.serverRelativePath,
shareFolder->remotePathTrailingSlash(),
shareFolder->path(),
shareFolder->journalDb(),
lockState);
lockState,
SyncFileItem::LockOwnerType::UserLock);

shareFolder->journalDb()->schedulePathForRemoteDiscovery(fileData.serverRelativePath);
shareFolder->scheduleThisFolderSoon();
Expand Down
5 changes: 3 additions & 2 deletions src/libsync/account.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -959,15 +959,16 @@ void Account::setLockFileState(const QString &serverRelativePath,
const QString &remoteSyncPathWithTrailingSlash,
const QString &localSyncPath,
SyncJournalDb * const journal,
const SyncFileItem::LockStatus lockStatus)
const SyncFileItem::LockStatus lockStatus,
const SyncFileItem::LockOwnerType lockOwnerType)
{
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);
auto job = std::make_unique<LockFileJob>(sharedFromThis(), journal, serverRelativePath, remoteSyncPathWithTrailingSlash, localSyncPath, lockStatus, lockOwnerType);
connect(job.get(), &LockFileJob::finishedWithoutError, this, [this, serverRelativePath, lockStatus]() {
removeLockStatusChangeInprogress(serverRelativePath, lockStatus);
Q_EMIT lockFileSuccess();
Expand Down
3 changes: 2 additions & 1 deletion src/libsync/account.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,8 @@ class OWNCLOUDSYNC_EXPORT Account : public QObject
const QString &remoteSyncPathWithTrailingSlash,
const QString &localSyncPath,
SyncJournalDb * const journal,
const SyncFileItem::LockStatus lockStatus);
const SyncFileItem::LockStatus lockStatus,
const SyncFileItem::LockOwnerType lockOwnerType);

SyncFileItem::LockStatus fileLockStatus(SyncJournalDb * const journal,
const QString &folderRelativePath) const;
Expand Down
5 changes: 5 additions & 0 deletions src/libsync/capabilities.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,11 @@ bool Capabilities::filesLockAvailable() const
return _capabilities["files"].toMap()["locking"].toByteArray() >= "1.0";
}

bool Capabilities::filesLockTypeAvailable() const
{
return _capabilities["files"].toMap()["api-feature-lock-type"].toByteArray() >= "1.0";
}

bool Capabilities::userStatus() const
{
if (!_capabilities.contains("user_status")) {
Expand Down
1 change: 1 addition & 0 deletions src/libsync/capabilities.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class OWNCLOUDSYNC_EXPORT Capabilities
[[nodiscard]] bool chunkingNg() const;
[[nodiscard]] bool bulkUpload() const;
[[nodiscard]] bool filesLockAvailable() const;
[[nodiscard]] bool filesLockTypeAvailable() const;
[[nodiscard]] bool userStatus() const;
[[nodiscard]] bool userStatusSupportsEmoji() const;
[[nodiscard]] QColor serverColor() const;
Expand Down
15 changes: 13 additions & 2 deletions src/libsync/lockfilejobs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ LockFileJob::LockFileJob(const AccountPtr account,
const QString &remoteSyncPathWithTrailingSlash,
const QString &localSyncPath,
const SyncFileItem::LockStatus requestedLockState,
const SyncFileItem::LockOwnerType lockOwnerType,
QObject *parent)
: AbstractNetworkJob(account, path, parent)
, _journal(journal)
, _requestedLockState(requestedLockState)
, _requestedLockOwnerType(lockOwnerType)
, _remoteSyncPathWithTrailingSlash(remoteSyncPathWithTrailingSlash)
, _localSyncPath(localSyncPath)
{
Expand All @@ -48,7 +50,14 @@ void LockFileJob::start()
qCInfo(lcLockFileJob()) << "start" << path() << _requestedLockState;

QNetworkRequest request;
request.setRawHeader("X-User-Lock", "1");
request.setRawHeader(QByteArrayLiteral("X-User-Lock"), QByteArrayLiteral("1"));
if (_account->capabilities().filesLockTypeAvailable()) {
if (_requestedLockOwnerType == SyncFileItem::LockOwnerType::UserLock) {
request.setRawHeader(QByteArrayLiteral("X-User-Lock-Type"), ("0"));
} else if (_requestedLockOwnerType == SyncFileItem::LockOwnerType::TokenLock) {
request.setRawHeader(QByteArrayLiteral("X-User-Lock-Type"), ("2"));
}
}

QByteArray verb;
switch(_requestedLockState)
Expand Down Expand Up @@ -174,7 +183,7 @@ SyncJournalFileRecord LockFileJob::handleReply()
if (_journal->getFileRecord(relativePathInDb, &record) && record.isValid()) {
setFileRecordLocked(record);
if ((_lockStatus == SyncFileItem::LockStatus::LockedItem)
&& (_lockOwnerType != SyncFileItem::LockOwnerType::UserLock || _userId != account()->davUser())) {
&& (_lockOwnerType == SyncFileItem::LockOwnerType::AppLock || _userId != account()->davUser())) {
FileSystem::setFileReadOnly(_localSyncPath + relativePathInDb, true);
}
const auto result = _journal->setFileRecord(record);
Expand Down Expand Up @@ -205,6 +214,8 @@ void LockFileJob::decodeStartElement(const QString &name,
const auto convertedValue = valueText.toInt(&isValid);
if (isValid) {
_lockOwnerType = static_cast<SyncFileItem::LockOwnerType>(convertedValue);
} else {
_lockOwnerType = SyncFileItem::LockOwnerType::UserLock;
}
} else if (name == QStringLiteral("lock-owner-displayname")) {
_userDisplayName = reader.readElementText();
Expand Down
2 changes: 2 additions & 0 deletions src/libsync/lockfilejobs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class OWNCLOUDSYNC_EXPORT LockFileJob : public AbstractNetworkJob
const QString &remoteSyncPathWithTrailingSlash,
const QString &localSyncPath,
const SyncFileItem::LockStatus requestedLockState,
const SyncFileItem::LockOwnerType lockOwnerType,
QObject *parent = nullptr);
void start() override;

Expand All @@ -48,6 +49,7 @@ class OWNCLOUDSYNC_EXPORT LockFileJob : public AbstractNetworkJob

SyncJournalDb* _journal = nullptr;
SyncFileItem::LockStatus _requestedLockState = SyncFileItem::LockStatus::LockedItem;
SyncFileItem::LockOwnerType _requestedLockOwnerType = SyncFileItem::LockOwnerType::UserLock;

SyncFileItem::LockStatus _lockStatus = SyncFileItem::LockStatus::UnlockedItem;
SyncFileItem::LockOwnerType _lockOwnerType = SyncFileItem::LockOwnerType::UserLock;
Expand Down
42 changes: 28 additions & 14 deletions test/testlockfile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ private slots:
QStringLiteral("/"),
fakeFolder.localPath(),
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem);
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QVERIFY(lockFileSuccessSpy.wait());
QCOMPARE(lockFileErrorSpy.count(), 0);
Expand Down Expand Up @@ -85,7 +86,8 @@ private slots:
QStringLiteral("/"),
fakeFolder.localPath(),
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem);
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QVERIFY(lockFileErrorSpy.wait());
QCOMPARE(lockFileSuccessSpy.count(), 0);
Expand All @@ -109,7 +111,8 @@ private slots:
QStringLiteral("/"),
fakeFolder.localPath(),
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem);
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QVERIFY(lockFileSuccessSpy.wait());
QCOMPARE(lockFileErrorSpy.count(), 0);
Expand All @@ -136,7 +139,8 @@ private slots:
QStringLiteral("/"),
fakeFolder.localPath(),
&fakeFolder.syncJournal(),
OCC::SyncFileItem::LockStatus::LockedItem);
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QVERIFY(lockFileSuccessSpy.wait());
QCOMPARE(lockFileErrorSpy.count(), 0);
Expand All @@ -160,7 +164,8 @@ private slots:
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::LockedItem);
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
Expand Down Expand Up @@ -198,7 +203,8 @@ private slots:
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::LockedItem);
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QSignalSpy lockFileJobSuccess(lockFileJob, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy lockFileJobFailure(lockFileJob, &OCC::LockFileJob::finishedWithError);
Expand All @@ -215,7 +221,8 @@ private slots:
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::UnlockedItem);
OCC::SyncFileItem::LockStatus::UnlockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QSignalSpy unlockFileJobSuccess(unlockFileJob, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy unlockFileJobFailure(unlockFileJob, &OCC::LockFileJob::finishedWithError);
Expand Down Expand Up @@ -274,7 +281,8 @@ private slots:
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::LockedItem);
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
Expand Down Expand Up @@ -327,7 +335,8 @@ private slots:
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::LockedItem);
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
Expand Down Expand Up @@ -380,7 +389,8 @@ private slots:
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::UnlockedItem);
OCC::SyncFileItem::LockStatus::UnlockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
Expand Down Expand Up @@ -431,7 +441,8 @@ private slots:
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::UnlockedItem);
OCC::SyncFileItem::LockStatus::UnlockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
Expand Down Expand Up @@ -469,7 +480,8 @@ private slots:
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::LockedItem);
OCC::SyncFileItem::LockStatus::LockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QSignalSpy lockFileJobSuccess(lockFileJob, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy lockFileJobFailure(lockFileJob, &OCC::LockFileJob::finishedWithError);
Expand All @@ -486,7 +498,8 @@ private slots:
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::UnlockedItem);
OCC::SyncFileItem::LockStatus::UnlockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QSignalSpy unlockFileJobSuccess(unlockFileJob, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy unlockFileJobFailure(unlockFileJob, &OCC::LockFileJob::finishedWithError);
Expand Down Expand Up @@ -539,7 +552,8 @@ private slots:
QStringLiteral("/") + testFileName,
QStringLiteral("/"),
fakeFolder.localPath(),
OCC::SyncFileItem::LockStatus::UnlockedItem);
OCC::SyncFileItem::LockStatus::UnlockedItem,
OCC::SyncFileItem::LockOwnerType::UserLock);

QSignalSpy jobSuccess(job, &OCC::LockFileJob::finishedWithoutError);
QSignalSpy jobFailure(job, &OCC::LockFileJob::finishedWithError);
Expand Down

0 comments on commit c959fe7

Please sign in to comment.