From aacc4bcb4a487871daae6717f77605aaba444341 Mon Sep 17 00:00:00 2001 From: Marc Deop Date: Sat, 2 Mar 2019 12:26:57 +0100 Subject: style: apply .clang-format to all .cpp and .h files --- lib/jobs/basejob.cpp | 436 ++++++++++++++++++++++++--------------------------- 1 file changed, 203 insertions(+), 233 deletions(-) (limited to 'lib/jobs/basejob.cpp') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 8c3381ae..a023d4f7 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -13,7 +13,7 @@ * * You should have received a copy of the GNU Lesser General Public * License along with this library; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */ #include "basejob.h" @@ -21,19 +21,18 @@ #include "connectiondata.h" #include "util.h" +#include +#include +#include #include -#include #include -#include -#include -#include +#include #include using namespace QMatrixClient; -struct NetworkReplyDeleter : public QScopedPointerDeleteLater -{ +struct NetworkReplyDeleter : public QScopedPointerDeleteLater { static inline void cleanup(QNetworkReply* reply) { if (reply && reply->isRunning()) @@ -45,51 +44,58 @@ struct NetworkReplyDeleter : public QScopedPointerDeleteLater 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, const QUrlQuery& q, - Data&& data, bool nt) - : verb(v), apiEndpoint(std::move(endpoint)), requestQuery(q) - , requestData(std::move(data)), needsToken(nt) - { } - - void sendRequest(bool inBackground); - const JobTimeoutConfig& getCurrentTimeoutConfig() const; - - const ConnectionData* connection = nullptr; - - // 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; - QByteArray rawResponse; - QUrl errorUrl; //< May contain a URL to help with some errors - - QTimer timer; - QTimer retryTimer; - - QVector errorStrategy = - { { 90, 5 }, { 90, 10 }, { 120, 30 } }; - int maxRetries = errorStrategy.size(); - int retriesTaken = 0; - - LoggingCategory logCat = JOBS; + // Using an idiom from clang-tidy: + // http://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html + Private(HttpVerb v, QString endpoint, const QUrlQuery& q, Data&& data, + bool nt) + : verb(v), + apiEndpoint(std::move(endpoint)), + requestQuery(q), + requestData(std::move(data)), + needsToken(nt) + { + } + + void sendRequest(bool inBackground); + const JobTimeoutConfig& getCurrentTimeoutConfig() const; + + const ConnectionData* connection = nullptr; + + // 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; + QByteArray rawResponse; + QUrl errorUrl; //< May contain a URL to help with some errors + + QTimer timer; + QTimer retryTimer; + + QVector errorStrategy = { { 90, 5 }, + { 90, 10 }, + { 120, 30 } }; + int maxRetries = errorStrategy.size(); + int retriesTaken = 0; + + LoggingCategory logCat = JOBS; }; -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, + bool needsToken) + : BaseJob(verb, name, endpoint, Query {}, Data {}, needsToken) +{ +} BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, const Query& query, Data&& data, bool needsToken) @@ -98,7 +104,7 @@ BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, setObjectName(name); setExpectedContentTypes({ "application/json" }); d->timer.setSingleShot(true); - connect (&d->timer, &QTimer::timeout, this, &BaseJob::timeout); + connect(&d->timer, &QTimer::timeout, this, &BaseJob::timeout); } BaseJob::~BaseJob() @@ -114,21 +120,20 @@ QUrl BaseJob::requestUrl() const bool BaseJob::isBackground() const { - return d->reply && d->reply->request().attribute( - QNetworkRequest::BackgroundRequestAttribute).toBool(); + return d->reply + && d->reply->request() + .attribute(QNetworkRequest::BackgroundRequestAttribute) + .toBool(); } -const QString& BaseJob::apiEndpoint() const -{ - return d->apiEndpoint; -} +const QString& BaseJob::apiEndpoint() const { return d->apiEndpoint; } void BaseJob::setApiEndpoint(const QString& apiEndpoint) { d->apiEndpoint = apiEndpoint; } -const BaseJob::headers_t&BaseJob::requestHeaders() const +const BaseJob::headers_t& BaseJob::requestHeaders() const { return d->requestHeaders; } @@ -144,25 +149,16 @@ void BaseJob::setRequestHeaders(const BaseJob::headers_t& headers) d->requestHeaders = headers; } -const QUrlQuery& BaseJob::query() const -{ - return d->requestQuery; -} +const QUrlQuery& BaseJob::query() const { return d->requestQuery; } void BaseJob::setRequestQuery(const QUrlQuery& query) { d->requestQuery = query; } -const BaseJob::Data& BaseJob::requestData() const -{ - return d->requestData; -} +const BaseJob::Data& BaseJob::requestData() const { return d->requestData; } -void BaseJob::setRequestData(Data&& data) -{ - std::swap(d->requestData, data); -} +void BaseJob::setRequestData(Data&& data) { std::swap(d->requestData, data); } const QByteArrayList& BaseJob::expectedContentTypes() const { @@ -179,22 +175,22 @@ void BaseJob::setExpectedContentTypes(const QByteArrayList& contentTypes) d->expectedContentTypes = contentTypes; } -QUrl BaseJob::makeRequestUrl(QUrl baseUrl, - const QString& path, const QUrlQuery& query) +QUrl BaseJob::makeRequestUrl(QUrl baseUrl, const QString& path, + const QUrlQuery& query) { auto pathBase = baseUrl.path(); if (!pathBase.endsWith('/') && !path.startsWith('/')) pathBase.push_back('/'); - baseUrl.setPath( pathBase + path ); + baseUrl.setPath(pathBase + path); baseUrl.setQuery(query); return baseUrl; } void BaseJob::Private::sendRequest(bool inBackground) { - QNetworkRequest req - { makeRequestUrl(connection->baseUrl(), apiEndpoint, requestQuery) }; + QNetworkRequest req { makeRequestUrl(connection->baseUrl(), apiEndpoint, + requestQuery) }; if (!requestHeaders.contains("Content-Type")) req.setHeader(QNetworkRequest::ContentTypeHeader, "application/json"); req.setRawHeader("Authorization", @@ -211,38 +207,34 @@ void BaseJob::Private::sendRequest(bool inBackground) #endif for (auto it = requestHeaders.cbegin(); it != requestHeaders.cend(); ++it) req.setRawHeader(it.key(), it.value()); - switch( verb ) - { - case HttpVerb::Get: - reply.reset( connection->nam()->get(req) ); - break; - case HttpVerb::Post: - reply.reset( connection->nam()->post(req, requestData.source()) ); - break; - case HttpVerb::Put: - reply.reset( connection->nam()->put(req, requestData.source()) ); - break; - case HttpVerb::Delete: - reply.reset( connection->nam()->deleteResource(req) ); - break; + switch (verb) { + case HttpVerb::Get: + reply.reset(connection->nam()->get(req)); + break; + case HttpVerb::Post: + reply.reset(connection->nam()->post(req, requestData.source())); + break; + case HttpVerb::Put: + reply.reset(connection->nam()->put(req, requestData.source())); + break; + case HttpVerb::Delete: + reply.reset(connection->nam()->deleteResource(req)); + break; } } -void BaseJob::beforeStart(const ConnectionData*) -{ } +void BaseJob::beforeStart(const ConnectionData*) {} -void BaseJob::afterStart(const ConnectionData*, QNetworkReply*) -{ } +void BaseJob::afterStart(const ConnectionData*, QNetworkReply*) {} -void BaseJob::beforeAbandon(QNetworkReply*) -{ } +void BaseJob::beforeAbandon(QNetworkReply*) {} void BaseJob::start(const ConnectionData* connData, bool inBackground) { d->connection = connData; d->retryTimer.setSingleShot(true); - connect (&d->retryTimer, &QTimer::timeout, - this, [this,inBackground] { sendRequest(inBackground); }); + connect(&d->retryTimer, &QTimer::timeout, this, + [this, inBackground] { sendRequest(inBackground); }); beforeStart(connData); if (status().good()) @@ -261,27 +253,23 @@ void BaseJob::sendRequest(bool inBackground) if (!d->requestQuery.isEmpty()) qCDebug(d->logCat) << " query:" << d->requestQuery.toString(); d->sendRequest(inBackground); - connect( d->reply.data(), &QNetworkReply::finished, this, &BaseJob::gotReply ); - if (d->reply->isRunning()) - { - connect( d->reply.data(), &QNetworkReply::metaDataChanged, - this, &BaseJob::checkReply); - connect( d->reply.data(), &QNetworkReply::uploadProgress, - this, &BaseJob::uploadProgress); - connect( d->reply.data(), &QNetworkReply::downloadProgress, - this, &BaseJob::downloadProgress); + connect(d->reply.data(), &QNetworkReply::finished, this, + &BaseJob::gotReply); + if (d->reply->isRunning()) { + connect(d->reply.data(), &QNetworkReply::metaDataChanged, this, + &BaseJob::checkReply); + 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(); - } - else + } else qCWarning(d->logCat) << this << "request could not start"; } -void BaseJob::checkReply() -{ - setStatus(doCheckReply(d->reply.data())); -} +void BaseJob::checkReply() { setStatus(doCheckReply(d->reply.data())); } void BaseJob::gotReply() { @@ -293,20 +281,18 @@ void BaseJob::gotReply() d->rawResponse = d->reply->readAll(); const auto jsonBody = d->reply->rawHeader("Content-Type") == "application/json"; - qCDebug(d->logCat).noquote() - << "Error body (truncated if long):" << d->rawResponse.left(500); - if (jsonBody) - { + qCDebug(d->logCat).noquote() << "Error body (truncated if long):" + << d->rawResponse.left(500); + if (jsonBody) { auto json = QJsonDocument::fromJson(d->rawResponse).object(); const auto errCode = json.value("errcode"_ls).toString(); - if (error() == TooManyRequestsError || - errCode == "M_LIMIT_EXCEEDED") - { + if (error() == TooManyRequestsError + || errCode == "M_LIMIT_EXCEEDED") { QString msg = tr("Too many requests"); auto retryInterval = json.value("retry_after_ms"_ls).toInt(-1); if (retryInterval != -1) msg += tr(", next retry advised after %1 ms") - .arg(retryInterval); + .arg(retryInterval); else // We still have to figure some reasonable interval retryInterval = getNextRetryInterval(); @@ -320,19 +306,16 @@ void BaseJob::gotReply() emit retryScheduled(d->retriesTaken, retryInterval); return; } - if (errCode == "M_CONSENT_NOT_GIVEN") - { + if (errCode == "M_CONSENT_NOT_GIVEN") { d->status.code = UserConsentRequiredError; d->errorUrl = json.value("consent_uri"_ls).toString(); - } - else if (errCode == "M_UNSUPPORTED_ROOM_VERSION" || - errCode == "M_INCOMPATIBLE_ROOM_VERSION") - { + } else if (errCode == "M_UNSUPPORTED_ROOM_VERSION" + || errCode == "M_INCOMPATIBLE_ROOM_VERSION") { d->status.code = UnsupportedRoomVersionError; if (json.contains("room_version")) d->status.message = - tr("Requested room version: %1") - .arg(json.value("room_version").toString()); + tr("Requested room version: %1") + .arg(json.value("room_version").toString()); } else if (!json.isEmpty()) // Not localisable on the client side setStatus(IncorrectRequestError, json.value("error"_ls).toString()); @@ -350,18 +333,18 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) // ignore possible appendixes of the content type const auto ctype = type.split(';').front(); - for (const auto& pattern: patterns) - { + for (const auto& pattern : patterns) { if (pattern.startsWith('*') || ctype == 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); + "BaseJob: Expected content type should have up to two" + " /-separated parts; violating pattern: " + + pattern); - if (ctype.split('/').front() == patternParts.front() && - patternParts.back() == "*") + if (ctype.split('/').front() == patternParts.front() + && patternParts.back() == "*") return true; // Exact match already went on fast lane } @@ -376,24 +359,26 @@ BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const // https://en.wikipedia.org/wiki/List_of_HTTP_status_codes const auto httpCodeHeader = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); - if (!httpCodeHeader.isValid()) - { + if (!httpCodeHeader.isValid()) { qCWarning(d->logCat) << this << "didn't get valid HTTP headers"; return { NetworkError, reply->errorString() }; } - const QString replyState = reply->isRunning() ? - QStringLiteral("(tentative)") : QStringLiteral("(final)"); + const QString replyState = reply->isRunning() + ? QStringLiteral("(tentative)") + : QStringLiteral("(final)"); const auto urlString = '|' + d->reply->url().toDisplayString(); const auto httpCode = httpCodeHeader.toInt(); const auto reason = - reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(); + reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute) + .toString(); if (httpCode / 100 == 2) // 2xx { qCDebug(d->logCat).noquote().nospace() << this << urlString; - qCDebug(d->logCat).noquote() << " " << httpCode << reason << replyState; + qCDebug(d->logCat).noquote() + << " " << httpCode << reason << replyState; if (!checkContentType(reply->rawHeader("Content-Type"), - d->expectedContentTypes)) + d->expectedContentTypes)) return { UnexpectedResponseTypeWarning, "Unexpected content type of the response" }; return NoError; @@ -402,29 +387,37 @@ BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const qCWarning(d->logCat).noquote().nospace() << this << urlString; qCWarning(d->logCat).noquote() << " " << httpCode << reason << replyState; return { [httpCode]() -> StatusCode { - if (httpCode / 10 == 41) - return httpCode == 410 ? IncorrectRequestError : NotFoundError; - switch (httpCode) - { - case 401: case 403: case 407: + if (httpCode / 10 == 41) + return httpCode == 410 ? IncorrectRequestError + : NotFoundError; + switch (httpCode) { + case 401: + case 403: + case 407: return ContentAccessError; case 404: return NotFoundError; - case 400: case 405: case 406: case 426: case 428: + case 400: + case 405: + case 406: + case 426: + case 428: case 505: case 494: // Unofficial nginx "Request header too large" case 497: // Unofficial nginx "HTTP request sent to HTTPS port" return IncorrectRequestError; case 429: return TooManyRequestsError; - case 501: case 510: + case 501: + case 510: return RequestNotImplementedError; case 511: return NetworkAuthRequiredError; default: return NetworkError; - } - }(), reply->errorString() }; + } + }(), + reply->errorString() }; } BaseJob::Status BaseJob::parseReply(QNetworkReply* reply) @@ -432,30 +425,25 @@ BaseJob::Status BaseJob::parseReply(QNetworkReply* reply) d->rawResponse = reply->readAll(); QJsonParseError error; const auto& json = QJsonDocument::fromJson(d->rawResponse, &error); - if( error.error == QJsonParseError::NoError ) + if (error.error == QJsonParseError::NoError) return parseJson(json); return { IncorrectResponseError, error.errorString() }; } -BaseJob::Status BaseJob::parseJson(const QJsonDocument&) -{ - return Success; -} +BaseJob::Status BaseJob::parseJson(const QJsonDocument&) { return Success; } void BaseJob::stop() { d->timer.stop(); - if (d->reply) - { + if (d->reply) { d->reply->disconnect(this); // Ignore whatever comes from the reply - if (d->reply->isRunning()) - { - qCWarning(d->logCat) << this << "stopped without ready network reply"; + if (d->reply->isRunning()) { + qCWarning(d->logCat) + << this << "stopped without ready network reply"; d->reply->abort(); } - } - else + } else qCWarning(d->logCat) << this << "stopped with empty network reply"; } @@ -463,16 +451,16 @@ void BaseJob::finishJob() { stop(); if ((error() == NetworkError || error() == TimeoutError) - && d->retriesTaken < d->maxRetries) - { + && d->retriesTaken < d->maxRetries) { // TODO: The whole retrying thing should be put to ConnectionManager // otherwise independently retrying jobs make a bit of notification // storm towards the UI. const auto retryInterval = error() == TimeoutError ? 0 : getNextRetryInterval(); ++d->retriesTaken; - qCWarning(d->logCat).nospace() << this << ": retry #" << d->retriesTaken - << " in " << retryInterval/1000 << " s"; + qCWarning(d->logCat).nospace() + << this << ": retry #" << d->retriesTaken << " in " + << retryInterval / 1000 << " s"; d->retryTimer.start(retryInterval); emit retryScheduled(d->retriesTaken, retryInterval); return; @@ -510,93 +498,78 @@ BaseJob::duration_t BaseJob::millisToRetry() const return d->retryTimer.isActive() ? d->retryTimer.remainingTime() : 0; } -int BaseJob::maxRetries() const -{ - return d->maxRetries; -} +int BaseJob::maxRetries() const { return d->maxRetries; } void BaseJob::setMaxRetries(int newMaxRetries) { d->maxRetries = newMaxRetries; } -BaseJob::Status BaseJob::status() const -{ - return d->status; -} +BaseJob::Status BaseJob::status() const { return d->status; } QByteArray BaseJob::rawData(int bytesAtMost) const { return bytesAtMost > 0 && d->rawResponse.size() > bytesAtMost - ? d->rawResponse.left(bytesAtMost) : d->rawResponse; + ? d->rawResponse.left(bytesAtMost) + : d->rawResponse; } QString BaseJob::rawDataSample(int bytesAtMost) const { auto data = rawData(bytesAtMost); Q_ASSERT(data.size() <= d->rawResponse.size()); - return data.size() == d->rawResponse.size() - ? data : data + tr("...(truncated, %Ln bytes in total)", - "Comes after trimmed raw network response", - d->rawResponse.size()); - + return data.size() == d->rawResponse.size() ? data + : data + + tr("...(truncated, %Ln bytes in total)", + "Comes after trimmed raw network response", + d->rawResponse.size()); } QString BaseJob::statusCaption() const { - switch (d->status.code) - { - case Success: - return tr("Success"); - case Pending: - return tr("Request still pending response"); - case UnexpectedResponseTypeWarning: - return tr("Warning: Unexpected response type"); - case Abandoned: - return tr("Request was abandoned"); - case NetworkError: - return tr("Network problems"); - case JsonParseError: - return tr("Response could not be parsed"); - case TimeoutError: - return tr("Request timed out"); - case ContentAccessError: - return tr("Access error"); - case NotFoundError: - return tr("Not found"); - case IncorrectRequestError: - return tr("Invalid request"); - case IncorrectResponseError: - return tr("Response could not be parsed"); - case TooManyRequestsError: - return tr("Too many requests"); - case RequestNotImplementedError: - return tr("Function not implemented by the server"); - case NetworkAuthRequiredError: - return tr("Network authentication required"); - case UserConsentRequiredError: - return tr("User consent required"); - case UnsupportedRoomVersionError: - return tr("The server does not support the needed room version"); - default: - return tr("Request failed"); + switch (d->status.code) { + case Success: + return tr("Success"); + case Pending: + return tr("Request still pending response"); + case UnexpectedResponseTypeWarning: + return tr("Warning: Unexpected response type"); + case Abandoned: + return tr("Request was abandoned"); + case NetworkError: + return tr("Network problems"); + case JsonParseError: + return tr("Response could not be parsed"); + case TimeoutError: + return tr("Request timed out"); + case ContentAccessError: + return tr("Access error"); + case NotFoundError: + return tr("Not found"); + case IncorrectRequestError: + return tr("Invalid request"); + case IncorrectResponseError: + return tr("Response could not be parsed"); + case TooManyRequestsError: + return tr("Too many requests"); + case RequestNotImplementedError: + return tr("Function not implemented by the server"); + case NetworkAuthRequiredError: + return tr("Network authentication required"); + case UserConsentRequiredError: + return tr("User consent required"); + case UnsupportedRoomVersionError: + return tr("The server does not support the needed room version"); + default: + return tr("Request failed"); } } -int BaseJob::error() const -{ - return d->status.code; -} +int BaseJob::error() const { return d->status.code; } -QString BaseJob::errorString() const -{ - return d->status.message; -} +QString BaseJob::errorString() const { return d->status.message; } -QUrl BaseJob::errorUrl() const -{ - return d->errorUrl; -} +QUrl BaseJob::errorUrl() const { return d->errorUrl; } void BaseJob::setStatus(Status s) { @@ -629,11 +602,8 @@ void BaseJob::abandon() void BaseJob::timeout() { - setStatus( TimeoutError, "The job has timed out" ); + setStatus(TimeoutError, "The job has timed out"); finishJob(); } -void BaseJob::setLoggingCategory(LoggingCategory lcf) -{ - d->logCat = lcf; -} +void BaseJob::setLoggingCategory(LoggingCategory lcf) { d->logCat = lcf; } -- cgit v1.2.3 From 5722ceaf4bd10c29f1091e3dc5a87f5650ea8c71 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 26 Jun 2019 07:51:08 +0900 Subject: BaseJob::Status: fromHttpCode --- lib/jobs/basejob.cpp | 61 +++++++++++++++++++++++++--------------------------- 1 file changed, 29 insertions(+), 32 deletions(-) (limited to 'lib/jobs/basejob.cpp') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index a0a3dc29..9c0b431c 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -348,6 +348,34 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) return false; } +BaseJob::Status BaseJob::Status::fromHttpCode(int httpCode, QString msg) +{ + // clang-format off + return { [httpCode]() -> StatusCode { + if (httpCode / 10 == 41) // 41x errors + return httpCode == 410 ? IncorrectRequestError : NotFoundError; + switch (httpCode) { + case 401: case 403: case 407: + return ContentAccessError; + case 404: + return NotFoundError; + case 400: case 405: case 406: case 426: case 428: case 505: + case 494: // Unofficial nginx "Request header too large" + case 497: // Unofficial nginx "HTTP request sent to HTTPS port" + return IncorrectRequestError; + case 429: + return TooManyRequestsError; + case 501: case 510: + return RequestNotImplementedError; + case 511: + return NetworkAuthRequiredError; + default: + return NetworkError; + } + }(), msg }; + // clang-format on +} + BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const { // QNetworkReply error codes seem to be flawed when it comes to HTTP; @@ -381,38 +409,7 @@ BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const qCWarning(d->logCat).noquote().nospace() << this << urlString; qCWarning(d->logCat).noquote() << " " << httpCode << reason << replyState; - return { [httpCode]() -> StatusCode { - if (httpCode / 10 == 41) - return httpCode == 410 ? IncorrectRequestError - : NotFoundError; - switch (httpCode) { - case 401: - case 403: - case 407: - return ContentAccessError; - case 404: - return NotFoundError; - case 400: - case 405: - case 406: - case 426: - case 428: - case 505: - case 494: // Unofficial nginx "Request header too large" - case 497: // Unofficial nginx "HTTP request sent to HTTPS port" - return IncorrectRequestError; - case 429: - return TooManyRequestsError; - case 501: - case 510: - return RequestNotImplementedError; - case 511: - return NetworkAuthRequiredError; - default: - return NetworkError; - } - }(), - reply->errorString() }; + return Status::fromHttpCode(httpCode, reply->errorString()); } BaseJob::Status BaseJob::parseReply(QNetworkReply* reply) -- cgit v1.2.3 From c05ade838f0fce81f2bbe80a3295618a8a26ff52 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 2 Aug 2019 19:59:40 +0900 Subject: Apply the new brace wrapping to source files --- lib/jobs/basejob.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'lib/jobs/basejob.cpp') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index e4a74954..a6471ece 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -32,8 +32,7 @@ using namespace QMatrixClient; -struct NetworkReplyDeleter : public QScopedPointerDeleteLater -{ +struct NetworkReplyDeleter : public QScopedPointerDeleteLater { static inline void cleanup(QNetworkReply* reply) { if (reply && reply->isRunning()) @@ -42,8 +41,7 @@ struct NetworkReplyDeleter : public QScopedPointerDeleteLater } }; -class BaseJob::Private -{ +class BaseJob::Private { public: // Using an idiom from clang-tidy: // http://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html -- cgit v1.2.3 From 7a5b359b8823646ce97cbaf05c251cb04c291466 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 7 Jul 2019 18:16:30 +0900 Subject: Rename zero-impact strings --- lib/jobs/basejob.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib/jobs/basejob.cpp') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index a6471ece..5615736e 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -321,7 +321,7 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const { // QNetworkReply error codes seem to be flawed when it comes to HTTP; - // see, e.g., https://github.com/QMatrixClient/libqmatrixclient/issues/200 + // see, e.g., https://github.com/quotient-im/libQuotient/issues/200 // so check genuine HTTP codes. The below processing is based on // https://en.wikipedia.org/wiki/List_of_HTTP_status_codes const auto httpCodeHeader = @@ -576,7 +576,7 @@ QUrl BaseJob::errorUrl() const { return d->errorUrl; } void BaseJob::setStatus(Status s) { // The crash that led to this code has been reported in - // https://github.com/QMatrixClient/Quaternion/issues/566 - basically, + // https://github.com/quotient-im/Quaternion/issues/566 - basically, // when cleaning up childrent of a deleted Connection, there's a chance // of pending jobs being abandoned, calling setStatus(Abandoned). // There's nothing wrong with this; however, the safety check for -- cgit v1.2.3 From 27ca32a1e5a56e09b9cc1d94224d2831004dcf3d Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 7 Jul 2019 19:32:34 +0900 Subject: Namespace: QMatrixClient -> Quotient (with back comp alias) --- lib/jobs/basejob.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/jobs/basejob.cpp') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 5615736e..ec6b8375 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -30,7 +30,7 @@ #include -using namespace QMatrixClient; +using namespace Quotient; struct NetworkReplyDeleter : public QScopedPointerDeleteLater { static inline void cleanup(QNetworkReply* reply) -- cgit v1.2.3 From fadce11be92abe76cecfe6356b3b38f25dd93e8d Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 13 Aug 2019 20:28:56 +0900 Subject: Support for server notices rooms (MSC1452) Closes #326. --- lib/jobs/basejob.cpp | 3 +++ 1 file changed, 3 insertions(+) (limited to 'lib/jobs/basejob.cpp') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index ec6b8375..5930e8b8 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -426,6 +426,9 @@ BaseJob::Status BaseJob::parseError(QNetworkReply* reply, ? tr("Requested room version: %1") .arg(errorJson.value("room_version"_ls).toString()) : errorJson.value("error"_ls).toString() }; + if (errCode == "M_CANNOT_LEAVE_SERVER_NOTICE_ROOM") + return { CannotLeaveRoom, + tr("It's not allowed to leave a server notices room") }; // Not localisable on the client side if (errorJson.contains("error"_ls)) -- cgit v1.2.3 From 8663c2e78407a0c0df872eaf9bb6b41de2fbdc9e Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 16 Aug 2019 13:40:09 +0900 Subject: BaseJob: support M_USER_DEACTIVATED error code Closes #344. --- lib/jobs/basejob.cpp | 2 ++ 1 file changed, 2 insertions(+) (limited to 'lib/jobs/basejob.cpp') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 5930e8b8..f3ba00b5 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -429,6 +429,8 @@ BaseJob::Status BaseJob::parseError(QNetworkReply* reply, if (errCode == "M_CANNOT_LEAVE_SERVER_NOTICE_ROOM") return { CannotLeaveRoom, tr("It's not allowed to leave a server notices room") }; + if (errCode == "M_USER_DEACTIVATED") + return { UserDeactivated }; // Not localisable on the client side if (errorJson.contains("error"_ls)) -- cgit v1.2.3 From ad159b5206de615762e22f95e97ae61400f11761 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 20 Aug 2019 19:56:54 +0900 Subject: BaseJob: Status/StatusCode tweaks, cleanup, mo' comments Notably, recovered Status::fromHttpCode() that was introduced in 5722ceaf4bd10c29f1091e3dc5a87f5650ea8c71 but fell victim of a careless merge (so much for introducing non-topical changes in feature branches). --- lib/jobs/basejob.cpp | 135 ++++++++++++++++++++++++++++----------------------- 1 file changed, 74 insertions(+), 61 deletions(-) (limited to 'lib/jobs/basejob.cpp') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index f3ba00b5..621762be 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -77,6 +77,8 @@ public: QByteArray rawResponse; QUrl errorUrl; //< May contain a URL to help with some errors + LoggingCategory logCat = JOBS; + QTimer timer; QTimer retryTimer; @@ -86,7 +88,12 @@ public: int maxRetries = errorStrategy.size(); int retriesTaken = 0; - LoggingCategory logCat = JOBS; + QString urlForLog() const + { + return reply + ? reply->url().toString(QUrl::RemoveQuery) + : makeRequestUrl(connection->baseUrl(), apiEndpoint).toString(); + } }; BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, @@ -112,7 +119,7 @@ BaseJob::~BaseJob() QUrl BaseJob::requestUrl() const { - return d->reply ? d->reply->request().url() : QUrl(); + return d->reply ? d->reply->url() : QUrl(); } bool BaseJob::isBackground() const @@ -193,17 +200,13 @@ void BaseJob::Private::sendRequest(bool inBackground) req.setRawHeader("Authorization", QByteArray("Bearer ") + connection->accessToken()); req.setAttribute(QNetworkRequest::BackgroundRequestAttribute, inBackground); -#if (QT_VERSION >= QT_VERSION_CHECK(5, 6, 0)) req.setAttribute(QNetworkRequest::FollowRedirectsAttribute, true); req.setMaximumRedirectsAllowed(10); -#endif req.setAttribute(QNetworkRequest::HttpPipeliningAllowedAttribute, true); -#if QT_VERSION >= QT_VERSION_CHECK(5, 9, 0) - // some sources claim that there are issues with QT 5.8 req.setAttribute(QNetworkRequest::HTTP2AllowedAttribute, true); -#endif for (auto it = requestHeaders.cbegin(); it != requestHeaders.cend(); ++it) req.setRawHeader(it.key(), it.value()); + switch (verb) { case HttpVerb::Get: reply.reset(connection->nam()->get(req)); @@ -318,6 +321,47 @@ bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) return false; } +BaseJob::Status BaseJob::Status::fromHttpCode(int httpCode, QString msg) +{ + // clang-format off + return { [httpCode]() -> StatusCode { + if (httpCode / 10 == 41) // 41x errors + return httpCode == 410 ? IncorrectRequestError : NotFoundError; + switch (httpCode) { + case 401: case 403: case 407: + return ContentAccessError; + case 404: + return NotFoundError; + case 400: case 405: case 406: case 426: case 428: case 505: + case 494: // Unofficial nginx "Request header too large" + case 497: // Unofficial nginx "HTTP request sent to HTTPS port" + return IncorrectRequestError; + case 429: + return TooManyRequestsError; + case 501: case 510: + return RequestNotImplementedError; + case 511: + return NetworkAuthRequiredError; + default: + return NetworkError; + } + }(), std::move(msg) }; + // clang-format on +} + +QDebug BaseJob::Status::dumpToLog(QDebug dbg) const +{ + QDebugStateSaver _s(dbg); + dbg.noquote().nospace(); + if (auto* const k = QMetaEnum::fromType().valueToKey(code)) { + const QByteArray b = k; + dbg << b.mid(b.lastIndexOf(':')); + } else + dbg << code; + return dbg << ": " << message; + +} + BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const { // QNetworkReply error codes seem to be flawed when it comes to HTTP; @@ -327,62 +371,30 @@ BaseJob::Status BaseJob::doCheckReply(QNetworkReply* reply) const const auto httpCodeHeader = reply->attribute(QNetworkRequest::HttpStatusCodeAttribute); if (!httpCodeHeader.isValid()) { - qCWarning(d->logCat) << this << "didn't get valid HTTP headers"; + qCWarning(d->logCat) << "No valid HTTP headers from" << d->urlForLog(); return { NetworkError, reply->errorString() }; } - const QString replyState = reply->isRunning() - ? QStringLiteral("(tentative)") - : QStringLiteral("(final)"); - const auto urlString = '|' + d->reply->url().toDisplayString(); const auto httpCode = httpCodeHeader.toInt(); - const auto reason = - reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString(); if (httpCode / 100 == 2) // 2xx { - qCDebug(d->logCat).noquote().nospace() << this << urlString; - qCDebug(d->logCat).noquote() << " " << httpCode << reason << replyState; + if (reply->isFinished()) + qCInfo(d->logCat) << httpCode << "<-" << d->urlForLog(); if (!checkContentType(reply->rawHeader("Content-Type"), d->expectedContentTypes)) return { UnexpectedResponseTypeWarning, "Unexpected content type of the response" }; return NoError; } + if (reply->isFinished()) + qCWarning(d->logCat) << httpCode << "<-" << d->urlForLog(); - qCWarning(d->logCat).noquote().nospace() << this << urlString; - qCWarning(d->logCat).noquote() << " " << httpCode << reason << replyState; - return { [httpCode]() -> StatusCode { - if (httpCode / 10 == 41) - return httpCode == 410 ? IncorrectRequestError - : NotFoundError; - switch (httpCode) { - case 401: - case 403: - case 407: - return ContentAccessError; - case 404: - return NotFoundError; - case 400: - case 405: - case 406: - case 426: - case 428: - case 505: - case 494: // Unofficial nginx "Request header too large" - case 497: // Unofficial nginx "HTTP request sent to HTTPS port" - return IncorrectRequestError; - case 429: - return TooManyRequestsError; - case 501: - case 510: - return RequestNotImplementedError; - case 511: - return NetworkAuthRequiredError; - default: - return NetworkError; - } - }(), - reply->errorString() }; + auto message = reply->errorString(); + if (message.isEmpty()) + message = reply->attribute(QNetworkRequest::HttpReasonPhraseAttribute) + .toString(); + + return Status::fromHttpCode(httpCode, message); } BaseJob::Status BaseJob::parseReply(QNetworkReply* reply) @@ -398,7 +410,7 @@ BaseJob::Status BaseJob::parseReply(QNetworkReply* reply) BaseJob::Status BaseJob::parseJson(const QJsonDocument&) { return Success; } -BaseJob::Status BaseJob::parseError(QNetworkReply* reply, +BaseJob::Status BaseJob::parseError(QNetworkReply* /*reply*/, const QJsonObject& errorJson) { const auto errCode = errorJson.value("errcode"_ls).toString(); @@ -441,7 +453,7 @@ BaseJob::Status BaseJob::parseError(QNetworkReply* reply, void BaseJob::stop() { - // This method is used to semi-finalise the job before retrying; so + // This method is (also) used to semi-finalise the job before retrying; so // stop the timeout timer but keep the retry timer running. d->timer.stop(); if (d->reply) { @@ -449,7 +461,7 @@ void BaseJob::stop() if (d->reply->isRunning()) { qCWarning(d->logCat) << this << "stopped without ready network reply"; - d->reply->abort(); + d->reply->abort(); // Keep the reply object in case clients need it } } else qCWarning(d->logCat) << this << "stopped with empty network reply"; @@ -474,10 +486,10 @@ void BaseJob::finishJob() return; } - // Notify those interested in any completion of the job (including killing) + // Notify those interested in any completion of the job including abandon() emit finished(this); - emit result(this); + emit result(this); // abandon() doesn't emit this if (error()) emit failure(this); else @@ -582,21 +594,22 @@ void BaseJob::setStatus(Status s) { // The crash that led to this code has been reported in // https://github.com/quotient-im/Quaternion/issues/566 - basically, - // when cleaning up childrent of a deleted Connection, there's a chance + // when cleaning up children of a deleted Connection, there's a chance // of pending jobs being abandoned, calling setStatus(Abandoned). // There's nothing wrong with this; however, the safety check for // cleartext access tokens below uses d->connection - which is a dangling // pointer. // To alleviate that, a stricter condition is applied, that for Abandoned // and to-be-Abandoned jobs the status message will be disregarded entirely. - // For 0.6 we might rectify the situation by making d->connection - // a QPointer<> (and derive ConnectionData from QObject, respectively). - if (d->status.code == Abandoned || s.code == Abandoned) - s.message.clear(); - + // We could rectify the situation by making d->connection a QPointer<> + // (and deriving ConnectionData from QObject, respectively) but it's + // a too edge case for the hassle. if (d->status == s) return; + if (d->status.code == Abandoned || s.code == Abandoned) + s.message.clear(); + if (!s.message.isEmpty() && d->connection && !d->connection->accessToken().isEmpty()) s.message.replace(d->connection->accessToken(), "(REDACTED)"); -- cgit v1.2.3 From 59c4996a602e9eeae4e3bfc0210ff15f957df38f Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 20 Aug 2019 20:09:09 +0900 Subject: BaseJob/ConnectionData: connection-wide rate-limiting As before, completely transparent for clients, driven by 529 errors from the server (but cases of rate limiting are signalled by BaseJob::rateLimited). That brings changes to BaseJob API: timeouts now use int64_t and also can be handled in std::chrono terms; aboutToStart() -> aboutToSendRequest(); started() -> sentRequest(). Closes #292. --- lib/jobs/basejob.cpp | 159 +++++++++++++++++++++++++++++---------------------- 1 file changed, 90 insertions(+), 69 deletions(-) (limited to 'lib/jobs/basejob.cpp') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 621762be..0a17431c 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -31,6 +31,8 @@ #include using namespace Quotient; +using std::chrono::seconds, std::chrono::milliseconds; +using std::chrono_literals::operator""s; struct NetworkReplyDeleter : public QScopedPointerDeleteLater { static inline void cleanup(QNetworkReply* reply) @@ -43,6 +45,11 @@ struct NetworkReplyDeleter : public QScopedPointerDeleteLater { class BaseJob::Private { public: + struct JobTimeoutConfig { + seconds jobTimeout; + seconds nextRetryInterval; + }; + // Using an idiom from clang-tidy: // http://clang.llvm.org/extra/clang-tidy/checks/modernize-pass-by-value.html Private(HttpVerb v, QString endpoint, const QUrlQuery& q, Data&& data, @@ -52,12 +59,14 @@ public: , requestQuery(q) , requestData(std::move(data)) , needsToken(nt) - {} + { + timer.setSingleShot(true); + retryTimer.setSingleShot(true); + } - void sendRequest(bool inBackground); - const JobTimeoutConfig& getCurrentTimeoutConfig() const; + void sendRequest(); - const ConnectionData* connection = nullptr; + ConnectionData* connection = nullptr; // Contents for the network request HttpVerb verb; @@ -67,13 +76,15 @@ public: Data requestData; bool needsToken; + bool inBackground = false; + // 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; + QByteArrayList expectedContentTypes { "application/json" }; QScopedPointer reply; - Status status = Pending; + Status status = Unprepared; QByteArray rawResponse; QUrl errorUrl; //< May contain a URL to help with some errors @@ -82,12 +93,18 @@ public: QTimer timer; QTimer retryTimer; - QVector errorStrategy = { { 90, 5 }, - { 90, 10 }, - { 120, 30 } }; - int maxRetries = errorStrategy.size(); + static constexpr std::array errorStrategy { + { { 90s, 5s }, { 90s, 10s }, { 120s, 30s } } + }; + int maxRetries = int(errorStrategy.size()); int retriesTaken = 0; + const JobTimeoutConfig& getCurrentTimeoutConfig() const + { + return errorStrategy[std::min(size_t(retriesTaken), + errorStrategy.size() - 1)]; + } + QString urlForLog() const { return reply @@ -106,9 +123,11 @@ BaseJob::BaseJob(HttpVerb verb, const QString& name, const QString& endpoint, : d(new Private(verb, endpoint, query, std::move(data), needsToken)) { setObjectName(name); - setExpectedContentTypes({ "application/json" }); - d->timer.setSingleShot(true); connect(&d->timer, &QTimer::timeout, this, &BaseJob::timeout); + connect(&d->retryTimer, &QTimer::timeout, this, [this] { + setStatus(Pending); + sendRequest(); + }); } BaseJob::~BaseJob() @@ -124,10 +143,7 @@ QUrl BaseJob::requestUrl() const bool BaseJob::isBackground() const { - return d->reply - && d->reply->request() - .attribute(QNetworkRequest::BackgroundRequestAttribute) - .toBool(); + return d->inBackground; } const QString& BaseJob::apiEndpoint() const { return d->apiEndpoint; } @@ -191,7 +207,7 @@ QUrl BaseJob::makeRequestUrl(QUrl baseUrl, const QString& path, return baseUrl; } -void BaseJob::Private::sendRequest(bool inBackground) +void BaseJob::Private::sendRequest() { QNetworkRequest req { makeRequestUrl(connection->baseUrl(), apiEndpoint, requestQuery) }; @@ -223,36 +239,31 @@ void BaseJob::Private::sendRequest(bool inBackground) } } -void BaseJob::beforeStart(const ConnectionData*) {} +void BaseJob::doPrepare() {} -void BaseJob::afterStart(const ConnectionData*, QNetworkReply*) {} +void BaseJob::onSentRequest(QNetworkReply*) {} void BaseJob::beforeAbandon(QNetworkReply*) {} -void BaseJob::start(const ConnectionData* connData, bool inBackground) +void BaseJob::prepare(ConnectionData* connData, bool inBackground) { + d->inBackground = inBackground; d->connection = connData; - d->retryTimer.setSingleShot(true); - connect(&d->retryTimer, &QTimer::timeout, this, - [this, inBackground] { sendRequest(inBackground); }); - - beforeStart(connData); - if (status().good()) - sendRequest(inBackground); - if (status().good()) - afterStart(connData, d->reply.data()); - if (!status().good()) + doPrepare(); + if (status().code != Unprepared && status().code != Pending) QTimer::singleShot(0, this, &BaseJob::finishJob); + setStatus(Pending); } -void BaseJob::sendRequest(bool inBackground) +void BaseJob::sendRequest() { - emit aboutToStart(); - d->retryTimer.stop(); // In case we were counting down at the moment - qCDebug(d->logCat) << this << "sending request to" << d->apiEndpoint; - if (!d->requestQuery.isEmpty()) - qCDebug(d->logCat) << " query:" << d->requestQuery.toString(); - d->sendRequest(inBackground); + if (status().code == Abandoned) + return; + Q_ASSERT(d->connection && status().code == Pending); + qCDebug(d->logCat) << "Making request to" << d->urlForLog(); + emit aboutToSendRequest(); + d->sendRequest(); + Q_ASSERT(d->reply); connect(d->reply.data(), &QNetworkReply::finished, this, &BaseJob::gotReply); if (d->reply->isRunning()) { connect(d->reply.data(), &QNetworkReply::metaDataChanged, this, @@ -262,10 +273,12 @@ void BaseJob::sendRequest(bool inBackground) connect(d->reply.data(), &QNetworkReply::downloadProgress, this, &BaseJob::downloadProgress); d->timer.start(getCurrentTimeout()); - qCDebug(d->logCat) << this << "request has been sent"; - emit started(); + qCInfo(d->logCat).noquote() << "Request sent to" << d->urlForLog(); + onSentRequest(d->reply.data()); + emit sentRequest(); } else - qCWarning(d->logCat) << this << "request could not start"; + qCWarning(d->logCat).noquote() + << "Request could not start:" << d->urlForLog(); } void BaseJob::checkReply() { setStatus(doCheckReply(d->reply.data())); } @@ -286,13 +299,7 @@ void BaseJob::gotReply() parseError(d->reply.data(), QJsonDocument::fromJson(d->rawResponse).object())); } - - if (status().code != TooManyRequestsError) - finishJob(); - else { - stop(); - emit retryScheduled(d->retriesTaken, d->retryTimer.interval()); - } + finishJob(); } bool checkContentType(const QByteArray& type, const QByteArrayList& patterns) @@ -416,14 +423,13 @@ BaseJob::Status BaseJob::parseError(QNetworkReply* /*reply*/, const auto errCode = errorJson.value("errcode"_ls).toString(); if (error() == TooManyRequestsError || errCode == "M_LIMIT_EXCEEDED") { QString msg = tr("Too many requests"); - auto retryInterval = errorJson.value("retry_after_ms"_ls).toInt(-1); - if (retryInterval != -1) - msg += tr(", next retry advised after %1 ms").arg(retryInterval); + int64_t retryAfterMs = errorJson.value("retry_after_ms"_ls).toInt(-1); + if (retryAfterMs >= 0) + msg += tr(", next retry advised after %1 ms").arg(retryAfterMs); else // We still have to figure some reasonable interval - retryInterval = getNextRetryInterval(); + retryAfterMs = getNextRetryMs(); - qCWarning(d->logCat) << this << "will retry in" << retryInterval << "ms"; - d->retryTimer.start(retryInterval); + d->connection->limitRate(milliseconds(retryAfterMs)); return { TooManyRequestsError, msg }; } @@ -470,19 +476,23 @@ void BaseJob::stop() void BaseJob::finishJob() { stop(); - if ((error() == NetworkError || error() == TimeoutError) + if (error() == TooManyRequests) { + emit rateLimited(); + setStatus(Pending); + d->connection->submit(this); + return; + } + if ((error() == NetworkError || error() == Timeout) && d->retriesTaken < d->maxRetries) { - // TODO: The whole retrying thing should be put to ConnectionManager + // TODO: The whole retrying thing should be put to Connection(Manager) // otherwise independently retrying jobs make a bit of notification // storm towards the UI. - const auto retryInterval = error() == TimeoutError - ? 0 - : getNextRetryInterval(); + const seconds retryIn = error() == Timeout ? 0s : getNextRetryInterval(); ++d->retriesTaken; qCWarning(d->logCat).nospace() << this << ": retry #" << d->retriesTaken - << " in " << retryInterval / 1000 << " s"; - d->retryTimer.start(retryInterval); - emit retryScheduled(d->retriesTaken, retryInterval); + << " in " << retryIn.count() << " s"; + d->retryTimer.start(retryIn); + emit retryScheduled(d->retriesTaken, milliseconds(retryIn).count()); return; } @@ -498,24 +508,35 @@ void BaseJob::finishJob() deleteLater(); } -const JobTimeoutConfig& BaseJob::Private::getCurrentTimeoutConfig() const +seconds BaseJob::getCurrentTimeout() const +{ + return d->getCurrentTimeoutConfig().jobTimeout; +} + +BaseJob::duration_ms_t BaseJob::getCurrentTimeoutMs() const +{ + return milliseconds(getCurrentTimeout()).count(); +} + +seconds BaseJob::getNextRetryInterval() const { - return errorStrategy[std::min(retriesTaken, errorStrategy.size() - 1)]; + return d->getCurrentTimeoutConfig().nextRetryInterval; } -BaseJob::duration_t BaseJob::getCurrentTimeout() const +BaseJob::duration_ms_t BaseJob::getNextRetryMs() const { - return d->getCurrentTimeoutConfig().jobTimeout * 1000; + return milliseconds(getNextRetryInterval()).count(); } -BaseJob::duration_t BaseJob::getNextRetryInterval() const +milliseconds BaseJob::timeToRetry() const { - return d->getCurrentTimeoutConfig().nextRetryInterval * 1000; + return d->retryTimer.isActive() ? d->retryTimer.remainingTimeAsDuration() + : 0s; } -BaseJob::duration_t BaseJob::millisToRetry() const +BaseJob::duration_ms_t BaseJob::millisToRetry() const { - return d->retryTimer.isActive() ? d->retryTimer.remainingTime() : 0; + return timeToRetry().count(); } int BaseJob::maxRetries() const { return d->maxRetries; } -- cgit v1.2.3 From 97cac4b919735eb59493dd1cd528992ba90e611b Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 27 Aug 2019 17:22:22 +0900 Subject: More compliant 'using' for chrono_literals Compilers warn on using 'using ...::operator""s' because they think we're redefining the reserved suffix. --- lib/jobs/basejob.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib/jobs/basejob.cpp') diff --git a/lib/jobs/basejob.cpp b/lib/jobs/basejob.cpp index 0a17431c..54931c83 100644 --- a/lib/jobs/basejob.cpp +++ b/lib/jobs/basejob.cpp @@ -32,7 +32,7 @@ using namespace Quotient; using std::chrono::seconds, std::chrono::milliseconds; -using std::chrono_literals::operator""s; +using namespace std::chrono_literals; struct NetworkReplyDeleter : public QScopedPointerDeleteLater { static inline void cleanup(QNetworkReply* reply) -- cgit v1.2.3