Back out the database hack and replace it with a temporary config setting

synapse/config/server.py

@@ -530,6 +530,16 @@ class ServerConfig(Config):
             "request_token_inhibit_3pid_errors", False,
         )
 
+        # List of users trialing the new experimental default push rules. This setting is
+        # not included in the sample configuration file on purpose as it's a temporary
+        # hack, so that some users can trial the new defaults without impacting every
+        # user on the homeserver.
+        self.users_new_default_push_rules = (
+            config.get("users_new_default_push_rules") or []
+        )
+        if not isinstance(self.users_new_default_push_rules, list):
+            raise ConfigError("'users_new_default_push_rules' must be a list")
+
     def has_tls_listener(self) -> bool:
         return any(listener.tls for listener in self.listeners)
 

synapse/replication/slave/storage/push_rule.py

@@ -34,7 +34,7 @@ class SlavedPushRuleStore(SlavedEventStore, PushRulesWorkerStore):
         if stream_name == PushRulesStream.NAME:
             self._push_rules_stream_id_gen.advance(token)
             for row in rows:
-                self._get_push_rules_for_user.invalidate((row.user_id,))
+                self.get_push_rules_for_user.invalidate((row.user_id,))
                 self.get_push_rules_enabled_for_user.invalidate((row.user_id,))
                 self.push_rules_stream_cache.entity_has_changed(row.user_id, token)
         return super().process_replication_rows(stream_name, instance_name, token, rows)

synapse/storage/data_stores/main/push_rule.py

@@ -105,6 +105,8 @@ class PushRulesWorkerStore(
             prefilled_cache=push_rules_prefill,
         )
 
+        self.users_new_default_push_rules = hs.config.users_new_default_push_rules
+
     @abc.abstractmethod
     def get_max_push_rules_stream_id(self):
         """Get the position of the push rules stream.
@@ -115,7 +117,7 @@ class PushRulesWorkerStore(
         raise NotImplementedError()
 
     @cachedInlineCallbacks(max_entries=5000)
-    def _get_push_rules_for_user(self, user_id, use_new_defaults=False):
+    def get_push_rules_for_user(self, user_id):
         rows = yield self.db.simple_select_list(
             table="push_rules",
             keyvalues={"user_name": user_id},
@@ -134,24 +136,12 @@ class PushRulesWorkerStore(
 
         enabled_map = yield self.get_push_rules_enabled_for_user(user_id)
 
+        use_new_defaults = user_id in self.users_new_default_push_rules
+
         rules = _load_rules(rows, enabled_map, use_new_defaults)
 
         return rules
 
-    @defer.inlineCallbacks
-    def get_push_rules_for_user(self, user_id):
-        # Temporary hack so we can use the new experimental default push rules to some
-        # users without impacting others.
-        use_new_defaults = yield self.db.simple_select_list(
-            table="new_push_rules_users_tmp",
-            keyvalues={"user_id": user_id},
-            retcols=("user_id",),
-            desc="get_user_new_default_push_rules",
-        )
-
-        rules = yield self._get_push_rules_for_user(user_id, bool(use_new_defaults))
-        return rules
-
     @cachedInlineCallbacks(max_entries=5000)
     def get_push_rules_enabled_for_user(self, user_id):
         results = yield self.db.simple_select_list(
@@ -181,7 +171,7 @@ class PushRulesWorkerStore(
             )
 
     @cachedList(
-        cached_method_name="_get_push_rules_for_user",
+        cached_method_name="get_push_rules_for_user",
         list_name="user_ids",
         num_args=1,
         inlineCallbacks=True,
@@ -208,17 +198,10 @@ class PushRulesWorkerStore(
         enabled_map_by_user = yield self.bulk_get_push_rules_enabled(user_ids)
 
         for user_id, rules in results.items():
-            # Temporary hack so we can use the new experimental default push rules to some
-            # users without impacting others.
-            use_new_defaults = yield self.db.simple_select_list(
-                table="new_push_rules_users_tmp",
-                keyvalues={"user_id": user_id},
-                retcols=("user_id",),
-                desc="get_user_new_default_push_rules",
-            )
+            use_new_defaults = user_id in self.users_new_default_push_rules
 
             results[user_id] = _load_rules(
-                rules, enabled_map_by_user.get(user_id, {}), bool(use_new_defaults),
+                rules, enabled_map_by_user.get(user_id, {}), use_new_defaults,
             )
 
         return results
@@ -768,7 +751,7 @@ class PushRuleStore(PushRulesWorkerStore):
 
         self.db.simple_insert_txn(txn, "push_rules_stream", values=values)
 
-        txn.call_after(self._get_push_rules_for_user.invalidate, (user_id,))
+        txn.call_after(self.get_push_rules_for_user.invalidate, (user_id,))
         txn.call_after(self.get_push_rules_enabled_for_user.invalidate, (user_id,))
         txn.call_after(
             self.push_rules_stream_cache.entity_has_changed, user_id, stream_id

synapse/storage/data_stores/main/schema/delta/58/13new_push_rules_tmp.sql

@@ -1,21 +0,0 @@
-/* Copyright 2020 The Matrix.org Foundation C.I.C
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *    http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
--- This is a temporary table in which we store the IDs of the users for which we need to
--- serve the new experimental default push rules. The purpose of this table is to help
--- test these new defaults, so it shall be dropped when the experimentation is done.
-CREATE TABLE IF NOT EXISTS new_push_rules_users_tmp (
-    user_id TEXT PRIMARY KEY
-);