Browse Source

Fix yields and copy instead of move push rules on room upgrade (#6144)

Copy push rules during a room upgrade from the old room to the new room, instead of deleting them from the old room.

For instance, we've defined upgrading of a room multiple times to be possible, and push rules won't be transferred on the second upgrade if they're deleted during the first.

Also fix some missing yields that probably broke things quite a bit.
Andrew Morgan 4 years ago
parent
commit
2a1470cd05
3 changed files with 9 additions and 12 deletions
  1. 1 0
      changelog.d/6144.bugfix
  2. 2 2
      synapse/handlers/room_member.py
  3. 6 10
      synapse/storage/push_rule.py

+ 1 - 0
changelog.d/6144.bugfix

@@ -0,0 +1 @@
+Prevent user push rules being deleted from a room when it is upgraded.

+ 2 - 2
synapse/handlers/room_member.py

@@ -216,8 +216,8 @@ class RoomMemberHandler(object):
                 self.copy_room_tags_and_direct_to_room(
                     predecessor["room_id"], room_id, user_id
                 )
-                # Move over old push rules
-                self.store.move_push_rules_from_room_to_room_for_user(
+                # Copy over push rules
+                yield self.store.copy_push_rules_from_room_to_room_for_user(
                     predecessor["room_id"], room_id, user_id
                 )
         elif event.membership == Membership.LEAVE:

+ 6 - 10
synapse/storage/push_rule.py

@@ -183,8 +183,8 @@ class PushRulesWorkerStore(
         return results
 
     @defer.inlineCallbacks
-    def move_push_rule_from_room_to_room(self, new_room_id, user_id, rule):
-        """Move a single push rule from one room to another for a specific user.
+    def copy_push_rule_from_room_to_room(self, new_room_id, user_id, rule):
+        """Copy a single push rule from one room to another for a specific user.
 
         Args:
             new_room_id (str): ID of the new room.
@@ -209,14 +209,11 @@ class PushRulesWorkerStore(
             actions=rule["actions"],
         )
 
-        # Delete push rule for the old room
-        yield self.delete_push_rule(user_id, rule["rule_id"])
-
     @defer.inlineCallbacks
-    def move_push_rules_from_room_to_room_for_user(
+    def copy_push_rules_from_room_to_room_for_user(
         self, old_room_id, new_room_id, user_id
     ):
-        """Move all of the push rules from one room to another for a specific
+        """Copy all of the push rules from one room to another for a specific
         user.
 
         Args:
@@ -227,15 +224,14 @@ class PushRulesWorkerStore(
         # Retrieve push rules for this user
         user_push_rules = yield self.get_push_rules_for_user(user_id)
 
-        # Get rules relating to the old room, move them to the new room, then
-        # delete them from the old room
+        # Get rules relating to the old room and copy them to the new room
         for rule in user_push_rules:
             conditions = rule.get("conditions", [])
             if any(
                 (c.get("key") == "room_id" and c.get("pattern") == old_room_id)
                 for c in conditions
             ):
-                self.move_push_rule_from_room_to_room(new_room_id, user_id, rule)
+                yield self.copy_push_rule_from_room_to_room(new_room_id, user_id, rule)
 
     @defer.inlineCallbacks
     def bulk_get_push_rules_for_room(self, event, context):