Blacklist: remember the X-Request-ID

Issue #6420
Store the X-Request-ID in the SyncFileItem and also in the blacklist.
Note that for consistency reason, the X-Request-ID is also in the
SyncFileItem if the request succeeds.

Currently there is no UI to access it, but it can be queried with sql
commands

14 files changed, 53 insertions, 16 deletions

diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp
index 8a41a4ac6..f8c1930d5 100644
--- a/src/common/syncjournaldb.cpp
+++ b/src/common/syncjournaldb.cpp
@@ -554,7 +554,7 @@ bool SyncJournalDb::checkConnect()
         return sqlFail("prepare _deleteUploadInfoQuery", _deleteUploadInfoQuery);
     }
 
-    QByteArray sql("SELECT lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget, errorCategory "
+    QByteArray sql("SELECT lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget, errorCategory, requestId "
                    "FROM blacklist WHERE path=?1");
     if (Utility::fsCasePreserving()) {
         // if the file system is case preserving we have to check the blacklist
@@ -753,6 +753,16 @@ bool SyncJournalDb::updateErrorBlacklistTableStructure()
         commitInternal("update database structure: add errorCategory col");
     }
 
+    if (columns.indexOf("requestId") == -1) {
+        SqlQuery query(_db);
+        query.prepare("ALTER TABLE blacklist ADD COLUMN requestId VARCHAR(36);");
+        if (!query.exec()) {
+            sqlFail("updateBlacklistTableStructure: Add requestId", query);
+            re = false;
+        }
+        commitInternal("update database structure: add errorCategory col");
+    }
+
     SqlQuery query(_db);
     query.prepare("CREATE INDEX IF NOT EXISTS blacklist_index ON blacklist(path collate nocase);");
     if (!query.exec()) {
@@ -1443,8 +1453,6 @@ SyncJournalErrorBlacklistRecord SyncJournalDb::errorBlacklistEntry(const QString
     if (file.isEmpty())
         return entry;
 
-    // SELECT lastTryEtag, lastTryModtime, retrycount, errorstring
-
     if (checkConnect()) {
         _getErrorBlacklistQuery.reset_and_clear_bindings();
         _getErrorBlacklistQuery.bindValue(1, file);
@@ -1459,6 +1467,7 @@ SyncJournalErrorBlacklistRecord SyncJournalDb::errorBlacklistEntry(const QString
                 entry._renameTarget = _getErrorBlacklistQuery.stringValue(6);
                 entry._errorCategory = static_cast<SyncJournalErrorBlacklistRecord::Category>(
                     _getErrorBlacklistQuery.intValue(7));
+                entry._requestId = _getErrorBlacklistQuery.baValue(8);
                 entry._file = file;
             }
         }
@@ -1578,8 +1587,8 @@ void SyncJournalDb::setErrorBlacklistEntry(const SyncJournalErrorBlacklistRecord
 
     if (!_setErrorBlacklistQuery.initOrReset(QByteArrayLiteral(
         "INSERT OR REPLACE INTO blacklist "
-        "(path, lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget, errorCategory) "
-        "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)"), _db)) {
+        "(path, lastTryEtag, lastTryModtime, retrycount, errorstring, lastTryTime, ignoreDuration, renameTarget, errorCategory, requestId) "
+        "VALUES ( ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)"), _db)) {
         return;
     }
 
@@ -1592,6 +1601,7 @@ void SyncJournalDb::setErrorBlacklistEntry(const SyncJournalErrorBlacklistRecord
     _setErrorBlacklistQuery.bindValue(7, item._ignoreDuration);
     _setErrorBlacklistQuery.bindValue(8, item._renameTarget);
     _setErrorBlacklistQuery.bindValue(9, item._errorCategory);
+    _setErrorBlacklistQuery.bindValue(10, item._requestId);
     _setErrorBlacklistQuery.exec();
 }
 
diff --git a/src/common/syncjournalfilerecord.h b/src/common/syncjournalfilerecord.h
index 43259c1e6..129ef1533 100644
--- a/src/common/syncjournalfilerecord.h
+++ b/src/common/syncjournalfilerecord.h
@@ -109,6 +109,9 @@ public:
     QString _file;
     QString _renameTarget;
 
+    /// The last X-Request-ID of the request that failled
+    QByteArray _requestId;
+
     bool isValid() const;
 };
 
diff --git a/src/libsync/abstractnetworkjob.cpp b/src/libsync/abstractnetworkjob.cpp
index d9d97ce92..b10a495af 100644
--- a/src/libsync/abstractnetworkjob.cpp
+++ b/src/libsync/abstractnetworkjob.cpp
@@ -243,6 +243,11 @@ QByteArray AbstractNetworkJob::responseTimestamp()
     return _responseTimestamp;
 }
 
