Improve ServerNoticeServlet to avoid duplicate requests (matrix-org#10679)

Fixes: matrix-org#9544

Author: dklimpel

changelog.d/10679.bugfix

@@ -0,0 +1 @@
+Improve ServerNoticeServlet to avoid duplicate requests and add unit tests.

synapse/rest/admin/__init__.py

@@ -223,7 +223,6 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
     RoomMembersRestServlet(hs).register(http_server)
     DeleteRoomRestServlet(hs).register(http_server)
     JoinRoomAliasServlet(hs).register(http_server)
-    SendServerNoticeServlet(hs).register(http_server)
     VersionServlet(hs).register(http_server)
     UserAdminServlet(hs).register(http_server)
     UserMembershipRestServlet(hs).register(http_server)
@@ -247,6 +246,10 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
     NewRegistrationTokenRestServlet(hs).register(http_server)
     RegistrationTokenRestServlet(hs).register(http_server)
 
+    # Some servlets only get registered for the main process.
+    if hs.config.worker_app is None:
+        SendServerNoticeServlet(hs).register(http_server)
+
 
 def register_servlets_for_client_rest_resource(
     hs: "HomeServer", http_server: HttpServer

synapse/rest/admin/server_notice_servlet.py

@@ -14,7 +14,7 @@
 from typing import TYPE_CHECKING, Optional, Tuple
 
 from synapse.api.constants import EventTypes
-from synapse.api.errors import SynapseError
+from synapse.api.errors import NotFoundError, SynapseError
 from synapse.http.server import HttpServer
 from synapse.http.servlet import (
     RestServlet,
@@ -53,6 +53,8 @@ class SendServerNoticeServlet(RestServlet):
     def __init__(self, hs: "HomeServer"):
         self.hs = hs
         self.auth = hs.get_auth()
+        self.server_notices_manager = hs.get_server_notices_manager()
+        self.admin_handler = hs.get_admin_handler()
         self.txns = HttpTransactionCache(hs)
 
     def register(self, json_resource: HttpServer):
@@ -79,19 +81,22 @@ async def on_POST(
         # We grab the server notices manager here as its initialisation has a check for worker processes,
         # but worker processes still need to initialise SendServerNoticeServlet (as it is part of the
         # admin api).
-        if not self.hs.get_server_notices_manager().is_enabled():
+        if not self.server_notices_manager.is_enabled():
             raise SynapseError(400, "Server notices are not enabled on this server")
 
-        user_id = body["user_id"]
-        UserID.from_string(user_id)
-        if not self.hs.is_mine_id(user_id):
+        target_user = UserID.from_string(body["user_id"])
+        if not self.hs.is_mine(target_user):
             raise SynapseError(400, "Server notices can only be sent to local users")
 
-        event = await self.hs.get_server_notices_manager().send_notice(
-            user_id=body["user_id"],
+        if not await self.admin_handler.get_user(target_user):
+            raise NotFoundError("User not found")
+
+        event = await self.server_notices_manager.send_notice(
+            user_id=target_user.to_string(),
             type=event_type,
             state_key=state_key,
             event_content=body["content"],
+            txn_id=txn_id,
         )
 
         return 200, {"event_id": event.event_id}

synapse/server_notices/server_notices_manager.py

@@ -12,26 +12,23 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import logging
-from typing import Optional
+from typing import TYPE_CHECKING, Optional
 
 from synapse.api.constants import EventTypes, Membership, RoomCreationPreset
 from synapse.events import EventBase
 from synapse.types import UserID, create_requester
 from synapse.util.caches.descriptors import cached
 
+if TYPE_CHECKING:
+    from synapse.server import HomeServer
+
 logger = logging.getLogger(__name__)
 
 SERVER_NOTICE_ROOM_TAG = "m.server_notice"
 
 
 class ServerNoticesManager:
-    def __init__(self, hs):
-        """
-
-        Args:
-            hs (synapse.server.HomeServer):
-        """
-
+    def __init__(self, hs: "HomeServer"):
         self._store = hs.get_datastore()
         self._config = hs.config
         self._account_data_handler = hs.get_account_data_handler()
@@ -58,6 +55,7 @@ async def send_notice(
         event_content: dict,
         type: str = EventTypes.Message,
         state_key: Optional[str] = None,
+        txn_id: Optional[str] = None,
     ) -> EventBase:
         """Send a notice to the given user
 
@@ -68,6 +66,7 @@ async def send_notice(
             event_content: content of event to send
             type: type of event
             is_state_event: Is the event a state event
+            txn_id: The transaction ID.
         """
         room_id = await self.get_or_create_notice_room_for_user(user_id)
         await self.maybe_invite_user_to_room(user_id, room_id)
@@ -90,7 +89,7 @@ async def send_notice(
             event_dict["state_key"] = state_key
 
         event, _ = await self._event_creation_handler.create_and_send_nonmember_event(
-            requester, event_dict, ratelimit=False
+            requester, event_dict, ratelimit=False, txn_id=txn_id
         )
         return event