Clarify that an appservice can be interested in local and remote users #14000
Conversation
… users `get_local_users_in_room` is way more performant since it looks at a single table (`local_current_membership`) and is looking through way less data since it only worries about the local users in the room instead of everyone in the room across the federation. Fix #13942 Related to: - #13605 - #13608 - #13606
MadLittleMods
added
the
T-Task
| def _notify_interested_services(self): | ||
| # This is normally set in `notify_interested_services` but we need to call the | ||
| # internal async version so the reactor gets pushed to completion. | ||
| self.hs.get_application_service_handler().current_max += 1 | ||
| self.get_success( | ||
| self.hs.get_application_service_handler()._notify_interested_services( | ||
| RoomStreamToken( | ||
| None, self.hs.get_application_service_handler().current_max | ||
| ) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
Is there a better, more clean way to do this dance?
Co-authored-by: Andrew Morgan <[email protected]>
Is there a legitimate use case for this though? I'm a bit wary as the behaviour will be different based on local or remote users. In particular, you'll only get stuff from remote users in rooms that the server is in, at which point you'll usually get that info as they'll be AS rooms. I just want to make sure that this is something we consciously want to support, rather than something that someone might want at some point, maybe. |
|
Thanks for questioning this @erikjohnston. After thinking about this more and asking in #matrix-spec, it indeed seems that there are no valid use cases for needing to specify a remote user in an application service's user namespace. By specifying local users, you will receive room events/presence updates from remote users anyhow. As @Half-Shot put it, defining remote users is merely a footgun for which an Application Service may assume that it'll receive all events sent by that user, even though it will only receive events in rooms that are shared with a local user. So by that logic, I'm fine with us defining (and limiting in the code) user namespaces to only cover local users. It would make sense to update the spec to include the same. Sorry for the run-around! |
|
Sounds great to me 👍 I'll get #13958 in shape again ⏩ |
Also created matrix-org/matrix-spec#1272 to track a spec clarification ⏩ |
Clarify that an appservice can be interested in local and remote users. Also adds tests that would actually fail if we attempted something like #13958 again.
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.Pull request includes a sign off(run the linters)