Use a single room per server-user pair for events #3910
Conversation
…realm-server-session
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cd147b2c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'JOIN realm_user_permissions rup', | ||
| 'ON rup.username = sr.matrix_user_id', | ||
| 'WHERE rup.realm_url =', | ||
| param(realmURL), | ||
| 'AND (rup.read = true OR rup.write = true)', |
There was a problem hiding this comment.
This query now only returns session rooms for users that have explicit realm_user_permissions rows (it joins on rup.username = sr.matrix_user_id). For realms that are world‑readable via the '*' permission row, users who only have public access will not match this join, so fetchAllSessionRooms returns no rooms for them. Because NodeAdapter.broadcastRealmEvent relies on this mapping, those public users will stop receiving realm events (e.g. incremental index invalidations) even though they can read the realm. This regression happens specifically for realms that are readable via '*' but have no per‑user permission rows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Yes. Only specifically mentioned users with read or write access are now going to be notified of updates. This is an expected and OK change. Pinging all users if we change catalog could be a huge issue, and should be solved in a different way so this drop of functionality is expected.
Preview deployments |
Host Test Results 1 files ± 0 1 suites ±0 5m 41s ⏱️ - 1h 35m 58s For more details on these failures and errors, see this check. Results for commit 7cd147b. ± Comparison against base commit 5a329a3. This pull request removes 1775 and adds 13 tests. Note that renamed tests count towards both. |
Currently we require that every realm has its own user added to matrix, and every realm-user pair has a unique room.
Instead, we can have a single server user, and a single dm room for events between the server and user.
This means that creating a new realm requires just creating a new realm, not a realm and user and room.