Server notices: Dissociate room creation/lookup from invite (#7199)

Fixes #6815

Before figuring out whether we should alert a user on MAU, we call get_notice_room_for_user to get some info on the existing server notices room for this user. This function, if the room doesn't exist, creates it and invites the user in it. This means that, if we decide later that no server notice is needed, the user gets invited in a room with no message in it. This happens at every restart of the server, since the room ID returned by get_notice_room_for_user is cached.

This PR fixes that by moving the inviting bit to a dedicated function, that's only called when the server actually needs to send a notice to the user. A potential issue with this approach is that the room that's created by get_notice_room_for_user doesn't match how that same function looks for an existing room (i.e. it creates a room that doesn't have an invite or a join for the current user in it, so it could lead to a new room being created each time a user syncs), but I'm not sure this is a problem given it's cached until the server restarts, so that function won't run very often.

It also renames get_notice_room_for_user into get_or_create_notice_room_for_user to make what it does clearer.

2 files changed, 45 insertions, 10 deletions

diff --git a/synapse/server_notices/resource_limits_server_notices.py b/synapse/server_notices/resource_limits_server_notices.py
index 9fae2e0afe..ce4a828894 100644
--- a/synapse/server_notices/resource_limits_server_notices.py
+++ b/synapse/server_notices/resource_limits_server_notices.py
@@ -80,7 +80,9 @@ class ResourceLimitsServerNotices(object):
             # In practice, not sure we can ever get here
             return
 
-        room_id = yield self._server_notices_manager.get_notice_room_for_user(user_id)
+        room_id = yield self._server_notices_manager.get_or_create_notice_room_for_user(
+            user_id
+        )
 
         if not room_id:
             logger.warning("Failed to get server notices room")
diff --git a/synapse/server_notices/server_notices_manager.py b/synapse/server_notices/server_notices_manager.py
index f7432c8d2f..bf0943f265 100644
--- a/synapse/server_notices/server_notices_manager.py
+++ b/synapse/server_notices/server_notices_manager.py
@@ -17,7 +17,7 @@ import logging
 from twisted.internet import defer
 
 from synapse.api.constants import EventTypes, Membership, RoomCreationPreset
-from synapse.types import create_requester
+from synapse.types import UserID, create_requester
 from synapse.util.caches.descriptors import cachedInlineCallbacks
 
 logger = logging.getLogger(__name__)
@@ -36,10 +36,12 @@ class ServerNoticesManager(object):
         self._store = hs.get_datastore()
         self._config = hs.config
         self._room_creation_handler = hs.get_room_creation_handler()
+        self._room_member_handler = hs.get_room_member_handler()
         self._event_creation_handler = hs.get_event_creation_handler()
         self._is_mine_id = hs.is_mine_id
 
         self._notifier = hs.get_notifier()
+        self.server_notices_mxid = self._config.server_notices_mxid
 
     def is_enabled(self):
         """Checks if server notices are enabled on this server.
@@ -66,7 +68,8 @@ class ServerNoticesManager(object):
         Returns:
             Deferred[FrozenEvent]
         """
-        room_id = yield self.get_notice_room_for_user(user_id)
+        room_id = yield self.get_or_create_notice_room_for_user(user_id)
+        yield self.maybe_invite_user_to_room(user_id, room_id)
 
         system_mxid = self._config.server_notices_mxid
         requester = create_requester(system_mxid)
@@ -89,10 +92,11 @@ class ServerNoticesManager(object):
         return res
 
     @cachedInlineCallbacks()
-    def get_notice_room_for_user(self, user_id):
+    def get_or_create_notice_room_for_user(self, user_id):
         """Get the room for notices for a given user
 
-        If we have not yet created a notice room for this user, create it
+        If we have not yet created a notice room for this user, create it, but don't
+        invite the user to it.
 
         Args:
             user_id (str): complete user id for the user we want a room for
@@ -108,7 +112,6 @@ class ServerNoticesManager(object):
         rooms = yield self._store.get_rooms_for_local_user_where_membership_is(
             user_id, [Membership.INVITE, Membership.JOIN]
         )
-        system_mxid = self._config.server_notices_mxid
         for room in rooms:
             # it's worth noting that there is an asymmetry here in that we
             # expect the user to be invited or joined, but the system user must
@@ -116,10 +119,14 @@ class ServerNoticesManager(object):
             # manages to invite the system user to a room, that doesn't make it
             # the server notices room.
             user_ids = yield self._store.get_users_in_room(room.room_id)
-            if system_mxid in user_ids:
+            if self.server_notices_mxid in user_ids:
                 # we found a room which our user shares with the system notice
                 # user
-                logger.info("Using room %s", room.room_id)
+                logger.info(
+                    "Using existing server notices room %s for user %s",
+                    room.room_id,
+                    user_id,
+                )
                 return room.room_id
 
         # apparently no existing notice room: create a new one
@@ -138,14 +145,13 @@ class ServerNoticesManager(object):
                 "avatar_url": self._config.server_notices_mxid_avatar_url,
             }
 
-        requester = create_requester(system_mxid)
+        requester = create_requester(self.server_notices_mxid)
         info = yield self._room_creation_handler.create_room(
             requester,
             config={
                 "preset": RoomCreationPreset.PRIVATE_CHAT,
                 "name": self._config.server_notices_room_name,
                 "power_level_content_override": {"users_default": -10},
-                "invite": (user_id,),
             },
             ratelimit=False,
             creator_join_profile=join_profile,
@@ -159,3 +165,30 @@ class ServerNoticesManager(object):
 
         logger.info("Created server notices room %s for %s", room_id, user_id)
         return room_id
+
+    @defer.inlineCallbacks
+    def maybe_invite_user_to_room(self, user_id: str, room_id: str):
+        """Invite the given user to the given server room, unless the user has already
+        joined or been invited to it.
+
+        Args:
+            user_id: The ID of the user to invite.
+            room_id: The ID of the room to invite the user to.
+        """
+        requester = create_requester(self.server_notices_mxid)
+
+        # Check whether the user has already joined or been invited to this room. If
+        # that's the case, there is no need to re-invite them.
+        joined_rooms = yield self._store.get_rooms_for_local_user_where_membership_is(
+            user_id, [Membership.INVITE, Membership.JOIN]
+        )
+        for room in joined_rooms:
+            if room.room_id == room_id:
+                return
+
+        yield self._room_member_handler.update_membership(
+            requester=requester,
+            target=UserID.from_string(user_id),
+            room_id=room_id,
+            action="invite",
+        )