From 9474a9afeb7ff63538ee85b4b59e172e5d32db32 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Fri, 23 Feb 2018 11:16:10 +0900 Subject: Avatar: provide common logic for uploading; don't store Connection Although the setting part of the work is class(User or Room)-specific, the uploading part is common, so Avatar provides it now. Also: there's no need to store Connection, as it's only used in get() and upload() - just pass it as the parameter to the methods. --- avatar.cpp | 84 +++++++++++++++++++++++++++++++++++++++++++++++--------------- avatar.h | 22 ++++++++++++---- room.cpp | 8 +++--- user.cpp | 40 ++++++++++++++---------------- user.h | 3 ++- 5 files changed, 105 insertions(+), 52 deletions(-) diff --git a/avatar.cpp b/avatar.cpp index 040bf9bb..9664199c 100644 --- a/avatar.cpp +++ b/avatar.cpp @@ -30,10 +30,13 @@ using namespace QMatrixClient; class Avatar::Private { public: - Private(Connection* c, QIcon di) : _connection(c), _defaultIcon(di) { } - QImage get(QSize size, Avatar::notifier_t notifier) const; + explicit Private(QIcon di, QUrl url = {}) + : _defaultIcon(di), _url(url) + { } + QImage get(Connection* connection, QSize size, + get_callback_t callback) const; + bool upload(UploadContentJob* job, upload_callback_t callback); - Connection* _connection; const QIcon _defaultIcon; QUrl _url; @@ -42,24 +45,52 @@ class Avatar::Private mutable std::vector> _scaledImages; mutable QSize _requestedSize; mutable bool _valid = false; - mutable QPointer _ongoingRequest = nullptr; - mutable std::vector notifiers; + mutable QPointer _thumbnailRequest = nullptr; + mutable QPointer _uploadRequest = nullptr; + mutable std::vector callbacks; + mutable get_callback_t uploadCallback; }; -Avatar::Avatar(Connection* connection, QIcon defaultIcon) - : d(new Private { connection, std::move(defaultIcon) }) +Avatar::Avatar(QIcon defaultIcon) + : d(std::make_unique(std::move(defaultIcon))) { } +Avatar::Avatar(QUrl url, QIcon defaultIcon) + : d(std::make_unique(std::move(defaultIcon), std::move(url))) +{ } + +Avatar::Avatar(Avatar&&) = default; + Avatar::~Avatar() = default; -QImage Avatar::get(int dimension, notifier_t notifier) const +Avatar& Avatar::operator=(Avatar&&) = default; + +QImage Avatar::get(Connection* connection, int dimension, + get_callback_t callback) const { - return d->get({dimension, dimension}, notifier); + return d->get(connection, {dimension, dimension}, callback); } -QImage Avatar::get(int width, int height, notifier_t notifier) const +QImage Avatar::get(Connection* connection, int width, int height, + get_callback_t callback) const { - return d->get({width, height}, notifier); + return d->get(connection, {width, height}, callback); +} + +bool Avatar::upload(Connection* connection, const QString& fileName, + upload_callback_t callback) const +{ + if (isJobRunning(d->_uploadRequest)) + return false; + return d->upload(connection->uploadFile(fileName), callback); +} + +bool Avatar::upload(Connection* connection, QIODevice* source, + upload_callback_t callback) const +{ + if (isJobRunning(d->_uploadRequest) || !source->isReadable()) + return false; + return d->upload(connection->uploadContent(source), callback); } QString Avatar::mediaId() const @@ -67,28 +98,29 @@ QString Avatar::mediaId() const return d->_url.authority() + d->_url.path(); } -QImage Avatar::Private::get(QSize size, Avatar::notifier_t notifier) const +QImage Avatar::Private::get(Connection* connection, QSize size, + get_callback_t callback) const { // 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) + if( ( !(_valid || _thumbnailRequest) || size.width() > _requestedSize.width() || size.height() > _requestedSize.height() ) && _url.isValid() ) { qCDebug(MAIN) << "Getting avatar from" << _url.toString(); _requestedSize = size; - if (isJobRunning(_ongoingRequest)) - _ongoingRequest->abandon(); - notifiers.emplace_back(std::move(notifier)); - _ongoingRequest = _connection->getThumbnail(_url, size); - QObject::connect( _ongoingRequest, &MediaThumbnailJob::success, [this] + if (isJobRunning(_thumbnailRequest)) + _thumbnailRequest->abandon(); + callbacks.emplace_back(std::move(callback)); + _thumbnailRequest = connection->getThumbnail(_url, size); + QObject::connect( _thumbnailRequest, &MediaThumbnailJob::success, [this] { _valid = true; - _originalImage = _ongoingRequest->scaledThumbnail(_requestedSize); + _originalImage = _thumbnailRequest->scaledThumbnail(_requestedSize); _scaledImages.clear(); - for (auto n: notifiers) + for (auto n: callbacks) n(); }); } @@ -111,6 +143,16 @@ QImage Avatar::Private::get(QSize size, Avatar::notifier_t notifier) const return result; } +bool Avatar::Private::upload(UploadContentJob* job, upload_callback_t callback) +{ + _uploadRequest = job; + if (!isJobRunning(_uploadRequest)) + return false; + _uploadRequest->connect(_uploadRequest, &BaseJob::success, + [job,callback] { callback(job->contentUri()); }); + return true; +} + QUrl Avatar::url() const { return d->_url; } bool Avatar::updateUrl(const QUrl& newUrl) @@ -127,6 +169,8 @@ bool Avatar::updateUrl(const QUrl& newUrl) } d->_url = newUrl; d->_valid = false; + if (isJobRunning(d->_thumbnailRequest)) + d->_thumbnailRequest->abandon(); return true; } diff --git a/avatar.h b/avatar.h index 4d476ea5..ecd5bc52 100644 --- a/avatar.h +++ b/avatar.h @@ -22,6 +22,7 @@ #include #include +#include namespace QMatrixClient { @@ -30,13 +31,24 @@ namespace QMatrixClient class Avatar { public: - explicit Avatar(Connection* connection, QIcon defaultIcon = {}); + explicit Avatar(QIcon defaultIcon = {}); + Avatar(QUrl url, QIcon defaultIcon = {}); + Avatar(Avatar&&); ~Avatar(); + Avatar& operator=(Avatar&&); - using notifier_t = std::function; + using get_callback_t = std::function; + using upload_callback_t = std::function; - QImage get(int dimension, notifier_t notifier) const; - QImage get(int w, int h, notifier_t notifier) const; + QImage get(Connection* connection, int dimension, + get_callback_t callback) const; + QImage get(Connection* connection, int w, int h, + get_callback_t callback) const; + + bool upload(Connection* connection, const QString& fileName, + upload_callback_t callback) const; + bool upload(Connection* connection, QIODevice* source, + upload_callback_t callback) const; QString mediaId() const; QUrl url() const; @@ -44,6 +56,6 @@ namespace QMatrixClient private: class Private; - QScopedPointer d; + std::unique_ptr d; }; } // namespace QMatrixClient diff --git a/room.cpp b/room.cpp index 73591c98..51c5f5cc 100644 --- a/room.cpp +++ b/room.cpp @@ -64,7 +64,7 @@ class Room::Private Private(Connection* c, QString id_, JoinState initialJoinState) : q(nullptr), connection(c), id(std::move(id_)) - , avatar(c), joinState(initialJoinState) + , joinState(initialJoinState) { } Room* q; @@ -287,7 +287,7 @@ QImage Room::avatar(int dimension) QImage Room::avatar(int width, int height) { if (!d->avatar.url().isEmpty()) - return d->avatar.get(width, height, [=] { emit avatarChanged(); }); + return d->avatar.get(connection(), width, height, [=] { emit avatarChanged(); }); // Use the other side's avatar for 1:1's if (d->membersMap.size() == 2) @@ -295,8 +295,8 @@ QImage Room::avatar(int width, int height) auto theOtherOneIt = d->membersMap.begin(); if (theOtherOneIt.value() == localUser()) ++theOtherOneIt; - return (*theOtherOneIt)->avatarObject() - .get(width, height, [=] { emit avatarChanged(); }); + return (*theOtherOneIt)->avatar(width, height, + [=] { emit avatarChanged(); }); } return {}; } diff --git a/user.cpp b/user.cpp index c80ec883..289f0bac 100644 --- a/user.cpp +++ b/user.cpp @@ -39,17 +39,15 @@ class User::Private public: Private(QString userId, Connection* connection) : userId(std::move(userId)), connection(connection) - , avatar(connection, QIcon::fromTheme(QStringLiteral("user-available"))) { } QString userId; QString name; QString bridged; Connection* connection; - Avatar avatar; - QPointer avatarUploadJob = nullptr; + Avatar avatar { QIcon::fromTheme(QStringLiteral("user-available")) }; - void setAvatar(UploadContentJob* job, User* q); + void setAvatar(QString contentUri, User* q); }; User::User(QString userId, Connection* connection) @@ -107,28 +105,20 @@ void User::rename(const QString& newName) bool User::setAvatar(const QString& fileName) { - if (isJobRunning(d->avatarUploadJob)) - return false; - d->setAvatar(d->connection->uploadFile(fileName), this); - return true; + return avatarObject().upload(d->connection, fileName, + [this] (QString contentUri) { d->setAvatar(contentUri, this); }); } bool User::setAvatar(QIODevice* source) { - if (isJobRunning(d->avatarUploadJob) || !source->isReadable()) - return false; - d->setAvatar(d->connection->uploadContent(source), this); - return true; + return avatarObject().upload(d->connection, source, + [this] (QString contentUri) { d->setAvatar(contentUri, this); }); } -void User::Private::setAvatar(UploadContentJob* job, User* q) +void User::Private::setAvatar(QString contentUri, User* q) { - avatarUploadJob = job; - connect(job, &BaseJob::success, q, [this,q] { - auto* j = connection->callApi( - userId, avatarUploadJob->contentUri()); - connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q); }); - }); + auto* j = connection->callApi(userId, contentUri); + connect(j, &BaseJob::success, q, [q] { emit q->avatarChanged(q); }); } QString User::displayname() const @@ -159,17 +149,23 @@ QImage User::avatar(int dimension) QImage User::avatar(int width, int height) { - return d->avatar.get(width, height, [=] { emit avatarChanged(this); }); + return avatar(width, height, []{}); +} + +QImage User::avatar(int width, int height, Avatar::get_callback_t callback) +{ + return avatarObject().get(d->connection, width, height, + [this,callback] { emit avatarChanged(this); callback(); }); } QString User::avatarMediaId() const { - return d->avatar.mediaId(); + return avatarObject().mediaId(); } QUrl User::avatarUrl() const { - return d->avatar.url(); + return avatarObject().url(); } void User::processEvent(Event* event) diff --git a/user.h b/user.h index 37977e08..fa85d778 100644 --- a/user.h +++ b/user.h @@ -84,7 +84,8 @@ namespace QMatrixClient const Avatar& avatarObject() const; Q_INVOKABLE QImage avatar(int dimension); - Q_INVOKABLE QImage avatar(int requestedWidth, int requestedHeight); + Q_INVOKABLE QImage avatar(int width, int height); + QImage avatar(int width, int height, Avatar::get_callback_t callback); QString avatarMediaId() const; QUrl avatarUrl() const; -- cgit v1.2.3