From 7b6ba76954f88558a638f174c68a87207fe4788d Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 13 Jan 2019 11:49:21 +0900 Subject: util.h: check for fallthrough attribute instead of C++ version --- lib/util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/util.h b/lib/util.h index 9c9a37ba..336248d3 100644 --- a/lib/util.h +++ b/lib/util.h @@ -27,7 +27,7 @@ #include #include -#if __cplusplus >= 201703L +#if __has_cpp_attribute(fallthrough) #define FALLTHROUGH [[fallthrough]] #elif __has_cpp_attribute(clang::fallthrough) #define FALLTHROUGH [[clang::fallthrough]] -- cgit v1.2.3 From d5c07b98cd708d0bf4590e7fd249aa972b090461 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 13 Jan 2019 13:32:04 +0900 Subject: Fix Omittables accidentally becoming non-omitted when compared with non-Omittable values --- lib/room.cpp | 5 +++-- lib/util.h | 21 ++++++++++++++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/room.cpp b/lib/room.cpp index 8f50607f..7ff8f5e9 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -97,7 +97,7 @@ class Room::Private Connection* connection; QString id; JoinState joinState; - RoomSummary summary; + RoomSummary summary = { none, 0, none }; /// The state of the room at timeline position before-0 /// \sa timelineBase std::unordered_map baseState; @@ -1065,7 +1065,8 @@ int Room::joinedCount() const int Room::invitedCount() const { // TODO: Store invited users in Room too - return d->summary.invitedMemberCount; + Q_ASSERT(!d->summary.invitedMemberCount.omitted()); + return d->summary.invitedMemberCount.value(); } int Room::totalMemberCount() const diff --git a/lib/util.h b/lib/util.h index 336248d3..77c2bfdb 100644 --- a/lib/util.h +++ b/lib/util.h @@ -107,6 +107,25 @@ namespace QMatrixClient return *this; } + bool operator==(const value_type& rhs) const + { + return !omitted() && value() == rhs; + } + friend bool operator==(const value_type& lhs, + const Omittable& rhs) + { + return rhs == lhs; + } + bool operator!=(const value_type& rhs) const + { + return !operator==(rhs); + } + friend bool operator!=(const value_type& lhs, + const Omittable& rhs) + { + return !(rhs == lhs); + } + bool omitted() const { return _omitted; } const value_type& value() const { @@ -136,7 +155,7 @@ namespace QMatrixClient } value_type&& release() { _omitted = true; return std::move(_value); } - operator value_type&() & { return editValue(); } + operator const value_type&() const & { return value(); } const value_type* operator->() const & { return &value(); } value_type* operator->() & { return &editValue(); } const value_type& operator*() const & { return value(); } -- cgit v1.2.3 From 8dc2a3273ac3a5bb518fa987b25e3df15106c4d2 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 13 Jan 2019 13:34:31 +0900 Subject: Connection::provideRoom: allow omitting join state --- lib/connection.cpp | 21 +++++++++++++++++---- lib/connection.h | 31 +++++++++++++++++++++---------- 2 files changed, 38 insertions(+), 14 deletions(-) diff --git a/lib/connection.cpp b/lib/connection.cpp index c17cbffc..18fa91e7 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -981,11 +981,12 @@ const ConnectionData* Connection::connectionData() const return d->data.get(); } -Room* Connection::provideRoom(const QString& id, JoinState joinState) +Room* Connection::provideRoom(const QString& id, Omittable joinState) { // TODO: This whole function is a strong case for a RoomManager class. Q_ASSERT_X(!id.isEmpty(), __FUNCTION__, "Empty room id"); + // If joinState.omitted(), all joinState == comparisons below are false. const auto roomKey = qMakePair(id, joinState == JoinState::Invite); auto* room = d->roomMap.value(roomKey, nullptr); if (room) @@ -995,10 +996,19 @@ Room* Connection::provideRoom(const QString& id, JoinState joinState) // and emit a signal. For Invite and Join, there's no such problem. if (room->joinState() == joinState && joinState != JoinState::Leave) return room; + } else if (joinState.omitted()) + { + // No Join and Leave, maybe Invite? + room = d->roomMap.value({id, true}, nullptr); + if (room) + return room; + // No Invite either, setup a new room object below } - else + + if (!room) { - room = roomFactory()(this, id, joinState); + room = roomFactory()(this, id, + joinState.omitted() ? JoinState::Join : joinState.value()); if (!room) { qCCritical(MAIN) << "Failed to create a room" << id; @@ -1010,6 +1020,9 @@ Room* Connection::provideRoom(const QString& id, JoinState joinState) this, &Connection::aboutToDeleteRoom); emit newRoom(room); } + if (joinState.omitted()) + return room; + if (joinState == JoinState::Invite) { // prev is either Leave or nullptr @@ -1018,7 +1031,7 @@ Room* Connection::provideRoom(const QString& id, JoinState joinState) } else { - room->setJoinState(joinState); + room->setJoinState(joinState.value()); // Preempt the Invite room (if any) with a room in Join/Leave state. auto* prevInvite = d->roomMap.take({id, true}); if (joinState == JoinState::Join) diff --git a/lib/connection.h b/lib/connection.h index ff3e2028..f2e10488 100644 --- a/lib/connection.h +++ b/lib/connection.h @@ -670,16 +670,27 @@ namespace QMatrixClient */ const ConnectionData* connectionData() const; - /** - * @brief Find a (possibly new) Room object for the specified id - * Use this method whenever you need to find a Room object in - * the local list of rooms. Note that this does not interact with - * the server; in particular, does not automatically create rooms - * on the server. - * @return a pointer to a Room object with the specified id; nullptr - * if roomId is empty or roomFactory() failed to create a Room object. - */ - Room* provideRoom(const QString& roomId, JoinState joinState); + /** Get a Room object for the given id in the given state + * + * Use this method when you need a Room object in the local list + * of rooms, with the given state. Note that this does not interact + * with the server; in particular, does not automatically create + * rooms on the server. This call performs necessary join state + * transitions; e.g., if it finds a room in Invite but + * `joinState == JoinState::Join` then the Invite room object + * will be deleted and a new room object with Join state created. + * In contrast, switching between Join and Leave happens within + * the same object. + * \param roomId room id (not alias!) + * \param joinState desired (target) join state of the room; if + * omitted, any state will be found and return unchanged, or a + * new Join room created. + * @return a pointer to a Room object with the specified id and the + * specified state; nullptr if roomId is empty or if roomFactory() + * failed to create a Room object. + */ + Room* provideRoom(const QString& roomId, + Omittable joinState = none); /** * Completes loading sync data. -- cgit v1.2.3 From 3f39c30bffcb4ebc8eefe9bd9feb1b71b1c13981 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 13 Jan 2019 16:00:35 +0900 Subject: Room::Room: initialise display name So that the room has at least some display name before any events come to it. --- lib/room.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/room.cpp b/lib/room.cpp index 7ff8f5e9..1931be49 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -89,11 +89,6 @@ class Room::Private Room* q; - // This updates the room displayname field (which is the way a room - // should be shown in the room list) It should be called whenever the - // list of members or the room name (m.room.name) or canonical alias change. - void updateDisplayname(); - Connection* connection; QString id; JoinState joinState; @@ -179,6 +174,14 @@ class Room::Private void renameMember(User* u, QString oldName); void removeMemberFromMap(const QString& username, User* u); + // This updates the room displayname field (which is the way a room + // should be shown in the room list); called whenever the list of + // members, the room name (m.room.name) or canonical alias change. + void updateDisplayname(); + // This is used by updateDisplayname() but only calculates the new name + // without any updates. + QString calculateDisplayname() const; + /// A point in the timeline corresponding to baseState rev_iter_t timelineBase() const { return q->findInTimeline(-1); } @@ -278,7 +281,6 @@ class Room::Private template users_shortlist_t buildShortlist(const ContT& users) const; users_shortlist_t buildShortlist(const QStringList& userIds) const; - QString calculateDisplayname() const; bool isLocalUser(const User* u) const { @@ -293,6 +295,7 @@ Room::Room(Connection* connection, QString id, JoinState initialJoinState) // See "Accessing the Public Class" section in // https://marcmutz.wordpress.com/translated-articles/pimp-my-pimpl-%E2%80%94-reloaded/ d->q = this; + d->displayname = d->calculateDisplayname(); // Set initial "Empty room" name qCDebug(MAIN) << "New" << toCString(initialJoinState) << "Room:" << id; } -- cgit v1.2.3 From a9bdc89f66ba283859fd9ca7383a7256198174ed Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 13 Jan 2019 13:36:57 +0900 Subject: Connection: fix/workaround glitches on joining/leaving Closes #273, in particular. --- lib/connection.cpp | 26 +++++++++++++++++++++++--- lib/connection.h | 7 ++++--- lib/room.cpp | 3 ++- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/connection.cpp b/lib/connection.cpp index 18fa91e7..c582cf94 100644 --- a/lib/connection.cpp +++ b/lib/connection.cpp @@ -84,6 +84,7 @@ class Connection::Private QHash, Room*> roomMap; QVector roomIdsToForget; QVector firstTimeRooms; + QVector pendingStateRoomIds; QMap userMap; DirectChatsMap directChats; DirectChatUsersMap directChatUsers; @@ -339,6 +340,7 @@ void Connection::onSyncSuccess(SyncData &&data, bool fromCache) { } if ( auto* r = provideRoom(roomData.roomId, roomData.joinState) ) { + d->pendingStateRoomIds.removeOne(roomData.roomId); r->updateData(std::move(roomData), fromCache); if (d->firstTimeRooms.removeOne(r)) emit loadedRoomState(r); @@ -427,14 +429,32 @@ JoinRoomJob* Connection::joinRoom(const QString& roomAlias, const QStringList& serverNames) { auto job = callApi(roomAlias, serverNames); + // Upon completion, ensure a room object in Join state is created but only + // if it's not already there due to a sync completing earlier. connect(job, &JoinRoomJob::success, - this, [this, job] { provideRoom(job->roomId(), JoinState::Join); }); + this, [this, job] { provideRoom(job->roomId()); }); return job; } -void Connection::leaveRoom(Room* room) +LeaveRoomJob* Connection::leaveRoom(Room* room) { - callApi(room->id()); + const auto& roomId = room->id(); + const auto job = callApi(roomId); + if (room->joinState() == JoinState::Invite) + { + // Workaround matrix-org/synapse#2181 - if the room is in invite state + // the invite may have been cancelled but Synapse didn't send it in + // `/sync`. See also #273 for the discussion in the library context. + d->pendingStateRoomIds.push_back(roomId); + connect(job, &LeaveRoomJob::success, this, [this,roomId] { + if (d->pendingStateRoomIds.removeOne(roomId)) + { + qCDebug(MAIN) << "Forcing the room to Leave status"; + provideRoom(roomId, JoinState::Leave); + } + }); + } + return job; } inline auto splitMediaId(const QString& mediaId) diff --git a/lib/connection.h b/lib/connection.h index f2e10488..cba57e3d 100644 --- a/lib/connection.h +++ b/lib/connection.h @@ -48,6 +48,7 @@ namespace QMatrixClient class DownloadFileJob; class SendToDeviceJob; class SendMessageJob; + class LeaveRoomJob; /** Create a single-shot connection that triggers on the signal and * then self-disconnects @@ -494,14 +495,14 @@ namespace QMatrixClient SendMessageJob* sendMessage(const QString& roomId, const RoomEvent& event) const; + /** \deprecated Do not use this directly, use Room::leaveRoom() instead */ + virtual LeaveRoomJob* leaveRoom( Room* room ); + // Old API that will be abolished any time soon. DO NOT USE. /** @deprecated Use callApi() or Room::postReceipt() instead */ virtual PostReceiptJob* postReceipt(Room* room, RoomEvent* event) const; - /** @deprecated Use callApi() or Room::leaveRoom() instead */ - virtual void leaveRoom( Room* room ); - signals: /** * @deprecated diff --git a/lib/room.cpp b/lib/room.cpp index 1931be49..d806183f 100644 --- a/lib/room.cpp +++ b/lib/room.cpp @@ -1633,7 +1633,8 @@ void Room::inviteToRoom(const QString& memberId) LeaveRoomJob* Room::leaveRoom() { - return connection()->callApi(id()); + // FIXME, #63: It should be RoomManager, not Connection + return connection()->leaveRoom(this); } SetRoomStateWithKeyJob*Room::setMemberState(const QString& memberId, const RoomMemberEvent& event) const -- cgit v1.2.3