chores: docstring update

Author: itsoyou

synapse/media/media_repository.py

@@ -925,11 +925,11 @@ async def copy_media(
             # Let existing methods handle creating the new file for us. By not passing a
             # media id, one will be created.
             new_mxc_uri = await self.create_or_update_content(
-                old_media_info.media_type,
-                old_media_info.upload_name,
-                io_object,
-                old_media_info.media_length,
-                auth_user,
+                media_type=old_media_info.media_type,
+                upload_name=old_media_info.upload_name,
+                content=io_object,
+                content_length=old_media_info.media_length,
+                auth_user=auth_user,
                 restricted=True,
             )
 
@@ -1340,7 +1340,6 @@ async def _get_remote_media_impl(
             await self.is_media_visible(requester.user, media_info)
 
         file_id = media_info.filesystem_id
-
         if not media_info.media_type:
             media_info = attr.evolve(media_info, media_type="application/octet-stream")
 

tests/media/test_media_repository.py

@@ -59,6 +59,7 @@ def create_resource_dict(self) -> dict[str, Resource]:
         return resources
 
     def _store_media_with_path(self, file_info: FileInfo, expected_path: str) -> None:
+        """Store media with given path."""
         assert isinstance(self.repo, MediaRepository)
         ctx = self.repo.media_storage.store_into_file(file_info)
         (f, fname) = self.get_success(ctx.__aenter__())
@@ -70,8 +71,8 @@ def _store_media_with_path(self, file_info: FileInfo, expected_path: str) -> Non
 
     def _create_local_media_with_media_id_path(self, user: str) -> MXCUri:
         """
-        Force creation of a media object at the specific place based on the media id
-        path. This does assert that the file exists where it is expected to be
+        Force creation of a local media object at the specific place based on the media
+        id path. This does assert that the file exists where it is expected to be
         """
         assert isinstance(self.repo, MediaRepository)
         media_id = random_string(24)
@@ -121,6 +122,7 @@ def _create_local_media_with_media_id_path(self, user: str) -> MXCUri:
         return MXCUri(server_name=self.hs.config.server.server_name, media_id=media_id)
 
     def _create_local_media_with_sha256_path(self, user: str) -> MXCUri:
+        """Creates local media object with sha256 path."""
         assert isinstance(self.repo, MediaRepository)
         media_id = random_string(24)
         # Curate a specialized FileInfo that includes sha256 data, then file will be
@@ -162,18 +164,17 @@ def _create_local_media_with_sha256_path(self, user: str) -> MXCUri:
                 media_id=media_id,
                 file_id=media_id,
                 media_type="image/png",
-                # We purposely do not use the sha256 here, as it directly causes the sha256
-                # path for thumbnails to be populated, and that is not what we are looking
-                # for.
                 sha256=SMALL_PNG_SHA256,
             )
         )
-
         return MXCUri(server_name=self.hs.config.server.server_name, media_id=media_id)
 
     def _create_remote_media_with_media_id_path(self, server_name: str) -> str:
+        """Creates remote media object with media id path."""
         media_id = random_string(24)
         upload_name = "other_server_media"
