Adding tests to check sync setting presence works as expected.

tests/handlers/test_presence.py

@@ -36,7 +36,7 @@
     handle_update,
 )
 from synapse.rest import admin
-from synapse.rest.client import room
+from synapse.rest.client import login, room
 from synapse.server import HomeServer
 from synapse.types import JsonDict, UserID, get_domain_from_id
 from synapse.util import Clock
@@ -514,6 +514,8 @@ def test_last_active(self) -> None:
 
 
 class PresenceHandlerTestCase(BaseMultiWorkerStreamTestCase):
+    servlets = [admin.register_servlets, login.register_servlets]
+
     def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         self.presence_handler = hs.get_presence_handler()
         self.clock = hs.get_clock()
@@ -826,11 +828,14 @@ def test_syncing_appropriately_handles_last_active_ts(self) -> None:
         """Test that syncing does not bump last_active_ts when already online"""
 
         user_id = "@test:server"
+        device_id = "device_whatever"
 
         # Start from a time that is not zero
         self.reactor.advance(2)
 
-        self._set_presencestate_with_status_msg(user_id, PresenceState.ONLINE, None)
+        self._set_presencestate_with_status_msg(
+            user_id, device_id, PresenceState.ONLINE, None
+        )
 
         prev_state = self.get_success(
             self.presence_handler.get_state(UserID.from_string(user_id))
@@ -839,7 +844,9 @@ def test_syncing_appropriately_handles_last_active_ts(self) -> None:
         # Poke the reactor so some time passes
         self.reactor.advance(2)
         self.get_success(
-            self.presence_handler.user_syncing(user_id, True, PresenceState.ONLINE)
+            self.presence_handler.user_syncing(
+                user_id, device_id, True, PresenceState.ONLINE
+            )
         )
 
         state = self.get_success(
@@ -848,6 +855,284 @@ def test_syncing_appropriately_handles_last_active_ts(self) -> None:
         # last_active_ts should not have changed as no pro-active event has occurred
         self.assertEqual(prev_state.last_active_ts, state.last_active_ts)
 
+    @parameterized.expand([(False,), (True,)])
+    def test_set_presence_with_syncing_from_offline_to_offline_explicit_single_client(
+        self, test_with_workers: bool
+    ) -> None:
+        """
+        Test that set_presence on /sync correctly
+        no-ops offline to offline explicitly
+        """
+        # This specifically is useful to test that direct messages pushed to device
+        # don't do weird things when calling sync with OFFLINE attached.
+        new_state = self._check_behaviour_of_presence_state_during_sync(
+            PresenceState.OFFLINE, PresenceState.OFFLINE, test_with_workers
+        )
+        self.assertEqual(new_state.state, PresenceState.OFFLINE)
+
+    @parameterized.expand([(False,), (True,)])
+    def test_set_presence_with_syncing_from_offline_to_online_explicit_single_client(
+        self, test_with_workers: bool
+    ) -> None:
+        """
+        Test that set_presence on /sync correctly
+        sets offline to online explicitly
+        """
+        new_state = self._check_behaviour_of_presence_state_during_sync(
+            PresenceState.OFFLINE, PresenceState.ONLINE, test_with_workers
+        )
+        self.assertEqual(new_state.state, PresenceState.ONLINE)
+
+    @parameterized.expand([(False,), (True,)])
+    def test_set_presence_with_syncing_from_offline_to_unavailable_explicit_single_client(
+        self, test_with_workers: bool
+    ) -> None:
+        """
+        Test that set_presence on /sync correctly
+        sets offline to unavailable explicitly
+        """
+        new_state = self._check_behaviour_of_presence_state_during_sync(
+            PresenceState.OFFLINE, PresenceState.UNAVAILABLE, test_with_workers
+        )
+        self.assertEqual(new_state.state, PresenceState.UNAVAILABLE)
+
+    @parameterized.expand([(False,), (True,)])
+    def test_set_presence_with_syncing_from_online_to_offline_explicit_single_client(
+        self, test_with_workers: bool
+    ) -> None:
+        """
+        Test that set_presence on /sync correctly
+        no-ops online to online
+        """
+        new_state = self._check_behaviour_of_presence_state_during_sync(
+            PresenceState.ONLINE, PresenceState.OFFLINE, test_with_workers
+        )
+        self.assertEqual(new_state.state, PresenceState.OFFLINE)
+
+    @parameterized.expand([(False,), (True,)])
+    def test_set_presence_with_syncing_from_online_to_online_explicit_single_client(
+        self, test_with_workers: bool
+    ) -> None:
+        """
+        Test that set_presence on /sync correctly
+        no-ops online to online
+        """
+        new_state = self._check_behaviour_of_presence_state_during_sync(
+            PresenceState.ONLINE, PresenceState.ONLINE, test_with_workers
+        )
+        self.assertEqual(new_state.state, PresenceState.ONLINE)
+
+    @parameterized.expand([(False,), (True,)])
+    def test_set_presence_with_syncing_from_online_to_unavailable_explicit_single_client(
+        self, test_with_workers: bool
+    ) -> None:
+        """
+        Test that set_presence on /sync correctly
+        sets online to unavailable explicitly
+        """
+        new_state = self._check_behaviour_of_presence_state_during_sync(
+            PresenceState.ONLINE, PresenceState.UNAVAILABLE, test_with_workers
+        )
+        self.assertEqual(new_state.state, PresenceState.UNAVAILABLE)
+
+    @parameterized.expand([(False,), (True,)])
+    def test_set_presence_with_syncing_from_unavailable_to_offline_explicit_single_client(
+        self, test_with_workers: bool
+    ) -> None:
+        """
+        Test that set_presence on /sync correctly
+        changes unavailable to offline explicitly
+        """
+        new_state = self._check_behaviour_of_presence_state_during_sync(
+            PresenceState.UNAVAILABLE, PresenceState.OFFLINE, test_with_workers
+        )
+        self.assertEqual(new_state.state, PresenceState.OFFLINE)
+
+    @parameterized.expand([(False,), (True,)])
+    def test_set_presence_with_syncing_from_unavailable_to_online_explicit_single_client(
+        self, test_with_workers: bool
+    ) -> None:
+        """
+        Test that set_presence on /sync correctly
+        sets unavailable to online explicitly
+        """
+        new_state = self._check_behaviour_of_presence_state_during_sync(
+            PresenceState.UNAVAILABLE, PresenceState.ONLINE, test_with_workers
+        )
+        self.assertEqual(new_state.state, PresenceState.ONLINE)
+
+    @parameterized.expand([(False,), (True,)])
+    def test_set_presence_with_syncing_from_unavailable_to_unavailable_explicit_single_client(
+        self, test_with_workers: bool
+    ) -> None:
+        """
+        Test that set_presence on /sync correctly
+        no-ops unavailable to unavailable
+        """
+        new_state = self._check_behaviour_of_presence_state_during_sync(
+            PresenceState.UNAVAILABLE, PresenceState.UNAVAILABLE, test_with_workers
+        )
+        self.assertEqual(new_state.state, PresenceState.UNAVAILABLE)
+
+    def _check_behaviour_of_presence_state_during_sync(
+        self, initial_state: str, requested_new_state: str, test_with_workers: bool
+    ) -> UserPresenceState:
+        """
+        Bulk test helper. When given an initial state, see what happens when requesting
+        a new/same state.
+
+        Args:
+            initial_state: The state to set at the beginning of the test
+            requested_new_state: The state to give to user_syncing
+
+        Returns:
+            UserPresenceState after user_syncing is complete.
+        """
+        user_id = "@test:server"
+        device_id = "device_whatever"
+
+        # 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": "synchrotron"}
+            )
+
+        self._set_presencestate_with_status_msg(user_id, device_id, initial_state, None)
+
+        # Test what happens to presence after user_syncing() is done.
+        self.get_success(
+            worker_to_sync_against.get_presence_handler().user_syncing(
+                user_id, device_id, True, requested_new_state
+            )
+        )
+
+        new_state = self.get_success(
+            self.presence_handler.get_state(UserID.from_string(user_id))
+        )
+        return new_state
+
+    def test_multi_device_online_to_online_with_offline(self) -> None:
+        new_state = self._check_behaviour_of_presence_state_during_sync_multi_device(
+            PresenceState.ONLINE, PresenceState.ONLINE, PresenceState.OFFLINE
+        )
+        self.assertEquals(new_state.state, PresenceState.ONLINE)
+
+    def test_multi_device_online_to_online_with_unavailable(self) -> None:
+        new_state = self._check_behaviour_of_presence_state_during_sync_multi_device(
+            PresenceState.ONLINE, PresenceState.ONLINE, PresenceState.UNAVAILABLE
+        )
+        self.assertEquals(new_state.state, PresenceState.ONLINE)
+
+    def test_multi_device_online_to_online_with_online(self) -> None:
+        new_state = self._check_behaviour_of_presence_state_during_sync_multi_device(
+            PresenceState.ONLINE, PresenceState.ONLINE, PresenceState.ONLINE
+        )
+        self.assertEquals(new_state.state, PresenceState.ONLINE)
+
+    def test_multi_device_online_to_unavailable_with_online(self) -> None:
+        new_state = self._check_behaviour_of_presence_state_during_sync_multi_device(
+            PresenceState.ONLINE, PresenceState.UNAVAILABLE, PresenceState.ONLINE
+        )
+        self.assertEquals(new_state.state, PresenceState.ONLINE)
+
+    def test_multi_device_online_to_unavailable_with_unavailable(self) -> None:
+        new_state = self._check_behaviour_of_presence_state_during_sync_multi_device(
+            PresenceState.ONLINE, PresenceState.UNAVAILABLE, PresenceState.UNAVAILABLE
+        )
+        self.assertEquals(new_state.state, PresenceState.UNAVAILABLE)
+
+    def test_multi_device_online_to_unavailable_with_offline(self) -> None:
+        new_state = self._check_behaviour_of_presence_state_during_sync_multi_device(
+            PresenceState.ONLINE, PresenceState.UNAVAILABLE, PresenceState.OFFLINE
+        )
+        self.assertEquals(new_state.state, PresenceState.UNAVAILABLE)
+
+    def test_multi_device_online_to_offline_with_online(self) -> None:
+        new_state = self._check_behaviour_of_presence_state_during_sync_multi_device(
+            PresenceState.ONLINE, PresenceState.OFFLINE, PresenceState.ONLINE
+        )
+        self.assertEquals(new_state.state, PresenceState.ONLINE)
+
+    def test_multi_device_online_to_offline_with_unavailable(self) -> None:
+        new_state = self._check_behaviour_of_presence_state_during_sync_multi_device(
+            PresenceState.ONLINE, PresenceState.OFFLINE, PresenceState.UNAVAILABLE
+        )
+        self.assertEquals(new_state.state, PresenceState.UNAVAILABLE)
+
+    def test_multi_device_online_to_offline_with_offline(self) -> None:
+        new_state = self._check_behaviour_of_presence_state_during_sync_multi_device(
+            PresenceState.ONLINE, PresenceState.OFFLINE, PresenceState.OFFLINE
+        )
+        self.assertEquals(new_state.state, PresenceState.OFFLINE)
+
+    def _check_behaviour_of_presence_state_during_sync_multi_device(
+        self,
+        initial_state: Optional[str] = None,
+        requested_new_state: str = PresenceState.ONLINE,
+        other_device_state: str = PresenceState.ONLINE,
+    ) -> UserPresenceState:
+        """
+        Bulk test helper. When given an initial state, see what happens when requesting
+        a new/same state.
+
+        The simulation is here is slightly complicated. We have one user with an extra
+        device. We test that our primary device requests a sync with the state provided,
+        and while that context manager is held open we sync on the other device. If
+        initial_state is provided, set that state before the first sync.
+
+        Args:
+            initial_state: The state to set at the beginning of the test
+            requested_new_state: The state to give to user_syncing
+            other_device_state:
+        Returns:
+            UserPresenceState from the other device.
+        """
+        user_name = "test"
+        password = "password2"
+        user_id = self.register_user(user_name, password)
+        device_id_1 = "device_id_1"
+        device_id_2 = "device_id_2"
+        # We probably will never need to actually use an access token, but capture it
+        self.login(user_name, password, device_id_1)
+        self.login(user_name, password, device_id_2)
+
+        # Just in case the login needs to process
+        self.reactor.pump([0])
+        if initial_state:
+            self._set_presencestate_with_status_msg(
+                user_id, device_id_1, initial_state, None
+            )
+
+        # Hold open the first sync
+        sctx = self.get_success(
+            self.presence_handler.user_syncing(
+                user_id, device_id_1, True, requested_new_state
+            )
+        )
+        with sctx:
+            # And do the second sync. Grab the state while in here to return it
+            prev_state = self.get_success(
+                self.presence_handler.get_state(UserID.from_string(user_id))
+            )
+            self.get_success(
+                self.presence_handler.user_syncing(
+                    user_id, device_id_2, True, other_device_state
+                )
+            )
+            state_to_return = self.get_success(
+                self.presence_handler.get_state(UserID.from_string(user_id))
+            )
+            # give the reactor some time, in case it needs to process anything
+            self.reactor.advance(5)
+
+        self.assertEqual(prev_state.state, requested_new_state)
+
+        return state_to_return
+
     def _set_presencestate_with_status_msg(
         self,
         user_id: str,
@@ -870,7 +1155,9 @@ def _set_presencestate_with_status_msg(
                 {"presence": state, "status_msg": status_msg},
             )
         )
-
+        # Have to chinch the system here, as the assert will expect something not None.
+        # If you use this for your tests, just don't let state be None.
+        state = state if state is not None else PresenceState.ONLINE
         new_state = self.get_success(
             self.presence_handler.get_state(UserID.from_string(user_id))
         )