PropagateUpload: Better messaging for 507 #5537

It now produces a summary error message indicating the problem.

Adjust blacklist database table to contain 'errorCategory'. This is
useful for two things:
  - Reestablishing summary messages based on blacklisted errors. For
    example if we don't retry a 507ed file, we still want to show the
    message about space on the server
  - Selectively wiping the blacklist: When we have ui for something like
    "I deleted some files, please retry all files now!", we want to
    delete all blacklist entries of a specific category only.

diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
index 96bb4ca2100986d5a9487aed209be96582709e18..016f0cdc84d7da110615a825dfc575ba8ba04ad8 100644 (file)
--- a/src/libsync/owncloudpropagator.cpp
+++ b/src/libsync/owncloudpropagator.cpp
@@ -129,15 +129,7 @@ static time_t getMaxBlacklistTime()
 static SyncJournalErrorBlacklistRecord createBlacklistEntry(
     const SyncJournalErrorBlacklistRecord &old, const SyncFileItem &item)
 {
-    SyncJournalErrorBlacklistRecord entry;
-
-    entry._errorString = item._errorString;
-    entry._lastTryModtime = item._modtime;
-    entry._lastTryEtag = item._etag;
-    entry._lastTryTime = Utility::qDateTimeToTime_t(QDateTime::currentDateTime());
-    entry._file = item._file;
-    entry._renameTarget = item._renameTarget;
-
+    auto entry = SyncJournalErrorBlacklistRecord::fromSyncFileItem(item);
     entry._retryCount = old._retryCount + 1;
 
     static time_t minBlacklistTime(getMinBlacklistTime());
@@ -162,6 +154,10 @@ static SyncJournalErrorBlacklistRecord createBlacklistEntry(
         entry._ignoreDuration = 0;
     }
 
+    if (item._httpErrorCode == 507) {
+        entry._errorCategory = SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage;
+    }
+
     return entry;
 }
 
@@ -189,7 +185,7 @@ static void blacklistUpdate(SyncJournalDb *journal, SyncFileItem &item)
     }
 
     auto newEntry = createBlacklistEntry(oldEntry, item);
-    journal->updateErrorBlacklistEntry(newEntry);
+    journal->setErrorBlacklistEntry(newEntry);
 
     // Suppress the error if it was and continues to be blacklisted.
     // An ignoreDuration of 0 mean we're tracking the error, but not actively
@@ -243,8 +239,13 @@ void PropagateItemJob::done(SyncFileItem::Status statusArg, const QString &error
     case SyncFileItem::SoftError:
     case SyncFileItem::FatalError:
     case SyncFileItem::NormalError:
+    case SyncFileItem::BlacklistedError:
         // Check the blacklist, possibly adjusting the item (including its status)
-        blacklistUpdate(propagator()->_journal, *_item);
+        // 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);
+        }
         break;
     case SyncFileItem::Success:
     case SyncFileItem::Restoration:
@@ -260,7 +261,6 @@ void PropagateItemJob::done(SyncFileItem::Status statusArg, const QString &error
     case SyncFileItem::Conflict:
     case SyncFileItem::FileIgnored:
     case SyncFileItem::NoStatus:
-    case SyncFileItem::BlacklistedError:
         // nothing
         break;
     }

diff --git a/src/libsync/owncloudpropagator.h b/src/libsync/owncloudpropagator.h
index 5b769d2f3cd8232e322d2b43b2821026102575d8..02145fa193f7cdee0ad25732d5decce46815fff2 100644 (file)
--- a/src/libsync/owncloudpropagator.h
+++ b/src/libsync/owncloudpropagator.h
@@ -445,6 +445,7 @@ signals:
     void touchedFile(const QString &fileName);
 
     void insufficientLocalStorage();
+    void insufficientRemoteStorage();
 
 private:
     AccountPtr _account;

diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp
index 041a944cb8e3c9a83bd4f1eae83326b0f678f6d3..757aee1c4485078005c6bbb1215c8ed139fa584e 100644 (file)
--- a/src/libsync/propagateupload.cpp
+++ b/src/libsync/propagateupload.cpp
@@ -542,6 +542,15 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job)
 
     SyncFileItem::Status status = classifyError(job->reply()->error(), _item->_httpErrorCode,
         &propagator()->_anotherSyncNeeded);
+
+    if (_item->_httpErrorCode == 507) {
+        // Insufficient remote storage.
+        _item->_errorMayBeBlacklisted = true;
+        status = SyncFileItem::BlacklistedError;
+        errorString = tr("Upload of %1 exceeds the quota for the folder").arg(Utility::octetsToString(_item->_size));
+        emit propagator()->insufficientRemoteStorage();
+    }
+
     abortWithError(status, errorString);
 }
 

diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index e73777f9647500bfcb1a31d10c28bc43132c299d..65fdd61135bb66cea5a012466417d23dcdbd4090 100644 (file)
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -272,6 +272,10 @@ bool SyncEngine::checkErrorBlacklisting(SyncFileItem &item)
     auto waitSecondsStr = Utility::durationToDescriptiveString1(1000 * waitSeconds);
     item._errorString = tr("%1 (skipped due to earlier error, trying again in %2)").arg(entry._errorString, waitSecondsStr);
 
+    if (entry._errorCategory == SyncJournalErrorBlacklistRecord::InsufficientRemoteStorage) {
+        slotInsufficientRemoteStorage();
+    }
+
     return true;
 }
 
@@ -1040,6 +1044,7 @@ void SyncEngine::slotDiscoveryJobFinished(int discoveryResult)
     connect(_propagator.data(), SIGNAL(seenLockedFile(QString)), SIGNAL(seenLockedFile(QString)));
     connect(_propagator.data(), SIGNAL(touchedFile(QString)), SLOT(slotAddTouchedFile(QString)));
     connect(_propagator.data(), SIGNAL(insufficientLocalStorage()), SLOT(slotInsufficientLocalStorage()));
+    connect(_propagator.data(), SIGNAL(insufficientRemoteStorage()), SLOT(slotInsufficientRemoteStorage()));
 
     // apply the network limits to the propagator
     setNetworkLimits(_uploadLimit, _downloadLimit);
@@ -1557,4 +1562,9 @@ void SyncEngine::slotInsufficientLocalStorage()
             .arg(Utility::octetsToString(freeSpaceLimit())));
 }
 
+void SyncEngine::slotInsufficientRemoteStorage()
+{
+    slotSummaryError(tr("There is insufficient space available on the server for some uploads."));
+}
+
 } // namespace OCC

diff --git a/src/libsync/syncengine.h b/src/libsync/syncengine.h
index deb87ef40928acc7ca96d3957203834eba69ac2f..e307a900b7c0bc9621b001eca938c0a7827c4ae2 100644 (file)
--- a/src/libsync/syncengine.h
+++ b/src/libsync/syncengine.h
@@ -167,6 +167,7 @@ private slots:
     void slotSummaryError(const QString &message);
 
     void slotInsufficientLocalStorage();
+    void slotInsufficientRemoteStorage();
 
 private:
     void handleSyncError(CSYNC *ctx, const char *state);

diff --git a/src/libsync/syncjournaldb.cpp b/src/libsync/syncjournaldb.cpp
index b6bcff4d0d48cd81e5efea57a8919788d78fab47..5b43a3595e7a7dc160cc2ee810b794b4a65c9aad 100644 (file)
--- a/src/libsync/syncjournaldb.cpp
+++ b/src/libsync/syncjournaldb.cpp
@@ -564,7 +564,7 @@ bool SyncJournalDb::checkConnect()
         return sqlFail("prepare _deleteFileRecordRecursively", *_deleteFileRecordRecursively);
     }
 
-    QString sql("SELECT lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget "
+    QString sql("SELECT lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget, errorCategory "
                 "FROM blacklist WHERE path=?1");
     if (Utility::fsCasePreserving()) {
         // if the file system is case preserving we have to check the blacklist
@@ -578,8 +578,8 @@ bool SyncJournalDb::checkConnect()
 
     _setErrorBlacklistQuery.reset(new SqlQuery(_db));
     if (_setErrorBlacklistQuery->prepare("INSERT OR REPLACE INTO blacklist "
-                                         "(path, lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget) "
-                                         "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)")) {
+                                         "(path, lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget, errorCategory) "
+                                         "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)")) {
         return sqlFail("prepare _setErrorBlacklistQuery", *_setErrorBlacklistQuery);
     }
 
@@ -800,7 +800,17 @@ bool SyncJournalDb::updateErrorBlacklistTableStructure()
             sqlFail("updateBlacklistTableStructure: Add renameTarget", query);
             re = false;
         }
-        commitInternal("update database structure: add lastTryTime, ignoreDuration cols");
+        commitInternal("update database structure: add renameTarget col");
+    }
+
+    if (columns.indexOf(QLatin1String("errorCategory")) == -1) {
+        SqlQuery query(_db);
+        query.prepare("ALTER TABLE blacklist ADD COLUMN errorCategory INTEGER(8);");
+        if (!query.exec()) {
+            sqlFail("updateBlacklistTableStructure: Add errorCategory", query);
+            re = false;
+        }
+        commitInternal("update database structure: add errorCategory col");
     }
 
     SqlQuery query(_db);
@@ -1410,6 +1420,8 @@ SyncJournalErrorBlacklistRecord SyncJournalDb::errorBlacklistEntry(const QString
                 entry._lastTryTime = _getErrorBlacklistQuery->int64Value(4);
                 entry._ignoreDuration = _getErrorBlacklistQuery->int64Value(5);
                 entry._renameTarget = _getErrorBlacklistQuery->stringValue(6);
+                entry._errorCategory = static_cast<SyncJournalErrorBlacklistRecord::Category>(
+                    _getErrorBlacklistQuery->intValue(7));
                 entry._file = file;
             }
             _getErrorBlacklistQuery->reset_and_clear_bindings();
