move experimental feature msc3881 (remotely toggle push) to per-user flag

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.