summary refs log tree commit diff
diff options
context:
space:
mode:
authorAndrew Morgan <1342360+anoadragon453@users.noreply.github.com>2021-02-17 17:31:37 +0000
committerGitHub <noreply@github.com>2021-02-17 17:31:37 +0000
commit3e5749b99fc47a28681178d96923519866b3ae5d (patch)
treec052d4e635cf241ef797965d2f80d293b6038c32
parentUpdate workers.md (diff)
downloadsynapse-3e5749b99fc47a28681178d96923519866b3ae5d.tar.xz
Fix only handling the last presence state for each user (#9425)
This is a small bug that I noticed while working on #8956.

We have a for-loop which attempts to strip all presence changes for each user except for the final one, as we don't really care about older presence:

https://github.com/matrix-org/synapse/blob/9e19c6aab4b5a99039f2ddc7d3120dd3b26c274b/synapse/handlers/presence.py#L368-L371

`new_states_dict` stores this stripped copy of latest presence state for each user, before it is... put into a new variable `new_state`, which is just overridden by the subsequent for loop.

I believe this was instead meant to override `new_states`. Without doing so, it effectively meant:

1. The for loop had no effect.
2. We were still processing old presence state for users.
-rw-r--r--changelog.d/9425.bugfix1
-rw-r--r--synapse/handlers/presence.py7
2 files changed, 6 insertions, 2 deletions
diff --git a/changelog.d/9425.bugfix b/changelog.d/9425.bugfix
new file mode 100644
index 0000000000..f5b8857cdb
--- /dev/null
+++ b/changelog.d/9425.bugfix
@@ -0,0 +1 @@
+Fix a long-standing bug in the deduplication of old presence, resulting in no deduplication.
\ No newline at end of file
diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py
index 7ba22d511f..ed90b5d457 100644
--- a/synapse/handlers/presence.py
+++ b/synapse/handlers/presence.py
@@ -349,10 +349,13 @@ class PresenceHandler(BasePresenceHandler):
                 [self.user_to_current_state[user_id] for user_id in unpersisted]
             )
 
-    async def _update_states(self, new_states):
+    async def _update_states(self, new_states: Iterable[UserPresenceState]) -> None:
         """Updates presence of users. Sets the appropriate timeouts. Pokes
         the notifier and federation if and only if the changed presence state
         should be sent to clients/servers.
+
+        Args:
+            new_states: The new user presence state updates to process.
         """
         now = self.clock.time_msec()
 
@@ -368,7 +371,7 @@ class PresenceHandler(BasePresenceHandler):
             new_states_dict = {}
             for new_state in new_states:
                 new_states_dict[new_state.user_id] = new_state
-            new_state = new_states_dict.values()
+            new_states = new_states_dict.values()
 
             for new_state in new_states:
                 user_id = new_state.user_id