From 986fc27e451b21cdd118e74da9e9ff22e275ef75 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 3 Jan 2018 12:19:25 +0900 Subject: BaseJob, MediaThumbnailJob: Support request and response headers Enable specifying headers in the request and checking/using headers in the response. --- jobs/basejob.cpp | 110 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 89 insertions(+), 21 deletions(-) (limited to 'jobs/basejob.cpp') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 9df3e430..980814c4 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -60,10 +60,16 @@ class BaseJob::Private // Contents for the network request HttpVerb verb; QString apiEndpoint; + QHash requestHeaders; QUrlQuery requestQuery; Data requestData; bool needsToken; + // There's no use of QMimeType here because we don't want to match + // content types against the known MIME type hierarchy; and at the same + // type QMimeType is of little help with MIME type globs (`text/*` etc.) + QByteArrayList expectedContentTypes; + QScopedPointer reply; Status status = Pending; @@ -117,6 +123,22 @@ void BaseJob::setApiEndpoint(const QString& apiEndpoint) d->apiEndpoint = apiEndpoint; } +const BaseJob::headers_t&BaseJob::requestHeaders() const +{ + return d->requestHeaders; +} + +void BaseJob::setRequestHeader(const headers_t::key_type& headerName, + const headers_t::mapped_type& headerValue) +{ + d->requestHeaders[headerName] = headerValue; +} + +void BaseJob::setRequestHeaders(const BaseJob::headers_t& headers) +{ + d->requestHeaders = headers; +} + const QUrlQuery& BaseJob::query() const { return d->requestQuery; @@ -137,6 +159,21 @@ void BaseJob::setRequestData(const BaseJob::Data& data) d->requestData = data; } +const QByteArrayList& BaseJob::expectedContentTypes() const +{ + return d->expectedContentTypes; +} + +void BaseJob::addExpectedContentType(const QByteArray& contentType) +{ + d->expectedContentTypes << contentType; +} + +void BaseJob::setExpectedContentTypes(const QByteArrayList& contentTypes) +{ + d->expectedContentTypes = contentTypes; +} + void BaseJob::Private::sendRequest() { QUrl url = connection->baseUrl(); @@ -148,13 +185,16 @@ void BaseJob::Private::sendRequest() url.setQuery(requestQuery); QNetworkRequest req {url}; - req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json"); + if (!requestHeaders.contains("Content-Type")) + req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json"); req.setRawHeader(QByteArray("Authorization"), QByteArray("Bearer ") + connection->accessToken()); #if (QT_VERSION >= QT_VERSION_CHECK(5, 6, 0)) req.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true); req.setMaximumRedirectsAllowed(10); #endif + for (auto it = requestHeaders.cbegin(); it != requestHeaders.cend(); ++it) + req.setRawHeader(it.key(), it.value()); switch( verb ) { case HttpVerb::Get: @@ -206,11 +246,34 @@ void BaseJob::gotReply() { setStatus(checkReply(d->reply.data())); if (status().good()) - setStatus(parseReply(d->reply->readAll())); + setStatus(parseReply(d->reply.data())); finishJob(); } +bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) +{ + if (patterns.isEmpty()) + return true; + + for (const auto& pattern: patterns) + { + if (pattern.startsWith('*') || type == pattern) // Fast lane + return true; + + auto patternParts = pattern.split('/'); + Q_ASSERT_X(patternParts.size() <= 2, __FUNCTION__, + "BaseJob: Expected content type should have up to two" + " /-separated parts; violating pattern: " + pattern); + + if (type.split('/').front() == patternParts.front() && + patternParts.back() == "*") + return true; // Exact match already went on fast lane + } + + return false; +} + BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const { qCDebug(d->logCat) << this << "returned from" << reply->url().toDisplayString(); @@ -218,30 +281,35 @@ BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const qCDebug(d->logCat) << this << "returned" << reply->error(); switch( reply->error() ) { - case QNetworkReply::NoError: - return NoError; - - case QNetworkReply::AuthenticationRequiredError: - case QNetworkReply::ContentAccessDenied: - case QNetworkReply::ContentOperationNotPermittedError: - return { ContentAccessError, reply->errorString() }; - - case QNetworkReply::ProtocolInvalidOperationError: - case QNetworkReply::UnknownContentError: - return { IncorrectRequestError, reply->errorString() }; - - case QNetworkReply::ContentNotFoundError: - return { NotFoundError, reply->errorString() }; - - default: - return { NetworkError, reply->errorString() }; + case QNetworkReply::NoError: + if (checkContentType(reply->rawHeader("Content-Type"), + d->expectedContentTypes)) + return NoError; + else + return { IncorrectResponseError, + "Incorrect content type of the response" }; + + case QNetworkReply::AuthenticationRequiredError: + case QNetworkReply::ContentAccessDenied: + case QNetworkReply::ContentOperationNotPermittedError: + return { ContentAccessError, reply->errorString() }; + + case QNetworkReply::ProtocolInvalidOperationError: + case QNetworkReply::UnknownContentError: + return { IncorrectRequestError, reply->errorString() }; + + case QNetworkReply::ContentNotFoundError: + return { NotFoundError, reply->errorString() }; + + default: + return { NetworkError, reply->errorString() }; } } -BaseJob::Status BaseJob::parseReply(QByteArray data) +BaseJob::Status BaseJob::parseReply(QNetworkReply* reply) { QJsonParseError error; - QJsonDocument json = QJsonDocument::fromJson(data, &error); + QJsonDocument json = QJsonDocument::fromJson(reply->readAll(), &error); if( error.error == QJsonParseError::NoError ) return parseJson(json); else -- cgit v1.2.3 From b142117d78a2a4ce21e818c62cb7a10cff80af0d Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 5 Jan 2018 19:21:54 +0900 Subject: BaseJob: Provide a simplified constructor --- jobs/basejob.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'jobs/basejob.cpp') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 980814c4..74031909 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -96,6 +96,10 @@ QDebug QMatrixClient::operator<<(QDebug dbg, const BaseJob::Status& s) << QString(s.message).replace(filter, "\\1 HIDDEN"); } +BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, bool needsToken) + : BaseJob(verb, name, endpoint, Query { }, Data { }, needsToken) +{ } + BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, const Query& query, const Data& data, bool needsToken) : d(new Private(verb, endpoint, query, data, needsToken)) -- cgit v1.2.3 From 01806d00977578681a401ad294a957ecec0a3d53 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 5 Jan 2018 19:46:41 +0900 Subject: jobs: expect application/json by default; set query in constructor body; properly convert numbers to strings in query The query should be set in constructor body because there's no reason to pass non-required parameters into the query. As for numbers to strings conversion - there was an attempt to use QJsonValue(a).toString() for that. That doesn't work; QJsonValue does not turn numbers to strings. --- jobs/basejob.cpp | 1 + 1 file changed, 1 insertion(+) (limited to 'jobs/basejob.cpp') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 74031909..dddff800 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -105,6 +105,7 @@ BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, : d(new Private(verb, endpoint, query, data, needsToken)) { setObjectName(name); + setExpectedContentTypes({ "application/json" }); d->timer.setSingleShot(true); connect (&d->timer, &QTimer::timeout, this, &BaseJob::timeout); d->retryTimer.setSingleShot(true); -- cgit v1.2.3 From 71b9445d7f696cdfabaf05ec7d26a52891dea873 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 12 Jan 2018 22:01:26 +0900 Subject: BaseJob::Data -> RequestData; support QIODevice* input/output --- jobs/basejob.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'jobs/basejob.cpp') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index f508026f..0a81ad5c 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -46,7 +46,7 @@ class BaseJob::Private public: // Using an idiom from clang-tidy: // http://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html - Private(HttpVerb v, QString endpoint, QUrlQuery q, Data data, bool nt) + Private(HttpVerb v, QString endpoint, QUrlQuery q, Data&& data, bool nt) : verb(v), apiEndpoint(std::move(endpoint)) , requestQuery(std::move(q)), requestData(std::move(data)) , needsToken(nt) @@ -101,8 +101,8 @@ BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, bo { } BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, - const Query& query, const Data& data, bool needsToken) - : d(new Private(verb, endpoint, query, data, needsToken)) + const Query& query, Data&& data, bool needsToken) + : d(new Private(verb, endpoint, query, std::move(data), needsToken)) { setObjectName(name); setExpectedContentTypes({ "application/json" }); @@ -159,9 +159,9 @@ const BaseJob::Data& BaseJob::requestData() const return d->requestData; } -void BaseJob::setRequestData(const BaseJob::Data& data) +void BaseJob::setRequestData(Data&& data) { - d->requestData = data; + std::swap(d->requestData, data); } const QByteArrayList& BaseJob::expectedContentTypes() const @@ -206,10 +206,10 @@ void BaseJob::Private::sendRequest() reply.reset( connection->nam()->get(req) ); break; case HttpVerb::Post: - reply.reset( connection->nam()->post(req, requestData.serialize()) ); + reply.reset( connection->nam()->post(req, requestData.source()) ); break; case HttpVerb::Put: - reply.reset( connection->nam()->put(req, requestData.serialize()) ); + reply.reset( connection->nam()->put(req, requestData.source()) ); break; case HttpVerb::Delete: reply.reset( connection->nam()->deleteResource(req) ); @@ -447,4 +447,3 @@ void BaseJob::setLoggingCategory(LoggingCategory lcf) { d->logCat = lcf; } - -- cgit v1.2.3 From b3ad6aa8fe62f461c99ae7728482bd9958e38909 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 12 Jan 2018 22:04:28 +0900 Subject: BaseJob: afterStart(), beforeAbandon(), up/downloadProgress() To support the upcoming DownloadFileJob --- jobs/basejob.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) (limited to 'jobs/basejob.cpp') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 0a81ad5c..720ac560 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -217,15 +217,21 @@ void BaseJob::Private::sendRequest() } } -void BaseJob::beforeStart(const ConnectionData* connData) -{ -} +void BaseJob::beforeStart(const ConnectionData*) +{ } + +void BaseJob::afterStart(const ConnectionData*, QNetworkReply*) +{ } + +void BaseJob::beforeAbandon(QNetworkReply*) +{ } void BaseJob::start(const ConnectionData* connData) { d->connection = connData; beforeStart(connData); sendRequest(); + afterStart(connData, d->reply.data()); } void BaseJob::sendRequest() @@ -239,6 +245,10 @@ void BaseJob::sendRequest() connect( d->reply.data(), &QNetworkReply::finished, this, &BaseJob::gotReply ); if (d->reply->isRunning()) { + connect( d->reply.data(), &QNetworkReply::uploadProgress, + this, &BaseJob::uploadProgress); + connect( d->reply.data(), &QNetworkReply::downloadProgress, + this, &BaseJob::downloadProgress); d->timer.start(getCurrentTimeout()); qCDebug(d->logCat) << this << "request has been sent"; emit started(); @@ -431,6 +441,7 @@ void BaseJob::setStatus(int code, QString message) void BaseJob::abandon() { + beforeAbandon(d->reply.data()); this->disconnect(); if (d->reply) d->reply->disconnect(this); -- cgit v1.2.3 From 84e183a70b831d1e18c373099988420f5050254b Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 25 Jan 2018 11:11:50 +0900 Subject: BaseJob::checkReply: log job returned status more explicitly --- jobs/basejob.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'jobs/basejob.cpp') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 720ac560..2d0d8b1b 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -291,9 +291,10 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) BaseJob::Status BaseJob::checkReply(QNetworkReply* reply) const { - qCDebug(d->logCat) << this << "returned from" << reply->url().toDisplayString(); - if (reply->error() != QNetworkReply::NoError) - qCDebug(d->logCat) << this << "returned" << reply->error(); + qCDebug(d->logCat) << this << "returned" + << (reply->error() == QNetworkReply::NoError ? + "Success" : reply->errorString()) + << "from" << reply->url().toDisplayString(); switch( reply->error() ) { case QNetworkReply::NoError: -- cgit v1.2.3