From 24e3d967d7c9029147202e10385b0b8e8881e4d9 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 9 Sep 2017 20:17:42 +0900 Subject: Kicking, inviting, exposing rooms in Invite state Kicking and inviting use generated job classes. Rooms in Invite state are stored separately in the hash from those in Join/Leave state because The Spec says so. For clients, this means that the same room may appear twice in the rooms map if it's been left and then the user was again invited to it. The code in Quaternion that properly processes this will arrive shortly. --- connection.cpp | 69 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 27 deletions(-) (limited to 'connection.cpp') diff --git a/connection.cpp b/connection.cpp index 2c9ee88a..5d8a42e3 100644 --- a/connection.cpp +++ b/connection.cpp @@ -50,7 +50,11 @@ class Connection::Private Connection* q; ConnectionData* data; - QHash roomMap; + // A complex key below is a pair of room name and whether its + // state is Invited. The spec mandates to keep Invited room state + // separately so we should, e.g., keep objects for Invite and + // Leave state of the same room. + QHash, Room*> roomMap; QHash userMap; QString username; QString password; @@ -160,7 +164,7 @@ void Connection::sync(int timeout) d->data->setLastEvent(job->nextBatch()); for( auto&& roomData: job->takeRoomData() ) { - if ( auto* r = provideRoom(roomData.roomId) ) + if ( auto* r = provideRoom(roomData.roomId, roomData.joinState) ) r->updateData(std::move(roomData)); } d->syncJob = nullptr; @@ -197,20 +201,12 @@ PostReceiptJob* Connection::postReceipt(Room* room, RoomEvent* event) const JoinRoomJob* Connection::joinRoom(const QString& roomAlias) { - auto job = callApi(roomAlias); - connect( job, &BaseJob::success, [=] () { - if ( Room* r = provideRoom(job->roomId()) ) - emit joinedRoom(r); - }); - return job; + return callApi(roomAlias); } void Connection::leaveRoom(Room* room) { - auto job = callApi(room->id()); - connect( job, &BaseJob::success, [=] () { - emit leftRoom(room); - }); + callApi(room->id()); } RoomMessagesJob* Connection::getMessages(Room* room, const QString& from) const @@ -275,7 +271,7 @@ int Connection::millisToReconnect() const return d->syncJob ? d->syncJob->millisToRetry() : 0; } -QHash< QString, Room* > Connection::roomMap() const +const QHash< QPair, Room* >& Connection::roomMap() const { return d->roomMap; } @@ -285,36 +281,55 @@ const ConnectionData* Connection::connectionData() const return d->data; } -Room* Connection::provideRoom(const QString& id) +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; } - if (d->roomMap.contains(id)) - return d->roomMap.value(id); - - // Not yet in the map, create a new one. - auto* room = createRoom(this, id); - if (room) + const auto roomKey = qMakePair(id, joinState == JoinState::Invite); + auto* room = d->roomMap.value(roomKey, nullptr); + if (!room) { - d->roomMap.insert( id, room ); + room = createRoom(this, id, joinState); + if (!room) + { + qCritical() << "Failed to create a room!!!" << id; + return nullptr; + } + qCDebug(MAIN) << "Created Room" << id << ", invited:" << roomKey.second; + + d->roomMap.insert(roomKey, room); emit newRoom(room); - } else { - qCritical() << "Failed to create a room!!!" << id; + } + else if (room->joinState() != joinState) + { + room->setJoinState(joinState); + if (joinState == JoinState::Leave) + emit leftRoom(room); + else if (joinState == JoinState::Join) + emit joinedRoom(room); + } + + if (joinState != JoinState::Invite && d->roomMap.contains({id, true})) + { + // Preempt the Invite room after it's been acted upon (joined or left). + qCDebug(MAIN) << "Deleting invited state"; + delete d->roomMap.take({id, true}); } return room; } -std::function Connection::createRoom = - [](Connection* c, const QString& id) { return new Room(c, id); }; +Connection::room_factory_t Connection::createRoom = + [](Connection* c, const QString& id, JoinState joinState) + { return new Room(c, id, joinState); }; -std::function Connection::createUser = +Connection::user_factory_t Connection::createUser = [](Connection* c, const QString& id) { return new User(id, c); }; - QByteArray Connection::generateTxnId() { return d->data->generateTxnId(); -- cgit v1.2.3 From 6b40c313f8e4a0964e857973d8f9636a9f833f9d Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Thu, 14 Sep 2017 20:19:41 +0900 Subject: Better API for clients to catch up on room list changes joinedRoom() and leftRoom() now pass the preempted Invite state of the room as well; roomMap() only returns Invite and Join rooms, not Leave. --- connection.cpp | 57 ++++++++++++++++++++++++++++++++++++++++++--------------- connection.h | 7 ++++--- 2 files changed, 46 insertions(+), 18 deletions(-) (limited to 'connection.cpp') diff --git a/connection.cpp b/connection.cpp index 5d8a42e3..fdf58e9e 100644 --- a/connection.cpp +++ b/connection.cpp @@ -271,9 +271,18 @@ int Connection::millisToReconnect() const return d->syncJob ? d->syncJob->millisToRetry() : 0; } -const QHash< QPair, Room* >& Connection::roomMap() const +QHash< QPair, Room* > Connection::roomMap() const { - return d->roomMap; + // Copy-on-write-and-remove-elements is faster than copying elements one by one. + QHash< QPair, Room* > roomMap = d->roomMap; + for (auto it = roomMap.begin(); it != roomMap.end(); ) + { + if (it.value()->joinState() == JoinState::Leave) + it = roomMap.erase(it); + else + ++it; + } + return roomMap; } const ConnectionData* Connection::connectionData() const @@ -290,6 +299,23 @@ Room* Connection::provideRoom(const QString& id, JoinState joinState) return nullptr; } + // Room transitions from the Connection standpoint: + // - none -> (new) Invite + // - none -> (new) Join + // - none -> (new) Leave + // - Invite -> (new) Join replaces Invite (deleted) + // - Invite -> (new) Leave (archived) replaces Invite (deleted) + // - Join -> (moves to) Leave + // - Leave -> (new) Invite, Leave + // - Leave -> (moves to) Join + // Room transitions from the user's standpoint (what's seen in signals): + // - none -> Invite: newRoom(Invite) + // - none -> Join: newRoom(Join) or Room::joinStateChanged(Join); joinedRoom + // - Invite -> Invite replaced with Join: + // newRoom(Join); joinedRoom; aboutToDeleteRoom(Invite) + // - Invite -> Invite replaced with Leave (none): + // newRoom(Leave); leftRoom; aboutToDeleteRoom(Invite) + // - Join -> Leave (none): leftRoom const auto roomKey = qMakePair(id, joinState == JoinState::Invite); auto* room = d->roomMap.value(roomKey, nullptr); if (!room) @@ -297,7 +323,7 @@ Room* Connection::provideRoom(const QString& id, JoinState joinState) room = createRoom(this, id, joinState); if (!room) { - qCritical() << "Failed to create a room!!!" << id; + qCCritical(MAIN) << "Failed to create a room" << id; return nullptr; } qCDebug(MAIN) << "Created Room" << id << ", invited:" << roomKey.second; @@ -305,20 +331,21 @@ Room* Connection::provideRoom(const QString& id, JoinState joinState) d->roomMap.insert(roomKey, room); emit newRoom(room); } - else if (room->joinState() != joinState) - { - room->setJoinState(joinState); - if (joinState == JoinState::Leave) - emit leftRoom(room); - else if (joinState == JoinState::Join) - emit joinedRoom(room); - } - if (joinState != JoinState::Invite && d->roomMap.contains({id, true})) + if (joinState != JoinState::Invite) { - // Preempt the Invite room after it's been acted upon (joined or left). - qCDebug(MAIN) << "Deleting invited state"; - delete d->roomMap.take({id, true}); + // Preempt the Invite room (if any) with a room in Join/Leave state. + auto prevInvite = d->roomMap.take({id, true}); + if (joinState == JoinState::Join) + joinedRoom(room, prevInvite); + else if (joinState == JoinState::Leave) + leftRoom(room, prevInvite); + if (prevInvite) + { + qCDebug(MAIN) << "Deleting Invite state for room" << prevInvite->id(); + emit aboutToDeleteRoom(prevInvite); + delete prevInvite; + } } return room; diff --git a/connection.h b/connection.h index b118ffb0..361e0870 100644 --- a/connection.h +++ b/connection.h @@ -52,7 +52,7 @@ namespace QMatrixClient Connection(); virtual ~Connection(); - const QHash, Room*>& roomMap() const; + QHash, Room*> roomMap() const; Q_INVOKABLE virtual void resolveServer(const QString& domain); Q_INVOKABLE virtual void connectToServer(const QString& user, @@ -128,8 +128,9 @@ namespace QMatrixClient void syncDone(); void newRoom(Room* room); - void joinedRoom(Room* room); - void leftRoom(Room* room); + void joinedRoom(Room* room, Room* prevInvite); + void leftRoom(Room* room, Room* prevInvite); + void aboutToDeleteRoom(Room* room); void loginError(QString error); void networkError(size_t nextAttempt, int inMilliseconds); -- cgit v1.2.3 From 726f8d464f4b29f6fd3dc92fa5493e239970b209 Mon Sep 17 00:00:00 2001 From: Kitsune Ral Date: Sat, 16 Sep 2017 20:39:36 +0900 Subject: provideRoom: Added invitedRoom() signal; fixed issues with some transitions Notably: * setJoinState() invocation has been missing from the previous code * processing invites did not take into account that a Leave state may already exist, thereby forcing clients that display left rooms to look through their records just in case they have to replace a Leave with Invite. * joinedRoom() was emitted even when the room is not newly joined. --- connection.cpp | 57 ++++++++++++++++++++++++++++++++------------------------- connection.h | 5 +++-- 2 files changed, 35 insertions(+), 27 deletions(-) (limited to 'connection.cpp') diff --git a/connection.cpp b/connection.cpp index fdf58e9e..513d9242 100644 --- a/connection.cpp +++ b/connection.cpp @@ -299,26 +299,28 @@ Room* Connection::provideRoom(const QString& id, JoinState joinState) return nullptr; } - // Room transitions from the Connection standpoint: - // - none -> (new) Invite - // - none -> (new) Join - // - none -> (new) Leave - // - Invite -> (new) Join replaces Invite (deleted) - // - Invite -> (new) Leave (archived) replaces Invite (deleted) - // - Join -> (moves to) Leave - // - Leave -> (new) Invite, Leave - // - Leave -> (moves to) Join - // Room transitions from the user's standpoint (what's seen in signals): - // - none -> Invite: newRoom(Invite) - // - none -> Join: newRoom(Join) or Room::joinStateChanged(Join); joinedRoom - // - Invite -> Invite replaced with Join: - // newRoom(Join); joinedRoom; aboutToDeleteRoom(Invite) - // - Invite -> Invite replaced with Leave (none): - // newRoom(Leave); leftRoom; aboutToDeleteRoom(Invite) - // - Join -> Leave (none): leftRoom + // Room transitions: + // 1. none -> Invite: r=createRoom, emit invitedRoom(r,null) + // 2. none -> Join: r=createRoom, emit joinedRoom(r,null) + // 3. none -> Leave: r=createRoom, emit leftRoom(r,null) + // 4. inv=Invite -> Join: r=createRoom, emit joinedRoom(r,inv), delete Invite + // 4a. Leave, inv=Invite -> Join: change state, emit joinedRoom(r,inv), delete Invite + // 5. inv=Invite -> Leave: r=createRoom, emit leftRoom(r,inv), delete Invite + // 5a. r=Leave, inv=Invite -> Leave: emit leftRoom(r,inv), delete Invite + // 6. Join -> Leave: change state + // 7. r=Leave -> Invite: inv=createRoom, emit invitedRoom(inv,r) + // 8. Leave -> (changes to) Join const auto roomKey = qMakePair(id, joinState == JoinState::Invite); auto* room = d->roomMap.value(roomKey, nullptr); - if (!room) + if (room) + { + // Leave is a special case because in transition (5a) above + // joinState == room->joinState but we still have to preempt the Invite + // and emit a signal. For Invite and Join, there's no such problem. + if (room->joinState() == joinState && joinState != JoinState::Leave) + return room; + } + else { room = createRoom(this, id, joinState); if (!room) @@ -326,20 +328,25 @@ Room* Connection::provideRoom(const QString& id, JoinState joinState) qCCritical(MAIN) << "Failed to create a room" << id; return nullptr; } - qCDebug(MAIN) << "Created Room" << id << ", invited:" << roomKey.second; - d->roomMap.insert(roomKey, room); + qCDebug(MAIN) << "Created Room" << id << ", invited:" << roomKey.second; emit newRoom(room); } - - if (joinState != JoinState::Invite) + if (joinState == JoinState::Invite) + { + // prev is either Leave or nullptr + auto* prev = d->roomMap.value({id, false}, nullptr); + emit invitedRoom(room, prev); + } + else { + room->setJoinState(joinState); // Preempt the Invite room (if any) with a room in Join/Leave state. - auto prevInvite = d->roomMap.take({id, true}); + auto* prevInvite = d->roomMap.take({id, true}); if (joinState == JoinState::Join) - joinedRoom(room, prevInvite); + emit joinedRoom(room, prevInvite); else if (joinState == JoinState::Leave) - leftRoom(room, prevInvite); + emit leftRoom(room, prevInvite); if (prevInvite) { qCDebug(MAIN) << "Deleting Invite state for room" << prevInvite->id(); diff --git a/connection.h b/connection.h index 361e0870..0dcb8c3f 100644 --- a/connection.h +++ b/connection.h @@ -128,8 +128,9 @@ namespace QMatrixClient void syncDone(); void newRoom(Room* room); - void joinedRoom(Room* room, Room* prevInvite); - void leftRoom(Room* room, Room* prevInvite); + void invitedRoom(Room* room, Room* prev); + void joinedRoom(Room* room, Room* prev); + void leftRoom(Room* room, Room* prev); void aboutToDeleteRoom(Room* room); void loginError(QString error); -- cgit v1.2.3