fix(upload): skip locked files during upload to avoid errors #9810
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,9 +113,9 @@ | |
| explicit DiscoverySingleDirectoryJob(const AccountPtr &account, | ||
| const QString &path, | ||
| const QString &remoteRootFolderPath, | ||
| /* TODO for topLevelE2eeFolderPaths, from review: I still do not get why giving the whole QSet instead of just the parent of the folder we are in | ||
| sounds to me like it would be much more efficient to just have the e2ee parent folder that we are | ||
| inside*/ | ||
|
Check warning on line 118 in src/libsync/discoveryphase.h
|
||
| const QSet<QString> &topLevelE2eeFolderPaths, | ||
| SyncFileItem::EncryptionStatus parentEncryptionStatus, | ||
| QObject *parent = nullptr); | ||
|
|
@@ -345,6 +345,9 @@ | |
| void addErrorToGui(const OCC::SyncFileItem::Status status, const QString &errorMessage, const QString &subject, const OCC::ErrorCategory category); | ||
|
|
||
| void remnantReadOnlyFolderDiscovered(const OCC::SyncFileItemPtr &item); | ||
|
|
||
| /** Emitted when propagation would have problems with a locked file. */ | ||
| void seenLockedFile(const QString &fileName); | ||
| private slots: | ||
| void slotItemDiscovered(const OCC::SyncFileItemPtr &item); | ||
| }; | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,7 +55,6 @@ | |
| // For failures, we want to add the file to the list so the next sync | ||
| // will be able to retry it. | ||
| if (item->_status == SyncFileItem::Success | ||
|
|
||
| || item->_status == SyncFileItem::Restoration | ||
| || item->_status == SyncFileItem::Conflict | ||
| || (item->_status == SyncFileItem::NoStatus | ||
|
|
@@ -66,7 +65,7 @@ | |
| if (!item->_renameTarget.isEmpty() && _previousLocalDiscoveryPaths.erase(item->_renameTarget.toUtf8())) | ||
| qCDebug(lcLocalDiscoveryTracker) << "wiped successful item" << item->_renameTarget; | ||
| } else { | ||
| _localDiscoveryPaths.insert(item->_file.toUtf8()); | ||
|
Check warning on line 68 in src/libsync/localdiscoverytracker.cpp
|
||
| qCWarning(lcLocalDiscoveryTracker) << "inserted error item" << item->_file; | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
| * SPDX-License-Identifier: GPL-2.0-or-later | ||
| */ | ||
|
|
||
| #include "config.h" | ||
| #include "propagateupload.h" | ||
| #include "owncloudpropagator_p.h" | ||
| #include "networkjobs.h" | ||
|
|
@@ -180,7 +180,7 @@ | |
| // we should remove the later chunks. Otherwise when we do dynamic chunk sizing, we may end up | ||
| // with corruptions if there are too many chunks, or if we abort and there are still stale chunks. | ||
| for (const auto &serverChunk : std::as_const(_serverChunks)) { | ||
| auto job = new DeleteJob(propagator()->account(), Utility::concatUrlPath(chunkUploadFolderUrl(), serverChunk.originalName), {}, this); | ||
|
Check warning on line 183 in src/libsync/propagateuploadng.cpp
|
||
| QObject::connect(job, &DeleteJob::finishedSignal, this, &PropagateUploadFileNG::slotDeleteJobFinished); | ||
| _jobs.append(job); | ||
| job->start(); | ||
|
|
@@ -355,15 +355,19 @@ | |
| } | ||
|
|
||
| const auto fileName = _fileToUpload._path; | ||
| if (FileSystem::isFileLocked(fileName, FileSystem::LockMode::SharedRead)) { | ||
| // If the file is currently locked, we want to retry the sync | ||
| // when it becomes available again. | ||
| emit propagator()->seenLockedFile(fileName); | ||
|
|
||
| // Soft error because this is likely caused by the user modifying his files while syncing | ||
| abortWithError(SyncFileItem::FileLocked, tr("File is locked preventing syncing it", "Generic warning message when a locked file cannot be synced")); | ||
| return; | ||
| } | ||
| auto device = std::make_unique<UploadDevice>(fileName, _sent, _currentChunkSize, &propagator()->_bandwidthManager); | ||
| if (!device->open(QIODevice::ReadOnly)) { | ||
| qCWarning(lcPropagateUploadNG) << "Could not prepare upload device: " << device->errorString(); | ||
|
|
||
| // Soft error because this is likely caused by the user modifying his files while syncing | ||
| abortWithError(SyncFileItem::SoftError, device->errorString()); | ||
| return; | ||
|
|
@@ -536,7 +540,7 @@ | |
| } | ||
|
|
||
| if (SyncJournalFileRecord oldRecord; propagator()->_journal->getFileRecord(_item->destination(), &oldRecord) && oldRecord.isValid()) { | ||
| if (oldRecord._etag != _item->_etag) { | ||
|
Check warning on line 543 in src/libsync/propagateuploadng.cpp
|
||
| _item->updateLockStateFromDbRecord(oldRecord); | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,10 @@ | ||
| /* | ||
| * SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-FileCopyrightText: 2016 ownCloud GmbH | ||
| * SPDX-License-Identifier: GPL-2.0-or-later | ||
| */ | ||
|
|
||
| #include "config.h" | ||
| #include "propagateupload.h" | ||
| #include "owncloudpropagator_p.h" | ||
| #include "networkjobs.h" | ||
|
|
@@ -74,7 +74,7 @@ | |
| startNextChunk(); | ||
| } | ||
|
|
||
| void PropagateUploadFileV1::startNextChunk() | ||
| { | ||
| if (propagator()->_abortRequested) | ||
| return; | ||
|
|
@@ -134,16 +134,19 @@ | |
| } | ||
|
|
||
| const QString fileName = _fileToUpload._path; | ||
| if (FileSystem::isFileLocked(fileName, FileSystem::LockMode::SharedRead)) { | ||
| emit propagator()->seenLockedFile(fileName); | ||
|
|
||
| // Soft error because this is likely caused by the user modifying his files while syncing | ||
| abortWithError(SyncFileItem::FileLocked, tr("File is locked preventing syncing it", "Generic warning message when a locked file cannot be synced")); | ||
| return; | ||
| } | ||
|
|
||
| auto device = std::make_unique<UploadDevice>( | ||
| fileName, chunkStart, currentChunkSize, &propagator()->_bandwidthManager); | ||
| if (!device->open(QIODevice::ReadOnly)) { | ||
| qCWarning(lcPropagateUploadV1) << "Could not prepare upload device: " << device->errorString(); | ||
|
|
||
| // Soft error because this is likely caused by the user modifying his files while syncing | ||
| abortWithError(SyncFileItem::SoftError, device->errorString()); | ||
| return; | ||
|
|
@@ -217,7 +220,7 @@ | |
| QNetworkReply::NetworkError err = job->reply()->error(); | ||
| if (err != QNetworkReply::NoError) { | ||
| commonErrorHandling(job); | ||
| const auto exceptionParsed = getExceptionFromReply(job->reply()); | ||
|
Check warning on line 223 in src/libsync/propagateuploadv1.cpp
|
||
| _item->_errorExceptionName = exceptionParsed.first; | ||
| _item->_errorExceptionMessage = exceptionParsed.second; | ||
| return; | ||
|
|
@@ -368,7 +371,7 @@ | |
| sender()->setProperty("byteWritten", sent); | ||
| if (_jobs.count() > 1) { | ||
| amount -= (_jobs.count() - 1) * chunkSize(); | ||
| for (QObject *j : std::as_const(_jobs)) { | ||
|
Check warning on line 374 in src/libsync/propagateuploadv1.cpp
|
||
| amount += j->property("byteWritten").toULongLong(); | ||
| } | ||
| } else { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -509,7 +509,7 @@ | |
| qCWarning(lcEngine()) << "decrypt failed"; | ||
| return; | ||
| } | ||
| // TODO: We need to rollback changes done to metadata in case we have an active lock, this needs to be implemented on the server first | ||
|
Check warning on line 512 in src/libsync/syncengine.cpp
|
||
| const auto unlockJob = new OCC::UnlockEncryptFolderApiJob(_account, folderId, *folderToken, _journal, this); | ||
| unlockJob->setShouldRollbackMetadataChanges(true); | ||
| unlockJob->start(); | ||
|
|
@@ -708,6 +708,7 @@ | |
| connect(_discoveryPhase.get(), &DiscoveryPhase::silentlyExcluded, | ||
| _syncFileStatusTracker.data(), &SyncFileStatusTracker::slotAddSilentlyExcluded); | ||
| connect(_discoveryPhase.get(), &DiscoveryPhase::remnantReadOnlyFolderDiscovered, this, &SyncEngine::remnantReadOnlyFolderDiscovered); | ||
| connect(_discoveryPhase.get(), &DiscoveryPhase::seenLockedFile, this, &SyncEngine::seenLockedFile); | ||
|
|
||
| ProcessDirectoryJob *discoveryJob = nullptr; | ||
|
|
||
|
|
@@ -1152,7 +1153,7 @@ | |
| return false; | ||
| } | ||
|
|
||
| bool SyncEngine::handleMassDeletion() | ||
|
Check failure on line 1156 in src/libsync/syncengine.cpp
|
||
| { | ||
| const auto displayDialog = ConfigFile().promptDeleteFiles() && !_syncOptions.isCmd(); | ||
| const auto allFilesDeleted = !_hasNoneFiles && _hasRemoveFile; | ||
|
|
@@ -1185,7 +1186,7 @@ | |
| } | ||
| } | ||
|
|
||
| promptUserBeforePropagation([this, side](auto &&callback){ | ||
|
Check failure on line 1189 in src/libsync/syncengine.cpp
|
||
| emit aboutToRemoveAllFiles(side >= 0 ? SyncFileItem::Down : SyncFileItem::Up, callback); | ||
| }); | ||
| return true; | ||
|
|
@@ -1210,7 +1211,7 @@ | |
| slotAddTouchedFile(_localPath + oneFolder->_file); | ||
|
|
||
| if (oneFolder->_type == ItemType::ItemTypeDirectory) { | ||
| const auto deletionCallback = [this] (const QString &deleteItem, bool) { | ||
|
Check warning on line 1214 in src/libsync/syncengine.cpp
|
||
| slotAddTouchedFile(deleteItem); | ||
| }; | ||
|
|
||
|
|
@@ -1239,7 +1240,7 @@ | |
| }; | ||
|
|
||
| lambda(callback); | ||
| } | ||
|
Check failure on line 1243 in src/libsync/syncengine.cpp
|
||
|
|
||
| void SyncEngine::slotAddTouchedFile(const QString &fn) | ||
| { | ||
|
|
@@ -1277,7 +1278,7 @@ | |
| _leadingAndTrailingSpacesFilesAllowed.append(filePath); | ||
| } | ||
|
|
||
| void SyncEngine::setLocalDiscoveryEnforceWindowsFileNameCompatibility(bool value) | ||
|
Check warning on line 1281 in src/libsync/syncengine.cpp
|
||
| { | ||
| _shouldEnforceWindowsFileNameCompatibility = value; | ||
| } | ||
|
|
@@ -1314,14 +1315,14 @@ | |
| // This invariant is used in SyncEngine::shouldDiscoverLocally | ||
| QString prev; | ||
| auto it = _localDiscoveryPaths.begin(); | ||
| while(it != _localDiscoveryPaths.end()) { | ||
| if (!prev.isNull() && it->startsWith(prev) && (prev.endsWith('/') || *it == prev || it->at(prev.size()) <= '/')) { | ||
| it = _localDiscoveryPaths.erase(it); | ||
| } else { | ||
| prev = *it; | ||
| ++it; | ||
| } | ||
| } | ||
|
Check warning on line 1325 in src/libsync/syncengine.cpp
|
||
| } | ||
|
|
||
| void SyncEngine::setSingleItemDiscoveryOptions(const SingleItemDiscoveryOptions &singleItemDiscoveryOptions) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ | |
| * any purpose. | ||
| */ | ||
|
|
||
| #include <QtTest> | ||
| #include "syncenginetestutils.h" | ||
| #include "lockwatcher.h" | ||
| #include <syncengine.h> | ||
|
|
@@ -137,7 +137,7 @@ | |
|
|
||
| fakeFolder.syncEngine().setLocalDiscoveryOptions(LocalDiscoveryStyle::DatabaseAndFilesystem, tracker.localDiscoveryPaths()); | ||
| tracker.startSyncPartialDiscovery(); | ||
| QVERIFY(fakeFolder.syncOnce()); | ||
|
|
||
| QVERIFY(seenLockedFiles.contains(fakeFolder.localPath() + "A/a1")); | ||
| QVERIFY(seenLockedFiles.size() == 1); | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
CSYNC_EXCLUDE_TYPEenum now has a more specific enum value for this case: