Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add a new module API to update user presence state. #16544

Merged
merged 7 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions docs/usage/configuration/config_documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
clokep marked this conversation as resolved.
Show resolved Hide resolved
---
### `require_auth_for_profile_requests`

Expand Down
16 changes: 8 additions & 8 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved

# 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"
Comment on lines +375 to +378
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think track_presence is very clear---thank you!

I think the phrase "Whether presence is enabled" is still a bit confusing. Can we call self.presence_enabled something like self.report_presence? (I'm guessing that this bool now controls the reporting of presence down /sync and in response to explicit presence GETs).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It ends up getting used a bit internally too, I'm not sure that presence_enabled is the best name, but I think report_presence doesn't quite capture it either.

So it gets used for:

  1. Whether to calculate presence for sync & initial sync.
  2. Whether to return real presence info from /presence.
  3. Whether to persist any presence information into the database.
  4. Whether the presence handler APIs can be called to update presence state.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think report_presence easily covers 1 & 2, it doesn't feel quite right for 3 & 4 though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blimey, that's a lot to juggle. Let's leave it as-is then. If nothing else, we'll have these comments as signposts for our future selves!


# Custom presence router module
# This is the legacy way of configuring it (the config should now be put in the modules section)
Expand Down
2 changes: 1 addition & 1 deletion synapse/federation/federation_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion synapse/federation/sender/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/initial_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
38 changes: 20 additions & 18 deletions synapse/handlers/presence.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down Expand Up @@ -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)
)
Expand All @@ -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
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -831,14 +832,17 @@ 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.
self.clock.call_later(
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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 2 additions & 4 deletions synapse/module_api/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down
6 changes: 2 additions & 4 deletions synapse/rest/client/presence.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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, {}
Expand Down
41 changes: 32 additions & 9 deletions tests/handlers/test_presence.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
IDLE_TIMER,
LAST_ACTIVE_GRANULARITY,
SYNC_ONLINE_TIMEOUT,
PresenceHandler,
handle_timeout,
handle_update,
)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
Loading
Loading