@@ -1501,13 +1513,14 @@ void SyncJournalDb::wipeErrorBlacklistEntry(const QString &file)
     }
 }
 
-void SyncJournalDb::updateErrorBlacklistEntry(const SyncJournalErrorBlacklistRecord &item)
+void SyncJournalDb::setErrorBlacklistEntry(const SyncJournalErrorBlacklistRecord &item)
 {
     QMutexLocker locker(&_mutex);
 
     qCInfo(lcDb) << "Setting blacklist entry for " << item._file << item._retryCount
                  << item._errorString << item._lastTryTime << item._ignoreDuration
-                 << item._lastTryModtime << item._lastTryEtag << item._renameTarget;
+                 << item._lastTryModtime << item._lastTryEtag << item._renameTarget
+                 << item._errorCategory;
 
     if (!checkConnect()) {
         return;
@@ -1521,6 +1534,7 @@ void SyncJournalDb::updateErrorBlacklistEntry(const SyncJournalErrorBlacklistRec
     _setErrorBlacklistQuery->bindValue(6, QString::number(item._lastTryTime));
     _setErrorBlacklistQuery->bindValue(7, QString::number(item._ignoreDuration));
     _setErrorBlacklistQuery->bindValue(8, item._renameTarget);
+    _setErrorBlacklistQuery->bindValue(9, item._errorCategory);
     _setErrorBlacklistQuery->exec();
     _setErrorBlacklistQuery->reset_and_clear_bindings();
 }

diff --git a/src/libsync/syncjournaldb.h b/src/libsync/syncjournaldb.h
index 3f6c89e43553ccc06ac65e58b35ec4e0fd70e3c8..15f1d01cab122ce5426c46210b63f1bdca36bb61 100644 (file)
--- a/src/libsync/syncjournaldb.h
+++ b/src/libsync/syncjournaldb.h
@@ -71,7 +71,7 @@ public:
 
     static qint64 getPHash(const QString &);
 
-    void updateErrorBlacklistEntry(const SyncJournalErrorBlacklistRecord &item);
+    void setErrorBlacklistEntry(const SyncJournalErrorBlacklistRecord &item);
     void wipeErrorBlacklistEntry(const QString &file);
     int wipeErrorBlacklist();
     int errorBlackListEntryCount();

diff --git a/src/libsync/syncjournalfilerecord.cpp b/src/libsync/syncjournalfilerecord.cpp
index 96bde3eb6d7aa30a3f017f58b686223cbc971aac..024c3274607d5abd78371f815f6575b7a7dbb198 100644 (file)
--- a/src/libsync/syncjournalfilerecord.cpp
+++ b/src/libsync/syncjournalfilerecord.cpp
@@ -120,6 +120,20 @@ QByteArray SyncJournalFileRecord::numericFileId() const
     return _fileId;
 }
 
+SyncJournalErrorBlacklistRecord SyncJournalErrorBlacklistRecord::fromSyncFileItem(
+    const SyncFileItem &item)
+{
+    SyncJournalErrorBlacklistRecord record;
+    record._file = item._file;
+    record._errorString = item._errorString;
+    record._lastTryModtime = item._modtime;
+    record._lastTryEtag = item._etag;
+    record._lastTryTime = Utility::qDateTimeToTime_t(QDateTime::currentDateTime());
+    record._renameTarget = item._renameTarget;
+    record._retryCount = 1;
+    return record;
+}
+
 bool SyncJournalErrorBlacklistRecord::isValid() const
 {
     return !_file.isEmpty()

diff --git a/src/libsync/syncjournalfilerecord.h b/src/libsync/syncjournalfilerecord.h
index 8f333318038b181bf3329a0939763f092481e048..6ef0ba169ea6094e04ac2f4758cde469667386ff 100644 (file)
--- a/src/libsync/syncjournalfilerecord.h
+++ b/src/libsync/syncjournalfilerecord.h
@@ -75,19 +75,32 @@ operator==(const SyncJournalFileRecord &lhs,
 class SyncJournalErrorBlacklistRecord
 {
 public:
+    enum Category {
+        /// Normal errors have no special behavior
+        Normal = 0,
+        /// These get a special summary message
+        InsufficientRemoteStorage
+    };
+
     SyncJournalErrorBlacklistRecord()
         : _retryCount(0)
+        , _errorCategory(Category::Normal)
         , _lastTryModtime(0)
         , _lastTryTime(0)
         , _ignoreDuration(0)
     {
     }
 
+    /// Create a record based on an item.
+    static SyncJournalErrorBlacklistRecord fromSyncFileItem(const SyncFileItem &item);
+
     /// The number of times the operation was unsuccessful so far.
     int _retryCount;
 
     /// The last error string.
     QString _errorString;
+    /// The error category. Sometimes used for special actions.
+    Category _errorCategory;
 
     time_t _lastTryModtime;
     QByteArray _lastTryEtag;