+        # Curate a specialized FileInfo that is lacking sha256 data, then file will be
+        # forced to the old media_id path
         file_info = FileInfo(
             server_name=server_name,
             file_id=media_id,
@@ -205,6 +206,7 @@ def _create_remote_media_with_media_id_path(self, server_name: str) -> str:
         return media_id
 
     def _create_remote_media_with_sha256_path(self, server_name: str) -> str:
+        """Creates remote media object with sha256 path."""
         media_id = random_string(24)
         upload_name = "other_server_media"
         file_info = FileInfo(
@@ -265,11 +267,12 @@ def test_create_or_update_content_creates_new_content_with_sha256_path(
         assert not os.path.exists(old_path)
 
     def test_create_or_update_content_updates_content_with_media_id_path(self) -> None:
-        """Test that `create_or_update_content` function can update existing media with media_id path"""
-        # Strictly speaking, this is not an operation that is supposed to be supported,
-        # but is currently possible. In the case it becomes necessary, the behavior
-        # should not be unexpected.
-
+        """
+        Test that `create_or_update_content` function can update existing media with
+        media_id path. Strictly speaking, this is not an operation that is supposed to
+        be supported, but is currently possible. In the case it becomes necessary, the
+        behavior should not be unexpected.
+        """
         # Create media with media_id path
         mxc_uri = self._create_local_media_with_media_id_path(self.creator)
         media_id = mxc_uri.media_id
@@ -291,11 +294,12 @@ def test_create_or_update_content_updates_content_with_media_id_path(self) -> No
         assert os.path.exists(self.repo.filepaths.local_media_filepath(media_id))
 
     def test_create_or_update_content_updates_content_with_sha256_path(self) -> None:
-        """Test that `create_or_update_content` function can update existing media with sha256 path"""
-        # Strictly speaking, this is not an operation that is supposed to be supported,
-        # but is currently possible. In the case it becomes necessary, the behavior
-        # should not be unexpected.
-
+        """
+        Test that `create_or_update_content` function can update existing media with
+        sha256 path. Strictly speaking, this is not an operation that is supposed to be
+        supported, but is currently possible. In the case it becomes necessary, the
+        behavior should not be unexpected.
+        """
         # Create media with sha256 path
         mxc_uri = self._create_local_media_with_sha256_path(self.creator)
         media_id = mxc_uri.media_id
@@ -392,6 +396,10 @@ def test_get_local_media_with_sha256_path(self) -> None:
         self.assertEqual(channel.result["body"], SMALL_PNG)
 
     def test_get_remote_media_impl_with_sha256_path_cache_hit(self) -> None:
+        """
+        Test `_get_remote_media_impl` can fetch the media with sha256 path successfully
+        in case of cache hit.
+        """
         server_name = "other_server.com"
         # Generate remote media with sha256 path
         media_id = self._create_remote_media_with_sha256_path(server_name)
@@ -423,7 +431,10 @@ def test_get_remote_media_impl_with_sha256_path_cache_hit(self) -> None:
     def test_get_remote_media_impl_with_sha256_path_cache_miss_no_federation(
         self,
     ) -> None:
-        """Test that `_get_remote_media_impl` can fetch the media with sha256 path successfully"""
+        """
+        Test that `_get_remote_media_impl` can fetch the media with sha256 path
+        successfully in case of cache miss, without using federation endpoint.
+        """
 
         # Mock the download media function of the client.
         async def _mock_download_media(
@@ -492,7 +503,7 @@ async def _mock_download_media(
         assert thumbnail is not None
 
     def test_remove_local_media_from_disk_with_media_id_path(self) -> None:
-        """Test that `_remove_local_media_from_disk` can remove media with media_id path"""
+        """Test that `_remove_local_media_from_disk` can remove media with media_id path."""
         # Generate 2 media with media_id path
         media1_mxc = self._create_local_media_with_media_id_path(self.creator)
         media1_id = media1_mxc.media_id
@@ -509,7 +520,7 @@ def test_remove_local_media_from_disk_with_media_id_path(self) -> None:
         assert not os.path.exists(self.repo.filepaths.local_media_filepath(media2_id))
 
     def test_remove_local_media_from_disk_with_sha256_path(self) -> None:
-        """Test that `_remove_local_media_from_disk` can remove media with sha256 path"""
+        """Test that `_remove_local_media_from_disk` can remove media with sha256 path."""
         # Generate 2 media with sha256 path with the same image.
         # There should be 2 rows in the table and only 1 media stored in filesystem.
         media1_mxc = self._create_local_media_with_sha256_path(self.creator)

tests/rest/client/test_media.py

@@ -5098,6 +5098,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
         self.tok = self.login("user", "pass")
 
     def _create_local_media_with_sha256_path(self) -> str:
+        """Create local media with sha256 path."""
         assert isinstance(self.repo, MediaRepository)
         media_id = random_string(24)
         # Curate a specialized FileInfo that includes sha256 data, then file will be
@@ -5139,16 +5140,13 @@ def _create_local_media_with_sha256_path(self) -> str:
                 media_id=media_id,
                 file_id=media_id,
                 media_type="image/png",
-                # We purposely do not use the sha256 here, as it directly causes the sha256
-                # path for thumbnails to be populated, and that is not what we are looking
-                # for.
                 sha256=SMALL_PNG_SHA256,
             )
         )
-
         return media_id
 
     def _create_local_media_with_media_id_path(self) -> str:
+        """Create local media with media id path."""
         assert isinstance(self.repo, MediaRepository)
         media_id = random_string(24)
         # Curate a specialized FileInfo that is lacking sha256 data, then file will be
@@ -5217,7 +5215,7 @@ async def _mock_federation_download_media(
         return 67, headers
 
     def test_download_remote_media_saves_file_in_sha256_path(self) -> None:
-        """Test that downloading remote media using sha256 path."""
+        """Test downloading remote media using sha256 path via download endpoint."""
         # Mock the federation download media
         self.repo.client.federation_download_media = (  # type: ignore
             self._mock_federation_download_media
@@ -5251,10 +5249,10 @@ def test_download_remote_media_saves_file_in_sha256_path(self) -> None:
     def test_download_local_media_with_media_id_path(
         self,
     ) -> None:
-        """Test that downloading local media with media_id path serves correct file."""
-        # Specifically, that a preexisting file on the legacy media paths can still be
-        # served from its present location.
-
+        """Test that downloading local media with media_id path serves correct file.
+        Specifically, that a preexisting file on the media id path(legacy) can still be
+        served from its present location.
+        """
         media_id = self._create_local_media_with_media_id_path()
         channel = self.make_request(
             "GET",
@@ -5325,7 +5323,7 @@ def test_copy_remote_media_with_sha256_path(self) -> None:
         )
 
     def test_copy_local_media_with_media_id(self) -> None:
-        """Test that copying media with media_id path works correctly, when sha256 path is enabled"""
+        """Test that copying media with media_id path works correctly, when sha256 path is enabled."""
         # Create local media with media_id path.
         media_id = self._create_local_media_with_media_id_path()