Fix an invalid comparison of UserPresenceState to str (#14393)

This commit is contained in:
Andrew Morgan 2022-11-16 15:25:35 +00:00 committed by GitHub
parent d8cc86eff4
commit 618e4ab81b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 46 additions and 8 deletions

1
changelog.d/14393.bugfix Normal file
View File

@ -0,0 +1 @@
Fix a bug introduced in 1.58.0 where a user with presence state 'org.matrix.msc3026.busy' would mistakenly be set to 'online' when calling `/sync` or `/events` on a worker process.

View File

@ -478,7 +478,7 @@ class WorkerPresenceHandler(BasePresenceHandler):
return _NullContextManager() return _NullContextManager()
prev_state = await self.current_state_for_user(user_id) prev_state = await self.current_state_for_user(user_id)
if prev_state != PresenceState.BUSY: if prev_state.state != PresenceState.BUSY:
# We set state here but pass ignore_status_msg = True as we don't want to # We set state here but pass ignore_status_msg = True as we don't want to
# cause the status message to be cleared. # cause the status message to be cleared.
# Note that this causes last_active_ts to be incremented which is not # Note that this causes last_active_ts to be incremented which is not

View File

@ -15,6 +15,7 @@
from typing import Optional from typing import Optional
from unittest.mock import Mock, call from unittest.mock import Mock, call
from parameterized import parameterized
from signedjson.key import generate_signing_key from signedjson.key import generate_signing_key
from synapse.api.constants import EventTypes, Membership, PresenceState from synapse.api.constants import EventTypes, Membership, PresenceState
@ -37,6 +38,7 @@ from synapse.rest.client import room
from synapse.types import UserID, get_domain_from_id from synapse.types import UserID, get_domain_from_id
from tests import unittest from tests import unittest
from tests.replication._base import BaseMultiWorkerStreamTestCase
class PresenceUpdateTestCase(unittest.HomeserverTestCase): class PresenceUpdateTestCase(unittest.HomeserverTestCase):
@ -505,7 +507,7 @@ class PresenceTimeoutTestCase(unittest.TestCase):
self.assertEqual(state, new_state) self.assertEqual(state, new_state)
class PresenceHandlerTestCase(unittest.HomeserverTestCase): class PresenceHandlerTestCase(BaseMultiWorkerStreamTestCase):
def prepare(self, reactor, clock, hs): def prepare(self, reactor, clock, hs):
self.presence_handler = hs.get_presence_handler() self.presence_handler = hs.get_presence_handler()
self.clock = hs.get_clock() self.clock = hs.get_clock()
@ -716,20 +718,47 @@ class PresenceHandlerTestCase(unittest.HomeserverTestCase):
# our status message should be the same as it was before # our status message should be the same as it was before
self.assertEqual(state.status_msg, status_msg) self.assertEqual(state.status_msg, status_msg)
def test_set_presence_from_syncing_keeps_busy(self): @parameterized.expand([(False,), (True,)])
"""Test that presence set by syncing doesn't affect busy status""" @unittest.override_config(
# while this isn't the default {
self.presence_handler._busy_presence_enabled = True "experimental_features": {
"msc3026_enabled": True,
},
}
)
def test_set_presence_from_syncing_keeps_busy(self, test_with_workers: bool):
"""Test that presence set by syncing doesn't affect busy status
Args:
test_with_workers: If True, check the presence state of the user by calling
/sync against a worker, rather than the main process.
"""
user_id = "@test:server" user_id = "@test:server"
status_msg = "I'm busy!" status_msg = "I'm busy!"
# By default, we call /sync against the main process.
worker_to_sync_against = self.hs
if test_with_workers:
# Create a worker and use it to handle /sync traffic instead.
# This is used to test that presence changes get replicated from workers
# to the main process correctly.
worker_to_sync_against = self.make_worker_hs(
"synapse.app.generic_worker", {"worker_name": "presence_writer"}
)
# Set presence to BUSY
self._set_presencestate_with_status_msg(user_id, PresenceState.BUSY, status_msg) self._set_presencestate_with_status_msg(user_id, PresenceState.BUSY, status_msg)
# Perform a sync with a presence state other than busy. This should NOT change
# our presence status; we only change from busy if we explicitly set it via
# /presence/*.
self.get_success( self.get_success(
self.presence_handler.user_syncing(user_id, True, PresenceState.ONLINE) worker_to_sync_against.get_presence_handler().user_syncing(
user_id, True, PresenceState.ONLINE
)
) )
# Check against the main process that the user's presence did not change.
state = self.get_success( state = self.get_success(
self.presence_handler.get_state(UserID.from_string(user_id)) self.presence_handler.get_state(UserID.from_string(user_id))
) )

View File

@ -778,8 +778,11 @@ def _test_sending_local_online_presence_to_local_user(
worker process. The test users will still sync with the main process. The purpose of testing worker process. The test users will still sync with the main process. The purpose of testing
with a worker is to check whether a Synapse module running on a worker can inform other workers/ with a worker is to check whether a Synapse module running on a worker can inform other workers/
the main process that they should include additional presence when a user next syncs. the main process that they should include additional presence when a user next syncs.
If this argument is True, `test_case` MUST be an instance of BaseMultiWorkerStreamTestCase.
""" """
if test_with_workers: if test_with_workers:
assert isinstance(test_case, BaseMultiWorkerStreamTestCase)
# Create a worker process to make module_api calls against # Create a worker process to make module_api calls against
worker_hs = test_case.make_worker_hs( worker_hs = test_case.make_worker_hs(
"synapse.app.generic_worker", {"worker_name": "presence_writer"} "synapse.app.generic_worker", {"worker_name": "presence_writer"}

View File

@ -542,8 +542,13 @@ class FakeRedisPubSubProtocol(Protocol):
self.send("OK") self.send("OK")
elif command == b"GET": elif command == b"GET":
self.send(None) self.send(None)
# Connection keep-alives.
elif command == b"PING":
self.send("PONG")
else: else:
raise Exception("Unknown command") raise Exception(f"Unknown command: {command}")
def send(self, msg): def send(self, msg):
"""Send a message back to the client.""" """Send a message back to the client."""