From df05042e7f4f45667428a5c1b114c8855f1906a2 Mon Sep 17 00:00:00 2001 From: Christian Kamm Date: Wed, 12 Jul 2017 12:38:53 +0200 Subject: [PATCH] Make DetailError different from BlacklistedError It's quite different in regard to blacklist handling and overall sync failure changes. --- src/gui/protocolwidget.cpp | 1 + src/libsync/owncloudpropagator.cpp | 15 +++++++-------- src/libsync/progressdispatcher.cpp | 2 +- src/libsync/propagatedownload.cpp | 4 ++-- src/libsync/propagateupload.cpp | 3 +-- src/libsync/syncfileitem.h | 20 ++++++++++++++++---- src/libsync/syncfilestatustracker.cpp | 1 + 7 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/gui/protocolwidget.cpp b/src/gui/protocolwidget.cpp index b2c82ce3c..75b8aaa52 100644 --- a/src/gui/protocolwidget.cpp +++ b/src/gui/protocolwidget.cpp @@ -147,6 +147,7 @@ QTreeWidgetItem *ProtocolWidget::createCompletedTreewidgetItem(const QString &fo QIcon icon; if (item._status == SyncFileItem::NormalError || item._status == SyncFileItem::FatalError + || item._status == SyncFileItem::DetailError || item._status == SyncFileItem::BlacklistedError) { icon = Theme::instance()->syncStateIcon(SyncResult::Error); } else if (Progress::isWarningKind(item._status)) { diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp index 2dcc7a26d..65fd70264 100644 --- a/src/libsync/owncloudpropagator.cpp +++ b/src/libsync/owncloudpropagator.cpp @@ -172,7 +172,8 @@ static void blacklistUpdate(SyncJournalDb *journal, SyncFileItem &item) bool mayBlacklist = item._errorMayBeBlacklisted // explicitly flagged for blacklisting || ((item._status == SyncFileItem::NormalError - || item._status == SyncFileItem::SoftError) + || item._status == SyncFileItem::SoftError + || item._status == SyncFileItem::DetailError) && item._httpErrorCode != 0 // or non-local error ); @@ -239,13 +240,9 @@ void PropagateItemJob::done(SyncFileItem::Status statusArg, const QString &error case SyncFileItem::SoftError: case SyncFileItem::FatalError: case SyncFileItem::NormalError: - case SyncFileItem::BlacklistedError: + case SyncFileItem::DetailError: // Check the blacklist, possibly adjusting the item (including its status) - // but not if this status comes from blacklisting in the first place - if (!(_item->_status == SyncFileItem::BlacklistedError - && _item->_instruction == CSYNC_INSTRUCTION_IGNORE)) { - blacklistUpdate(propagator()->_journal, *_item); - } + blacklistUpdate(propagator()->_journal, *_item); break; case SyncFileItem::Success: case SyncFileItem::Restoration: @@ -261,6 +258,7 @@ void PropagateItemJob::done(SyncFileItem::Status statusArg, const QString &error case SyncFileItem::Conflict: case SyncFileItem::FileIgnored: case SyncFileItem::NoStatus: + case SyncFileItem::BlacklistedError: // nothing break; } @@ -805,7 +803,8 @@ void PropagatorCompositeJob::slotSubJobFinished(SyncFileItem::Status status) if (status == SyncFileItem::FatalError || status == SyncFileItem::NormalError - || status == SyncFileItem::SoftError) { + || status == SyncFileItem::SoftError + || status == SyncFileItem::DetailError) { _hasError = status; } diff --git a/src/libsync/progressdispatcher.cpp b/src/libsync/progressdispatcher.cpp index d5b3f20f5..df6417e4a 100644 --- a/src/libsync/progressdispatcher.cpp +++ b/src/libsync/progressdispatcher.cpp @@ -91,7 +91,7 @@ bool Progress::isWarningKind(SyncFileItem::Status kind) return kind == SyncFileItem::SoftError || kind == SyncFileItem::NormalError || kind == SyncFileItem::FatalError || kind == SyncFileItem::FileIgnored || kind == SyncFileItem::Conflict || kind == SyncFileItem::Restoration - || kind == SyncFileItem::BlacklistedError; + || kind == SyncFileItem::DetailError || kind == SyncFileItem::BlacklistedError; } bool Progress::isIgnoredKind(SyncFileItem::Status kind) diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index f5b199024..e98278e0c 100644 --- a/src/libsync/propagatedownload.cpp +++ b/src/libsync/propagatedownload.cpp @@ -429,10 +429,10 @@ void PropagateDownloadFile::startDownload() const auto diskSpaceResult = propagator()->diskSpaceCheck(); if (diskSpaceResult != OwncloudPropagator::DiskSpaceOk) { if (diskSpaceResult == OwncloudPropagator::DiskSpaceFailure) { - // Using BlacklistedError here will make the error not pop up in the account + // Using DetailError here will make the error not pop up in the account // tab: instead we'll generate a general "disk space low" message and show // these detail errors only in the error view. - done(SyncFileItem::BlacklistedError, + done(SyncFileItem::DetailError, tr("The download would reduce free local disk space below the limit")); emit propagator()->insufficientLocalStorage(); } else if (diskSpaceResult == OwncloudPropagator::DiskSpaceCritical) { diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index ac6c68259..73002c427 100644 --- a/src/libsync/propagateupload.cpp +++ b/src/libsync/propagateupload.cpp @@ -524,8 +524,7 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job) if (_item->_httpErrorCode == 507) { // Insufficient remote storage. - _item->_errorMayBeBlacklisted = true; - status = SyncFileItem::BlacklistedError; + status = SyncFileItem::DetailError; errorString = tr("Upload of %1 exceeds the quota for the folder").arg(Utility::octetsToString(_item->_size)); emit propagator()->insufficientRemoteStorage(); } diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h index 571eabc19..45660f83f 100644 --- a/src/libsync/syncfileitem.h +++ b/src/libsync/syncfileitem.h @@ -64,12 +64,24 @@ public: FileIgnored, ///< The file is in the ignored list (or blacklisted with no retries left) Restoration, ///< The file was restored because what should have been done was not allowed - /** For files whose errors were blacklisted. + /** For errors that should only appear in the error view. * - * If _instruction == IGNORE, the file wasn't even reattempted. + * Some errors also produce a summary message. Usually displaying that message is + * sufficient, but the individual errors should still appear in the issues tab. * - * These errors should usually be shown as NormalErrors, but not be as loud - * as them. + * These errors do cause the sync to fail. + * + * A NormalError that isn't as prominent. + */ + DetailError, + + /** For files whose errors were blacklisted + * + * If an file is blacklisted due to an error it isn't even reattempted. These + * errors should appear in the issues tab, but not on the account settings and + * should not cause the sync run to fail. + * + * A DetailError that doesn't cause sync failure. */ BlacklistedError }; diff --git a/src/libsync/syncfilestatustracker.cpp b/src/libsync/syncfilestatustracker.cpp index 32e3ccb00..c0b315372 100644 --- a/src/libsync/syncfilestatustracker.cpp +++ b/src/libsync/syncfilestatustracker.cpp @@ -94,6 +94,7 @@ static inline bool showErrorInSocketApi(const SyncFileItem &item) return item._instruction == CSYNC_INSTRUCTION_ERROR || status == SyncFileItem::NormalError || status == SyncFileItem::FatalError + || status == SyncFileItem::DetailError || status == SyncFileItem::BlacklistedError || item._hasBlacklistEntry; }