Repository navigation
fix(chat): show typing only from participants of the current conversation - #6905
ToteMeiSter wants to merge 13 commits into
Conversation
The signaling connection of an account is shared by all conversations, but the session list was only filled by join events and never cleared when the room changed. It kept the sessions of earlier rooms, so typing messages went to sessions of other rooms, and typing messages from such sessions were shown as "Guest" because their name is unknown. The sessions are now held in RoomSessionRegistry. It is cleared when the server answers with another room, when the room is left, after a hello without resume, and on no_such_session / no_such_room. A resumed connection keeps it. The registry is thread safe, the own session is not a typing target, and a typing message is only accepted from a session of the current room. A real guest still joins without a user id and stays in the registry. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
…ssions After the connection left a room the room is unknown, so the answer to the leave (an empty room) clears the sessions as well. Join events of the left room which arrive between the leave and its answer are dropped with it. The registry tests are named after what they check. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
The leave event of the room now reaches the chat through the conversation message listener. A session which left cannot send a stop message any more (it is not a known session of the room), so its indicator stayed until the 15 second timeout. The typing participants are changed on the UI thread only, also when a stop message arrives. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 16.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 10 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: restricting typing indicators to participants in the current conversation. Description check The description is complete and follows the repository template. It explains the problem, cause, changes, tests, manual verification, TODO status, and checklist. The backport and milestone items remai… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description is complete and follows the repository template. It explains the problem, cause, changes, tests, manual verification, TODO status, and checklist. The backport and milestone items remain unchecked, but these are non-critical metadata items.
-
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
1c7b5586-699b-423c-b95c-264054a80196
📒 Files selected for processing: 10
app/src/main/java/com/nextcloud/talk/chat/ChatActivity.ktapp/src/main/java/com/nextcloud/talk/chat/MessageInputFragment.ktapp/src/main/java/com/nextcloud/talk/chat/TypingParticipant.ktapp/src/main/java/com/nextcloud/talk/signaling/ConversationMessageNotifier.ktapp/src/main/java/com/nextcloud/talk/signaling/SignalingMessageReceiver.ktapp/src/main/java/com/nextcloud/talk/webrtc/RoomSessionRegistry.ktapp/src/main/java/com/nextcloud/talk/webrtc/WebSocketInstance.ktapp/src/test/java/com/nextcloud/talk/chat/TypingParticipantTest.ktapp/src/test/java/com/nextcloud/talk/signaling/SignalingMessageReceiverSessionsLeftTest.ktapp/src/test/java/com/nextcloud/talk/webrtc/RoomSessionRegistryTest.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
…r types A user can be in the room with several signaling sessions (phone and laptop). The typing participant kept only the first session, so the leave event of that session hid the indicator although the second session still typed. The participant now holds all sessions which sent a typing message and is removed when the last of them left. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
… room After the connection left a room (or lost its session) the registry accepted a late join event of that room until the answer to the leave arrived. Such a session received our typing messages and passed the check of incoming typing messages. The server sends the answer to a join before the list of the sessions, so a join without a known room is always a late event of a room which was left. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
A user may type from several devices. A stoppedTyping message of one session removed the whole participant, and the single timer of the participant removed it 15 s after the last start of any session, so the indicator vanished while another session still typed. Every session now has its own 15 s timer and stoppedTyping forgets only its session. The participant is removed with its last session. Events without a session id are ignored, as they are not accepted anyway. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
da5969f1-13d9-4629-8c8f-b3b1801fc10f
📒 Files selected for processing: 5
app/src/main/java/com/nextcloud/talk/chat/ChatActivity.ktapp/src/main/java/com/nextcloud/talk/chat/TypingParticipant.ktapp/src/main/java/com/nextcloud/talk/webrtc/RoomSessionRegistry.ktapp/src/test/java/com/nextcloud/talk/chat/TypingParticipantTest.ktapp/src/test/java/com/nextcloud/talk/webrtc/RoomSessionRegistryTest.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.
|
I added one more commit: 1517c84 "fix(chat): stop and expire typing per session, not per participant". de29297 keeps all sessions of a typing user, but only the leave event used them. A Now every session has its own 15 s timer, and Tests: How to test: log in with the same user on two devices, open the conversation on a third device. Type on both devices, then stop typing on one of them. "… is typing" stays visible. Stop on the second device: the indicator disappears. |
…nRegistry reset() runs on the UI thread when a room is left, onJoin() on the OkHttp thread. onJoin() could pass its check of the room, reset() then cleared the registry, and onJoin() still stored its session. The session of the left room stayed until the answer to the leave arrived, so it received our typing messages and passed the check of incoming ones. onRoomResponse(), reset() and onJoin() now share one lock. Reads stay lock-free. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
The new test makes the class reach the detekt limit of functions, as in the other large test classes of the project. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
2320d593-5685-463c-9573-ca56421955b7
📒 Files selected for processing: 2
app/src/main/java/com/nextcloud/talk/webrtc/RoomSessionRegistry.ktapp/src/test/java/com/nextcloud/talk/webrtc/RoomSessionRegistryTest.kt
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.
…yTest Without the lock on onRoomResponse() no test failed: a join of the previous room could store its session after the answer for the next room cleared the registry. Share the paused-join setup between the reset() and the onRoomResponse() race tests. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
…istryTest fails Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
The constructor takes an optional client, event bus and connection helper; when the client and the bus are given, the injection through the application component is skipped. This lets a unit test run WebSocketInstance against a mock web server. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
…g frames The tests feed the frames of a signaling server into a real WebSocketInstance: switching the room, leaving, resume, a new session without resume, already_joined, no_such_session, no_such_room, the leave event and two accounts. They fail when the instance stops clearing the sessions on a room change or on a new session. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
…tInstance A join event of the room being left, sent before the answer to the leave, must neither make that session a typing sender nor a typing target; after the answer and the join of the next room only its sessions are known. Also fix four tests of the class which waited for a barrier join event: the registry ignores join events while the connection is in no room (bcb9d83), so they timed out. They wait for the leave event of an unknown session now. Assisted-by: Claude-Code:claude-sonnet-5-5 Signed-off-by: Krainov Gleb <krajnov.g@kontentplus.ru>
1 participant
Problem
A conversation shows "Guest is typing…". The person who types is not a guest. They type in another conversation.
Cause
WebSocketInstance(usersHashMap) was filled by join events and cleared only by leave events. It was not cleared when the room changed, when the room was left, after a hello without resume, or onno_such_session/no_such_room. So it kept the sessions of earlier rooms.MessageInputFragmentsentstartedTyping/stoppedTypingto every session in the map.sessionto that session in any room. In nextcloud-spreed-signaling,server/hub.goprocessMessageMsgchecks only "same backend" and "not to self". It does not check the room of the recipient.src/utils/SignalingTypingHandler.jsin spreed sends typing only to the participants of the current room and ignores typing from sessions which are not in that list.Changes
RoomSessionRegistryreplacesusersHashMap. It is thread safe. It is cleared when the server answers with another room, when the room is left, after a hello without resume, and onno_such_session/no_such_room. A resumed connection keeps it.WebSocketInstancegives a snapshot of the room sessions without the own session, and checks if a session is in the current room.MessageInputFragmentsends typing only to the sessions of the current room.ChatActivityaccepts typing only from a session of the current room. A real guest still joins without a user id, stays in the registry and is still shown as "Guest".SignalingMessageReceiverandConversationMessageNotifierpass the leave event to the chat listener (newonSessionsLeft, empty by default).TypingParticipantstores the session and can remove the participants of left sessions.ChatActivitychanges the typing participants on the UI thread only, also for a stop message.stoppedTypingand the timeout forget only that session; the participant is removed with its last session.Tests
RoomSessionRegistryTestTypingParticipantTestSignalingMessageReceiverSessionsLeftTestHow to test
Tested on a Huawei DEL-LX9 and a Samsung SM-S926B (Android 17): "Guest is typing…" from another conversation no longer appears.
🖼️ Screenshots
No screenshots. The UI change is the absence of a wrong typing line.
🚧 TODO
🏁 Checklist
/backport to stable-xx.x🤖 AI (if applicable)
The code and the tests were written by an AI agent (Claude Code, model claude-sonnet-5-5). I tested the change on a device and I am responsible for it. The commits carry the trailer
Assisted-by: Claude-Code:claude-sonnet-5-5. This description was also prepared with AI help and proofread by me.