From b60b543cb1cec3487dbae2449c29ae3eb28eff50 Mon Sep 17 00:00:00 2001 From: Hannah von Reth Date: Tue, 22 Jun 2021 09:26:23 +0200 Subject: [PATCH] Check a wehter file is locked before we move Fixes: #8765, #8766 --- changelog/unreleased/8761 | 4 ++++ src/common/filesystembase.cpp | 10 ++++++++++ src/libsync/owncloudpropagator.cpp | 15 ++++++++------- src/libsync/propagatedownload.cpp | 27 +++++++++++++++++---------- src/libsync/propagateremotemove.cpp | 5 ----- src/libsync/propagatorjobs.cpp | 10 +++++++--- 6 files changed, 46 insertions(+), 25 deletions(-) diff --git a/changelog/unreleased/8761 b/changelog/unreleased/8761 index 52d6f09d949..112e851c9d1 100644 --- a/changelog/unreleased/8761 +++ b/changelog/unreleased/8761 @@ -4,3 +4,7 @@ We fixed an issue where files locked by office etc, where not correctly synced, when Windows Virtual files are enabled. https://github.com/owncloud/client/issues/8761 +https://github.com/owncloud/client/issues/8765 +https://github.com/owncloud/client/issues/8766 +https://github.com/owncloud/client/pull/8763 +https://github.com/owncloud/client/pull/8768 diff --git a/src/common/filesystembase.cpp b/src/common/filesystembase.cpp index 8cb1b53dd7b..5994b1ec02b 100644 --- a/src/common/filesystembase.cpp +++ b/src/common/filesystembase.cpp @@ -133,6 +133,11 @@ bool FileSystem::rename(const QString &originFileName, bool success = false; QString error; #ifdef Q_OS_WIN + if (FileSystem::isFileLocked(originFileName, FileSystem::LockMode::Exclusive)) { + *errorString = QCoreApplication::tr("FileSystem", "Can't rename locked file %1").arg(originFileName); + qCWarning(lcFileSystem) << "Renaming failed: " << *errorString; + return false; + } QString orig = longWinPath(originFileName); QString dest = longWinPath(destinationFileName); @@ -191,6 +196,11 @@ bool FileSystem::uncheckedRenameReplace(const QString &originFileName, } #else //Q_OS_WIN + if (FileSystem::isFileLocked(originFileName, FileSystem::LockMode::Exclusive)) { + *errorString = QCoreApplication::tr("FileSystem", "Can't rename locked file %1").arg(originFileName); + qCWarning(lcFileSystem) << "Renaming failed: " << *errorString; + return false; + } // You can not overwrite a read-only file on windows. if (!QFileInfo(destinationFileName).isWritable()) { setFileReadOnly(destinationFileName, false); diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 149dbe0485c..8dec507ebd9 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -691,15 +691,16 @@ bool OwncloudPropagator::createConflict(const SyncFileItemPtr &item, emit touchedFile(fn); emit touchedFile(conflictFilePath); + // If the file is locked, we want to retry this sync when it + // becomes available again. + if (FileSystem::isFileLocked(fn, FileSystem::LockMode::Exclusive)) { + emit seenLockedFile(fn, FileSystem::LockMode::Exclusive); + if (error) + *error = tr("File %1 is locked").arg(fn); + return false; + } if (!FileSystem::rename(fn, conflictFilePath, &renameError)) { // If the rename fails, don't replace it. - - // If the file is locked, we want to retry this sync when it - // becomes available again. - if (FileSystem::isFileLocked(fn, FileSystem::LockMode::Exclusive)) { - emit seenLockedFile(fn, FileSystem::LockMode::Exclusive); - } - if (error) *error = renameError; return false; diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index e2492254493..9b4c7ec3657 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -458,7 +458,14 @@ void PropagateDownloadFile::startDownload() done(SyncFileItem::NormalError, tr("File %1 can not be downloaded because of a local file name clash!").arg(QDir::toNativeSeparators(_item->_file))); return; } - + // If the file is locked, we want to retry this sync when it + // becomes available again + const auto targetFile = propagator()->fullLocalPath(_item->_file); + if (FileSystem::isFileLocked(targetFile, FileSystem::LockMode::Exclusive)) { + emit propagator()->seenLockedFile(targetFile, FileSystem::LockMode::Exclusive); + done(SyncFileItem::SoftError, tr("File %1 is locked").arg(QDir::toNativeSeparators(_item->_file))); + return; + } propagator()->reportProgress(*_item, 0); QString tmpFileName; @@ -940,20 +947,20 @@ void PropagateDownloadFile::downloadFinished() return; } } + // If the file is locked, we want to retry this sync when it + // becomes available again + if (FileSystem::isFileLocked(fn, FileSystem::LockMode::Exclusive)) { + emit propagator()->seenLockedFile(fn, FileSystem::LockMode::Exclusive); + done(SyncFileItem::SoftError, tr("File %1 is locked").arg(fn)); + return; + } QString error; emit propagator()->touchedFile(fn); // The fileChanged() check is done above to generate better error messages. if (!FileSystem::uncheckedRenameReplace(_tmpFile.fileName(), fn, &error)) { - qCWarning(lcPropagateDownload) << QStringLiteral("Rename failed: %1 => %2").arg(_tmpFile.fileName()).arg(fn); - // If the file is locked, we want to retry this sync when it - // becomes available again, otherwise try again directly - if (FileSystem::isFileLocked(fn, FileSystem::LockMode::Exclusive)) { - emit propagator()->seenLockedFile(fn, FileSystem::LockMode::Exclusive); - } else { - propagator()->_anotherSyncNeeded = true; - } - + qCWarning(lcPropagateDownload) << "Rename failed:" << _tmpFile.fileName() << "=>" << fn; + propagator()->_anotherSyncNeeded = true; done(SyncFileItem::SoftError, error); return; } diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp index 7f45eb0509f..58d021ca7b3 100644 --- a/src/libsync/propagateremotemove.cpp +++ b/src/libsync/propagateremotemove.cpp @@ -80,9 +80,6 @@ void PropagateRemoteMove::start() QString origin = propagator()->adjustRenamedPath(_item->_file); qCDebug(lcPropagateRemoteMove) << origin << _item->_renameTarget; - - QString targetFile(propagator()->fullLocalPath(_item->_renameTarget)); - if (origin == _item->_renameTarget) { // The parent has been renamed already so there is nothing more to do. finalize(); @@ -147,8 +144,6 @@ void PropagateRemoteMove::start() << folderTargetAlt << "to" << folderTarget; } } - qCDebug(lcPropagateRemoteMove) << remoteSource << remoteDestination; - _job = new MoveJob(propagator()->account(), remoteSource, remoteDestination, this); connect(_job.data(), &MoveJob::finishedSignal, this, &PropagateRemoteMove::slotMoveJobFinished); propagator()->_activeJobList.append(this); diff --git a/src/libsync/propagatorjobs.cpp b/src/libsync/propagatorjobs.cpp index 3e926588bee..39612dfa778 100644 --- a/src/libsync/propagatorjobs.cpp +++ b/src/libsync/propagatorjobs.cpp @@ -215,11 +215,15 @@ void PropagateLocalRename::start() // it would have to come out the localFileNameClash function done(SyncFileItem::NormalError, tr("File %1 can not be renamed to %2 because of a local file name clash") - .arg(QDir::toNativeSeparators(_item->_file)) - .arg(QDir::toNativeSeparators(_item->_renameTarget))); + .arg(QDir::toNativeSeparators(_item->_file), + QDir::toNativeSeparators(_item->_renameTarget))); + return; + } + if (FileSystem::isFileLocked(existingFile, FileSystem::LockMode::Exclusive)) { + emit propagator()->seenLockedFile(existingFile, FileSystem::LockMode::Exclusive); + done(SyncFileItem::SoftError, tr("Could not rename %1 to %2, file is locked").arg(existingFile, targetFile)); return; } - emit propagator()->touchedFile(existingFile); emit propagator()->touchedFile(targetFile); QString renameError;