Port RequestEtagJob -> to PropFindJob

Fixes: #10179

11 files changed, 92 insertions, 109 deletions

diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp
index 66b7c65cf..3d4fc15ef 100644
--- a/src/gui/folder.cpp
+++ b/src/gui/folder.cpp
@@ -432,24 +432,24 @@ void Folder::slotRunEtagJob()
     _requestEtagJob = new RequestEtagJob(account, webDavUrl(), remotePath(), this);
     _requestEtagJob->setTimeout(60s);
     // check if the etag is different when retrieved
-    QObject::connect(_requestEtagJob.data(), &RequestEtagJob::etagRetreived, this, &Folder::etagRetreived);
-    QObject::connect(_requestEtagJob.data(), &RequestEtagJob::finishedWithResult, this, [=](const HttpResult<QByteArray> &) { _timeSinceLastEtagCheckDone.start(); });
-    FolderMan::instance()->slotScheduleETagJob(_requestEtagJob);
-    // The _requestEtagJob is auto deleting itself on finish. Our guard pointer _requestEtagJob will then be null.
-}
-
-void Folder::etagRetreived(const QByteArray &etag, const QDateTime &tp)
-{
-    // re-enable sync if it was disabled because network was down
-    FolderMan::instance()->setSyncEnabled(true);
+    QObject::connect(_requestEtagJob.data(), &RequestEtagJob::finishedSignal, this, [this] {
+        if (_requestEtagJob->httpStatusCode() == 207) {
+            // re-enable sync if it was disabled because network was down
+            FolderMan::instance()->setSyncEnabled(true);
+
+            if (_lastEtag != _requestEtagJob->etag()) {
+                qCInfo(lcFolder) << "Compare etag with previous etag: last:" << _lastEtag << ", received:" << _requestEtagJob->etag() << "-> CHANGED";
+                _lastEtag = _requestEtagJob->etag();
+                slotScheduleThisFolder();
+            }
 
-    if (_lastEtag != etag) {
-        qCInfo(lcFolder) << "Compare etag with previous etag: last:" << _lastEtag << ", received:" << etag << "-> CHANGED";
-        _lastEtag = etag;
-        slotScheduleThisFolder();
-    }
+            _accountState->tagLastSuccessfullETagRequest(_requestEtagJob->responseQTimeStamp());
+        }
+        _timeSinceLastEtagCheckDone.start();
+    });
 
-    _accountState->tagLastSuccessfullETagRequest(tp);
+    FolderMan::instance()->slotScheduleETagJob(_requestEtagJob);
+    // The _requestEtagJob is auto deleting itself on finish. Our guard pointer _requestEtagJob will then be null.
 }
 
 void Folder::etagRetrievedFromSyncEngine(const QByteArray &etag, const QDateTime &time)
diff --git a/src/gui/folder.h b/src/gui/folder.h
index e185ac6f3..848e26a25 100644
--- a/src/gui/folder.h
+++ b/src/gui/folder.h
@@ -447,7 +447,6 @@ private slots:
 
     void slotItemCompleted(const SyncFileItemPtr &);
 
-    void etagRetreived(const QByteArray &, const QDateTime &tp);
     void etagRetrievedFromSyncEngine(const QByteArray &, const QDateTime &time);
 
     void slotEmitFinishedDelayed();
diff --git a/src/libsync/abstractnetworkjob.cpp b/src/libsync/abstractnetworkjob.cpp
index 6a34354e7..b5f6ebc1f 100644
--- a/src/libsync/abstractnetworkjob.cpp
+++ b/src/libsync/abstractnetworkjob.cpp
@@ -226,12 +226,17 @@ void AbstractNetworkJob::slotFinished()
     deleteLater();
 }
 
-QByteArray AbstractNetworkJob::responseTimestamp()
+QByteArray AbstractNetworkJob::responseTimestamp() const
 {
     OC_ASSERT(!_responseTimestamp.isEmpty() || _aborted || (reply() && reply()->error() == QNetworkReply::RemoteHostClosedError));
     return _responseTimestamp;
 }
 
+QDateTime OCC::AbstractNetworkJob::responseQTimeStamp() const
+{
+    return QDateTime::fromString(QString::fromUtf8(responseTimestamp()), Qt::RFC2822Date);
+}
+
 QByteArray AbstractNetworkJob::requestId()
 {
     return  _reply ? _reply->request().rawHeader("X-Request-ID") : QByteArray();
@@ -384,6 +389,11 @@ QNetworkRequest::Priority AbstractNetworkJob::priority() const
     return _priority;
 }
 
+int AbstractNetworkJob::httpStatusCode() const
+{
+    return reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
+}
+
 } // namespace OCC
 
 QDebug operator<<(QDebug debug, const OCC::AbstractNetworkJob *job)
