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.

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)
+        // 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;
     }

src/libsync/owncloudpropagator.h

@@ -445,6 +445,7 @@ signals:
     void touchedFile(const QString &fileName);
 
     void insufficientLocalStorage();
+    void insufficientRemoteStorage();
 
 private:
     AccountPtr _account;

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);
 }
 

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

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);

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();
 }

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();

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()

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;