From 37e807b7e4b8991353802b38da226ef47b8848ec Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 25 Dec 2017 20:00:24 +0900 Subject: BaseJob: consolidate job timeout configuration This prepares the backend to enable timeouts/retry intervals configurable from clients. --- jobs/basejob.cpp | 18 ++++++++++++------ jobs/basejob.h | 6 ++++++ 2 files changed, 18 insertions(+), 6 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 9d5c5ed6..3ea13c83 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -53,6 +53,7 @@ class BaseJob::Private { } void sendRequest(); + const JobTimeoutConfig& getCurrentTimeoutConfig() const; const ConnectionData* connection = nullptr; @@ -69,8 +70,10 @@ class BaseJob::Private QTimer timer; QTimer retryTimer; - size_t maxRetries = 3; - size_t retriesTaken = 0; + QVector errorStrategy = + { { 90, 5 }, { 90, 10 }, { 120, 30 } }; + int maxRetries = errorStrategy.size(); + int retriesTaken = 0; LoggingCategory logCat = JOBS; }; @@ -297,16 +300,19 @@ void BaseJob::finishJob() deleteLater(); } +const JobTimeoutConfig& BaseJob::Private::getCurrentTimeoutConfig() const +{ + return errorStrategy[std::min(retriesTaken, errorStrategy.size() - 1)]; +} + BaseJob::duration_t BaseJob::getCurrentTimeout() const { - static const std::array timeouts = { 90, 90, 120, 120 }; - return timeouts[std::min(d->retriesTaken, timeouts.size() - 1)] * 1000; + return d->getCurrentTimeoutConfig().jobTimeout * 1000; } BaseJob::duration_t BaseJob::getNextRetryInterval() const { - static const std::array intervals = { 5, 10, 30 }; - return intervals[std::min(d->retriesTaken, intervals.size() - 1)] * 1000; + return d->getCurrentTimeoutConfig().nextRetryInterval * 1000; } BaseJob::duration_t BaseJob::millisToRetry() const diff --git a/jobs/basejob.h b/jobs/basejob.h index 66812774..aa4894bd 100644 --- a/jobs/basejob.h +++ b/jobs/basejob.h @@ -36,6 +36,12 @@ namespace QMatrixClient enum class HttpVerb { Get, Put, Post, Delete }; + struct JobTimeoutConfig + { + int jobTimeout; + int nextRetryInterval; + }; + class BaseJob: public QObject { Q_OBJECT -- cgit v1.2.3 From 9335a3beba0c15a64478458f418b648834779683 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 25 Dec 2017 21:23:52 +0900 Subject: BaseJob: further minor code cleanup --- jobs/basejob.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.h b/jobs/basejob.h index aa4894bd..0d669b78 100644 --- a/jobs/basejob.h +++ b/jobs/basejob.h @@ -168,7 +168,7 @@ namespace QMatrixClient * @param nextAttempt the 1-based number of attempt (will always be more than 1) * @param inMilliseconds the interval after which the next attempt will be taken */ - void retryScheduled(size_t nextAttempt, int inMilliseconds); + void retryScheduled(int nextAttempt, int inMilliseconds); /** * Emitted when the job is finished, in any case. It is used to notify @@ -185,7 +185,6 @@ namespace QMatrixClient * to avoid dangling pointers in your list. * * @param job the job that emitted this signal - * @internal * * @see success, failure */ -- cgit v1.2.3 From a3b9fe1ddd2d3b0a0cbb07ffc42317b30a1a3899 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 28 Dec 2017 11:26:59 +0900 Subject: Switch from QPixmap to QImage; add convenience avatar() overloads to Room and User The switch is necessary because MediaThumbnailJob is supposed to return something that can be worked on in non-GUI threads (as is the case of QML image providers), and QPixmap is not supposed for usage out of the main thread. --- avatar.cpp | 43 +++++++++++++++++++++++++------------------ avatar.h | 3 ++- jobs/mediathumbnailjob.cpp | 11 ++++++----- jobs/mediathumbnailjob.h | 6 +++--- room.cpp | 7 ++++++- room.h | 13 +++++++++++-- user.cpp | 7 ++++++- user.h | 3 ++- 8 files changed, 61 insertions(+), 32 deletions(-) (limited to 'jobs') diff --git a/avatar.cpp b/avatar.cpp index b2f50a67..9490347d 100644 --- a/avatar.cpp +++ b/avatar.cpp @@ -22,20 +22,22 @@ #include "events/eventcontent.h" #include "connection.h" +#include + using namespace QMatrixClient; class Avatar::Private { public: Private(Connection* c, QIcon di) : _connection(c), _defaultIcon(di) { } - QPixmap get(int width, int height, Avatar::notifier_t notifier); + QImage get(QSize size, Avatar::notifier_t notifier); Connection* _connection; const QIcon _defaultIcon; QUrl _url; - QPixmap _originalPixmap; + QImage _originalImage; - std::vector> _scaledPixmaps; + std::vector> _scaledImages; QSize _requestedSize; bool _valid = false; @@ -49,22 +51,25 @@ Avatar::Avatar(Connection* connection, QIcon defaultIcon) Avatar::~Avatar() = default; -QPixmap Avatar::get(int width, int height, Avatar::notifier_t notifier) +QImage Avatar::get(int dimension, Avatar::notifier_t notifier) { - return d->get(width, height, notifier); + return d->get({dimension, dimension}, notifier); } -QPixmap Avatar::Private::get(int width, int height, Avatar::notifier_t notifier) +QImage Avatar::get(int width, int height, Avatar::notifier_t notifier) { - QSize size(width, height); + return d->get({width, height}, notifier); +} +QImage Avatar::Private::get(QSize size, Avatar::notifier_t notifier) +{ // FIXME: Alternating between longer-width and longer-height requests // is a sure way to trick the below code into constantly getting another // image from the server because the existing one is alleged unsatisfactory. // This is plain abuse by the client, though; so not critical for now. if( ( !(_valid || _ongoingRequest) - || width > _requestedSize.width() - || height > _requestedSize.height() ) && _url.isValid() ) + || size.width() > _requestedSize.width() + || size.height() > _requestedSize.height() ) && _url.isValid() ) { qCDebug(MAIN) << "Getting avatar from" << _url.toString(); _requestedSize = size; @@ -77,8 +82,9 @@ QPixmap Avatar::Private::get(int width, int height, Avatar::notifier_t notifier) if (_ongoingRequest->status().good()) { _valid = true; - _originalPixmap = _ongoingRequest->scaledThumbnail(_requestedSize); - _scaledPixmaps.clear(); + _originalImage = + _ongoingRequest->scaledThumbnail(_requestedSize); + _scaledImages.clear(); for (auto n: notifiers) n(); } @@ -86,21 +92,22 @@ QPixmap Avatar::Private::get(int width, int height, Avatar::notifier_t notifier) }); } - if( _originalPixmap.isNull() ) + if( _originalImage.isNull() ) { if (_defaultIcon.isNull()) - return _originalPixmap; + return _originalImage; - _originalPixmap = _defaultIcon.pixmap(size); + QPainter p { &_originalImage }; + _defaultIcon.paint(&p, { QPoint(), _defaultIcon.actualSize(size) }); } - for (auto p: _scaledPixmaps) + for (auto p: _scaledImages) if (p.first == size) return p.second; - auto pixmap = _originalPixmap.scaled(size, + auto result = _originalImage.scaled(size, Qt::KeepAspectRatio, Qt::SmoothTransformation); - _scaledPixmaps.emplace_back(size, pixmap); - return pixmap; + _scaledImages.emplace_back(size, result); + return result; } QUrl Avatar::url() const { return d->_url; } diff --git a/avatar.h b/avatar.h index e71fecd7..d8b4b206 100644 --- a/avatar.h +++ b/avatar.h @@ -36,7 +36,8 @@ namespace QMatrixClient using notifier_t = std::function; - QPixmap get(int w, int h, notifier_t notifier); + QImage get(int dimension, notifier_t notifier); + QImage get(int w, int h, notifier_t notifier); QUrl url() const; bool updateUrl(const QUrl& newUrl); diff --git a/jobs/mediathumbnailjob.cpp b/jobs/mediathumbnailjob.cpp index 5945493a..c0d67a63 100644 --- a/jobs/mediathumbnailjob.cpp +++ b/jobs/mediathumbnailjob.cpp @@ -36,19 +36,20 @@ MediaThumbnailJob::MediaThumbnailJob(QUrl url, QSize requestedSize, })) { } -QPixmap MediaThumbnailJob::thumbnail() const +QImage MediaThumbnailJob::thumbnail() const { - return pixmap; + return _thumbnail; } -QPixmap MediaThumbnailJob::scaledThumbnail(QSize toSize) const +QImage MediaThumbnailJob::scaledThumbnail(QSize toSize) const { - return pixmap.scaled(toSize, Qt::KeepAspectRatio, Qt::SmoothTransformation); + return _thumbnail.scaled(toSize, + Qt::KeepAspectRatio, Qt::SmoothTransformation); } BaseJob::Status MediaThumbnailJob::parseReply(QByteArray data) { - if( !pixmap.loadFromData(data) ) + if( !_thumbnail.loadFromData(data) ) { qCDebug(JOBS) << "MediaThumbnailJob: could not read image data"; } diff --git a/jobs/mediathumbnailjob.h b/jobs/mediathumbnailjob.h index 292e7f14..f8f36fe9 100644 --- a/jobs/mediathumbnailjob.h +++ b/jobs/mediathumbnailjob.h @@ -32,13 +32,13 @@ namespace QMatrixClient MediaThumbnailJob(QUrl url, QSize requestedSize, ThumbnailType thumbnailType = ThumbnailType::Scale); - QPixmap thumbnail() const; - QPixmap scaledThumbnail(QSize toSize) const; + QImage thumbnail() const; + QImage scaledThumbnail(QSize toSize) const; protected: Status parseReply(QByteArray data) override; private: - QPixmap pixmap; + QImage _thumbnail; }; } // namespace QMatrixClient diff --git a/room.cpp b/room.cpp index 0024683d..1e159fd5 100644 --- a/room.cpp +++ b/room.cpp @@ -214,7 +214,12 @@ QString Room::topic() const return d->topic; } -QPixmap Room::avatar(int width, int height) +QImage Room::avatar(int dimension) +{ + return avatar(dimension, dimension); +} + +QImage Room::avatar(int width, int height) { if (!d->avatar.url().isEmpty()) return d->avatar.get(width, height, [=] { emit avatarChanged(); }); diff --git a/room.h b/room.h index 9a458c4e..f863d41b 100644 --- a/room.h +++ b/room.h @@ -108,11 +108,20 @@ namespace QMatrixClient Q_INVOKABLE int timelineSize() const; /** - * Returns a room avatar and requests it from the network if needed + * Returns a square room avatar with the given size and requests it + * from the network if needed * @return a pixmap with the avatar or a placeholder if there's none * available yet */ - Q_INVOKABLE QPixmap avatar(int width, int height); + Q_INVOKABLE QImage avatar(int dimension); + /** + * Returns a room avatar with the given dimensions and requests it + * from the network if needed + * @return a pixmap with the avatar or a placeholder if there's none + * available yet + */ + Q_INVOKABLE QImage avatar(int width, int height); + /** * @brief Produces a disambiguated name for a given user in * the context of the room diff --git a/user.cpp b/user.cpp index faad6231..6d2a2030 100644 --- a/user.cpp +++ b/user.cpp @@ -95,7 +95,12 @@ Avatar& User::avatarObject() return d->avatar; } -QPixmap User::avatar(int width, int height) +QImage User::avatar(int dimension) +{ + return avatar(dimension, dimension); +} + +QImage User::avatar(int width, int height) { return d->avatar.get(width, height, [=] { emit avatarChanged(this); }); } diff --git a/user.h b/user.h index 148ed64d..b7d67fb2 100644 --- a/user.h +++ b/user.h @@ -54,7 +54,8 @@ namespace QMatrixClient Q_INVOKABLE QString bridged() const; Avatar& avatarObject(); - QPixmap avatar(int requestedWidth, int requestedHeight); + QImage avatar(int dimension); + QImage avatar(int requestedWidth, int requestedHeight); QUrl avatarUrl() const; -- cgit v1.2.3 From 2c440249052b0d518fccd953a7dc657f9eed7ab7 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 30 Dec 2017 17:50:07 +0900 Subject: BaseJob: do not suppress SSL errors It is the application's responsibility to properly display the error and get confirmation from the user about it. --- jobs/basejob.cpp | 9 --------- jobs/basejob.h | 1 - 2 files changed, 10 deletions(-) (limited to 'jobs') diff --git a/jobs/basejob.cpp b/jobs/basejob.cpp index 3ea13c83..8d116c26 100644 --- a/jobs/basejob.cpp +++ b/jobs/basejob.cpp @@ -191,7 +191,6 @@ void BaseJob::sendRequest() if (!d->requestQuery.isEmpty()) qCDebug(d->logCat) << " query:" << d->requestQuery.toString(); d->sendRequest(); - connect( d->reply.data(), &QNetworkReply::sslErrors, this, &BaseJob::sslErrors ); connect( d->reply.data(), &QNetworkReply::finished, this, &BaseJob::gotReply ); if (d->reply->isRunning()) { @@ -371,14 +370,6 @@ void BaseJob::timeout() finishJob(); } -void BaseJob::sslErrors(const QList& errors) -{ - foreach (const QSslError &error, errors) { - qCWarning(d->logCat) << "SSL ERROR" << error.errorString(); - } - d->reply->ignoreSslErrors(); // TODO: insecure! should prompt user first -} - void BaseJob::setLoggingCategory(LoggingCategory lcf) { d->logCat = lcf; diff --git a/jobs/basejob.h b/jobs/basejob.h index 0d669b78..e3a379fa 100644 --- a/jobs/basejob.h +++ b/jobs/basejob.h @@ -267,7 +267,6 @@ namespace QMatrixClient virtual ~BaseJob(); protected slots: void timeout(); - void sslErrors(const QList& errors); private slots: void sendRequest(); -- cgit v1.2.3