MSC3911: ap? automatic pending media deletion (#130)

# Linked Media MSC3911 AP?: Automatic media deletion
[#3365](famedly/product-management#3365)

Background job to automatically delete unattached media. This should
likely use the same 24h as asynchronous media uploads.

# Acceptance criteria

- [x] Background job to delete pending media after 24h

Author: itsoyou

docker/complement/conf/workers-shared-extra.yaml.j2

@@ -130,7 +130,11 @@ experimental_features:
   # Invite filtering
   msc4155_enabled: true
   # Media Attachment
-  msc3911_enabled: true
+  msc3911:
+    enabled: true
+    block_unrestricted_media_upload: false
+    purge_pending_unattached_media: true
+    pending_media_cleanup_interval_ms: 86400000 # 1 day
 
 server_notices:
   system_mxid_localpart: _server

synapse/config/experimental.py

@@ -365,6 +365,22 @@ class MSC3866Config:
     require_approval_for_new_accounts: bool = False
 
 
+@attr.s(auto_attribs=True, frozen=True, slots=True)
+class MSC3911Config:
+    """Configuration for MSC3911 (Linking Media to Events)"""
+
+    # MSC3911 is enabled
+    enabled: bool = False
+
+    # Disable the current media create and upload endpoints
+    block_unrestricted_media_upload: bool = False
+
+    # Delete pending media that is older than certain interval and not attached to any events.
+    purge_pending_unattached_media: bool = False
+    # This configures how often the cleanup loop run in milliseconds
+    pending_media_cleanup_interval_ms: int = 24 * 60 * 60 * 1000  # 1 day
+
+
 class ExperimentalConfig(Config):
     """Config section for enabling experimental features"""
 
@@ -590,9 +606,5 @@ def read_config(
         self.msc4306_enabled: bool = experimental.get("msc4306_enabled", False)
 
         # MSC3911: Linking Media to Events
-        self.msc3911_enabled: bool = experimental.get("msc3911_enabled", False)
-
-        # Disable the current media create and upload endpoints
-        self.msc3911_unrestricted_media_upload_disabled: bool = experimental.get(
-            "msc3911_unrestricted_media_upload_disabled", False
-        )
+        raw_msc3911_config = experimental.get("msc3911", {})
+        self.msc3911 = MSC3911Config(**raw_msc3911_config)

synapse/federation/transport/server/federation.py

@@ -79,7 +79,7 @@ def __init__(
     ):
         super().__init__(hs, authenticator, ratelimiter, server_name)
         self.handler = hs.get_federation_server()
-        self.enable_restricted_media = hs.config.experimental.msc3911_enabled
+        self.enable_restricted_media = hs.config.experimental.msc3911.enabled
 
 
 class FederationSendServlet(BaseFederationServerServlet):

synapse/handlers/profile.py

@@ -80,9 +80,9 @@ def __init__(self, hs: "HomeServer"):
 
         self._third_party_rules = hs.get_module_api_callbacks().third_party_event_rules
 
-        self.enable_restricted_media = hs.config.experimental.msc3911_enabled
-        self.disable_unrestricted_media = (
-            hs.config.experimental.msc3911_unrestricted_media_upload_disabled
+        self.enable_restricted_media = hs.config.experimental.msc3911.enabled
+        self.block_unrestricted_media = (
+            hs.config.experimental.msc3911.block_unrestricted_media_upload
         )
 
     async def get_profile(self, user_id: str, ignore_backoff: bool = True) -> JsonDict:
@@ -334,7 +334,7 @@ async def validate_avatar_url(self, avatar_url: str, requester: Requester) -> No
             # event, it will either be copied/passed along/dropped depending on the
             # above circumstances
             if not media_info:
-                if self.disable_unrestricted_media:
+                if self.block_unrestricted_media:
                     # The user should have done a COPY on this media previous to this
                     # attempt to set
                     raise SynapseError(
@@ -345,7 +345,7 @@ async def validate_avatar_url(self, avatar_url: str, requester: Requester) -> No
                 # For backwards compatible behavior, treat the media as unrestricted
                 return
 
-        if self.disable_unrestricted_media and not media_info.restricted:
+        if self.block_unrestricted_media and not media_info.restricted:
             raise SynapseError(
                 HTTPStatus.BAD_REQUEST,
                 f"The media attachment request is invalid as the media '{mxc_uri.media_id}' is not restricted",

synapse/handlers/room.py

@@ -1232,7 +1232,7 @@ async def create_room(
         room_avatar = initial_state.get((EventTypes.RoomAvatar, ""))
         # It is unfortunate that this conditional block has to be run twice: once here
         # to validate the data and again to actually attach the media reference.
-        if room_avatar is not None and self.config.experimental.msc3911_enabled:
+        if room_avatar is not None and self.config.experimental.msc3911.enabled:
             # this should be an mxc, but the spec does not specifically say it has to be
             extracted_media_id: Optional[str] = room_avatar.get("url")
             # It may be that "url" is set to either an empty string or None. Accept
@@ -1550,7 +1550,7 @@ async def create_event(
 
             mxc_restrictions = None
             if (
-                self.config.experimental.msc3911_enabled
+                self.config.experimental.msc3911.enabled
                 and etype == EventTypes.RoomAvatar
             ):
                 # this should be an mxc, but the spec does not specifically say it has to be

synapse/handlers/room_member.py

@@ -109,9 +109,9 @@ def __init__(self, hs: "HomeServer"):
         self.account_data_handler = hs.get_account_data_handler()
         self.event_auth_handler = hs.get_event_auth_handler()
         self._worker_lock_handler = hs.get_worker_locks_handler()
-        self.enable_restricted_media = hs.config.experimental.msc3911_enabled
+        self.enable_restricted_media = hs.config.experimental.msc3911.enabled
         self.allow_legacy_media = (
-            not hs.config.experimental.msc3911_unrestricted_media_upload_disabled
+            not hs.config.experimental.msc3911.block_unrestricted_media_upload
         )
 
         self._membership_types_to_include_profile_data_in = {

synapse/media/media_repository.py

@@ -112,7 +112,7 @@ def __init__(self, hs: "HomeServer"):
         self.server_name = hs.hostname
         self.store = hs.get_datastores().main
         self._is_mine_server_name = hs.is_mine_server_name
-        self.enable_media_restriction = self.hs.config.experimental.msc3911_enabled
+        self.enable_media_restriction = self.hs.config.experimental.msc3911.enabled
 
     @trace
     async def create_media_id_without_expiration(
@@ -643,6 +643,16 @@ def __init__(self, hs: "HomeServer"):
             key=lambda limit: limit.time_period_ms, reverse=True
         )
 
+        if (
+            self.hs.config.experimental.msc3911.purge_pending_unattached_media
+            and hs.config.media.media_instance_running_background_jobs
+            == hs.config.worker.worker_name
+        ):
+            self.clock.looping_call(
+                self._pending_media_cleanup,
+                self.hs.config.experimental.msc3911.pending_media_cleanup_interval_ms,
+            )
+
     def _start_update_recently_accessed(self) -> Deferred:
         return run_as_background_process(
             "update_recently_accessed_media", self._update_recently_accessed
@@ -664,6 +674,13 @@ async def _update_recently_accessed(self) -> None:
             local_media, remote_media, self.clock.time_msec()
         )
 
+    async def _pending_media_cleanup(self) -> None:
+        pending_media_ids = await self.store.get_pending_media_ids(
+            self.hs.config.experimental.msc3911.pending_media_cleanup_interval_ms
+        )
+        if pending_media_ids:
+            await self.delete_local_media_ids(pending_media_ids)
+
     def mark_recently_accessed(self, server_name: Optional[str], media_id: str) -> None:
         """Mark the given media as recently accessed.
 

synapse/media/thumbnailer.py

@@ -271,7 +271,7 @@ def __init__(
         self.media_storage = media_storage
         self.store = hs.get_datastores().main
         self.dynamic_thumbnails = hs.config.media.dynamic_thumbnails
-        self.enable_media_restriction = self.hs.config.experimental.msc3911_enabled
+        self.enable_media_restriction = self.hs.config.experimental.msc3911.enabled
 
     async def respond_local_thumbnail(
         self,

synapse/rest/client/media.py

@@ -410,7 +410,7 @@ def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
     MediaConfigResource(hs).register(http_server)
     ThumbnailResource(hs, media_repo, media_repo.media_storage).register(http_server)
     DownloadResource(hs, media_repo).register(http_server)
-    if hs.config.experimental.msc3911_enabled:
+    if hs.config.experimental.msc3911.enabled:
         CreateResource(hs, media_repo, restricted=True).register(http_server)
         UploadRestrictedResource(hs, media_repo).register(http_server)
         CopyResource(hs, media_repo).register(http_server)

synapse/rest/client/profile.py

@@ -109,7 +109,7 @@ def __init__(self, hs: "HomeServer"):
         self.hs = hs
         self.profile_handler = hs.get_profile_handler()
         self.auth = hs.get_auth()
-        self.enable_restricted_media = hs.config.experimental.msc3911_enabled
+        self.enable_restricted_media = hs.config.experimental.msc3911.enabled
 
     async def on_GET(
         self, request: SynapseRequest, user_id: str, field_name: str
@@ -178,7 +178,7 @@ async def on_PUT(
 
         content = parse_json_object_from_request(request)
         try:
-            new_value = content[field_name]
+            input_value = content[field_name]
         except KeyError:
             raise SynapseError(
                 400, f"Missing key '{field_name}'", errcode=Codes.MISSING_PARAM
@@ -200,24 +200,24 @@ async def on_PUT(
             )
         if field_name == ProfileFields.DISPLAYNAME:
             await self.profile_handler.set_displayname(
-                user, requester, new_value, is_admin, propagate=propagate
+                user, requester, input_value, is_admin, propagate=propagate
             )
         elif field_name == ProfileFields.AVATAR_URL:
-            if self.enable_restricted_media and new_value:
+            if self.enable_restricted_media and input_value:
                 current_avatar_url = (
                     await self.profile_handler.store.get_profile_avatar_url(
                         requester.user
                     )
                 )
                 # If new_value is the same as existing one, keep the function idempotent
-                if current_avatar_url and str(current_avatar_url) == new_value:
+                if current_avatar_url and str(current_avatar_url) == input_value:
                     return 200, {}
             await self.profile_handler.set_avatar_url(
-                user, requester, new_value, is_admin, propagate=propagate
+                user, requester, input_value, is_admin, propagate=propagate
             )
         else:
             await self.profile_handler.set_profile_field(
-                user, requester, field_name, new_value, is_admin
+                user, requester, field_name, input_value, is_admin
             )
 
         return 200, {}