From 5f85a2bfc5d5fee00fcdfb1230af32344376e39a Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 19 Aug 2017 18:01:19 +0900 Subject: BaseJob::Data: Small update to better match Qt API Also: Query and Data constructors from initialization_list<> are no more explicit, as clang-tidy recommends. --- jobs/basejob.h | 7 +++---- jobs/passwordlogin.cpp | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.h b/jobs/basejob.h index 0ec40a7a..5df03b32 100644 --- a/jobs/basejob.h +++ b/jobs/basejob.h @@ -63,7 +63,7 @@ namespace QMatrixClient public: using QUrlQuery::QUrlQuery; Query() = default; - explicit Query(const std::initializer_list< QPair >& l) + Query(const std::initializer_list< QPair >& l) { setQueryItems(l); } @@ -78,11 +78,10 @@ namespace QMatrixClient { public: Data() = default; - Data(const QJsonObject& o) : QJsonObject(o) { } - Data(QJsonObject&& o) : QJsonObject(std::move(o)) { } + explicit Data(const QJsonObject& o) : QJsonObject(o) { } #if (QT_VERSION < QT_VERSION_CHECK(5, 4, 0)) // This method exists in QJsonObject of newer Qt versions - explicit Data(const std::initializer_list< QPair >& l) + Data(const std::initializer_list< QPair >& l) { for (auto i: l) insert(i.first, i.second); diff --git a/jobs/passwordlogin.cpp b/jobs/passwordlogin.cpp index 081e19bc..09108215 100644 --- a/jobs/passwordlogin.cpp +++ b/jobs/passwordlogin.cpp @@ -33,7 +33,7 @@ PasswordLogin::PasswordLogin(const ConnectionData* connection, QString user, QSt , "_matrix/client/r0/login" , Query() , Data( - { { "type", "m.login.password" } + { { "type", QStringLiteral("m.login.password") } , { "user", user } , { "password", password } }) -- cgit v1.2.3 From 45c138903c20d32a8a69b5637a72898bc690f1f1 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 1 Sep 2017 10:29:33 +0900 Subject: BaseJob::Data: expose constructors from QJsonObject We had a stupid situation when this class has less features when compiled with newer Qt because we explicitly added a constructor from std::initializer_list for older Qt versions but did not reuse the same constructor from QJsonObject for newer versions. --- jobs/basejob.h | 1 + 1 file changed, 1 insertion(+) (limited to 'jobs') diff --git a/jobs/basejob.h b/jobs/basejob.h index 5df03b32..b8cc9511 100644 --- a/jobs/basejob.h +++ b/jobs/basejob.h @@ -77,6 +77,7 @@ namespace QMatrixClient class Data : public QJsonObject { public: + using QJsonObject::QJsonObject; Data() = default; explicit Data(const QJsonObject& o) : QJsonObject(o) { } #if (QT_VERSION < QT_VERSION_CHECK(5, 4, 0)) -- cgit v1.2.3 From da975f68f6a8503bf5466292dcdceed8c6f7fa6f Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 5 Sep 2017 19:23:13 +0900 Subject: Initialize more properly to fix a warning --- jobs/basejob.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 26ceb268..157ac034 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -263,13 +263,13 @@ void BaseJob::finishJob() BaseJob::duration_t BaseJob::getCurrentTimeout() const { - static const std::array timeouts = { 90, 90, 120, 120 }; + static const std::array timeouts ({ 90, 90, 120, 120 }); return timeouts[std::min(d->retriesTaken, timeouts.size() - 1)] * 1000; } BaseJob::duration_t BaseJob::getNextRetryInterval() const { - static const std::array intervals = { 5, 10, 30 }; + static const std::array intervals ({ 5, 10, 30 }); return intervals[std::min(d->retriesTaken, intervals.size() - 1)] * 1000; } -- cgit v1.2.3 From 180d62e094a1a6b6fc69c99b291ef075dc649135 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 8 Sep 2017 16:51:03 +0900 Subject: Revert previous commit as it breaks building with VC 2015 This reverts commit da975f68f6a8503bf5466292dcdceed8c6f7fa6f. --- jobs/basejob.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 157ac034..26ceb268 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -263,13 +263,13 @@ void BaseJob::finishJob() BaseJob::duration_t BaseJob::getCurrentTimeout() const { - static const std::array timeouts ({ 90, 90, 120, 120 }); + static const std::array timeouts = { 90, 90, 120, 120 }; return timeouts[std::min(d->retriesTaken, timeouts.size() - 1)] * 1000; } BaseJob::duration_t BaseJob::getNextRetryInterval() const { - static const std::array intervals ({ 5, 10, 30 }); + static const std::array intervals = { 5, 10, 30 }; return intervals[std::min(d->retriesTaken, intervals.size() - 1)] * 1000; } -- cgit v1.2.3 From cb3848c1e7fe09c2e778d38139c399b9f0484ce5 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Wed, 13 Sep 2017 21:10:36 +0900 Subject: MediaThumbnailJob: get rid of useless pimpl; add scaledThumbnail() Also use scaledThumbnail() in User::requestAvatar() --- jobs/mediathumbnailjob.cpp | 17 +++++------------ jobs/mediathumbnailjob.h | 5 ++--- user.cpp | 5 ++--- 3 files changed, 9 insertions(+), 18 deletions(-) (limited to 'jobs') diff --git a/jobs/mediathumbnailjob.cpp b/jobs/mediathumbnailjob.cpp index 9bb731b9..9579f6b2 100644 --- a/jobs/mediathumbnailjob.cpp +++ b/jobs/mediathumbnailjob.cpp @@ -23,12 +23,6 @@ using namespace QMatrixClient; -class MediaThumbnailJob::Private -{ - public: - QPixmap thumbnail; -}; - MediaThumbnailJob::MediaThumbnailJob(const ConnectionData* data, QUrl url, QSize requestedSize, ThumbnailType thumbnailType) : BaseJob(data, HttpVerb::Get, "MediaThumbnailJob", @@ -39,22 +33,21 @@ MediaThumbnailJob::MediaThumbnailJob(const ConnectionData* data, QUrl url, QSize , { "method", thumbnailType == ThumbnailType::Scale ? "scale" : "crop" } })) - , d(new Private) { } -MediaThumbnailJob::~MediaThumbnailJob() +QPixmap MediaThumbnailJob::thumbnail() { - delete d; + return pixmap; } -QPixmap MediaThumbnailJob::thumbnail() +QPixmap MediaThumbnailJob::scaledThumbnail(QSize toSize) { - return d->thumbnail; + return pixmap.scaled(toSize, Qt::KeepAspectRatio, Qt::SmoothTransformation); } BaseJob::Status MediaThumbnailJob::parseReply(QByteArray data) { - if( !d->thumbnail.loadFromData(data) ) + if( !pixmap.loadFromData(data) ) { qCDebug(JOBS) << "MediaThumbnailJob: could not read image data"; } diff --git a/jobs/mediathumbnailjob.h b/jobs/mediathumbnailjob.h index 307d0a99..186da829 100644 --- a/jobs/mediathumbnailjob.h +++ b/jobs/mediathumbnailjob.h @@ -31,15 +31,14 @@ namespace QMatrixClient public: MediaThumbnailJob(const ConnectionData* data, QUrl url, QSize requestedSize, ThumbnailType thumbnailType=ThumbnailType::Scale); - virtual ~MediaThumbnailJob(); QPixmap thumbnail(); + QPixmap scaledThumbnail(QSize toSize); protected: Status parseReply(QByteArray data) override; private: - class Private; - Private* d; + QPixmap pixmap; }; } diff --git a/user.cpp b/user.cpp index 8d37eef6..f9f48539 100644 --- a/user.cpp +++ b/user.cpp @@ -170,12 +170,11 @@ void User::requestAvatar() void User::Private::requestAvatar() { - MediaThumbnailJob* job = connection->getThumbnail(avatarUrl, requestedSize); + auto* job = connection->callApi(avatarUrl, requestedSize); connect( job, &MediaThumbnailJob::success, [=]() { avatarOngoingRequest = false; avatarValid = true; - avatar = job->thumbnail().scaled(requestedSize, - Qt::KeepAspectRatio, Qt::SmoothTransformation); + avatar = job->scaledThumbnail(requestedSize); scaledAvatars.clear(); emit q->avatarChanged(q); }); -- cgit v1.2.3 From d4cb62523585137dee7879f2143ae1b4dd62513d Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 19 Sep 2017 11:39:30 +0900 Subject: Fix a race condition leading to a crash on close It seems that some reply processing still might have happened after BaseJob::abandon() (caused in turn by destroying a Connection object), probably because the event from QNetworkReply landed in the event queue after BaseJob::abandon() but before actual deletion of a job object. Now countered by disconnecting from QNetworkReply signals in abandon() and stop(). --- jobs/basejob.cpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 26ceb268..8c9ef222 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -219,16 +219,17 @@ BaseJob::Status BaseJob::parseJson(const QJsonDocument&) void BaseJob::stop() { d->timer.stop(); - if (!d->reply) + if (d->reply) { - qCWarning(d->logCat) << this << "stopped with empty network reply"; - } - else if (d->reply->isRunning()) - { - qCWarning(d->logCat) << this << "stopped without ready network reply"; d->reply->disconnect(this); // Ignore whatever comes from the reply - d->reply->abort(); + if (d->reply->isRunning()) + { + qCWarning(d->logCat) << this << "stopped without ready network reply"; + d->reply->abort(); + } } + else + qCWarning(d->logCat) << this << "stopped with empty network reply"; } void BaseJob::finishJob() @@ -320,6 +321,9 @@ void BaseJob::setStatus(int code, QString message) void BaseJob::abandon() { + this->disconnect(); + if (d->reply) + d->reply->disconnect(this); deleteLater(); } -- cgit v1.2.3 From 0b11b06379fe668063ea5658a261f53f1dcf117a Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 19 Sep 2017 12:56:29 +0900 Subject: BaseJob: improved logging Your QT_LOGGING_RULES (especially useful with Qt 5.6 and newer) should work a bit better now: * "Job" prefix is no more needed because the Qt logging prefix (libqmatrixclient.jobs) says it already; * The "created" record didn't follow the logging category if overridden from the concrete job class (see SyncJob); so instead of "created" there's now much more useful "sending request" record. --- jobs/basejob.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 8c9ef222..3057ed75 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include @@ -76,7 +77,7 @@ class BaseJob::Private inline QDebug operator<<(QDebug dbg, const BaseJob* j) { - return dbg << "Job" << j->objectName(); + return dbg << j->objectName(); } BaseJob::BaseJob(const ConnectionData* connection, HttpVerb verb, @@ -89,7 +90,6 @@ BaseJob::BaseJob(const ConnectionData* connection, HttpVerb verb, connect (&d->timer, &QTimer::timeout, this, &BaseJob::timeout); d->retryTimer.setSingleShot(true); connect (&d->retryTimer, &QTimer::timeout, this, &BaseJob::start); - qCDebug(d->logCat) << this << "created"; } BaseJob::~BaseJob() @@ -159,6 +159,8 @@ void BaseJob::start() { emit aboutToStart(); d->retryTimer.stop(); // In case we were counting down at the moment + qCDebug(d->logCat) << this << "sending request to" + << d->apiEndpoint % '?' % d->requestQuery.toString(); d->sendRequest(); connect( d->reply.data(), &QNetworkReply::sslErrors, this, &BaseJob::sslErrors ); connect( d->reply.data(), &QNetworkReply::finished, this, &BaseJob::gotReply ); -- cgit v1.2.3