diff --git a/src/libsync/abstractnetworkjob.h b/src/libsync/abstractnetworkjob.h
index 54724b01f..0ef8e5a17 100644
--- a/src/libsync/abstractnetworkjob.h
+++ b/src/libsync/abstractnetworkjob.h
@@ -73,7 +73,11 @@ public:
     void setIgnoreCredentialFailure(bool ignore);
     bool ignoreCredentialFailure() const { return _ignoreCredentialFailure; }
 
-    QByteArray responseTimestamp();
+    QByteArray responseTimestamp() const;
+    QDateTime responseQTimeStamp() const;
+
+    int httpStatusCode() const;
+
     /* Content of the X-Request-ID header. (Only set after the request is sent) */
     QByteArray requestId();
 
diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index b89be8453..3dceb7365 100644
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -604,24 +604,26 @@ void ProcessDirectoryJob::processFileAnalyzeRemoteInfo(
             // we need to make a request to the server to know that the original file is deleted on the server
             _pendingAsyncJobs++;
             auto job = new RequestEtagJob(_discoveryData->_account, _discoveryData->_baseUrl, _discoveryData->_remoteFolder + originalPath, this);
-            connect(job, &RequestEtagJob::finishedWithResult, this, [=](const HttpResult<QByteArray> &etag) mutable {
+            connect(job, &RequestEtagJob::finishedSignal, this, [=]() mutable {
                 _pendingAsyncJobs--;
                 QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs);
-                if (etag || etag.error().code != 404 ||
+                if (job->httpStatusCode() == 207 ||
                     // Somehow another item claimed this original path, consider as if it existed
                     _discoveryData->isRenamed(originalPath)) {
                     // If the file exist or if there is another error, consider it is a new file.
                     postProcessServerNew();
                     return;
-                }
-
-                // The file do not exist, it is a rename
+                } else if (OC_ENSURE(job->httpStatusCode() == 404)) {
+                    // The file do not exist, it is a rename
 
-                // In case the deleted item was discovered in parallel
-                _discoveryData->findAndCancelDeletedJob(originalPath);
+                    // In case the deleted item was discovered in parallel
+                    _discoveryData->findAndCancelDeletedJob(originalPath);
 
-                postProcessRename(path);
-                processFileFinalize(item, path, item->isDirectory(), item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist, _queryServer);
+                    postProcessRename(path);
+                    processFileFinalize(item, path, item->isDirectory(), item->_instruction == CSYNC_INSTRUCTION_RENAME ? NormalQuery : ParentDontExist, _queryServer);
+                    return;
+                }
+                abort();
             });
             job->start();
             done = true; // Ideally, if the origin still exist on the server, we should continue searching...  but that'd be difficult
@@ -1002,9 +1004,9 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
             serverOriginalPath = chopVirtualFileSuffix(serverOriginalPath);
         }
         auto job = new RequestEtagJob(_discoveryData->_account, _discoveryData->_baseUrl, serverOriginalPath, this);
-        connect(job, &RequestEtagJob::finishedWithResult, this,
-            [recurseQueryServer, path = path, postProcessLocalNew, processRename, base, item, originalPath, this](const HttpResult<QByteArray> &etag) {
-                if (!etag || (etag.get() != base._etag && !item->isDirectory()) || _discoveryData->isRenamed(originalPath)) {
+        connect(job, &RequestEtagJob::finishedSignal, this,
+            [job, recurseQueryServer, path = path, postProcessLocalNew, processRename, base, item, originalPath, this] {
+                if (job->httpStatusCode() == 404 || (job->etag() != base._etag && !item->isDirectory()) || _discoveryData->isRenamed(originalPath)) {
                     qCInfo(lcDisco) << "Can't rename because the etag has changed or the directory is gone" << originalPath;
                     // Can't be a rename, leave it as a new.
                     postProcessLocalNew(path);
@@ -1012,7 +1014,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
                 } else {
                     // In case the deleted item was discovered in parallel
                     _discoveryData->findAndCancelDeletedJob(originalPath);
-                    processFileFinalize(item, processRename(path), item->isDirectory(), NormalQuery, etag.get() == base._etag ? ParentNotChanged : NormalQuery);
+                    processFileFinalize(item, processRename(path), item->isDirectory(), NormalQuery, job->etag() == base._etag ? ParentNotChanged : NormalQuery);
                 }
                 _pendingAsyncJobs--;
                 QTimer::singleShot(0, _discoveryData, &DiscoveryPhase::scheduleMoreJobs);
diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
index 67b341a43..9e2c479b5 100644
--- a/src/libsync/discoveryphase.cpp
+++ b/src/libsync/discoveryphase.cpp
@@ -454,7 +454,7 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(const QString &fi
     //This works in concerto with the RequestEtagJob and the Folder object to check if the remote folder changed.
     if (_firstEtag.isEmpty()) {
         if (auto it = Utility::optionalFind(map, QStringLiteral("getetag"))) {
-            _firstEtag = parseEtag(it->value().toUtf8()); // for directory itself
+            _firstEtag = parseEtag(it->value()).toUtf8(); // for directory itself
         }
     }
 }
diff --git a/src/libsync/networkjobs.cpp b/src/libsync/networkjobs.cpp
index bff9213e7..e58625265 100644
--- a/src/libsync/networkjobs.cpp
+++ b/src/libsync/networkjobs.cpp
@@ -46,83 +46,53 @@ Q_LOGGING_CATEGORY(lcAvatarJob, "sync.networkjob.avatar", QtInfoMsg)
 Q_LOGGING_CATEGORY(lcMkColJob, "sync.networkjob.mkcol", QtInfoMsg)
 Q_LOGGING_CATEGORY(lcDetermineAuthTypeJob, "sync.networkjob.determineauthtype", QtInfoMsg)
 
-QByteArray parseEtag(const QByteArray &header)
+QString parseEtag(QStringView header)
 {
-    if (header.isEmpty())
-        return QByteArray();
-    QByteArray arr = header;
+    if (header.isEmpty()) {
+        return {};
+    }
 
     // Weak E-Tags can appear when gzip compression is on, see #3946
-    if (arr.startsWith("W/"))
-        arr = arr.mid(2);
+    if (header.startsWith(QLatin1String("W/"))) {
+        header = header.mid(2);
+    }
 
     // https://github.com/owncloud/client/issues/1195
-    arr.replace("-gzip", "");
+    const QLatin1String gzipSuffix("-gzip");
+    if (header.endsWith(gzipSuffix)) {
+        header.chop(gzipSuffix.size());
+    }
 
-    if (arr.length() >= 2 && arr.startsWith('"') && arr.endsWith('"')) {
-        arr = arr.mid(1, arr.length() - 2);
+    if (header.startsWith(QLatin1Char('"'))) {
+        header = header.mid(1);
+        if (OC_ENSURE(header.endsWith(QLatin1Char('"')))) {
+            header.chop(1);
+        }
     }
-    return arr;
+    return header.toString();
 }
 
 RequestEtagJob::RequestEtagJob(AccountPtr account, const QUrl &rootUrl, const QString &path, QObject *parent)
-    : AbstractNetworkJob(account, rootUrl, path, parent)
+    : PropfindJob(account, rootUrl, path, PropfindJob::Depth::Zero, parent)
 {
+    setProperties({ QByteArrayLiteral("getetag") });
+    connect(this, &PropfindJob::directoryListingIterated, this, [this](const QString &, const QMap<QString, QString> &value) {
+        if (reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt() != 207) {
+            Q_EMIT finishedWithError(reply());
+            return;
+        }
+        _etag = parseEtag(value.value(QStringLiteral("getetag"))).toUtf8();
+        // the server returned a 207 but no etag, something is wrong
+        if (!OC_ENSURE(!_etag.isEmpty())) {
+            abort();
+        }
+    });
 }
 
-void RequestEtagJob::start()
-{
-    QNetworkRequest req;
-    req.setRawHeader(QByteArrayLiteral("Depth"), QByteArrayLiteral("0"));
-    req.setRawHeader(QByteArrayLiteral("Prefer"), QByteArrayLiteral("return=minimal"));
-
-    const QByteArray xml = QByteArrayLiteral("<?xml version=\"1.0\" ?>\n"
-                                             "<d:propfind xmlns:d=\"DAV:\">\n"
-                                             "  <d:prop>\n"
-                                             "    <d:getetag/>\n"
-                                             "  </d:prop>\n"
-                                             "</d:propfind>\n");
-    QBuffer *buf = new QBuffer(this);
-    buf->setData(xml);
-    buf->open(QIODevice::ReadOnly);
-    // assumes ownership
-    sendRequest("PROPFIND", req, buf);
-    AbstractNetworkJob::start();
-}
-
-void RequestEtagJob::finished()
+const QByteArray &RequestEtagJob::etag() const
 {
-    qCInfo(lcEtagJob) << "Request Etag of" << reply()->request().url() << "FINISHED WITH STATUS"
-                      <<  replyStatusString();
-
-    auto httpCode = reply()->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
-    if (httpCode == 207) {
-        // Parse DAV response
-        QXmlStreamReader reader(reply());
-        reader.addExtraNamespaceDeclaration(QXmlStreamNamespaceDeclaration(QStringLiteral("d"), QStringLiteral("DAV:")));
-        QByteArray etag;
-        while (!reader.atEnd()) {
-            reader.readNextStartElement();
-            if (reader.namespaceUri() == QLatin1String("DAV:")) {
-                QString name = reader.name().toString();
-                if (name == QLatin1String("getetag")) {
-                    auto etagText = reader.readElementText();
-                    auto parsedTag = parseEtag(etagText.toUtf8());
-                    if (!parsedTag.isEmpty()) {
-                        etag += parsedTag;
-                    } else {
-                        etag += etagText.toUtf8();
-                    }
-                }
-            }
-        }
-        emit etagRetreived(etag, QDateTime::fromString(QString::fromUtf8(_responseTimestamp), Qt::RFC2822Date));
-        emit finishedWithResult(etag);
-    } else {
-        emit finishedWithResult(HttpError{ httpCode, errorString() });
-    }
+    return _etag;
 }
-
 /*********************************************************************************************/
 
 MkColJob::MkColJob(AccountPtr account, const QUrl &url, const QString &path,
diff --git a/src/libsync/networkjobs.h b/src/libsync/networkjobs.h
index 5cc007ddf..776ea6085 100644
--- a/src/libsync/networkjobs.h
+++ b/src/libsync/networkjobs.h
@@ -27,7 +27,7 @@ class QUrl;
 namespace OCC {
 
 /** Strips quotes and gzip annotations */
-OWNCLOUDSYNC_EXPORT QByteArray parseEtag(const QByteArray &header);
+OWNCLOUDSYNC_EXPORT QString parseEtag(QStringView header);
 
 struct HttpError
 {
@@ -174,19 +174,16 @@ private:
 /**
  * @brief The RequestEtagJob class
  */
-class OWNCLOUDSYNC_EXPORT RequestEtagJob : public AbstractNetworkJob
+class OWNCLOUDSYNC_EXPORT RequestEtagJob : public PropfindJob
 {
     Q_OBJECT
 public:
     explicit RequestEtagJob(AccountPtr account, const QUrl &rootUrl, const QString &path, QObject *parent = nullptr);
-    void start() override;
 
-signals:
-    void etagRetreived(const QByteArray &etag, const QDateTime &time);
-    void finishedWithResult(const HttpResult<QByteArray> &etag);
+    const QByteArray &etag() const;
 
-private slots:
-    void finished() override;
+private:
+    QByteArray _etag;
 };
 
 /**
diff --git a/src/libsync/owncloudpropagator_p.h b/src/libsync/owncloudpropagator_p.h
index 3c2e2444b..12150189a 100644
--- a/src/libsync/owncloudpropagator_p.h
+++ b/src/libsync/owncloudpropagator_p.h
@@ -25,11 +25,11 @@ namespace OCC {
 
 inline QByteArray getEtagFromReply(QNetworkReply *reply)
 {
-    QByteArray ret = parseEtag(reply->rawHeader("OC-ETag"));
-    if (ret.isEmpty()) {
-        ret = parseEtag(reply->rawHeader("ETag"));
+    QByteArray rawEtag = reply->rawHeader("OC-ETag");
+    if (rawEtag.isEmpty()) {
+        rawEtag = reply->rawHeader("ETag");
     }
-    return ret;
+    return parseEtag(QString::fromUtf8(rawEtag)).toUtf8();
 }
 
 /**
diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp
index 1734be186..e96f6c1c3 100644
--- a/src/libsync/propagatedownload.cpp
+++ b/src/libsync/propagatedownload.cpp
@@ -663,7 +663,8 @@ void PropagateDownloadFile::slotGetFinished()
     if (!job->etag().isEmpty()) {
         // The etag will be empty if we used a direct download URL.
         // (If it was really empty by the server, the GETFileJob will have errored
-        _item->_etag = parseEtag(job->etag());
+        Q_ASSERT(job->etag() == parseEtag(QString::fromUtf8(job->etag())).toUtf8());
+        _item->_etag = job->etag();
     }
     if (job->lastModified()) {
         // It is possible that the file was modified on the server since we did the discovery phase
diff --git a/test/testowncloudpropagator.cpp b/test/testowncloudpropagator.cpp
index e1a580c45..85aa8cf75 100644
--- a/test/testowncloudpropagator.cpp
+++ b/test/testowncloudpropagator.cpp
@@ -73,7 +73,7 @@ private slots:
         tests.append(Test("W/\"foo\"", "foo"));
 
         for (const auto &test : qAsConst(tests)) {
-            QCOMPARE(parseEtag(test.first), QByteArray(test.second));
+            QCOMPARE(parseEtag(QString::fromUtf8(test.first)).toUtf8(), QByteArray(test.second));
         }
     }
 };