+QByteArray AbstractNetworkJob::requestId()
+{
+    return  _reply ? _reply->request().rawHeader("X-Request-ID") : QByteArray();
+}
+
 QString AbstractNetworkJob::errorString() const
 {
     if (_timedout) {
diff --git a/src/libsync/abstractnetworkjob.h b/src/libsync/abstractnetworkjob.h
index bbbc58e2f..6f61d26fd 100644
--- a/src/libsync/abstractnetworkjob.h
+++ b/src/libsync/abstractnetworkjob.h
@@ -69,6 +69,8 @@ public:
     bool followRedirects() const { return _followRedirects; }
 
     QByteArray responseTimestamp();
+    /* Content of the X-Request-ID header. (Only set after the request is sent) */
+    QByteArray requestId();
 
     qint64 timeoutMsec() const { return _timer.interval(); }
     bool timedOut() const { return _timedout; }
diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
index 3cd8ab0be..f1c1512d1 100644
--- a/src/libsync/owncloudpropagator.cpp
+++ b/src/libsync/owncloudpropagator.cpp
@@ -141,6 +141,7 @@ static SyncJournalErrorBlacklistRecord createBlacklistEntry(
     entry._lastTryTime = Utility::qDateTimeToTime_t(QDateTime::currentDateTimeUtc());
     entry._renameTarget = item._renameTarget;
     entry._retryCount = old._retryCount + 1;
+    entry._requestId = item._requestId;
 
     static qint64 minBlacklistTime(getMinBlacklistTime());
     static qint64 maxBlacklistTime(qMax(getMaxBlacklistTime(), minBlacklistTime));
diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index d1479b887..59de69758 100644
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -550,9 +550,12 @@ void PropagateDownloadFile::slotGetFinished()
     GETFileJob *job = _job;
     ASSERT(job);
 
+    _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+    _item->_responseTimeStamp = job->responseTimestamp();
+    _item->_requestId = job->requestId();
+
     QNetworkReply::NetworkError err = job->reply()->error();
     if (err != QNetworkReply::NoError) {
-        _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
         // If we sent a 'Range' header and get 416 back, we want to retry
         // without the header.
@@ -625,7 +628,6 @@ void PropagateDownloadFile::slotGetFinished()
         // so make sure we have the up-to-date time
         _item->_modtime = job->lastModified();
     }
-    _item->_responseTimeStamp = job->responseTimestamp();
 
     _tmpFile.close();
     _tmpFile.flush();
diff --git a/src/libsync/propagateremotedelete.cpp b/src/libsync/propagateremotedelete.cpp
index 9b3ea8958..61ee3edee 100644
--- a/src/libsync/propagateremotedelete.cpp
+++ b/src/libsync/propagateremotedelete.cpp
@@ -93,6 +93,8 @@ void PropagateRemoteDelete::slotDeleteJobFinished()
     QNetworkReply::NetworkError err = _job->reply()->error();
     const int httpStatus = _job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
     _item->_httpErrorCode = httpStatus;
+    _item->_responseTimeStamp = _job->responseTimestamp();
+    _item->_requestId = _job->requestId();
 
     if (err != QNetworkReply::NoError && err != QNetworkReply::ContentNotFoundError) {
         SyncFileItem::Status status = classifyError(err, _item->_httpErrorCode,
@@ -101,8 +103,6 @@ void PropagateRemoteDelete::slotDeleteJobFinished()
         return;
     }
 
-    _item->_responseTimeStamp = _job->responseTimestamp();
-
     // A 404 reply is also considered a success here: We want to make sure
     // a file is gone from the server. It not being there in the first place
     // is ok. This will happen for files that are in the DB but not on
diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp
index 98a72d48a..d44f2b2da 100644
--- a/src/libsync/propagateremotemkdir.cpp
+++ b/src/libsync/propagateremotemkdir.cpp
@@ -83,6 +83,8 @@ void PropagateRemoteMkdir::slotMkcolJobFinished()
 
     QNetworkReply::NetworkError err = _job->reply()->error();
     _item->_httpErrorCode = _job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+    _item->_responseTimeStamp = _job->responseTimestamp();
+    _item->_requestId = _job->requestId();
 
     if (_item->_httpErrorCode == 405) {
         // This happens when the directory already exists. Nothing to do.
@@ -102,7 +104,6 @@ void PropagateRemoteMkdir::slotMkcolJobFinished()
         return;
     }
 
-    _item->_responseTimeStamp = _job->responseTimestamp();
     _item->_fileId = _job->reply()->rawHeader("OC-FileId");
 
     if (_item->_fileId.isEmpty()) {
diff --git a/src/libsync/propagateremotemove.cpp b/src/libsync/propagateremotemove.cpp
index c08cacfeb..e4b726201 100644
--- a/src/libsync/propagateremotemove.cpp
+++ b/src/libsync/propagateremotemove.cpp
@@ -116,6 +116,8 @@ void PropagateRemoteMove::slotMoveJobFinished()
 
     QNetworkReply::NetworkError err = _job->reply()->error();
     _item->_httpErrorCode = _job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+    _item->_responseTimeStamp = _job->responseTimestamp();
+    _item->_requestId = _job->requestId();
 
     if (err != QNetworkReply::NoError) {
         SyncFileItem::Status status = classifyError(err, _item->_httpErrorCode,
@@ -124,8 +126,6 @@ void PropagateRemoteMove::slotMoveJobFinished()
         return;
     }
 
-    _item->_responseTimeStamp = _job->responseTimestamp();
-
     if (_item->_httpErrorCode != 201) {
         // Normally we expect "201 Created"
         // If it is not the case, it might be because of a proxy or gateway intercepting the request, so we must
diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp
index 712a08603..81d9f37ea 100644
--- a/src/libsync/propagateupload.cpp
+++ b/src/libsync/propagateupload.cpp
@@ -107,6 +107,7 @@ bool PollJob::finished()
     QNetworkReply::NetworkError err = reply()->error();
     if (err != QNetworkReply::NoError) {
         _item->_httpErrorCode = reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+        _item->_requestId = requestId();
         _item->_status = classifyError(err, _item->_httpErrorCode);
         _item->_errorString = errorString();
 
diff --git a/src/libsync/propagateuploadng.cpp b/src/libsync/propagateuploadng.cpp
index a76f3ac38..aec4efca8 100644
--- a/src/libsync/propagateuploadng.cpp
+++ b/src/libsync/propagateuploadng.cpp
@@ -184,6 +184,7 @@ void PropagateUploadFileNG::slotPropfindFinishedWithError()
     auto httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
     auto status = classifyError(err, httpErrorCode, &propagator()->_anotherSyncNeeded);
     if (status == SyncFileItem::FatalError) {
+        _item->_requestId = job->requestId();
         propagator()->_activeJobList.removeOne(this);
         abortWithError(status, job->errorStringParsingBody());
         return;
@@ -202,6 +203,7 @@ void PropagateUploadFileNG::slotDeleteJobFinished()
         const int httpStatus = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
         SyncFileItem::Status status = classifyError(err, httpStatus);
         if (status == SyncFileItem::FatalError) {
+            _item->_requestId = job->requestId();
             abortWithError(status, job->errorString());
             return;
         } else {
@@ -258,6 +260,7 @@ void PropagateUploadFileNG::slotMkColFinished(QNetworkReply::NetworkError)
     _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
     if (err != QNetworkReply::NoError || _item->_httpErrorCode != 201) {
+        _item->_requestId = job->requestId();
         SyncFileItem::Status status = classifyError(err, _item->_httpErrorCode,
             &propagator()->_anotherSyncNeeded);
         abortWithError(status, job->errorStringParsingBody());
@@ -361,6 +364,7 @@ void PropagateUploadFileNG::slotPutFinished()
 
     if (err != QNetworkReply::NoError) {
         _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+        _item->_requestId = job->requestId();
         commonErrorHandling(job);
         return;
     }
@@ -441,6 +445,8 @@ void PropagateUploadFileNG::slotMoveJobFinished()
     slotJobDestroyed(job); // remove it from the _jobs list
     QNetworkReply::NetworkError err = job->reply()->error();
     _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+    _item->_responseTimeStamp = job->responseTimestamp();
+    _item->_requestId = job->requestId();
 
     if (err != QNetworkReply::NoError) {
         commonErrorHandling(job);
@@ -471,7 +477,6 @@ void PropagateUploadFileNG::slotMoveJobFinished()
         abortWithError(SyncFileItem::NormalError, tr("Missing ETag from server"));
         return;
     }
-    _item->_responseTimeStamp = job->responseTimestamp();
     finalize();
 }
 
diff --git a/src/libsync/propagateuploadv1.cpp b/src/libsync/propagateuploadv1.cpp
index f8069dec0..7e2917840 100644
--- a/src/libsync/propagateuploadv1.cpp
+++ b/src/libsync/propagateuploadv1.cpp
@@ -198,6 +198,8 @@ void PropagateUploadFileV1::slotPutFinished()
     }
 
     _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+    _item->_responseTimeStamp = job->responseTimestamp();
+    _item->_requestId = job->requestId();
     QNetworkReply::NetworkError err = job->reply()->error();
     if (err != QNetworkReply::NoError) {
         commonErrorHandling(job);
@@ -299,8 +301,6 @@ void PropagateUploadFileV1::slotPutFinished()
 
     _item->_etag = etag;
 
-    _item->_responseTimeStamp = job->responseTimestamp();
-
     if (job->reply()->rawHeader("X-OC-MTime") != "accepted") {
         // X-OC-MTime is supported since owncloud 5.0.   But not when chunking.
         // Normally Owncloud 6 always puts X-OC-MTime
diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h
index 88421c39d..39e1b6add 100644
--- a/src/libsync/syncfileitem.h
+++ b/src/libsync/syncfileitem.h
@@ -223,6 +223,7 @@ public:
     RemotePermissions _remotePerm;
     QString _errorString; // Contains a string only in case of error
     QByteArray _responseTimeStamp;
+    QByteArray _requestId; // X-Request-Id of the failed request
     quint32 _affectedItems; // the number of affected items by the operation on this item.
     // usually this value is 1, but for removes on dirs, it might be much higher.
 
diff --git a/test/testblacklist.cpp b/test/testblacklist.cpp
index f571affa4..cad9f35f2 100644
--- a/test/testblacklist.cpp
+++ b/test/testblacklist.cpp
@@ -51,7 +51,9 @@ private slots:
         auto &modifier = remote ? fakeFolder.remoteModifier() : fakeFolder.localModifier();
 
         int counter = 0;
-        fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &, QIODevice *) -> QNetworkReply * {
+        QByteArray reqId;
+        fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *) -> QNetworkReply * {
+            reqId = req.rawHeader("X-Request-ID");
             if (!remote && op == QNetworkAccessManager::PutOperation)
                 ++counter;
             if (remote && op == QNetworkAccessManager::GetOperation)
@@ -82,6 +84,7 @@ private slots:
             QCOMPARE(entry._retryCount, 1);
             QCOMPARE(counter, 1);
             QVERIFY(entry._ignoreDuration > 0);
+            QCOMPARE(entry._requestId, reqId);
 
             if (remote)
                 QCOMPARE(journalRecord(fakeFolder, "A")._etag, initialEtag);
@@ -102,6 +105,7 @@ private slots:
             QCOMPARE(entry._retryCount, 1);
             QCOMPARE(counter, 1);
             QVERIFY(entry._ignoreDuration > 0);
+            QCOMPARE(entry._requestId, reqId);
 
             if (remote)
                 QCOMPARE(journalRecord(fakeFolder, "A")._etag, initialEtag);
@@ -128,6 +132,7 @@ private slots:
             QCOMPARE(entry._retryCount, 2);
             QCOMPARE(counter, 2);
             QVERIFY(entry._ignoreDuration > 0);
+            QCOMPARE(entry._requestId, reqId);
 
             if (remote)
                 QCOMPARE(journalRecord(fakeFolder, "A")._etag, initialEtag);
@@ -149,6 +154,7 @@ private slots:
             QCOMPARE(entry._retryCount, 3);
             QCOMPARE(counter, 3);
             QVERIFY(entry._ignoreDuration > 0);
+            QCOMPARE(entry._requestId, reqId);
 
             if (remote)
                 QCOMPARE(journalRecord(fakeFolder, "A")._etag, initialEtag);