From 99aa721e5ed33c6ef4c7e9f41ad23e161c2fb77a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 24 Oct 2023 09:19:55 -0400 Subject: [PATCH 1/6] Add a new module API to update user presence state. --- synapse/module_api/__init__.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 0786d2063565..9f3ddad60ebc 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -23,6 +23,7 @@ Generator, Iterable, List, + Mapping, Optional, Tuple, TypeVar, @@ -39,6 +40,7 @@ from synapse.api import errors from synapse.api.errors import SynapseError +from synapse.api.presence import UserPresenceState from synapse.config import ConfigError from synapse.events import EventBase from synapse.events.presence_router import ( @@ -1184,6 +1186,37 @@ async def send_local_online_presence_to(self, users: Iterable[str]) -> None: presence_events, [destination] ) + async def set_presence_for_users( + self, users: Mapping[str, Tuple[str, Optional[str]]] + ) -> None: + """ + Update the internal presence state of users. + + Note that this can be used for either local or remote users. + + Note that this method can only be run on the process that is configured to write to the + presence stream. By default, this is the main process. + + Added in Synapse v1.96.0. + """ + + # We pull out the presence handler here to break a cyclic + # dependency between the presence router and module API. + presence_handler = self._hs.get_presence_handler() + + from synapse.handlers.presence import PresenceHandler + + assert isinstance(presence_handler, PresenceHandler) + + states = await presence_handler.current_state_for_users(users.keys()) + for user_id, (state, status_msg) in users.items(): + prev_state = states.setdefault(user_id, UserPresenceState.default(user_id)) + states[user_id] = prev_state.copy_and_replace( + state=state, status_msg=status_msg + ) + + await presence_handler._update_states(states.values(), force_notify=True) + def looping_background_call( self, f: Callable, From be16e5b434eaae30b5a235d1f4357e883f8fb6b8 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 24 Oct 2023 09:20:16 -0400 Subject: [PATCH 2/6] Selectively enable presence only for sync. --- .../configuration/config_documentation.md | 3 + synapse/config/server.py | 5 ++ synapse/handlers/initial_sync.py | 2 +- synapse/handlers/presence.py | 46 ++++++---- synapse/handlers/sync.py | 2 +- synapse/module_api/__init__.py | 4 +- tests/handlers/test_presence.py | 86 +++++++++++++++++-- 7 files changed, 120 insertions(+), 28 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 92e00c138086..2fc7b9ba6b30 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -230,6 +230,9 @@ Example configuration: presence: enabled: false ``` + +The `enabled_for_sync` sub-option can be used to selectively enable/disable +returning presence information in `/sync` response. --- ### `require_auth_for_profile_requests` diff --git a/synapse/config/server.py b/synapse/config/server.py index 72d30da30082..60a3ab163590 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -372,6 +372,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: if self.use_presence is None: self.use_presence = config.get("use_presence", True) + # Selectively enable syncing of presence, even if it is disabled. + self.use_presence_for_sync = presence_config.get( + "enabled_for_sync", self.use_presence + ) + # Custom presence router module # This is the legacy way of configuring it (the config should now be put in the modules section) self.presence_router_module_class = None diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index c34bd7db954f..7e9a93cbc99f 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -439,7 +439,7 @@ async def _room_initial_sync_joined( async def get_presence() -> List[JsonDict]: # If presence is disabled, return an empty list - if not self.hs.config.server.use_presence: + if not self.hs.config.server.use_presence_for_sync: return [] states = await presence_handler.get_states( diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index dfc0b9db070f..b8cc7ef9e19c 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -905,7 +905,10 @@ async def _persist_unpersisted_changes(self) -> None: ) async def _update_states( - self, new_states: Iterable[UserPresenceState], force_notify: bool = False + self, + new_states: Iterable[UserPresenceState], + force_notify: bool = False, + override: bool = False, ) -> None: """Updates presence of users. Sets the appropriate timeouts. Pokes the notifier and federation if and only if the changed presence state @@ -917,8 +920,9 @@ async def _update_states( even if it doesn't change the state of a user's presence (e.g online -> online). This is currently used to bump the max presence stream ID without changing any user's presence (see PresenceHandler.add_users_to_send_full_presence_to). + override: Whether to set the presence state even if presence is disabled. """ - if not self._presence_enabled: + if not self._presence_enabled and not override: # We shouldn't get here if presence is disabled, but we check anyway # to ensure that we don't a) send out presence federation and b) # don't add things to the wheel timer that will never be handled. @@ -957,6 +961,7 @@ async def _update_states( is_mine=self.is_mine_id(user_id), wheel_timer=self.wheel_timer, now=now, + override=override, ) if force_notify: @@ -2118,6 +2123,7 @@ def handle_update( is_mine: bool, wheel_timer: WheelTimer, now: int, + override: bool, ) -> Tuple[UserPresenceState, bool, bool]: """Given a presence update: 1. Add any appropriate timers. @@ -2129,6 +2135,8 @@ def handle_update( is_mine: Whether the user is ours wheel_timer now: Time now in ms + override: True if this state should persist until another update occurs. + Skips insertion into wheel timers. Returns: 3-tuple: `(new_state, persist_and_notify, federation_ping)` where: @@ -2146,14 +2154,15 @@ def handle_update( if is_mine: if new_state.state == PresenceState.ONLINE: # Idle timer - wheel_timer.insert( - now=now, obj=user_id, then=new_state.last_active_ts + IDLE_TIMER - ) + if not override: + wheel_timer.insert( + now=now, obj=user_id, then=new_state.last_active_ts + IDLE_TIMER + ) active = now - new_state.last_active_ts < LAST_ACTIVE_GRANULARITY new_state = new_state.copy_and_replace(currently_active=active) - if active: + if active and not override: wheel_timer.insert( now=now, obj=user_id, @@ -2162,11 +2171,12 @@ def handle_update( if new_state.state != PresenceState.OFFLINE: # User has stopped syncing - wheel_timer.insert( - now=now, - obj=user_id, - then=new_state.last_user_sync_ts + SYNC_ONLINE_TIMEOUT, - ) + if not override: + wheel_timer.insert( + now=now, + obj=user_id, + then=new_state.last_user_sync_ts + SYNC_ONLINE_TIMEOUT, + ) last_federate = new_state.last_federation_update_ts if now - last_federate > FEDERATION_PING_INTERVAL: @@ -2174,7 +2184,7 @@ def handle_update( new_state = new_state.copy_and_replace(last_federation_update_ts=now) federation_ping = True - if new_state.state == PresenceState.BUSY: + if new_state.state == PresenceState.BUSY and not override: wheel_timer.insert( now=now, obj=user_id, @@ -2182,11 +2192,13 @@ def handle_update( ) else: - wheel_timer.insert( - now=now, - obj=user_id, - then=new_state.last_federation_update_ts + FEDERATION_TIMEOUT, - ) + # An update for a remote user was received. + if not override: + wheel_timer.insert( + now=now, + obj=user_id, + then=new_state.last_federation_update_ts + FEDERATION_TIMEOUT, + ) # Check whether the change was something worth notifying about if should_notify(prev_state, new_state, is_mine): diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index f131c0e8e0c7..10825653d2da 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1512,7 +1512,7 @@ async def generate_sync_result( # Presence data is included if the server has it enabled and not filtered out. include_presence_data = bool( - self.hs_config.server.use_presence + self.hs_config.server.use_presence_for_sync and not sync_config.filter_collection.blocks_all_presence() ) # Device list updates are sent if a since token is provided. diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 9f3ddad60ebc..854b2906887b 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -1215,7 +1215,9 @@ async def set_presence_for_users( state=state, status_msg=status_msg ) - await presence_handler._update_states(states.values(), force_notify=True) + await presence_handler._update_states( + states.values(), force_notify=True, override=True + ) def looping_background_call( self, diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 41c8c44e0241..57f67498d9fe 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -11,7 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - +import itertools from typing import Optional, cast from unittest.mock import Mock, call @@ -66,7 +66,12 @@ def test_offline_to_online(self) -> None: ) state, persist_and_notify, federation_ping = handle_update( - prev_state, new_state, is_mine=True, wheel_timer=wheel_timer, now=now + prev_state, + new_state, + is_mine=True, + wheel_timer=wheel_timer, + now=now, + override=False, ) self.assertTrue(persist_and_notify) @@ -108,7 +113,12 @@ def test_online_to_online(self) -> None: ) state, persist_and_notify, federation_ping = handle_update( - prev_state, new_state, is_mine=True, wheel_timer=wheel_timer, now=now + prev_state, + new_state, + is_mine=True, + wheel_timer=wheel_timer, + now=now, + override=False, ) self.assertFalse(persist_and_notify) @@ -153,7 +163,12 @@ def test_online_to_online_last_active_noop(self) -> None: ) state, persist_and_notify, federation_ping = handle_update( - prev_state, new_state, is_mine=True, wheel_timer=wheel_timer, now=now + prev_state, + new_state, + is_mine=True, + wheel_timer=wheel_timer, + now=now, + override=False, ) self.assertFalse(persist_and_notify) @@ -196,7 +211,12 @@ def test_online_to_online_last_active(self) -> None: new_state = prev_state.copy_and_replace(state=PresenceState.ONLINE) state, persist_and_notify, federation_ping = handle_update( - prev_state, new_state, is_mine=True, wheel_timer=wheel_timer, now=now + prev_state, + new_state, + is_mine=True, + wheel_timer=wheel_timer, + now=now, + override=False, ) self.assertTrue(persist_and_notify) @@ -231,7 +251,12 @@ def test_remote_ping_timer(self) -> None: new_state = prev_state.copy_and_replace(state=PresenceState.ONLINE) state, persist_and_notify, federation_ping = handle_update( - prev_state, new_state, is_mine=False, wheel_timer=wheel_timer, now=now + prev_state, + new_state, + is_mine=False, + wheel_timer=wheel_timer, + now=now, + override=False, ) self.assertFalse(persist_and_notify) @@ -265,7 +290,12 @@ def test_online_to_offline(self) -> None: new_state = prev_state.copy_and_replace(state=PresenceState.OFFLINE) state, persist_and_notify, federation_ping = handle_update( - prev_state, new_state, is_mine=True, wheel_timer=wheel_timer, now=now + prev_state, + new_state, + is_mine=True, + wheel_timer=wheel_timer, + now=now, + override=False, ) self.assertTrue(persist_and_notify) @@ -287,7 +317,12 @@ def test_online_to_idle(self) -> None: new_state = prev_state.copy_and_replace(state=PresenceState.UNAVAILABLE) state, persist_and_notify, federation_ping = handle_update( - prev_state, new_state, is_mine=True, wheel_timer=wheel_timer, now=now + prev_state, + new_state, + is_mine=True, + wheel_timer=wheel_timer, + now=now, + override=False, ) self.assertTrue(persist_and_notify) @@ -347,6 +382,41 @@ def test_persisting_presence_updates(self) -> None: # They should be identical. self.assertEqual(presence_states_compare, db_presence_states) + @parameterized.expand( + itertools.permutations( + ( + PresenceState.BUSY, + PresenceState.ONLINE, + PresenceState.UNAVAILABLE, + PresenceState.OFFLINE, + ), + 2, + ) + ) + def test_override(self, initial_state: str, final_state: str) -> None: + """Overridden statuses should not go into the wheel timer.""" + wheel_timer = Mock() + user_id = "@foo:bar" + now = 5000000 + + prev_state = UserPresenceState.default(user_id) + prev_state = prev_state.copy_and_replace( + state=initial_state, last_active_ts=now, currently_active=True + ) + + new_state = prev_state.copy_and_replace(state=final_state, last_active_ts=now) + + handle_update( + prev_state, + new_state, + is_mine=True, + wheel_timer=wheel_timer, + now=now, + override=True, + ) + + wheel_timer.insert.assert_not_called() + class PresenceTimeoutTestCase(unittest.TestCase): """Tests different timers and that the timer does not change `status_msg` of user.""" From a44f1c64cf00f3d691039c2b07ca8af04cade26d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 24 Oct 2023 09:54:45 -0400 Subject: [PATCH 3/6] Newsfragment --- changelog.d/16544.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16544.feature diff --git a/changelog.d/16544.feature b/changelog.d/16544.feature new file mode 100644 index 000000000000..92bf701be649 --- /dev/null +++ b/changelog.d/16544.feature @@ -0,0 +1 @@ +Add a new module API for controller presence. From b1c55ee7f33308e37ff19cb175a2f823f3b997c5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 25 Oct 2023 12:55:47 -0400 Subject: [PATCH 4/6] Rename variable. --- synapse/handlers/presence.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index b8cc7ef9e19c..059d86b526a3 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -947,7 +947,7 @@ async def _update_states( for new_state in new_states: user_id = new_state.user_id - # Its fine to not hit the database here, as the only thing not in + # It's fine to not hit the database here, as the only thing not in # the current state cache are OFFLINE states, where the only field # of interest is last_active which is safe enough to assume is 0 # here. @@ -961,7 +961,9 @@ async def _update_states( is_mine=self.is_mine_id(user_id), wheel_timer=self.wheel_timer, now=now, - override=override, + # When overriding disabled presence, don't kick off all the + # wheel timers. + persist=override, ) if force_notify: @@ -2123,7 +2125,7 @@ def handle_update( is_mine: bool, wheel_timer: WheelTimer, now: int, - override: bool, + persist: bool, ) -> Tuple[UserPresenceState, bool, bool]: """Given a presence update: 1. Add any appropriate timers. @@ -2135,7 +2137,7 @@ def handle_update( is_mine: Whether the user is ours wheel_timer now: Time now in ms - override: True if this state should persist until another update occurs. + persist: True if this state should persist until another update occurs. Skips insertion into wheel timers. Returns: @@ -2154,7 +2156,7 @@ def handle_update( if is_mine: if new_state.state == PresenceState.ONLINE: # Idle timer - if not override: + if not persist: wheel_timer.insert( now=now, obj=user_id, then=new_state.last_active_ts + IDLE_TIMER ) @@ -2162,7 +2164,7 @@ def handle_update( active = now - new_state.last_active_ts < LAST_ACTIVE_GRANULARITY new_state = new_state.copy_and_replace(currently_active=active) - if active and not override: + if active and not persist: wheel_timer.insert( now=now, obj=user_id, @@ -2171,7 +2173,7 @@ def handle_update( if new_state.state != PresenceState.OFFLINE: # User has stopped syncing - if not override: + if not persist: wheel_timer.insert( now=now, obj=user_id, @@ -2184,7 +2186,7 @@ def handle_update( new_state = new_state.copy_and_replace(last_federation_update_ts=now) federation_ping = True - if new_state.state == PresenceState.BUSY and not override: + if new_state.state == PresenceState.BUSY and not persist: wheel_timer.insert( now=now, obj=user_id, @@ -2193,7 +2195,7 @@ def handle_update( else: # An update for a remote user was received. - if not override: + if not persist: wheel_timer.insert( now=now, obj=user_id, From eef5b4188e3b86698af337853484dcabdd2b1ea7 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 25 Oct 2023 13:45:35 -0400 Subject: [PATCH 5/6] Update the configuration to a tri-state, instead of two booleans. This switches enabled to be true/false/"untracked" instead of two different booleans. Most places that used use_presence actually want to only kick in if presence is being internally tracked. The few spots which don't are updated to use the new presence_enabled, which is true if presence enabled = true or "untracked". --- .../configuration/config_documentation.md | 5 ++- synapse/config/server.py | 16 ++++---- synapse/federation/federation_server.py | 2 +- synapse/federation/sender/__init__.py | 2 +- synapse/handlers/initial_sync.py | 2 +- synapse/handlers/presence.py | 38 +++++++++-------- synapse/handlers/sync.py | 2 +- synapse/module_api/__init__.py | 6 +-- synapse/rest/client/presence.py | 6 +-- tests/handlers/test_presence.py | 41 +++++++++++++++---- tests/rest/client/test_presence.py | 19 ++++++++- 11 files changed, 88 insertions(+), 51 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 2fc7b9ba6b30..dbeb239d061c 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -231,8 +231,9 @@ presence: enabled: false ``` -The `enabled_for_sync` sub-option can be used to selectively enable/disable -returning presence information in `/sync` response. +`enabled` can also be set to a special value of "untracked" which ignores updates +received via clients and federation, while still accepting updates from the +[module API](../../modules/index.md). --- ### `require_auth_for_profile_requests` diff --git a/synapse/config/server.py b/synapse/config/server.py index 60a3ab163590..f9e18d2053d3 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -368,14 +368,14 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # Whether to enable user presence. presence_config = config.get("presence") or {} - self.use_presence = presence_config.get("enabled") - if self.use_presence is None: - self.use_presence = config.get("use_presence", True) - - # Selectively enable syncing of presence, even if it is disabled. - self.use_presence_for_sync = presence_config.get( - "enabled_for_sync", self.use_presence - ) + presence_enabled = presence_config.get("enabled") + if presence_enabled is None: + presence_enabled = config.get("use_presence", True) + + # Whether presence is enabled *at all*. + self.presence_enabled = bool(presence_enabled) + # Whether to internally track presence, requires that presence is enabled, + self.track_presence = self.presence_enabled and presence_enabled != "untracked" # Custom presence router module # This is the legacy way of configuring it (the config should now be put in the modules section) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 6ac8d1609585..3b27925517a9 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -1395,7 +1395,7 @@ def register_instances_for_edu( self._edu_type_to_instance[edu_type] = instance_names async def on_edu(self, edu_type: str, origin: str, content: dict) -> None: - if not self.config.server.use_presence and edu_type == EduTypes.PRESENCE: + if not self.config.server.track_presence and edu_type == EduTypes.PRESENCE: return # Check if we have a handler on this instance diff --git a/synapse/federation/sender/__init__.py b/synapse/federation/sender/__init__.py index 7b6b1da090b1..7980d1a322c6 100644 --- a/synapse/federation/sender/__init__.py +++ b/synapse/federation/sender/__init__.py @@ -844,7 +844,7 @@ async def send_presence_to_destinations( destinations (list[str]) """ - if not states or not self.hs.config.server.use_presence: + if not states or not self.hs.config.server.track_presence: # No-op if presence is disabled. return diff --git a/synapse/handlers/initial_sync.py b/synapse/handlers/initial_sync.py index 7e9a93cbc99f..cda1fbd11293 100644 --- a/synapse/handlers/initial_sync.py +++ b/synapse/handlers/initial_sync.py @@ -439,7 +439,7 @@ async def _room_initial_sync_joined( async def get_presence() -> List[JsonDict]: # If presence is disabled, return an empty list - if not self.hs.config.server.use_presence_for_sync: + if not self.hs.config.server.presence_enabled: return [] states = await presence_handler.get_states( diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index 059d86b526a3..202beee73802 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -192,7 +192,8 @@ def __init__(self, hs: "HomeServer"): self.state = hs.get_state_handler() self.is_mine_id = hs.is_mine_id - self._presence_enabled = hs.config.server.use_presence + self._presence_enabled = hs.config.server.presence_enabled + self._track_presence = hs.config.server.track_presence self._federation = None if hs.should_send_federation(): @@ -512,7 +513,7 @@ def __init__(self, hs: "HomeServer"): ) async def _on_shutdown(self) -> None: - if self._presence_enabled: + if self._track_presence: self.hs.get_replication_command_handler().send_command( ClearUserSyncsCommand(self.instance_id) ) @@ -524,7 +525,7 @@ def send_user_sync( is_syncing: bool, last_sync_ms: int, ) -> None: - if self._presence_enabled: + if self._track_presence: self.hs.get_replication_command_handler().send_user_sync( self.instance_id, user_id, device_id, is_syncing, last_sync_ms ) @@ -571,7 +572,7 @@ async def user_syncing( Called by the sync and events servlets to record that a user has connected to this worker and is waiting for some events. """ - if not affect_presence or not self._presence_enabled: + if not affect_presence or not self._track_presence: return _NullContextManager() # Note that this causes last_active_ts to be incremented which is not @@ -702,8 +703,8 @@ async def set_state( user_id = target_user.to_string() - # If presence is disabled, no-op - if not self._presence_enabled: + # If tracking of presence is disabled, no-op + if not self._track_presence: return # Proxy request to instance that writes presence @@ -723,7 +724,7 @@ async def bump_presence_active_time( with the app. """ # If presence is disabled, no-op - if not self._presence_enabled: + if not self._track_presence: return # Proxy request to instance that writes presence @@ -760,7 +761,7 @@ def __init__(self, hs: "HomeServer"): ] = {} now = self.clock.time_msec() - if self._presence_enabled: + if self._track_presence: for state in self.user_to_current_state.values(): # Create a psuedo-device to properly handle time outs. This will # be overridden by any "real" devices within SYNC_ONLINE_TIMEOUT. @@ -831,7 +832,7 @@ def __init__(self, hs: "HomeServer"): self.external_sync_linearizer = Linearizer(name="external_sync_linearizer") - if self._presence_enabled: + if self._track_presence: # Start a LoopingCall in 30s that fires every 5s. # The initial delay is to allow disconnected clients a chance to # reconnect before we treat them as offline. @@ -839,6 +840,9 @@ def __init__(self, hs: "HomeServer"): 30, self.clock.looping_call, self._handle_timeouts, 5000 ) + # Presence information is persisted, whether or not it is being tracked + # internally. + if self._presence_enabled: self.clock.call_later( 60, self.clock.looping_call, @@ -854,7 +858,7 @@ def __init__(self, hs: "HomeServer"): ) # Used to handle sending of presence to newly joined users/servers - if self._presence_enabled: + if self._track_presence: self.notifier.add_replication_callback(self.notify_new_event) # Presence is best effort and quickly heals itself, so lets just always @@ -908,7 +912,6 @@ async def _update_states( self, new_states: Iterable[UserPresenceState], force_notify: bool = False, - override: bool = False, ) -> None: """Updates presence of users. Sets the appropriate timeouts. Pokes the notifier and federation if and only if the changed presence state @@ -920,9 +923,8 @@ async def _update_states( even if it doesn't change the state of a user's presence (e.g online -> online). This is currently used to bump the max presence stream ID without changing any user's presence (see PresenceHandler.add_users_to_send_full_presence_to). - override: Whether to set the presence state even if presence is disabled. """ - if not self._presence_enabled and not override: + if not self._presence_enabled: # We shouldn't get here if presence is disabled, but we check anyway # to ensure that we don't a) send out presence federation and b) # don't add things to the wheel timer that will never be handled. @@ -963,7 +965,7 @@ async def _update_states( now=now, # When overriding disabled presence, don't kick off all the # wheel timers. - persist=override, + persist=not self._track_presence, ) if force_notify: @@ -1079,7 +1081,7 @@ async def bump_presence_active_time( with the app. """ # If presence is disabled, no-op - if not self._presence_enabled: + if not self._track_presence: return user_id = user.to_string() @@ -1131,7 +1133,7 @@ async def user_syncing( client that is being used by a user. presence_state: The presence state indicated in the sync request """ - if not affect_presence or not self._presence_enabled: + if not affect_presence or not self._track_presence: return _NullContextManager() curr_sync = self._user_device_to_num_current_syncs.get((user_id, device_id), 0) @@ -1291,7 +1293,7 @@ async def _persist_and_notify(self, states: List[UserPresenceState]) -> None: async def incoming_presence(self, origin: str, content: JsonDict) -> None: """Called when we receive a `m.presence` EDU from a remote server.""" - if not self._presence_enabled: + if not self._track_presence: return now = self.clock.time_msec() @@ -1366,7 +1368,7 @@ async def set_state( raise SynapseError(400, "Invalid presence state") # If presence is disabled, no-op - if not self._presence_enabled: + if not self._track_presence: return user_id = target_user.to_string() diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 10825653d2da..21e0f76421f4 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1512,7 +1512,7 @@ async def generate_sync_result( # Presence data is included if the server has it enabled and not filtered out. include_presence_data = bool( - self.hs_config.server.use_presence_for_sync + self.hs_config.server.presence_enabled and not sync_config.filter_collection.blocks_all_presence() ) # Device list updates are sent if a since token is provided. diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 854b2906887b..09ea6bdecb79 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -1192,7 +1192,7 @@ async def set_presence_for_users( """ Update the internal presence state of users. - Note that this can be used for either local or remote users. + This can be used for either local or remote users. Note that this method can only be run on the process that is configured to write to the presence stream. By default, this is the main process. @@ -1215,9 +1215,7 @@ async def set_presence_for_users( state=state, status_msg=status_msg ) - await presence_handler._update_states( - states.values(), force_notify=True, override=True - ) + await presence_handler._update_states(states.values(), force_notify=True) def looping_background_call( self, diff --git a/synapse/rest/client/presence.py b/synapse/rest/client/presence.py index d578faa96984..054a391f2630 100644 --- a/synapse/rest/client/presence.py +++ b/synapse/rest/client/presence.py @@ -42,15 +42,13 @@ def __init__(self, hs: "HomeServer"): self.clock = hs.get_clock() self.auth = hs.get_auth() - self._use_presence = hs.config.server.use_presence - async def on_GET( self, request: SynapseRequest, user_id: str ) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) user = UserID.from_string(user_id) - if not self._use_presence: + if not self.hs.config.server.presence_enabled: return 200, {"presence": "offline"} if requester.user != user: @@ -96,7 +94,7 @@ async def on_PUT( except Exception: raise SynapseError(400, "Unable to parse state") - if self._use_presence: + if self.hs.config.server.track_presence: await self.presence_handler.set_state(user, requester.device_id, state) return 200, {} diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 57f67498d9fe..173b14521a15 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -33,6 +33,7 @@ IDLE_TIMER, LAST_ACTIVE_GRANULARITY, SYNC_ONLINE_TIMEOUT, + PresenceHandler, handle_timeout, handle_update, ) @@ -71,7 +72,7 @@ def test_offline_to_online(self) -> None: is_mine=True, wheel_timer=wheel_timer, now=now, - override=False, + persist=False, ) self.assertTrue(persist_and_notify) @@ -118,7 +119,7 @@ def test_online_to_online(self) -> None: is_mine=True, wheel_timer=wheel_timer, now=now, - override=False, + persist=False, ) self.assertFalse(persist_and_notify) @@ -168,7 +169,7 @@ def test_online_to_online_last_active_noop(self) -> None: is_mine=True, wheel_timer=wheel_timer, now=now, - override=False, + persist=False, ) self.assertFalse(persist_and_notify) @@ -216,7 +217,7 @@ def test_online_to_online_last_active(self) -> None: is_mine=True, wheel_timer=wheel_timer, now=now, - override=False, + persist=False, ) self.assertTrue(persist_and_notify) @@ -256,7 +257,7 @@ def test_remote_ping_timer(self) -> None: is_mine=False, wheel_timer=wheel_timer, now=now, - override=False, + persist=False, ) self.assertFalse(persist_and_notify) @@ -295,7 +296,7 @@ def test_online_to_offline(self) -> None: is_mine=True, wheel_timer=wheel_timer, now=now, - override=False, + persist=False, ) self.assertTrue(persist_and_notify) @@ -322,7 +323,7 @@ def test_online_to_idle(self) -> None: is_mine=True, wheel_timer=wheel_timer, now=now, - override=False, + persist=False, ) self.assertTrue(persist_and_notify) @@ -412,7 +413,7 @@ def test_override(self, initial_state: str, final_state: str) -> None: is_mine=True, wheel_timer=wheel_timer, now=now, - override=True, + persist=True, ) wheel_timer.insert.assert_not_called() @@ -808,7 +809,6 @@ class PresenceHandlerTestCase(BaseMultiWorkerStreamTestCase): def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.presence_handler = hs.get_presence_handler() - self.clock = hs.get_clock() def test_external_process_timeout(self) -> None: """Test that if an external process doesn't update the records for a while @@ -1541,6 +1541,29 @@ def _set_presencestate_with_status_msg( self.assertEqual(new_state.state, state) self.assertEqual(new_state.status_msg, status_msg) + @unittest.override_config({"presence": {"enabled": "untracked"}}) + def test_untracked_does_not_idle(self) -> None: + """Untracked presence should not idle.""" + + # Mark user as online, this needs to reach into internals in order to + # bypass checks. + state = self.get_success(self.presence_handler.get_state(self.user_id_obj)) + assert isinstance(self.presence_handler, PresenceHandler) + self.get_success( + self.presence_handler._update_states( + [state.copy_and_replace(state=PresenceState.ONLINE)] + ) + ) + + # Ensure the update took. + state = self.get_success(self.presence_handler.get_state(self.user_id_obj)) + self.assertEqual(state.state, PresenceState.ONLINE) + + # The timeout should not fire and the state should be the same. + self.reactor.advance(SYNC_ONLINE_TIMEOUT) + state = self.get_success(self.presence_handler.get_state(self.user_id_obj)) + self.assertEqual(state.state, PresenceState.ONLINE) + class PresenceFederationQueueTestCase(unittest.HomeserverTestCase): def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: diff --git a/tests/rest/client/test_presence.py b/tests/rest/client/test_presence.py index 66b387cea37e..4e89107e54cf 100644 --- a/tests/rest/client/test_presence.py +++ b/tests/rest/client/test_presence.py @@ -50,7 +50,7 @@ def test_put_presence(self) -> None: PUT to the status endpoint with use_presence enabled will call set_state on the presence handler. """ - self.hs.config.server.use_presence = True + self.hs.config.server.presence_enabled = True body = {"presence": "here", "status_msg": "beep boop"} channel = self.make_request( @@ -63,7 +63,22 @@ def test_put_presence(self) -> None: @unittest.override_config({"use_presence": False}) def test_put_presence_disabled(self) -> None: """ - PUT to the status endpoint with use_presence disabled will NOT call + PUT to the status endpoint with presence disabled will NOT call + set_state on the presence handler. + """ + + body = {"presence": "here", "status_msg": "beep boop"} + channel = self.make_request( + "PUT", "/presence/%s/status" % (self.user_id,), body + ) + + self.assertEqual(channel.code, HTTPStatus.OK) + self.assertEqual(self.presence_handler.set_state.call_count, 0) + + @unittest.override_config({"presence": {"enabled": "untracked"}}) + def test_put_presence_untracked(self) -> None: + """ + PUT to the status endpoint with presence untracked will NOT call set_state on the presence handler. """ From d512c87b20f54f5faf58bea86ec0d9ec2eaa00cb Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 26 Oct 2023 13:46:45 -0400 Subject: [PATCH 6/6] Update documentation to note when added. --- docs/usage/configuration/config_documentation.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index af2a8530e85d..a1ca5fa98c22 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -234,6 +234,9 @@ presence: `enabled` can also be set to a special value of "untracked" which ignores updates received via clients and federation, while still accepting updates from the [module API](../../modules/index.md). + +*The "untracked" option was added in Synapse 1.96.0.* + --- ### `require_auth_for_profile_requests`