Notify users, rather than rooms, of device list updates

[anoadragon453]

When a device list update occurs, Notifier.on_new_event is called with the key device_list. This method determines the set of users to notify of this update from both a collection of users and a collection of rooms.

Users are consolidated before notifying (waking up sync streams etc.) occurs. Notifier.rooms_to_users_state is used to translate from a given room ID to a set of users in the room. Those users are then merged into the list of those to notify.

This all works pretty well, until we hooked into this method to send EDUs to AS's:

synapse/synapse/notifier.py

Lines 461 to 472 in 610578e

	# Notify appservices.
	try:
	self.appservice_handler.notify_interested_services_ephemeral(
	stream_key,
	new_token,
	users,
	rooms,
	)
	except Exception:
	logger.exception(
	"Error notifying application services of ephemeral events"
	)

This method only receives the original collection of users, not the one derived from rooms. Eventually, it should accept a list of room IDs as well (#11152), but not in the case of device list updates.

Technically speaking, device list updates aren't tied to a room. We just use rooms to determine who to send our device list updates to.

Therefore, this PR aims to fix both problems:

• For device list changes, on_new_event should receive a collection of users, derived from the rooms the requester is in. Relevant users should then be notified.
• AppserviceHandler.notify_interested_services_ephemeral will automatically receive a complete list of users who should be notified, allowing us to send device list updates to AS's.

[clokep]

From our conversation over Matrix @anoadragon453 is going to attempt to add a test for this, if it seems non-trivial we'll re-evaluate.

[anoadragon453]

I wrote a test to check that device list updates come down to local users when they share a room in Complement at matrix-org/complement#305.

However part-way through I realised that there wouldn't be a simple way of verifying with Complement that the user stream would be updated properly. In Complement, we often check if something comes down /sync eventually - something we do to prevent race conditions.

But in this case we're trying to check that the current sync stream is woken up; something that you can really only do with a unit test.

So not wasted effort. But a unit test will be written up tomorrow instead.

[anoadragon453]

Closing this as per #11905 (comment).

[anoadragon453]

On further thought, this PR is what we want. It's just taken me a few times round in circles to reach that conclusion.

The PR description has been updated with a clearer overview of the changes and why they're needed.

[clokep]

I think this is OK.

My follow-up question is if on_new_event needs to take room IDs, but it seems to be used in my places still! 👍

[anoadragon453]

My follow-up question is if on_new_event needs to take room IDs, but it seems to be used in my places still!

Yes indeed, even for the notify_interested_services_ephemeral use case! For events that are tied to a room, the room ID should be included. This could then be used to notify appservices that explicitly subscribe to updates from a room, which they can do even if they haven't specified a user namespace.