Merge branch 'kitsune-better-forget'

1 files changed, 31 insertions, 13 deletions

diff --git a/connection.cpp b/connection.cpp
index efe47f86..4ed7384f 100644
--- a/connection.cpp
+++ b/connection.cpp
@@ -59,6 +59,7 @@ class Connection::Private
         // separately so we should, e.g., keep objects for Invite and
         // Leave state of the same room.
         QHash<QPair<QString, bool>, Room*> roomMap;
+        QVector<QString> roomIdsToForget;
         QHash<QString, User*> userMap;
         QString userId;
 
@@ -256,6 +257,21 @@ void Connection::onSyncSuccess(SyncData &&data) {
     d->data->setLastEvent(data.nextBatch());
     for( auto&& roomData: data.takeRoomData() )
     {
+        const auto forgetIdx = d->roomIdsToForget.indexOf(roomData.roomId);
+        if (forgetIdx != -1)
+        {
+            d->roomIdsToForget.removeAt(forgetIdx);
+            if (roomData.joinState == JoinState::Leave)
+            {
+                qDebug(MAIN) << "Room" << roomData.roomId
+                    << "has been forgotten, ignoring /sync response for it";
+                continue;
+            }
+            qWarning(MAIN) << "Room" << roomData.roomId
+                 << "has just been forgotten but /sync returned it in"
+                 << toCString(roomData.joinState)
+                 << "state - suspiciously fast turnaround";
+        }
         if ( auto* r = provideRoom(roomData.roomId, roomData.joinState) )
             r->updateData(std::move(roomData));
     }
@@ -377,20 +393,26 @@ ForgetRoomJob* Connection::forgetRoom(const QString& id)
     // a ForgetRoomJob is created in advance and can be returned in a probably
     // not-yet-started state (it will start once /leave completes).
     auto forgetJob = new ForgetRoomJob(id);
-    auto joinedRoom = d->roomMap.value({id, false});
-    if (joinedRoom && joinedRoom->joinState() == JoinState::Join)
+    auto room = d->roomMap.value({id, false});
+    if (!room)
+        room = d->roomMap.value({id, true});
+    if (room && room->joinState() != JoinState::Leave)
     {
-        auto leaveJob = joinedRoom->leaveRoom();
-        connect(leaveJob, &BaseJob::success,
-                this, [this, forgetJob] { forgetJob->start(connectionData()); });
+        auto leaveJob = room->leaveRoom();
+        connect(leaveJob, &BaseJob::success, this, [this, forgetJob, room] {
+            forgetJob->start(connectionData());
+            // If the matching /sync response hasn't arrived yet, mark the room
+            // for explicit deletion
+            if (room->joinState() != JoinState::Leave)
+                d->roomIdsToForget.push_back(room->id());
+        });
         connect(leaveJob, &BaseJob::failure, forgetJob, &BaseJob::abandon);
     }
     else
         forgetJob->start(connectionData());
-    connect(forgetJob, &BaseJob::success, this, [this, &id]
+    connect(forgetJob, &BaseJob::success, this, [this, id]
     {
-        // If the room happens to be in the map (possible in both forms),
-        // delete the found object(s).
+        // If the room is in the map (possibly in both forms), delete all forms.
         for (auto f: {false, true})
             if (auto r = d->roomMap.take({ id, f }))
             {
@@ -477,11 +499,7 @@ const ConnectionData* Connection::connectionData() const
 Room* Connection::provideRoom(const QString& id, JoinState joinState)
 {
     // TODO: This whole function is a strong case for a RoomManager class.
-    if (id.isEmpty())
-    {
-        qCDebug(MAIN) << "Connection::provideRoom() with empty id, doing nothing";
-        return nullptr;
-    }
+    Q_ASSERT_X(!id.isEmpty(), __FUNCTION__, "Empty room id");
 
     const auto roomKey = qMakePair(id, joinState == JoinState::Invite);
     auto* room = d->roomMap.value(roomKey, nullptr);