From b385baadc8e73ff3c499a0111e2a553d35dd29b6 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Mar 2018 12:30:23 -0700 Subject: Direct chat (un)marking: update internal structure synchronously The asynchronous update first implemented was more verbose and caused more problems than provided solutions. The idea was that the internal directChats map would better reflect the server state if updated asynchronously. However, it also causes a local race condition; e.g., to quickly remove rooms from direct chats one after another becomes very non-trivial (one has to wait until the previous operation succeeds). So after some playing with the code, hitting pitfalls along the way, I decided to align the logic with the one for room tags; synchronously issued signals look uglyish but at least work predictably. And race conditions between several clients generally cannot be cleanly resolved anyway. --- connection.cpp | 50 ++++++++++++++++++++++++-------------------------- 1 file changed, 24 insertions(+), 26 deletions(-) (limited to 'connection.cpp') diff --git a/connection.cpp b/connection.cpp index df9fd35f..b32f38ea 100644 --- a/connection.cpp +++ b/connection.cpp @@ -80,7 +80,7 @@ class Connection::Private void connectWithToken(const QString& user, const QString& accessToken, const QString& deviceId); - void applyDirectChatUpdates(const DirectChatsMap& newMap); + void broadcastDirectChatUpdates(); }; Connection::Connection(const QUrl& server, QObject* parent) @@ -655,16 +655,11 @@ QJsonObject toJson(const DirectChatsMap& directChats) return json; } -void Connection::Private::applyDirectChatUpdates(const DirectChatsMap& newMap) +void Connection::Private::broadcastDirectChatUpdates() { - auto j = q->callApi(userId, "m.direct", toJson(newMap)); - connect(j, &BaseJob::success, q, [this, newMap] { - if (directChats != newMap) - { - directChats = newMap; - emit q->directChatsListChanged(); - } - }); + auto j = q->callApi(userId, QStringLiteral("m.direct"), + toJson(directChats)); + emit q->directChatsListChanged(); } void Connection::addToDirectChats(const Room* room, const User* user) @@ -672,29 +667,32 @@ void Connection::addToDirectChats(const Room* room, const User* user) Q_ASSERT(room != nullptr && user != nullptr); if (d->directChats.contains(user, room->id())) return; - auto newMap = d->directChats; - newMap.insert(user, room->id()); - d->applyDirectChatUpdates(newMap); + d->directChats.insert(user, room->id()); + d->broadcastDirectChatUpdates(); } -void Connection::removeFromDirectChats(const Room* room, const User* user) +void Connection::removeFromDirectChats(const QString& roomId, const User* user) { - Q_ASSERT(room != nullptr); - if ((user != nullptr && !d->directChats.contains(user, room->id())) || - d->directChats.key(room->id()) == nullptr) + Q_ASSERT(!roomId.isEmpty()); + if ((user != nullptr && !d->directChats.contains(user, roomId)) || + d->directChats.key(roomId) == nullptr) return; - DirectChatsMap newMap; - for (auto it = d->directChats.begin(); it != d->directChats.end(); ++it) - { - if (it.value() != room->id() || (user != nullptr && it.key() != user)) - newMap.insert(it.key(), it.value()); - } - d->applyDirectChatUpdates(newMap); + if (user != nullptr) + d->directChats.remove(user, roomId); + else + for (auto it = d->directChats.begin(); it != d->directChats.end();) + { + if (it.value() == roomId) + it = d->directChats.erase(it); + else + ++it; + } + d->broadcastDirectChatUpdates(); } -bool Connection::isDirectChat(const Room* room) const +bool Connection::isDirectChat(const QString& roomId) const { - return d->directChats.key(room->id()) != nullptr; + return d->directChats.key(roomId) != nullptr; } QMap Connection::users() const -- cgit v1.2.3 From 381ab25563cce26be8e3983b3fb3b8090385a766 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Mon, 26 Mar 2018 12:35:49 +0900 Subject: Connection::doInDirectChat() and refactored direct chat requesting logic Basically, the whole requestDirectChat() body has been moved and generalised to doInDirectChat(), and requestDirectChat() delegates to doInDirectChat(). The logic has been updated to cope with formerly left/forgotten rooms present in the list of direct chats (cleaning up the list along the way). --- connection.cpp | 65 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 36 insertions(+), 29 deletions(-) (limited to 'connection.cpp') diff --git a/connection.cpp b/connection.cpp index b32f38ea..2a748eb1 100644 --- a/connection.cpp +++ b/connection.cpp @@ -436,38 +436,45 @@ CreateRoomJob* Connection::createRoom(RoomVisibility visibility, void Connection::requestDirectChat(const QString& userId) { - auto roomId = d->directChats.value(user(userId)); - if (roomId.isEmpty()) - { - auto j = createDirectChat(userId); - connect(j, &BaseJob::success, this, [this,j,userId,roomId] { - qCDebug(MAIN) << "Direct chat with" << userId - << "has been created as" << roomId; - emit directChatAvailable(roomMap().value({j->roomId(), false})); - }); - return; - } + doInDirectChat(userId, [this] (Room* r) { emit directChatAvailable(r); }); +} - auto room = roomMap().value({roomId, false}, nullptr); - if (room) - { - Q_ASSERT(room->id() == roomId); - qCDebug(MAIN) << "Requested direct chat with" << userId - << "is already available as" << room->id(); - emit directChatAvailable(room); - return; - } - room = roomMap().value({roomId, true}, nullptr); - if (room) +void Connection::doInDirectChat(const QString& userId, + std::function operation) +{ + // There can be more than one DC; find the first valid, and delete invalid + // (left/forgotten) ones along the way. + for (auto roomId: d->directChats.values(user(userId))) { - Q_ASSERT(room->id() == roomId); - auto j = joinRoom(room->id()); - connect(j, &BaseJob::success, this, [this,j,roomId,userId] { - qCDebug(MAIN) << "Joined the already invited direct chat with" - << userId << "as" << roomId; - emit directChatAvailable(roomMap().value({roomId, false})); - }); + if (auto r = room(roomId, JoinState::Join)) + { + Q_ASSERT(r->id() == roomId); + qCDebug(MAIN) << "Requested direct chat with" << userId + << "is already available as" << r->id(); + operation(r); + return; + } + if (auto ir = invitation(roomId)) + { + Q_ASSERT(ir->id() == roomId); + auto j = joinRoom(ir->id()); + connect(j, &BaseJob::success, this, [this,roomId,userId,operation] { + qCDebug(MAIN) << "Joined the already invited direct chat with" + << userId << "as" << roomId; + operation(room(roomId, JoinState::Join)); + }); + } + qCWarning(MAIN) << "Direct chat with" << userId << "known as room" + << roomId << "is not valid, discarding it"; + removeFromDirectChats(roomId); } + + auto j = createDirectChat(userId); + connect(j, &BaseJob::success, this, [this,j,userId,operation] { + qCDebug(MAIN) << "Direct chat with" << userId + << "has been created as" << j->roomId(); + operation(room(j->roomId(), JoinState::Join)); + }); } CreateRoomJob* Connection::createDirectChat(const QString& userId, -- cgit v1.2.3 From a73bb17418a6dfea556a837c48463e454c8db130 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sun, 25 Mar 2018 22:06:46 -0700 Subject: Minor cleanup --- connection.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'connection.cpp') diff --git a/connection.cpp b/connection.cpp index 2a748eb1..db8bb085 100644 --- a/connection.cpp +++ b/connection.cpp @@ -664,8 +664,8 @@ QJsonObject toJson(const DirectChatsMap& directChats) void Connection::Private::broadcastDirectChatUpdates() { - auto j = q->callApi(userId, QStringLiteral("m.direct"), - toJson(directChats)); + q->callApi(userId, QStringLiteral("m.direct"), + toJson(directChats)); emit q->directChatsListChanged(); } -- cgit v1.2.3