diff options
author | Hannah von Reth <hannah.vonreth@owncloud.com> | 2022-01-25 18:14:24 +0300 |
---|---|---|
committer | Hannah von Reth <vonreth@kde.org> | 2022-01-26 17:18:23 +0300 |
commit | cc3b63f9dea76f1b164ebb8f316745f5afa8213d (patch) | |
tree | db03f8d459a13dbf4671d113600f472718c3f01b | |
parent | fae3a39cc235154e69c2fc6156d4de976a71ea2f (diff) |
Introduce new error category that is not handled by the ignore list
Fixes: #9382
-rw-r--r-- | changelog/unreleased/9382 | 5 | ||||
-rw-r--r-- | src/gui/issueswidget.cpp | 28 | ||||
-rw-r--r-- | src/libsync/localdiscoverytracker.cpp | 51 | ||||
-rw-r--r-- | src/libsync/owncloudpropagator.cpp | 1 | ||||
-rw-r--r-- | src/libsync/propagateupload.cpp | 19 | ||||
-rw-r--r-- | src/libsync/syncfileitem.cpp | 3 | ||||
-rw-r--r-- | src/libsync/syncfileitem.h | 5 |
7 files changed, 78 insertions, 34 deletions
diff --git a/changelog/unreleased/9382 b/changelog/unreleased/9382 new file mode 100644 index 000000000..ba5f5bc2d --- /dev/null +++ b/changelog/unreleased/9382 @@ -0,0 +1,5 @@ +Bugfix: Immediately retry upload if file changed during sync + +If a file changed during discovery and the actual upload for multiple retries in a row, changes of it were ignored for a period of time. + +https://github.com/owncloud/client/issues/9382 diff --git a/src/gui/issueswidget.cpp b/src/gui/issueswidget.cpp index 41a7b006a..e53c7d794 100644 --- a/src/gui/issueswidget.cpp +++ b/src/gui/issueswidget.cpp @@ -302,14 +302,15 @@ std::function<void(void)> IssuesWidget::addStatusFilter(QMenu *menu) const auto initialFilter = _statusSortModel->filter(); { // Add all errors under 1 action: - const std::array<SyncFileItem::Status, 5> ErrorStatusItems = { + const std::vector<SyncFileItem::Status> ErrorStatusItems = { SyncFileItem::Status::FatalError, SyncFileItem::Status::NormalError, SyncFileItem::Status::SoftError, - SyncFileItem::Status::DetailError + SyncFileItem::Status::DetailError, }; - auto action = menu->addAction(SyncFileItem::statusEnumDisplayName(SyncFileItem::NormalError), [this, ErrorStatusItems](bool checked) { + + auto action = menu->addAction(SyncFileItem::statusEnumDisplayName(SyncFileItem::NormalError), this, [this, ErrorStatusItems](bool checked) { auto currentFilter = _statusSortModel->filter(); for (const auto &item : ErrorStatusItems) { currentFilter[item] = checked; @@ -320,23 +321,34 @@ std::function<void(void)> IssuesWidget::addStatusFilter(QMenu *menu) action->setChecked(initialFilter[ErrorStatusItems[0]]); statusFilterGroup->addAction(action); } + menu->addSeparator(); // Add the other non-error items: - const std::array<SyncFileItem::Status, 5> OtherStatusItems = { + const std::vector<SyncFileItem::Status> OtherStatusItems = { SyncFileItem::Status::Conflict, SyncFileItem::Status::FileIgnored, SyncFileItem::Status::Restoration, SyncFileItem::Status::BlacklistedError, - SyncFileItem::Status::Excluded + SyncFileItem::Status::Excluded, + SyncFileItem::Status::Message }; + // list of OtherStatusItems with the localised name + std::vector<std::pair<QString, SyncFileItem::Status>> otherStatusItems; + otherStatusItems.reserve(OtherStatusItems.size()); for (const auto &item : OtherStatusItems) { - auto action = menu->addAction(SyncFileItem::statusEnumDisplayName(item), [this, item](bool checked) { + otherStatusItems.emplace_back(SyncFileItem::statusEnumDisplayName(item), item); + } + std::sort(otherStatusItems.begin(), otherStatusItems.end(), [](const auto &a, const auto &b) { + return a.first < b.first; + }); + for (const auto &item : otherStatusItems) { + auto action = menu->addAction(item.first, this, [this, item](bool checked) { auto currentFilter = _statusSortModel->filter(); - currentFilter[item] = checked; + currentFilter[item.second] = checked; _statusSortModel->setFilter(currentFilter); }); action->setCheckable(true); - action->setChecked(initialFilter[item]); + action->setChecked(initialFilter[item.second]); statusFilterGroup->addAction(action); } diff --git a/src/libsync/localdiscoverytracker.cpp b/src/libsync/localdiscoverytracker.cpp index 44cb909ca..cfc626a80 100644 --- a/src/libsync/localdiscoverytracker.cpp +++ b/src/libsync/localdiscoverytracker.cpp @@ -64,23 +64,48 @@ void LocalDiscoveryTracker::slotItemCompleted(const SyncFileItemPtr &item) // // 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::FileIgnored - || item->_status == SyncFileItem::Restoration - || item->_status == SyncFileItem::Conflict - || (item->_status == SyncFileItem::NoStatus - && (item->_instruction == CSYNC_INSTRUCTION_NONE - || item->_instruction == CSYNC_INSTRUCTION_UPDATE_METADATA))) { - if (_previousLocalDiscoveryPaths.erase(item->_file)) + + switch (item->_status) { + case SyncFileItem::NoStatus: + // we can't use the flags operator with CSYNC_INSTRUCTION_NONE + if (item->_instruction != CSYNC_INSTRUCTION_NONE && item->_instruction != CSYNC_INSTRUCTION_UPDATE_METADATA) { + break; + } + Q_FALLTHROUGH(); + case SyncFileItem::Success: + Q_FALLTHROUGH(); + case SyncFileItem::FileIgnored: + Q_FALLTHROUGH(); + case SyncFileItem::Restoration: + Q_FALLTHROUGH(); + case SyncFileItem::Conflict: + Q_FALLTHROUGH(); + case OCC::SyncFileItem::Message: + if (_previousLocalDiscoveryPaths.erase(item->_file)) { qCDebug(lcLocalDiscoveryTracker) << "wiped successful item" << item->_file; - if (!item->_renameTarget.isEmpty() && _previousLocalDiscoveryPaths.erase(item->_renameTarget)) + } + if (!item->_renameTarget.isEmpty() && _previousLocalDiscoveryPaths.erase(item->_renameTarget)) { qCDebug(lcLocalDiscoveryTracker) << "wiped successful item" << item->_renameTarget; - } else if (item->_status == SyncFileItem::StatusCount) { + } + return; + case OCC::SyncFileItem::FatalError: + Q_FALLTHROUGH(); + case OCC::SyncFileItem::NormalError: + Q_FALLTHROUGH(); + case OCC::SyncFileItem::SoftError: + Q_FALLTHROUGH(); + case OCC::SyncFileItem::DetailError: + Q_FALLTHROUGH(); + case OCC::SyncFileItem::BlacklistedError: + Q_FALLTHROUGH(); + case OCC::SyncFileItem::Excluded: + break; + case SyncFileItem::StatusCount: Q_UNREACHABLE(); - } else { - _localDiscoveryPaths.insert(item->_file); - qCDebug(lcLocalDiscoveryTracker) << "inserted error item" << item->_file; } + + _localDiscoveryPaths.insert(item->_file); + qCDebug(lcLocalDiscoveryTracker) << "inserted error item" << item->_file; } void LocalDiscoveryTracker::slotSyncFinished(bool success) diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 933dcc523..3836f5e14 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -258,6 +258,7 @@ void PropagateItemJob::done(SyncFileItem::Status statusArg, const QString &error case SyncFileItem::FatalError: case SyncFileItem::NormalError: case SyncFileItem::DetailError: + case SyncFileItem::Message: // Check the blacklist, possibly adjusting the item (including its status) blacklistUpdate(propagator()->_journal, *_item); break; diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index eb54aeb6e..eb147a13c 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -225,26 +225,19 @@ void PropagateUploadFileCommon::slotStartUpload(const QByteArray &transmissionCh done(SyncFileItem::SoftError, tr("File Removed")); return; } - time_t prevModtime = _item->_modtime; // the _item value was set in PropagateUploadFile::start() + _item->_size = FileSystem::getSize(fullFilePath); + + const time_t prevModtime = _item->_modtime; // the _item value was set in PropagateUploadFile::start() // but a potential checksum calculation could have taken some time during which the file could // have been changed again, so better check again here. - _item->_modtime = FileSystem::getModTime(fullFilePath); - if (prevModtime != _item->_modtime) { - propagator()->_anotherSyncNeeded = true; - done(SyncFileItem::SoftError, tr("Local file changed during syncing. It will be resumed.")); - return; - } - - qint64 fileSize = FileSystem::getSize(fullFilePath); - _item->_size = fileSize; - // But skip the file if the mtime is too close to 'now'! // That usually indicates a file that is still being changed // or not yet fully copied to the destination. - if (fileIsStillChanging(*_item)) { + _item->_modtime = FileSystem::getModTime(fullFilePath); + if (prevModtime != _item->_modtime || fileIsStillChanging(*_item)) { propagator()->_anotherSyncNeeded = true; - done(SyncFileItem::SoftError, tr("Local file changed during sync.")); + done(SyncFileItem::Message, tr("Local file changed during sync. It will be resumed.")); return; } diff --git a/src/libsync/syncfileitem.cpp b/src/libsync/syncfileitem.cpp index 9e1ac350c..1ca287e0d 100644 --- a/src/libsync/syncfileitem.cpp +++ b/src/libsync/syncfileitem.cpp @@ -77,6 +77,7 @@ SyncFileItemPtr SyncFileItem::fromSyncJournalFileRecord(const SyncJournalFileRec QString SyncFileItem::statusEnumDisplayName(Status s) { + // TODO: 2.11 enumDisplayName https://github.com/owncloud/client/issues/9043 switch (s) { case SyncFileItem::NoStatus: return QCoreApplication::translate("SyncFileItem::Status", "Undefined"); @@ -100,6 +101,8 @@ QString SyncFileItem::statusEnumDisplayName(Status s) return QCoreApplication::translate("SyncFileItem::Status", "Blacklisted"); case OCC::SyncFileItem::Excluded: return QCoreApplication::translate("SyncFileItem::Status", "Excluded"); + case OCC::SyncFileItem::Message: + return QCoreApplication::translate("SyncFileItem::Status", "Message"); case OCC::SyncFileItem::StatusCount: Q_UNREACHABLE(); } diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index be400bbe1..fd7247829 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -93,6 +93,11 @@ public: */ Excluded, + /** + * Similar to SoftError but will not cause any error handling + */ + Message, + /** For use in an array or vector for the number of items in this enum. */ StatusCount |