From de1f9a320bdabf4859a89d2686873aea622c1354 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 19 Aug 2017 17:58:57 +0900 Subject: Room::localUser() facility method --- room.cpp | 30 +++++++++++++++++++----------- room.h | 4 +++- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/room.cpp b/room.cpp index cfdd33ac..992411c0 100644 --- a/room.cpp +++ b/room.cpp @@ -128,6 +128,10 @@ class Room::Private void insertEvent(RoomEvent* e, Timeline::iterator where, TimelineItem::index_t index); + bool isLocalUser(const User* u) const + { + return u == connection->user(); + } }; Room::Room(Connection* connection, QString id) @@ -197,7 +201,7 @@ void Room::Private::setLastReadEvent(User* u, const QString& eventId) { lastReadEventIds.insert(u, eventId); emit q->lastReadEventChanged(u); - if (u == connection->user()) + if (isLocalUser(u)) emit q->readMarkerMoved(); } @@ -219,7 +223,7 @@ Room::Private::promoteReadMarker(User* u, Room::rev_iter_t newMarker) [=](const TimelineItem& ti) { return ti->senderId() != u->id(); }); setLastReadEvent(u, (*(eagerMarker - 1))->id()); - if (u == connection->user() && unreadMessages) + if (isLocalUser(u) && unreadMessages) { auto stillUnreadMessagesCount = count_if(eagerMarker, timeline.cend(), @@ -242,8 +246,7 @@ Room::Private::promoteReadMarker(User* u, Room::rev_iter_t newMarker) void Room::markMessagesAsRead(Room::rev_iter_t upToMarker) { - auto localUser = connection()->user(); - Private::rev_iter_pair_t markers = d->promoteReadMarker(localUser, upToMarker); + Private::rev_iter_pair_t markers = d->promoteReadMarker(localUser(), upToMarker); if (markers.first != markers.second) qCDebug(MAIN) << "Marked messages as read until" << *readMarker(); @@ -252,7 +255,7 @@ void Room::markMessagesAsRead(Room::rev_iter_t upToMarker) // until the previous last-read message, whichever comes first. for (; markers.second < markers.first; ++markers.second) { - if ((*markers.second)->senderId() != localUser->id()) + if ((*markers.second)->senderId() != localUser()->id()) { connection()->callApi(id(), (*markers.second)->id()); break; @@ -319,12 +322,12 @@ Room::rev_iter_t Room::readMarker(const User* user) const Room::rev_iter_t Room::readMarker() const { - return readMarker(connection()->user()); + return readMarker(localUser()); } QString Room::readMarkerEventId() const { - return d->lastReadEventIds.value(d->connection->user()); + return d->lastReadEventIds.value(localUser()); } int Room::notificationCount() const @@ -612,9 +615,15 @@ void Room::Private::dropDuplicateEvents(RoomEvents* events) const Connection* Room::connection() const { + Q_ASSERT(d->connection); return d->connection; } +User* Room::localUser() const +{ + return connection()->user(); +} + void Room::addNewMessageEvents(RoomEvents events) { d->dropDuplicateEvents(&events); @@ -718,7 +727,7 @@ void Room::processStateEvents(const RoomEvents& events) case EventType::RoomMember: { auto memberEvent = static_cast(event); // Can't use d->member() below because the user may be not a member (yet) - User* u = d->connection->user(memberEvent->userId()); + auto u = d->connection->user(memberEvent->userId()); u->processEvent(event); if( memberEvent->membership() == MembershipType::Join ) { @@ -810,12 +819,11 @@ QString Room::Private::roomNameFromMemberNames(const QList &userlist) co first_two.begin(), first_two.end(), [this](const User* u1, const User* u2) { // Filter out the "me" user so that it never hits the room name - return u2 == connection->user() || - (u1 != connection->user() && u1->id() < u2->id()); + return isLocalUser(u2) || (!isLocalUser(u1) && u1->id() < u2->id()); } ); - // i. One-on-one chat. first_two[1] == connection->user() in this case. + // i. One-on-one chat. first_two[1] == localUser() in this case. if (userlist.size() == 2) return q->roomMembername(first_two[0]); diff --git a/room.h b/room.h index 03827a55..a3eb189c 100644 --- a/room.h +++ b/room.h @@ -64,7 +64,8 @@ namespace QMatrixClient class Room: public QObject { Q_OBJECT - Q_PROPERTY(const Connection* connection READ connection CONSTANT) + Q_PROPERTY(Connection* connection READ connection CONSTANT) + Q_PROPERTY(User* localUser READ localUser CONSTANT) Q_PROPERTY(QString id READ id CONSTANT) Q_PROPERTY(QString name READ name NOTIFY namesChanged) Q_PROPERTY(QStringList aliases READ aliases NOTIFY namesChanged) @@ -80,6 +81,7 @@ namespace QMatrixClient virtual ~Room(); Connection* connection() const; + User* localUser() const; QString id() const; QString name() const; QStringList aliases() const; -- cgit v1.2.3 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(-) 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 72f7d49f89c14965be7d667f83a20c9d527708cb Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 28 Aug 2017 10:28:58 +0900 Subject: Use -pedantic with compilers that support it --- CMakeLists.txt | 6 +++++- logging.h | 2 +- util.h | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9254e029..5a345e06 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -23,7 +23,11 @@ endif() CHECK_CXX_COMPILER_FLAG("-Wall" WALL_FLAG_SUPPORTED) if ( WALL_FLAG_SUPPORTED AND NOT CMAKE_CXX_FLAGS MATCHES "(^| )-Wall($| )") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") -endif ( WALL_FLAG_SUPPORTED ) +endif ( ) +CHECK_CXX_COMPILER_FLAG("-Wpedantic" PEDANTIC_FLAG_SUPPORTED) +if ( PEDANTIC_FLAG_SUPPORTED AND NOT CMAKE_CXX_FLAGS MATCHES "(^| )pedantic($| )") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wpedantic") +endif ( ) if ( CMAKE_VERSION VERSION_LESS "3.1" ) CHECK_CXX_COMPILER_FLAG("-std=c++11" STD_FLAG_SUPPORTED) diff --git a/logging.h b/logging.h index e2a4c9a7..aaeceeac 100644 --- a/logging.h +++ b/logging.h @@ -50,7 +50,7 @@ namespace QMatrixClient #else return debug_object.noquote(); #endif - }; + } /** * @brief A helper operator to facilitate usage of formatJson (and possibly diff --git a/util.h b/util.h index c9e2d1c9..1f9e3f0b 100644 --- a/util.h +++ b/util.h @@ -232,7 +232,7 @@ namespace QMatrixClient Dispatch dispatch(ArgTs&& ... args) { return Dispatch(std::forward(args)...); - }; + } // The below enables pretty-printing of enums in logs #if (QT_VERSION >= QT_VERSION_CHECK(5, 5, 0)) -- cgit v1.2.3 From 3ce6dd3db9480b017f89a977d4333f01f25f2cff Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 19 Jun 2017 19:40:00 +0900 Subject: Better documentation Thanks to CII Best Practices Badge project for the hints and for the original CONTRIBUTING.md --- CONTRIBUTING.md | 200 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ README.md | 4 ++ 2 files changed, 204 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00000000..81c9d819 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,200 @@ +# Contributing + +Feedback and contributions are very welcome! + +Here's help on how to make contributions, divided into the following sections: + +* general information, +* vulnerability reporting, +* code changes, +* documentation, +* how to check proposed changes before submitting them, +* reuse (supply chain for third-party components, including updating them), and +* keeping up with external changes. + +## General information + +For specific proposals, please provide them as +[pull requests](https://github.com/QMatrixClient/libqmatrixclient/pulls) +or +[issues](https://github.com/QMatrixClient/libqmatrxclient/issues) +For general discussion, feel free to use our Matrix room: +[#quaternion:matrix.org](https://matrix.to/#/#quaternion:matrix.org). + +If you're new to the project (or FLOSS in general), +[issues tagged as simple](https://github.com/QMatrixClient/libqmatrixclient/labels/simple) +are smaller tasks that may typically take 1-3 days. +You are welcome aboard! + +### Pull requests and different branches recommended + +Pull requests are preferred, since they are specific. +See the GitHub Help [articles about pull requests](https://help.github.com/articles/using-pull-requests/) +to learn how to deal with them. + +We recommend creating different branches for different (logical) +changes, and creating a pull request when you're done into the master branch. +See the GitHub documentation on +[creating branches](https://help.github.com/articles/creating-and-deleting-branches-within-your-repository/) +and +[using pull requests](https://help.github.com/articles/using-pull-requests/). + +### How we handle proposals + +We use GitHub to track all changes via its +[issue tracker](https://github.com/QMatrixClient/libqmatrixclient/issues) and +[pull requests](https://github.com/QMatrixClient/libqmatrixclient/pulls). +Specific changes are proposed using those mechanisms. +Issues are assigned to an individual, who works it and then marks it complete. +If there are questions or objections, the conversation area of that +issue or pull request is used to resolve it. + + +### License + +Unless a contributor explicitly specifies otherwise, we assume that all contributed code is released under [the same license as libqmatrixclient itself](./COPYING), which is LGPL v2.1 as of the time of this writing. + + +Any components proposed for reuse should have a license that permits releasing a derivative work under LGPL v2.1. Moreover, the license of a proposed component should be approved by OSI, no exceptions. + +## Vulnerability reporting (security issues) + +If you find a significant vulnerability, or evidence of one, +use either of the following contacts: +* send an email to Kitsune Ral +* reach out in Matrix to #kitsune:matrix.org (with encryption switched on) + +In any of these two options, _indicate that you have such +information_ (do not share the information yet), and we'll tell you the next steps. + +We will gladly give credit to anyone who reports a vulnerability so that we can fix it. If you want to remain anonymous or pseudonymous instead, please let us know that; we will gladly respect your wishes. + +## Documentation changes + +Most of the documentation is in "markdown" format. +All markdown files use the .md filename extension. + +Where reasonable, limit yourself to Markdown +that will be accepted by different markdown processors +(e.g., what is specified by CommonMark or the original Markdown). +In practice we use the version of Markdown implemented by GitHub when it renders .md files, and you can use its extensions (in particular, mark code snippets with the programming language used). +This version of markdown is sometimes called +[GitHub-flavored markdown](https://help.github.com/articles/github-flavored-markdown/). +In particular, blank lines separate paragraphs; newlines inside a paragraph do *not* force a line break. +Beware - this is *not* the same markdown algorithm used by GitHub when it renders issue or pull comments; in those cases [newlines in paragraph-like content are considered as real line breaks](https://help.github.com/articles/writing-on-github/); unfortunately this other algorithm is *also* called GitHub rendered markdown. (Yes, it'd be better if there were standard different names +for different things.) + +In your markdown, please don't use tab characters and avoid "bare" URLs (in a hypertext link, the link text and URL should be on the same line). We do not care about the line length in markdown texts (and more often than not put the whole paragraph into one line). This is actually negotiable, and absolutely not enforceable. If you want to fit in a 70-character limit, go ahead, just don't reformat the whole text along the way. Take care to not break URLs when breaking lines. + +Do not use trailing two spaces for line breaks, since these cannot be seen and may be silently removed by some tools. Instead, use <br /> (an HTML break). + +## Code changes + +The code should strive to be DRY (don't repeat yourself), clear, and obviously correct. +Some technical debt is inevitable, just don't bankrupt us with it. +Improved refactorizations are welcome. + +Below are guidelines for specific languages. + +### Qt-flavoured C++ + +This is our primary language. We don't have a particular code style _as of yet_ but some rules-of-thumb are below: +* 4-space indents, no tabs, no trailing spaces, no last empty lines. If you spot abusing code in the existing code base - we'll thank you for fixing it. +* A notable deviation from the usual Qt code format conventions is an extra indent inside _classes_ (access specifiers go at +4 spaces to the base, members at +8 spaces) but not _structs_ (members at +4 spaces). +* While we are at it - please don't make hypocritical structs with protected or private members. Just do them classes instead. +* + +### Automated tests + +There's no testing framework as of now; either Catch or QTest will be used eventually (PRs welcome, just don't expect a quick merge of one - we'll hunt you down to actually write some tests first :-D ). + +### Security, privacy, and performance + +Pay attention to security, and work *with* (not against) the usual security hardening mechanisms (however few in C++). `char *` and similar unchecked C-style read/write arrays are forbidden - use Qt containers or at the very least std::array<> instead. +Protect private information, in particular passwords and email addresses. Do not forget about local access to data (in particular, be very careful when storing something in temporary files, let alone permanent configuration or state). +Avoid mechanisms that could be used for tracking where possible +(we do need to verify people are logged in for some operations), +and ensure that third parties can't use interactions for tracking. + +We want the software to have decent performance for typical users. And the same time we keep libqmatrixclient single-threaded as much as possible, to keep the code simple. That means being cautious about operation complexity (read about big-O notation if you need a kickstart on the topic). This especially refers to operations on the whole timeline - it can easily be several thousands elements long so even operations with linear complexity, if heavy enough, can produce noticeable GUI freezing. +Having said that, there's always a trade-off between various attributes; in particular, readability and maintainability of the code is more important than squeezing every bit out of that clumsy algorithm. + +## How to check proposed changes before submitting them + +Checking the code on at least one configuration is essential; if you only have a hasty fix that doesn't even compile, better make an issue and put a link to your commit into it (with an explanation what it is about and why). + +### Standard checks + +-Wall -pedantic is used with GCC and Clang. We don't turn those warnings to errors but please treat them as such. + +### Continuous Integration + +We use Travis CI to check buildability on Linux (GCC, Clang) and MacOS (Clang), and AppVeyor CI to build on Windows (MSVC). Every PR will go through these, and you'll see the traffic lights from them on the PR page. Failure on any platform will most likely entail a request to you for a fix before merging a PR. + +### Other tools + +If you know how to use clang-tidy, here's a list of checks we do and do not use (a leading hyphen means a disabled check, an asterisk is a wildcard): `*,cert-env33-c,-cppcoreguidelines-pro-bounds-array-to-pointer-decay,-cppcoreguidelines-pro-bounds-constant-array-index,-cppcoreguidelines-pro-bounds-pointer-arithmetic,-cppcoreguidelines-pro-type-const-cast,-cppcoreguidelines-pro-type-union-access,-cppcoreguidelines-special-member-functions,-google-build-using-namespace,-google-readability-braces-around-statements,-hicpp-*,-llvm-*,-misc-unused-parameters,-misc-noexcept-moveconstructor,-modernize-use-using,-readability-braces-around-statements,readability-identifier-naming,-readability-implicit-bool-cast,-clang-diagnostic-*,-clang-analyzer-*`. If you're on CLion (which makes clang-tidy usage a no-brainer), you can simple copy-paste the above list into the Clang-Tidy inspection configuration. + +## Git commit messages + +When writing git commit messages, try to follow the guidelines in +[How to Write a Git Commit Message](https://chris.beams.io/posts/git-commit/): + +1. Separate subject from body with a blank line +2. Be reasonable on the subject line length, because this is what we see in commit logs. Try to fit in 50 characters whenever possible. +3. Capitalize the subject line +4. Do not end the subject line with a period +5. Use the imperative mood in the subject line (*command* form) + (we don't always practice this ourselves but let's try). +6. Use the body to explain what and why vs. how + (git tracks how it was changed in detail, don't repeat that). Sometimes a quick overview of "how" is acceptable if a commit is huge - but maybe split a commit into smaller ones, to begin with? + +## Reuse (libraries, frameworks, etc.) + +C++ is unfortunately not very coherent about SDK/package management, and we try to keep building the library as easy as possible. Because of that we are very (and it means _very_) conservative about adding dependencies to libqmatrixclient. That relates to both additional Qt components and even more to other libraries. + +Regardless of the above paragraph (and as mentioned earlier in the text), we're now looking at possible options for automated testing, so PRs onboarding a test framework will be considered with much gratitude. Just don't forget to bootstrap with at least some tests on the existing codebase, rather than just throw in a Git submodule. + +Some cases need additional explanation: +* Before rolling out your own super-optimised container or algorithm written from scratch, take a good long look through documentation on Qt and C++ standard library (including the experimental/future sections). Please try to reuse the existing facilities as much as possible. +* You should have a very good reason (or better several ones) to add a component from KDE Frameworks. We don't rule this out and there's no prejudice against KDE; it just so happened that KDE Frameworks is one of most obvious reuse candidates for us but so far none of these components survived as libqmatrixclient deps. +* Never forget that libqmatrixclient is aimed to be a non-visual library; QtGui in dependencies is only driven by (entirely offscreen) dealing with QPixmaps. While there's a bunch of visual code (in C++ and QML) shared between libqmatrixclient-enabled _applications_, this is likely to end up in a separate (libqmatrixclient-enabled) library, rather than libqmatrixclient. + +## Attribution + +This text is largely based on CONTRIBUTING.md from CII Best Practices Badge project, which is a collective work of its contributors. The text itself is licensed under CC-BY-4.0. diff --git a/README.md b/README.md index b2536797..089ba492 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,8 @@ # Libqmatrixclient + +[![license](https://img.shields.io/github/license/QMatrixClient/libqmatrixclient.svg)](https://github.com/QMatrixClient/libqmatrixclient/blob/master/COPYING) +[![PRs Welcome](https://img.shields.io/badge/PRs-welcome-brightgreen.svg?style=flat-square)](http://makeapullrequest.com) + libqmatrixclient is a Qt-based library to make IM clients for the [Matrix](https://matrix.org) protocol. It is used by the Quaternion client and is a part of the Quaternion project. The below instructions are the same for Quaternion and libqmatrixclient (the source tree of Quaternion has most up-to-date instructions but this source tree strives to closely follow). ## Contacts -- 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(+) 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 b62149fdb0b5c247c075a1070bf4b75c1a3d057e Mon Sep 17 00:00:00 2001 From: Josip Delic Date: Fri, 1 Sep 2017 20:59:15 +0200 Subject: Add Connection::leftRoom signal --- connection.cpp | 5 ++++- connection.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/connection.cpp b/connection.cpp index f9f1490c..7920125d 100644 --- a/connection.cpp +++ b/connection.cpp @@ -207,7 +207,10 @@ JoinRoomJob* Connection::joinRoom(const QString& roomAlias) void Connection::leaveRoom(Room* room) { - callApi(room->id()); + auto job = callApi(room->id()); + connect( job, &BaseJob::success, [=] () { + emit leftRoom(room); + }); } RoomMessagesJob* Connection::getMessages(Room* room, const QString& from) const diff --git a/connection.h b/connection.h index e3f33155..0b8500b9 100644 --- a/connection.h +++ b/connection.h @@ -105,6 +105,7 @@ namespace QMatrixClient void syncDone(); void newRoom(Room* room); void joinedRoom(Room* room); + void leftRoom(Room* room); void loginError(QString error); void networkError(size_t nextAttempt, int inMilliseconds); -- cgit v1.2.3 From c0bd6c511b54eb9154e8f42427a62f5ac202cb4a Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 5 Sep 2017 18:34:58 +0900 Subject: Room: memberCount(); slight optimization and reformatting --- room.cpp | 12 ++++++++---- room.h | 1 + 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/room.cpp b/room.cpp index 992411c0..547b74c4 100644 --- a/room.cpp +++ b/room.cpp @@ -371,16 +371,20 @@ QList< User* > Room::users() const return d->membersMap.values(); } -QStringList Room::memberNames() const { +QStringList Room::memberNames() const +{ QStringList res; - - for (auto u : d->membersMap.values()) { + for (auto u : d->membersMap) res.append( this->roomMembername(u) ); - } return res; } +int Room::memberCount() const +{ + return d->membersMap.size(); +} + void Room::Private::insertMemberIntoMap(User *u) { auto namesakes = membersMap.values(u->name()); diff --git a/room.h b/room.h index a3eb189c..23a1412d 100644 --- a/room.h +++ b/room.h @@ -94,6 +94,7 @@ namespace QMatrixClient Q_INVOKABLE QList users() const; Q_INVOKABLE QStringList memberNames() const; + Q_INVOKABLE int memberCount() const; /** * @brief Produces a disambiguated name for a given user in -- 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(-) 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(-) 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 c4f690edf4cc2e11f19370f80ddd7428aba0b536 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 30 May 2017 08:56:59 +0900 Subject: Connection: Room and User factories are std::functions now Instead of createUser() and createRoom() virtual functions, use std::function<> to store predefined lambdas that would create respective descendants from User and Room, respectively. No more need QuaternionConnection just for the sake of creating a QuaternionRoom. --- connection.cpp | 16 ++++++---------- connection.h | 29 +++++++++++++++++++---------- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/connection.cpp b/connection.cpp index 7920125d..2c9ee88a 100644 --- a/connection.cpp +++ b/connection.cpp @@ -238,7 +238,7 @@ User* Connection::user(const QString& userId) { if( d->userMap.contains(userId) ) return d->userMap.value(userId); - User* user = createUser(userId); + auto* user = createUser(this, userId); d->userMap.insert(userId, user); return user; } @@ -297,7 +297,7 @@ Room* Connection::provideRoom(const QString& id) return d->roomMap.value(id); // Not yet in the map, create a new one. - Room* room = createRoom(id); + auto* room = createRoom(this, id); if (room) { d->roomMap.insert( id, room ); @@ -309,15 +309,11 @@ Room* Connection::provideRoom(const QString& id) return room; } -User* Connection::createUser(const QString& userId) -{ - return new User(userId, this); -} +std::function Connection::createRoom = + [](Connection* c, const QString& id) { return new Room(c, id); }; -Room* Connection::createRoom(const QString& roomId) -{ - return new Room(this, roomId); -} +std::function Connection::createUser = + [](Connection* c, const QString& id) { return new User(id, c); }; QByteArray Connection::generateTxnId() { diff --git a/connection.h b/connection.h index 0b8500b9..4b0413e3 100644 --- a/connection.h +++ b/connection.h @@ -22,6 +22,8 @@ #include #include +#include + namespace QMatrixClient { class Room; @@ -96,6 +98,20 @@ namespace QMatrixClient */ Q_INVOKABLE QByteArray generateTxnId(); + template + static void setRoomType() + { + createRoom = + [](Connection* c, const QString& id) { return new T(c, id); }; + } + + template + static void setUserType() + { + createUser = + [](Connection* c, const QString& id) { return new T(id, c); }; + } + signals: void resolved(); void connected(); @@ -130,18 +146,11 @@ namespace QMatrixClient */ Room* provideRoom(const QString& roomId); - /** - * makes it possible for derived classes to have its own User class - */ - virtual User* createUser(const QString& userId); - - /** - * makes it possible for derived classes to have its own Room class - */ - virtual Room* createRoom(const QString& roomId); - private: class Private; Private* d; + + static std::function createRoom; + static std::function createUser; }; } // namespace QMatrixClient -- 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(-) 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 be258954da33ea3f96fa947569bf617caae68452 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 19 Sep 2017 10:26:41 +0900 Subject: Connection: More deprecations; documented callApi<>() --- connection.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/connection.h b/connection.h index 4b0413e3..08d216d1 100644 --- a/connection.h +++ b/connection.h @@ -65,13 +65,16 @@ namespace QMatrixClient /** @deprecated Use callApi() or Room::postReceipt() instead */ Q_INVOKABLE virtual PostReceiptJob* postReceipt(Room* room, RoomEvent* event) const; + /** @deprecated Use callApi() instead */ Q_INVOKABLE virtual JoinRoomJob* joinRoom(const QString& roomAlias); /** @deprecated Use callApi() or Room::leaveRoom() instead */ Q_INVOKABLE virtual void leaveRoom( Room* room ); Q_INVOKABLE virtual RoomMessagesJob* getMessages(Room* room, const QString& from) const; + /** @deprecated Use callApi() instead */ virtual MediaThumbnailJob* getThumbnail(const QUrl& url, QSize requestedSize) const; + /** @deprecated Use callApi() instead */ MediaThumbnailJob* getThumbnail(const QUrl& url, int requestedWidth, int requestedHeight) const; @@ -85,6 +88,12 @@ namespace QMatrixClient Q_INVOKABLE SyncJob* syncJob() const; Q_INVOKABLE int millisToReconnect() const; + /** + * This is a universal method to start a job of a type passed + * as a template parameter. Arguments to callApi() are arguments + * to the job constructor _except_ the first ConnectionData* + * argument - callApi() will pass it automatically. + */ template JobT* callApi(JobArgTs... jobArgs) const { -- cgit v1.2.3 From 1b11a6ee708291db37b0c7879eb103d81d70a6b7 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 19 Sep 2017 10:10:18 +0900 Subject: Event::originalJsonObject(), RoomEvent validations commented out * Event::originalJsonObject() exposes the original JSON for the event without converting it to QByteArray. This is useful to quickly dump an event into a bigger JSON without reconstructing a JSON object. * Validations in RoomEvent::RoomEvent() do more harm than good. The rest of the library tolerates absence of those attributes pretty well (it wouldn't be able to do much with that anyway); at the same time, dumping JSON to logs turns out to be pretty heavy, and throwing many invalid events at a client is a good way to hit its performance. --- events/event.cpp | 35 ++++++++++++++++++++--------------- events/event.h | 1 + 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/events/event.cpp b/events/event.cpp index 8a6de822..d718306d 100644 --- a/events/event.cpp +++ b/events/event.cpp @@ -48,6 +48,11 @@ QByteArray Event::originalJson() const return QJsonDocument(_originalJson).toJson(); } +QJsonObject Event::originalJsonObject() const +{ + return _originalJson; +} + QDateTime Event::toTimestamp(const QJsonValue& v) { Q_ASSERT(v.isDouble() || v.isNull() || v.isUndefined()); @@ -97,21 +102,21 @@ RoomEvent::RoomEvent(Type type, const QJsonObject& rep) , _senderId(rep["sender"].toString()) , _txnId(rep["unsigned"].toObject().value("transactionId").toString()) { - if (_id.isEmpty()) - { - qCWarning(EVENTS) << "Can't find event_id in a room event"; - qCWarning(EVENTS) << formatJson << rep; - } - if (!rep.contains("origin_server_ts")) - { - qCWarning(EVENTS) << "Can't find server timestamp in a room event"; - qCWarning(EVENTS) << formatJson << rep; - } - if (_senderId.isEmpty()) - { - qCWarning(EVENTS) << "Can't find sender in a room event"; - qCWarning(EVENTS) << formatJson << rep; - } +// if (_id.isEmpty()) +// { +// qCWarning(EVENTS) << "Can't find event_id in a room event"; +// qCWarning(EVENTS) << formatJson << rep; +// } +// if (!rep.contains("origin_server_ts")) +// { +// qCWarning(EVENTS) << "Can't find server timestamp in a room event"; +// qCWarning(EVENTS) << formatJson << rep; +// } +// if (_senderId.isEmpty()) +// { +// qCWarning(EVENTS) << "Can't find sender in a room event"; +// qCWarning(EVENTS) << formatJson << rep; +// } if (!_txnId.isEmpty()) qCDebug(EVENTS) << "Event transactionId:" << _txnId; } diff --git a/events/event.h b/events/event.h index 8760aa28..7db14100 100644 --- a/events/event.h +++ b/events/event.h @@ -43,6 +43,7 @@ namespace QMatrixClient Type type() const { return _type; } QByteArray originalJson() const; + QJsonObject originalJsonObject() const; // According to the CS API spec, every event also has // a "content" object; but since its structure is different for -- cgit v1.2.3 From 4e23da3de66e425997506c75204a9e3ea22ccfa5 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Tue, 19 Sep 2017 10:43:10 +0900 Subject: Room: Slight optimization of makeErrorStr() The previous version constructed QString from const char* and QByteArray parts, only to convert it back to QByteArray; the current version does the whole thing in QByteArray terms. --- room.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/room.cpp b/room.cpp index 547b74c4..85953a5c 100644 --- a/room.cpp +++ b/room.cpp @@ -406,10 +406,9 @@ void Room::Private::removeMemberFromMap(const QString& username, User* u) emit q->memberRenamed(formerNamesakes[0]); } -inline QByteArray makeErrorStr(const Event* e, const char* msg) +inline QByteArray makeErrorStr(const Event* e, QByteArray msg) { - return QString("%1; event dump follows:\n%2") - .arg(msg, QString(e->originalJson())).toUtf8(); + return msg.append("; event dump follows:\n").append(e->originalJson()); } void Room::Private::insertEvent(RoomEvent* e, Timeline::iterator where, -- 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(-) 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(-) 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