From b8dd6812fea501661d9a8eca94b125cc38f793f5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 4 Aug 2023 15:43:44 -0400 Subject: [PATCH] Handle devices going offline. --- synapse/handlers/presence.py | 89 ++++++++++++++++++++++++++++----- tests/handlers/test_presence.py | 21 +++++++- 2 files changed, 96 insertions(+), 14 deletions(-) diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index 02405a4847d4..a8663271f5e8 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -948,6 +948,35 @@ async def _handle_timeouts(self) -> None: ) self.external_process_last_updated_ms.pop(process_id) + # Check if any devices for these users should be discarded. + new_states = [] + for user_id in users_to_check: + user_devices = self.user_to_device_to_current_state.get(user_id, {}) + # Only keep the devices that have been seen recently. + new_user_devices = { + device_id: device + for device_id, device in user_devices.items() + if now - max(device.last_user_sync_ts, device.last_active_ts) + <= IDLE_TIMER + } + + # If any device has timed out, ensure that the presence state and status msg + # is up-to-date. + if len(user_devices) != len(new_user_devices): + # Note that we don't need to worry about resetting other information as + # it will also be the most up-to-date anyway. + presence, status_msg = _combine_device_states(new_user_devices.values()) + + new_states.append( + self.user_to_current_state.get( + user_id, UserPresenceState.default(user_id) + ).copy_and_replace(state=presence, status_msg=status_msg) + ) + + self.user_to_device_to_current_state[user_id] = new_user_devices + if new_states: + await self._update_states(new_states) + states = [ self.user_to_current_state.get(user_id, UserPresenceState.default(user_id)) for user_id in users_to_check @@ -1296,21 +1325,13 @@ async def set_state( if presence: device_state.status_msg = status_msg device_state.last_active_ts = self.clock.time_msec() + # TODO This would get set for non-syncs (also see above). device_state.last_user_sync_ts = self.clock.time_msec() # Based on (all) the user's devices calculate the new presence state. - presence_by_priority = { - PresenceState.BUSY: 4, - PresenceState.ONLINE: 3, - PresenceState.UNAVAILABLE: 2, - PresenceState.OFFLINE: 1, - } - for device_state in self.user_to_device_to_current_state[user_id].values(): - if ( - presence_by_priority[device_state.state] - > presence_by_priority[presence] - ): - presence = device_state.state + presence, status_msg = _combine_device_states( + self.user_to_device_to_current_state[user_id].values() + ) # The newly updated status as an amalgamation of all the device statuses. new_fields = {"state": presence} @@ -2072,6 +2093,50 @@ def handle_update( return new_state, persist_and_notify, federation_ping +PRESENCE_BY_PRIORITY = { + PresenceState.BUSY: 4, + PresenceState.ONLINE: 3, + PresenceState.UNAVAILABLE: 2, + PresenceState.OFFLINE: 1, +} + + +def _combine_device_states( + device_states: Iterable[UserDevicePresenceState], +) -> Tuple[str, Optional[str]]: + """ + Find the device to use presence information from. + + Orders devices by priority, then last_active_ts, then device ID. + + Args: + device_states: An iterable of device presence states + + Return: + A two-tuple of the combined presence information: + + * The new presence state + * The new status message + """ + + # Based on (all) the user's devices calculate the new presence state. + presence = PresenceState.OFFLINE + last_active_ts = -1 + status_msg = None + + # Find the device to use presen priority device based on the presence priority, but tie-break with how recently the device has synced. + for device_state in device_states: + if (PRESENCE_BY_PRIORITY[device_state.state], device_state.last_active_ts) > ( + PRESENCE_BY_PRIORITY[presence], + last_active_ts, + ): + presence = device_state.state + last_active_ts = device_state.last_active_ts + status_msg = device_state.status_msg + + return presence, status_msg + + async def get_interested_parties( store: DataStore, presence_router: PresenceRouter, states: List[UserPresenceState] ) -> Tuple[Dict[str, List[UserPresenceState]], Dict[str, List[UserPresenceState]]]: diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 31f8ee3787ad..41151de59a60 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -713,7 +713,9 @@ def test_set_presence_from_syncing_is_set(self) -> None: def test_set_presence_from_syncing_multi_device(self) -> None: """Test that presence is set to the highest priority of all devices.""" - user_id = "@test:server" + user_id = f"@test:{self.hs.config.server.server_name}" + + # Create 2 devices, the first is online, the second unavailable. self.get_success( self.presence_handler.user_syncing( @@ -721,18 +723,33 @@ def test_set_presence_from_syncing_multi_device(self) -> None: ) ) + # Add some time between syncs. + self.reactor.advance(10) + self.reactor.pump([0.1]) + self.get_success( self.presence_handler.user_syncing( user_id, "dev-2", True, PresenceState.UNAVAILABLE ) ) + # Online should win. state = self.get_success( self.presence_handler.get_state(UserID.from_string(user_id)) ) - # we should now be online self.assertEqual(state.state, PresenceState.ONLINE) + # Advance such that the first device should be discarded, then pump so + # _handle_timeouts function to called. + self.reactor.advance(IDLE_TIMER / 1000 - 10) + self.reactor.pump([5]) + + # Unavailable. + state = self.get_success( + self.presence_handler.get_state(UserID.from_string(user_id)) + ) + self.assertEqual(state.state, PresenceState.UNAVAILABLE) + def test_set_presence_from_syncing_keeps_status(self) -> None: """Test that presence set by syncing retains status message""" user_id = "@test:server"