From 1739ce698a48e8abb11175c8c98397ce9c585b9d Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 28 Mar 2023 14:56:14 -0700 Subject: move experimental feature msc3881 (remotely toggle push) to per-user flag --- synapse/config/experimental.py | 3 --- synapse/rest/client/pusher.py | 13 +++++++++---- synapse/rest/client/versions.py | 2 +- tests/push/test_http.py | 27 ++++++++++++++++++++++----- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 43f544c34e..e0a84e56d3 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -124,9 +124,6 @@ class ExperimentalConfig(Config): raw_msc3866_config = experimental.get("msc3866", {}) self.msc3866 = MSC3866Config(**raw_msc3866_config) - # MSC3881: Remotely toggle push notifications for another client - self.msc3881_enabled: bool = experimental.get("msc3881_enabled", False) - # MSC3882: Allow an existing session to sign in a new session self.msc3882_enabled: bool = experimental.get("msc3882_enabled", False) self.msc3882_ui_auth: bool = experimental.get("msc3882_ui_auth", True) diff --git a/synapse/rest/client/pusher.py b/synapse/rest/client/pusher.py index 1a8f5292ac..0cc63dc5de 100644 --- a/synapse/rest/client/pusher.py +++ b/synapse/rest/client/pusher.py @@ -42,7 +42,6 @@ class PushersRestServlet(RestServlet): super().__init__() self.hs = hs self.auth = hs.get_auth() - self._msc3881_enabled = self.hs.config.experimental.msc3881_enabled async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) @@ -55,7 +54,9 @@ class PushersRestServlet(RestServlet): pusher_dicts = [p.as_dict() for p in pushers] for pusher in pusher_dicts: - if self._msc3881_enabled: + if await self.hs.get_datastores().main.get_feature_enabled( + user.to_string(), "msc3881" + ): pusher["org.matrix.msc3881.enabled"] = pusher["enabled"] pusher["org.matrix.msc3881.device_id"] = pusher["device_id"] del pusher["enabled"] @@ -73,7 +74,6 @@ class PushersSetRestServlet(RestServlet): self.auth = hs.get_auth() self.notifier = hs.get_notifier() self.pusher_pool = self.hs.get_pusherpool() - self._msc3881_enabled = self.hs.config.experimental.msc3881_enabled async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]: requester = await self.auth.get_user_by_req(request) @@ -113,7 +113,12 @@ class PushersSetRestServlet(RestServlet): append = content["append"] enabled = True - if self._msc3881_enabled and "org.matrix.msc3881.enabled" in content: + if ( + await self.hs.get_datastores().main.get_feature_enabled( + user.to_string(), "msc3881" + ) + and "org.matrix.msc3881.enabled" in content + ): enabled = content["org.matrix.msc3881.enabled"] if not append: diff --git a/synapse/rest/client/versions.py b/synapse/rest/client/versions.py index 1606250e98..ae2101ce83 100644 --- a/synapse/rest/client/versions.py +++ b/synapse/rest/client/versions.py @@ -115,7 +115,7 @@ class VersionsRestServlet(RestServlet): # Adds support for login token requests as per MSC3882 "org.matrix.msc3882": self.config.experimental.msc3882_enabled, # Adds support for remotely enabling/disabling pushers, as per MSC3881 - "org.matrix.msc3881": self.config.experimental.msc3881_enabled, + "org.matrix.msc3881": True, # Adds support for filtering /messages by event relation. "org.matrix.msc3874": self.config.experimental.msc3874_enabled, # Adds support for simple HTTP rendezvous as per MSC3886 diff --git a/tests/push/test_http.py b/tests/push/test_http.py index 99cec0836b..67c1934eaa 100644 --- a/tests/push/test_http.py +++ b/tests/push/test_http.py @@ -819,7 +819,6 @@ class HTTPPusherTests(HomeserverTestCase): self.helper.send(room, body="Hello", tok=access_token) self.assertEqual(len(self.push_attempts), 1) - @override_config({"experimental_features": {"msc3881_enabled": True}}) def test_disable(self) -> None: """Tests that disabling a pusher means it's not pushed to anymore.""" user_id, access_token = self._make_user_with_pusher("user") @@ -828,6 +827,11 @@ class HTTPPusherTests(HomeserverTestCase): room = self.helper.create_room_as(user_id, tok=access_token) self.helper.join(room=room, user=other_user_id, tok=other_access_token) + # enable msc3881 per_user flag + self.get_success( + self.hs.get_datastores().main.set_feature_for_user(user_id, "msc3881", True) + ) + # Send a message and check that it generated a push. self.helper.send(room, body="Hi!", tok=other_access_token) self.assertEqual(len(self.push_attempts), 1) @@ -848,7 +852,6 @@ class HTTPPusherTests(HomeserverTestCase): self.assertFalse(enabled) self.assertTrue(isinstance(enabled, bool)) - @override_config({"experimental_features": {"msc3881_enabled": True}}) def test_enable(self) -> None: """Tests that enabling a disabled pusher means it gets pushed to.""" # Create the user with the pusher already disabled. @@ -858,6 +861,11 @@ class HTTPPusherTests(HomeserverTestCase): room = self.helper.create_room_as(user_id, tok=access_token) self.helper.join(room=room, user=other_user_id, tok=other_access_token) + # enable msc3881 per_user flag + self.get_success( + self.hs.get_datastores().main.set_feature_for_user(user_id, "msc3881", True) + ) + # Send a message and check that it did not generate a push. self.helper.send(room, body="Hi!", tok=other_access_token) self.assertEqual(len(self.push_attempts), 0) @@ -878,7 +886,7 @@ class HTTPPusherTests(HomeserverTestCase): self.assertTrue(enabled) self.assertTrue(isinstance(enabled, bool)) - @override_config({"experimental_features": {"msc3881_enabled": True}}) + # @override_config({"experimental_features": {"msc3881_enabled": True}}) def test_null_enabled(self) -> None: """Tests that a pusher that has an 'enabled' column set to NULL (eg pushers created before the column was introduced) is considered enabled. @@ -887,6 +895,11 @@ class HTTPPusherTests(HomeserverTestCase): # database. user_id, access_token = self._make_user_with_pusher("user", enabled=None) # type: ignore[arg-type] + # enable msc3881 per_user flag + self.get_success( + self.hs.get_datastores().main.set_feature_for_user(user_id, "msc3881", True) + ) + channel = self.make_request("GET", "/pushers", access_token=access_token) self.assertEqual(channel.code, 200) self.assertEqual(len(channel.json_body["pushers"]), 1) @@ -922,14 +935,18 @@ class HTTPPusherTests(HomeserverTestCase): self.assertEqual(len(pushers), 1) self.assertEqual(pushers[0].device_id, device_id) - @override_config({"experimental_features": {"msc3881_enabled": True}}) def test_device_id(self) -> None: """Tests that a pusher created with a given device ID shows that device ID in GET /pushers requests. """ - self.register_user("user", "pass") + user = self.register_user("user", "pass") access_token = self.login("user", "pass") + # enable msc3881 per_user flag + self.get_success( + self.hs.get_datastores().main.set_feature_for_user(user, "msc3881", True) + ) + # We create the pusher with an HTTP request rather than with # _make_user_with_pusher so that we can test the device ID is correctly set when # creating a pusher via an API call. -- cgit 1.4.1