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

diff --git a/src/common/syncjournaldb.cpp b/src/common/syncjournaldb.cpp
index 7a6bee7e9de5bd8fb53559fc3dea60a5b060c839..41cf3e0bfcc8a6cd374034d562aa86fe226249d9 100644 (file)
--- a/src/common/syncjournaldb.cpp
+++ b/src/common/syncjournaldb.cpp
@@ -568,7 +568,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
@@ -798,6 +798,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()) {
@@ -1538,8 +1548,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);
@@ -1554,6 +1562,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;
             }
         }
@@ -1673,8 +1682,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;
     }
 
@@ -1687,6 +1696,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 2bff920220e1a85a3cf44750bcfe20194de15dce..91e7fb870b89fc408a4f9e83d25bbc6ecc949981 100644 (file)
--- a/src/common/syncjournalfilerecord.h
+++ b/src/common/syncjournalfilerecord.h
@@ -102,6 +102,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 0168c90532be99b38b782e177323bf1ac9df7e7a..f5695fec856ba84fcd042c9a4085e2a8349001ef 100644 (file)
--- a/src/libsync/abstractnetworkjob.cpp
+++ b/src/libsync/abstractnetworkjob.cpp
@@ -275,6 +275,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 bf35a07927a2ab08560478bf239ba27b0e76a217..a05711916fc0f2b015fab95248ebca8420af1548 100644 (file)
--- 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 5584c8652eeee86bdd0762d2134baa4952b6b5e4..14f3c03f432df21a365afecc8398f23360c91ad1 100644 (file)
--- a/src/libsync/owncloudpropagator.cpp
+++ b/src/libsync/owncloudpropagator.cpp
@@ -139,6 +139,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 32f5840ac8d9452f41009f9a15d6f66c1887dc57..69ba22bd74864da3a8b068243a1c1e1f803a963c 100644 (file)
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -590,9 +590,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.
@@ -671,7 +674,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 931bbf00fa1a3188ffecda8dcde46d6729288bd7..848d943280e00a9e16941a9e0e78e0c58e6ba39d 100644 (file)
--- a/src/libsync/propagateremotedelete.cpp
+++ b/src/libsync/propagateremotedelete.cpp
@@ -130,6 +130,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,
@@ -138,8 +140,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 d19bcf4818315c9b016a58e651435e2e43ba9b6b..d2fd737001b22d9a27edd7087ea6332c2f3e9b08 100644 (file)
--- a/src/libsync/propagateremotemkdir.cpp
+++ b/src/libsync/propagateremotemkdir.cpp
@@ -193,6 +193,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.
@@ -212,7 +214,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 39fb1a0328aff9171e758f206daccd2dbf4ca7cc..45fa23e15c13244872ad349a5989e15f7fc9a2e3 100644 (file)
--- 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 19067957ec0aceda83f8bf8cb65e86f2a26e7686..60bcef43c9a067c2e601b1941583819d6e20ff59 100644 (file)
--- a/src/libsync/propagateupload.cpp
+++ b/src/libsync/propagateupload.cpp
@@ -113,6 +113,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 1ab60b3235c38e8d175bf68030e25ac80e5d3336..7732af3a6b230dd3c224d144999356bf2127b8b5 100644 (file)
--- a/src/libsync/propagateuploadng.cpp
+++ b/src/libsync/propagateuploadng.cpp
@@ -185,6 +185,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;
@@ -203,6 +204,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 {
@@ -262,6 +264,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());
@@ -368,6 +371,7 @@ void PropagateUploadFileNG::slotPutFinished()
 
     if (err != QNetworkReply::NoError) {
         _item->_httpErrorCode = job->reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+        _item->_requestId = job->requestId();
         commonErrorHandling(job);
         return;
     }
@@ -448,6 +452,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);
@@ -478,7 +484,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 2214d459c6d950f2615960375136ec2cb194e8f6..e28a7be625ef0466297fc44fab5dc50f93ca4dfc 100644 (file)
--- a/src/libsync/propagateuploadv1.cpp
+++ b/src/libsync/propagateuploadv1.cpp
@@ -201,6 +201,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);
@@ -306,8 +308,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 adcd4ae4d821df6bd80bd9b632a0e26d61dd5555..f98ea8c0ab43a96a0ac0866eacde5fd14cd0b18e 100644 (file)
--- a/src/libsync/syncfileitem.h
+++ b/src/libsync/syncfileitem.h
@@ -221,6 +221,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 = 1; // 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 f571affa4aa5ba6d80d5229073ce39d0d6ffe496..cad9f35f2fa6cfa9403a71195ef9de4090054a94 100644 (file)
--- 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);