Merge branch 'kitsune-workaround-stuck-invites'

4 files changed, 99 insertions, 30 deletions

diff --git a/lib/connection.cpp b/lib/connection.cpp
index c17cbffc..c582cf94 100644
--- a/lib/connection.cpp
+++ b/lib/connection.cpp
@@ -84,6 +84,7 @@ class Connection::Private
         QHash<QPair<QString, bool>, Room*> roomMap;
         QVector<QString> roomIdsToForget;
         QVector<Room*> firstTimeRooms;
+        QVector<QString> pendingStateRoomIds;
         QMap<QString, User*> 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<JoinRoomJob>(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<LeaveRoomJob>(room->id());
+    const auto& roomId = room->id();
+    const auto job = callApi<LeaveRoomJob>(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)
@@ -981,11 +1001,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> 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 +1016,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 +1040,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 +1051,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 98e8dced..9e4121f4 100644
--- a/lib/connection.h
+++ b/lib/connection.h
@@ -49,6 +49,7 @@ namespace QMatrixClient
     class DownloadFileJob;
     class SendToDeviceJob;
     class SendMessageJob;
+    class LeaveRoomJob;
 
     class Connection;
 
@@ -475,14 +476,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<PostReceiptJob>() or Room::postReceipt() instead */
             virtual PostReceiptJob* postReceipt(Room* room,
                                                 RoomEvent* event) const;
-            /** @deprecated Use callApi<LeaveRoomJob>() or Room::leaveRoom() instead */
-            virtual void leaveRoom( Room* room );
-
         signals:
             /**
              * @deprecated
@@ -651,16 +652,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> joinState = none);
 
             /**
              * Completes loading sync data.
diff --git a/lib/room.cpp b/lib/room.cpp
index 8f50607f..d806183f 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -89,15 +89,10 @@ 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;
-        RoomSummary summary;
+        RoomSummary summary = { none, 0, none };
         /// The state of the room at timeline position before-0
         /// \sa timelineBase
         std::unordered_map<StateEventKey, StateEventPtr> baseState;
@@ -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<typename ContT>
         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;
 }
 
@@ -1065,7 +1068,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
@@ -1629,7 +1633,8 @@ void Room::inviteToRoom(const QString& memberId)
 
 LeaveRoomJob* Room::leaveRoom()
 {
-    return connection()->callApi<LeaveRoomJob>(id());
+    // FIXME, #63: It should be RoomManager, not Connection
+    return connection()->leaveRoom(this);
 }
 
 SetRoomStateWithKeyJob*Room::setMemberState(const QString& memberId, const RoomMemberEvent& event) const
diff --git a/lib/util.h b/lib/util.h
index bae7f93f..ade6e8c2 100644
--- a/lib/util.h
+++ b/lib/util.h
@@ -108,6 +108,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<value_type>& rhs)
+            {
+                return rhs == lhs;
+            }
+            bool operator!=(const value_type& rhs) const
+            {
+                return !operator==(rhs);
+            }
+            friend bool operator!=(const value_type& lhs,
+                                   const Omittable<value_type>& rhs)
+            {
+                return !(rhs == lhs);
+            }
+
             bool omitted() const { return _omitted; }
             const value_type& value() const
             {
@@ -137,7 +156,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(); }