From 1e703b5f3c2540cfa08569247bdf0f054dd0aab8 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 15 Aug 2023 08:58:11 -0400 Subject: [PATCH] Properly bump the last_active_ts. --- synapse/api/presence.py | 13 +++++++ synapse/handlers/message.py | 9 +++-- synapse/handlers/presence.py | 56 +++++++++++++++++----------- synapse/replication/http/presence.py | 7 ++-- synapse/rest/client/read_marker.py | 4 +- synapse/rest/client/receipts.py | 4 +- synapse/rest/client/room.py | 4 +- 7 files changed, 67 insertions(+), 30 deletions(-) diff --git a/synapse/api/presence.py b/synapse/api/presence.py index 7b60c0f5bfce..35b14cd39e19 100644 --- a/synapse/api/presence.py +++ b/synapse/api/presence.py @@ -28,6 +28,19 @@ class UserDevicePresenceState: last_active_ts: int last_sync_ts: int + @classmethod + def default( + cls, user_id: str, device_id: Optional[str] + ) -> "UserDevicePresenceState": + """Returns a default presence state.""" + return cls( + user_id=user_id, + device_id=device_id, + state=PresenceState.OFFLINE, + last_active_ts=0, + last_sync_ts=0, + ) + @attr.s(slots=True, frozen=True, auto_attribs=True) class UserPresenceState: diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index d485f21e49f1..3885880f450b 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -1921,7 +1921,10 @@ async def persist_and_notify_client_events( # We don't want to block sending messages on any presence code. This # matters as sometimes presence code can take a while. run_as_background_process( - "bump_presence_active_time", self._bump_active_time, requester.user + "bump_presence_active_time", + self._bump_active_time, + requester.user, + requester.device_id, ) async def _notify() -> None: @@ -1958,10 +1961,10 @@ async def _maybe_kick_guest_users( logger.info("maybe_kick_guest_users %r", current_state) await self.hs.get_room_member_handler().kick_guest_users(current_state) - async def _bump_active_time(self, user: UserID) -> None: + async def _bump_active_time(self, user: UserID, device_id: Optional[str]) -> None: try: presence = self.hs.get_presence_handler() - await presence.bump_presence_active_time(user) + await presence.bump_presence_active_time(user, device_id) except Exception: logger.exception("Error bumping presence active time") diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index c876fa542c39..9e8fab62f9c6 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -278,7 +278,9 @@ async def set_state( """ @abc.abstractmethod - async def bump_presence_active_time(self, user: UserID) -> None: + async def bump_presence_active_time( + self, user: UserID, device_id: Optional[str] + ) -> None: """We've seen the user do something that indicates they're interacting with the app. """ @@ -652,7 +654,9 @@ async def set_state( is_sync=is_sync, ) - async def bump_presence_active_time(self, user: UserID) -> None: + async def bump_presence_active_time( + self, user: UserID, device_id: Optional[str] + ) -> None: """We've seen the user do something that indicates they're interacting with the app. """ @@ -663,7 +667,9 @@ async def bump_presence_active_time(self, user: UserID) -> None: # Proxy request to instance that writes presence user_id = user.to_string() await self._bump_active_client( - instance_name=self._presence_writer_instance, user_id=user_id + instance_name=self._presence_writer_instance, + user_id=user_id, + device_id=device_id, ) @@ -977,7 +983,9 @@ async def _handle_timeouts(self) -> None: return await self._update_states(changes) - async def bump_presence_active_time(self, user: UserID) -> None: + async def bump_presence_active_time( + self, user: UserID, device_id: Optional[str] + ) -> None: """We've seen the user do something that indicates they're interacting with the app. """ @@ -989,11 +997,26 @@ async def bump_presence_active_time(self, user: UserID) -> None: bump_active_time_counter.inc() - prev_state = await self.current_state_for_user(user_id) + now = self.clock.time_msec() - new_fields: Dict[str, Any] = {"last_active_ts": self.clock.time_msec()} - if prev_state.state == PresenceState.UNAVAILABLE: - new_fields["state"] = PresenceState.ONLINE + # Update the device information & mark the device as online if it was + # unavailable. + devices = self.user_to_device_to_current_state.setdefault(user_id, {}) + device_state = devices.setdefault( + device_id, + UserDevicePresenceState.default(user_id, device_id), + ) + device_state.last_active_ts = now + if device_state.state == PresenceState.UNAVAILABLE: + device_state.state = PresenceState.ONLINE + + # Update the user state, if this will always update last_active_ts and + # might update the presence state. + prev_state = await self.current_state_for_user(user_id) + new_fields: Dict[str, Any] = { + "last_active_ts": now, + "presence": _combine_device_states(devices.values()), + } await self._update_states([prev_state.copy_and_replace(**new_fields)]) @@ -1272,17 +1295,10 @@ async def set_state( prev_state = await self.current_state_for_user(user_id) # Always update the device specific information. - device_state = self.user_to_device_to_current_state.setdefault( - user_id, {} - ).setdefault( + devices = self.user_to_device_to_current_state.setdefault(user_id, {}) + device_state = devices.setdefault( device_id, - UserDevicePresenceState( - user_id, - device_id, - presence, - last_active_ts=self.clock.time_msec(), - last_sync_ts=0, - ), + UserDevicePresenceState.default(user_id, device_id), ) device_state.state = presence device_state.last_active_ts = now @@ -1290,9 +1306,7 @@ async def set_state( device_state.last_sync_ts = now # Based on (all) the user's devices calculate the new presence state. - presence = _combine_device_states( - self.user_to_device_to_current_state[user_id].values() - ) + presence = _combine_device_states(devices.values()) # The newly updated status as an amalgamation of all the device statuses. new_fields = {"state": presence} diff --git a/synapse/replication/http/presence.py b/synapse/replication/http/presence.py index 84bd8986daa8..1493d05d0b80 100644 --- a/synapse/replication/http/presence.py +++ b/synapse/replication/http/presence.py @@ -51,14 +51,15 @@ def __init__(self, hs: "HomeServer"): self._presence_handler = hs.get_presence_handler() @staticmethod - async def _serialize_payload(user_id: str) -> JsonDict: # type: ignore[override] - return {} + async def _serialize_payload(user_id: str, device_id: Optional[str]) -> JsonDict: # type: ignore[override] + return {"device_id": device_id} async def _handle_request( # type: ignore[override] self, request: Request, content: JsonDict, user_id: str ) -> Tuple[int, JsonDict]: await self._presence_handler.bump_presence_active_time( - UserID.from_string(user_id) + UserID.from_string(user_id), + content.get("device_id"), ) return (200, {}) diff --git a/synapse/rest/client/read_marker.py b/synapse/rest/client/read_marker.py index 4f96e51eeb93..1707e519723a 100644 --- a/synapse/rest/client/read_marker.py +++ b/synapse/rest/client/read_marker.py @@ -52,7 +52,9 @@ async def on_POST( ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) - await self.presence_handler.bump_presence_active_time(requester.user) + await self.presence_handler.bump_presence_active_time( + requester.user, requester.device_id + ) body = parse_json_object_from_request(request) diff --git a/synapse/rest/client/receipts.py b/synapse/rest/client/receipts.py index 316e7b99821e..869a37445950 100644 --- a/synapse/rest/client/receipts.py +++ b/synapse/rest/client/receipts.py @@ -94,7 +94,9 @@ async def on_POST( Codes.INVALID_PARAM, ) - await self.presence_handler.bump_presence_active_time(requester.user) + await self.presence_handler.bump_presence_active_time( + requester.user, requester.device_id + ) if receipt_type == ReceiptTypes.FULLY_READ: await self.read_marker_handler.received_client_read_marker( diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index dc498001e450..553938ce9d13 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -1229,7 +1229,9 @@ async def on_PUT( content = parse_json_object_from_request(request) - await self.presence_handler.bump_presence_active_time(requester.user) + await self.presence_handler.bump_presence_active_time( + requester.user, requester.device_id + ) # Limit timeout to stop people from setting silly typing timeouts. timeout = min(content.get("timeout", 30000), 120000)