Skip to content

Commit

Permalink
Merge pull request #4962 from nextcloud/bugfix/files-not-unlocking
Browse files Browse the repository at this point in the history
  • Loading branch information
@claucambra
claucambra authored Oct 4, 2022
2 parents 311469a + 2f57133 commit b9f6d91
Show file tree
Hide file tree
Showing 7 changed files with 363 additions and 20 deletions.
13 changes: 11 additions & 2 deletions src/common/syncjournaldb.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1351,7 +1351,7 @@ bool SyncJournalDb::updateFileRecordChecksum(const QString &filename,
}

bool SyncJournalDb::updateLocalMetadata(const QString &filename,
qint64 modtime, qint64 size, quint64 inode)
qint64 modtime, qint64 size, quint64 inode, const SyncJournalFileLockInfo &lockInfo)

{
QMutexLocker locker(&_mutex);
Expand All @@ -1365,7 +1365,9 @@ bool SyncJournalDb::updateLocalMetadata(const QString &filename,
}

const auto query = _queryManager.get(PreparedSqlQueryManager::SetFileRecordLocalMetadataQuery, QByteArrayLiteral("UPDATE metadata"
" SET inode=?2, modtime=?3, filesize=?4"
" SET inode=?2, modtime=?3, filesize=?4, lock=?5, lockType=?6,"
" lockOwnerDisplayName=?7, lockOwnerId=?8, lockOwnerEditor = ?9,"
" lockTime=?10, lockTimeout=?11"
" WHERE phash == ?1;"),
_db);
if (!query) {
Expand All @@ -1376,6 +1378,13 @@ bool SyncJournalDb::updateLocalMetadata(const QString &filename,
query->bindValue(2, inode);
query->bindValue(3, modtime);
query->bindValue(4, size);
query->bindValue(5, lockInfo._locked ? 1 : 0);
query->bindValue(6, lockInfo._lockOwnerType);
query->bindValue(7, lockInfo._lockOwnerDisplayName);
query->bindValue(8, lockInfo._lockOwnerId);
query->bindValue(9, lockInfo._lockEditorApp);
query->bindValue(10, lockInfo._lockTime);
query->bindValue(11, lockInfo._lockTimeout);
return query->exec();
}

Expand Down
2 changes: 1 addition & 1 deletion src/common/syncjournaldb.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class OCSYNC_EXPORT SyncJournalDb : public QObject
const QByteArray &contentChecksum,
const QByteArray &contentChecksumType);
[[nodiscard]] bool updateLocalMetadata(const QString &filename,
qint64 modtime, qint64 size, quint64 inode);
qint64 modtime, qint64 size, quint64 inode, const SyncJournalFileLockInfo &lockInfo);

/// Return value for hasHydratedOrDehydratedFiles()
struct HasHydratedDehydrated
Expand Down
46 changes: 40 additions & 6 deletions src/libsync/discovery.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,29 @@ 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)
Expand Down Expand Up @@ -498,6 +521,15 @@ 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;

if(isServerEntryLocked != dbEntry._lockstate._locked) {
item->_instruction = CSYNC_INSTRUCTION_UPDATE_METADATA;
}
};

// The file is known in the db already
if (dbEntry.isValid()) {
const bool isDbEntryAnE2EePlaceholder = dbEntry.isVirtualFile() && !dbEntry.e2eMangledName().isEmpty();
Expand Down Expand Up @@ -579,10 +611,12 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
return ParentNotChanged;
}();

checkFileLockState();
processFileAnalyzeLocalInfo(item, path, localEntry, serverEntry, dbEntry, serverQueryMode);
return;
}

checkFileLockState();
processFileAnalyzeLocalInfo(item, path, localEntry, serverEntry, dbEntry, _queryServer);
return;
}
Expand Down Expand Up @@ -815,7 +849,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(

bool serverModified = item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC
|| item->_instruction == CSYNC_INSTRUCTION_RENAME || item->_instruction == CSYNC_INSTRUCTION_TYPE_CHANGE;

qCDebug(lcDisco) << "File" << item->_file << "- servermodified:" << serverModified
<< "noServerEntry:" << noServerEntry;

Expand Down Expand Up @@ -1029,7 +1063,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
item->_size = localEntry.size;
item->_modtime = localEntry.modtime;
_childModified = true;

qCDebug(lcDisco) << "Local file was changed: File" << item->_file
<< "item->_instruction:" << item->_instruction
<< "noServerEntry:" << noServerEntry
Expand Down Expand Up @@ -1316,7 +1350,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
chopVirtualFileSuffix(serverOriginalPath);
auto job = new RequestEtagJob(_discoveryData->_account, serverOriginalPath, this);
connect(job, &RequestEtagJob::finishedWithResult, this, [=](const HttpResult<QByteArray> &etag) mutable {


if (!etag || (etag.get() != base._etag && !item->isDirectory()) || _discoveryData->isRenamed(originalPath)
|| (isAnyParentBeingRestored(originalPath) && !isRename(originalPath))) {
Expand Down Expand Up @@ -1382,7 +1416,7 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, Proce
<< "localEntry.modtime:" << localEntry.modtime;
return;
}

if (!serverEntry.checksumHeader.isEmpty()) {
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_CONFLICT: File" << item->_file << "if (!serverEntry.checksumHeader.isEmpty())";
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_CONFLICT: serverEntry.size:" << serverEntry.size
Expand Down Expand Up @@ -1425,7 +1459,7 @@ void ProcessDirectoryJob::processFileConflict(const SyncFileItemPtr &item, Proce
}
return;
}

if (!up._valid || up._contentChecksum != serverEntry.checksumHeader) {
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_SYNC: File" << item->_file << "if (!up._valid && up._contentChecksum != serverEntry.checksumHeader)";
qCDebug(lcDisco) << "CSYNC_INSTRUCTION_SYNC: up._valid:" << up._valid
Expand Down Expand Up @@ -1636,7 +1670,7 @@ bool ProcessDirectoryJob::isRename(const QString &originalPath) const

/* TODO: This was needed at some point to cover an edge case which I am no longer to reproduce and it might no longer be the case.
* Still, leaving this here just in case the edge case is caught at some point in future.
*
*
OCC::SyncJournalFileRecord base;
// are we allowed to rename?
if (!_discoveryData || !_discoveryData->_statedb || !_discoveryData->_statedb->getFileRecord(originalPath, &base)) {
Expand Down
2 changes: 2 additions & 0 deletions src/libsync/discoveryphase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,8 @@ class DiscoveryPhase : public QObject
// output
QByteArray _dataFingerprint;
bool _anotherSyncNeeded = false;
QHash<QString, long long> _filesNeedingScheduledSync;
QVector<QString> _filesUnscheduleSync;

signals:
void fatalError(const QString &errorString);
Expand Down
Loading

0 comments on commit b9f6d91

Please sign in to comment.