From 1f27d2675d935d7856ee0e801bae89c289028e19 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 8 Sep 2021 14:08:23 -0400 Subject: [PATCH 1/7] Convert FileInfo to attrs. --- synapse/rest/media/v1/_base.py | 57 ++++++++++++---------------------- 1 file changed, 20 insertions(+), 37 deletions(-) diff --git a/synapse/rest/media/v1/_base.py b/synapse/rest/media/v1/_base.py index 90364ebcf70d..452a0fe8315a 100644 --- a/synapse/rest/media/v1/_base.py +++ b/synapse/rest/media/v1/_base.py @@ -18,6 +18,8 @@ import urllib from typing import Awaitable, Dict, Generator, List, Optional, Tuple +import attr + from twisted.internet.interfaces import IConsumer from twisted.protocols.basic import FileSender from twisted.web.server import Request @@ -287,44 +289,25 @@ def __exit__(self, exc_type, exc_val, exc_tb): pass +@attr.s(slots=True, auto_attribs=True) class FileInfo: - """Details about a requested/uploaded file. - - Attributes: - server_name (str): The server name where the media originated from, - or None if local. - file_id (str): The local ID of the file. For local files this is the - same as the media_id - url_cache (bool): If the file is for the url preview cache - thumbnail (bool): Whether the file is a thumbnail or not. - thumbnail_width (int) - thumbnail_height (int) - thumbnail_method (str) - thumbnail_type (str): Content type of thumbnail, e.g. image/png - thumbnail_length (int): The size of the media file, in bytes. - """ - - def __init__( - self, - server_name, - file_id, - url_cache=False, - thumbnail=False, - thumbnail_width=None, - thumbnail_height=None, - thumbnail_method=None, - thumbnail_type=None, - thumbnail_length=None, - ): - self.server_name = server_name - self.file_id = file_id - self.url_cache = url_cache - self.thumbnail = thumbnail - self.thumbnail_width = thumbnail_width - self.thumbnail_height = thumbnail_height - self.thumbnail_method = thumbnail_method - self.thumbnail_type = thumbnail_type - self.thumbnail_length = thumbnail_length + """Details about a requested/uploaded file.""" + + # The server name where the media originated from, or None if local. + server_name: Optional[str] + # The local ID of the file. For local files this is the same as the media_id + file_id: str + # If the file is for the url preview cache + url_cache: bool = False + # Whether the file is a thumbnail or not. + thumbnail: bool = False + thumbnail_width: Optional[int] = None + thumbnail_height: Optional[int] = None + thumbnail_method: Optional[str] = None + # Content type of thumbnail, e.g. image/png + thumbnail_type: Optional[str] = None + # The size of the media file, in bytes. + thumbnail_length: Optional[int] = None def get_filename_from_headers(headers: Dict[bytes, List[bytes]]) -> Optional[str]: From 54f569b2519193b4d5236a597c6b5f12da6536dd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 8 Sep 2021 14:22:49 -0400 Subject: [PATCH 2/7] Fix-up type hints. --- synapse/rest/media/v1/_base.py | 2 +- synapse/rest/media/v1/media_repository.py | 4 +-- synapse/rest/media/v1/media_storage.py | 28 +++++++++++++++++++ synapse/rest/media/v1/preview_url_resource.py | 6 ++-- synapse/rest/media/v1/thumbnail_resource.py | 7 +++++ 5 files changed, 41 insertions(+), 6 deletions(-) diff --git a/synapse/rest/media/v1/_base.py b/synapse/rest/media/v1/_base.py index 452a0fe8315a..473cc2e9bb68 100644 --- a/synapse/rest/media/v1/_base.py +++ b/synapse/rest/media/v1/_base.py @@ -298,7 +298,7 @@ class FileInfo: # The local ID of the file. For local files this is the same as the media_id file_id: str # If the file is for the url preview cache - url_cache: bool = False + url_cache: Optional[str] = None # Whether the file is a thumbnail or not. thumbnail: bool = False thumbnail_width: Optional[int] = None diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index 0f5ce41ff880..80152e1915b3 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -585,7 +585,7 @@ async def generate_remote_exact_thumbnail( t_type: str, ) -> Optional[str]: input_path = await self.media_storage.ensure_media_is_in_local_cache( - FileInfo(server_name, file_id, url_cache=False) + FileInfo(server_name, file_id) ) try: @@ -655,7 +655,7 @@ async def _generate_thumbnails( media_id: str, file_id: str, media_type: str, - url_cache: bool = False, + url_cache: Optional[str] = None, ) -> Optional[dict]: """Generate and store thumbnails for an image. diff --git a/synapse/rest/media/v1/media_storage.py b/synapse/rest/media/v1/media_storage.py index 56cdc1b4edd3..318c04d10d7d 100644 --- a/synapse/rest/media/v1/media_storage.py +++ b/synapse/rest/media/v1/media_storage.py @@ -172,6 +172,11 @@ async def fetch_media(self, file_info: FileInfo) -> Optional[Responder]: # fallback for remote thumbnails with no method in the filename if file_info.thumbnail and file_info.server_name: + # These must exist if it is a thumbnail. + assert file_info.thumbnail_width is not None + assert file_info.thumbnail_height is not None + assert file_info.thumbnail_type is not None + paths.append( self.filepaths.remote_media_thumbnail_rel_legacy( server_name=file_info.server_name, @@ -217,6 +222,11 @@ async def ensure_media_is_in_local_cache(self, file_info: FileInfo) -> str: # Fallback for paths without method names # Should be removed in the future if file_info.thumbnail and file_info.server_name: + # These must exist if it is a thumbnail. + assert file_info.thumbnail_width is not None + assert file_info.thumbnail_height is not None + assert file_info.thumbnail_type is not None + legacy_path = self.filepaths.remote_media_thumbnail_rel_legacy( server_name=file_info.server_name, file_id=file_info.file_id, @@ -253,6 +263,12 @@ def _file_info_to_path(self, file_info: FileInfo) -> str: """ if file_info.url_cache: if file_info.thumbnail: + # These must exist if it is a thumbnail. + assert file_info.thumbnail_width is not None + assert file_info.thumbnail_height is not None + assert file_info.thumbnail_type is not None + assert file_info.thumbnail_method is not None + return self.filepaths.url_cache_thumbnail_rel( media_id=file_info.file_id, width=file_info.thumbnail_width, @@ -264,6 +280,12 @@ def _file_info_to_path(self, file_info: FileInfo) -> str: if file_info.server_name: if file_info.thumbnail: + # These must exist if it is a thumbnail. + assert file_info.thumbnail_width is not None + assert file_info.thumbnail_height is not None + assert file_info.thumbnail_type is not None + assert file_info.thumbnail_method is not None + return self.filepaths.remote_media_thumbnail_rel( server_name=file_info.server_name, file_id=file_info.file_id, @@ -277,6 +299,12 @@ def _file_info_to_path(self, file_info: FileInfo) -> str: ) if file_info.thumbnail: + # These must exist if it is a thumbnail. + assert file_info.thumbnail_width is not None + assert file_info.thumbnail_height is not None + assert file_info.thumbnail_type is not None + assert file_info.thumbnail_method is not None + return self.filepaths.local_media_thumbnail_rel( media_id=file_info.file_id, width=file_info.thumbnail_width, diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py index f108da05db55..ad70d504b675 100644 --- a/synapse/rest/media/v1/preview_url_resource.py +++ b/synapse/rest/media/v1/preview_url_resource.py @@ -261,7 +261,7 @@ async def _do_preview(self, url: str, user: str, ts: int) -> bytes: if _is_media(media_info.media_type): file_id = media_info.filesystem_id dims = await self.media_repo._generate_thumbnails( - None, file_id, file_id, media_info.media_type, url_cache=True + None, file_id, file_id, media_info.media_type, url_cache=url ) og = { @@ -300,7 +300,7 @@ async def _do_preview(self, url: str, user: str, ts: int) -> bytes: # TODO: make sure we don't choke on white-on-transparent images file_id = image_info.filesystem_id dims = await self.media_repo._generate_thumbnails( - None, file_id, file_id, image_info.media_type, url_cache=True + None, file_id, file_id, image_info.media_type, url_cache=url ) if dims: og["og:image:width"] = dims["width"] @@ -355,7 +355,7 @@ async def _download_url(self, url: str, user: str) -> MediaInfo: file_id = datetime.date.today().isoformat() + "_" + random_string(16) - file_info = FileInfo(server_name=None, file_id=file_id, url_cache=True) + file_info = FileInfo(server_name=None, file_id=file_id, url_cache=url) # If this URL can be accessed via oEmbed, use that instead. url_to_download: Optional[str] = url diff --git a/synapse/rest/media/v1/thumbnail_resource.py b/synapse/rest/media/v1/thumbnail_resource.py index 12bd745cb21c..aa96b0bc6d6b 100644 --- a/synapse/rest/media/v1/thumbnail_resource.py +++ b/synapse/rest/media/v1/thumbnail_resource.py @@ -318,6 +318,13 @@ async def _select_and_respond_with_thumbnail( respond_404(request) return + # These must exist if it is a thumbnail. + assert file_info.thumbnail_width is not None + assert file_info.thumbnail_height is not None + assert file_info.thumbnail_type is not None + assert file_info.thumbnail_method is not None + assert file_info.thumbnail_length is not None + responder = await self.media_storage.fetch_media(file_info) if responder: await respond_with_responder( From f7b6e451f6cd6185af70bc80092643363eb630c3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 8 Sep 2021 14:29:52 -0400 Subject: [PATCH 3/7] Newsfragment --- changelog.d/10785.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10785.misc diff --git a/changelog.d/10785.misc b/changelog.d/10785.misc new file mode 100644 index 000000000000..3d7f91d516de --- /dev/null +++ b/changelog.d/10785.misc @@ -0,0 +1 @@ +Convert the internal `FileInfo` class to attrs and add type hints. From 5a127cc6c45752a2ebf865d81800bb1fecffec07 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 8 Sep 2021 14:32:29 -0400 Subject: [PATCH 4/7] Freeze the FileInfo class. --- synapse/rest/media/v1/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/media/v1/_base.py b/synapse/rest/media/v1/_base.py index 473cc2e9bb68..4142961058c5 100644 --- a/synapse/rest/media/v1/_base.py +++ b/synapse/rest/media/v1/_base.py @@ -289,7 +289,7 @@ def __exit__(self, exc_type, exc_val, exc_tb): pass -@attr.s(slots=True, auto_attribs=True) +@attr.s(slots=True, frozen=True, auto_attribs=True) class FileInfo: """Details about a requested/uploaded file.""" From 6f0ad04fad0e69fd18c65908e7511b13b977599a Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 10 Sep 2021 08:43:28 -0400 Subject: [PATCH 5/7] Create a ThumbnailInfo type. --- synapse/rest/media/v1/_base.py | 51 +++++++++++++--- synapse/rest/media/v1/media_repository.py | 34 ++++++----- synapse/rest/media/v1/media_storage.py | 64 ++++++------------- synapse/rest/media/v1/thumbnail_resource.py | 68 ++++++++++----------- 4 files changed, 114 insertions(+), 103 deletions(-) diff --git a/synapse/rest/media/v1/_base.py b/synapse/rest/media/v1/_base.py index 4142961058c5..2180459e23ed 100644 --- a/synapse/rest/media/v1/_base.py +++ b/synapse/rest/media/v1/_base.py @@ -289,6 +289,17 @@ def __exit__(self, exc_type, exc_val, exc_tb): pass +@attr.s(slots=True, frozen=True, auto_attribs=True) +class ThumbnailInfo: + width: int + height: int + method: str + # Content type of thumbnail, e.g. image/png + type: str + # The size of the media file, in bytes. + length: Optional[int] = None + + @attr.s(slots=True, frozen=True, auto_attribs=True) class FileInfo: """Details about a requested/uploaded file.""" @@ -300,14 +311,38 @@ class FileInfo: # If the file is for the url preview cache url_cache: Optional[str] = None # Whether the file is a thumbnail or not. - thumbnail: bool = False - thumbnail_width: Optional[int] = None - thumbnail_height: Optional[int] = None - thumbnail_method: Optional[str] = None - # Content type of thumbnail, e.g. image/png - thumbnail_type: Optional[str] = None - # The size of the media file, in bytes. - thumbnail_length: Optional[int] = None + thumbnail: Optional[ThumbnailInfo] = None + + # The below properties exist to maintain compatibility with third-party modules. + @property + def thumbnail_width(self): + if not self.thumbnail: + return None + return self.thumbnail.width + + @property + def thumbnail_height(self): + if not self.thumbnail: + return None + return self.thumbnail.height + + @property + def thumbnail_method(self): + if not self.thumbnail: + return None + return self.thumbnail.method + + @property + def thumbnail_type(self): + if not self.thumbnail: + return None + return self.thumbnail.type + + @property + def thumbnail_length(self): + if not self.thumbnail: + return None + return self.thumbnail.length def get_filename_from_headers(headers: Dict[bytes, List[bytes]]) -> Optional[str]: diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index 80152e1915b3..4aefc41cae5c 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -42,6 +42,7 @@ from ._base import ( FileInfo, Responder, + ThumbnailInfo, get_filename_from_headers, respond_404, respond_with_responder, @@ -548,11 +549,12 @@ async def generate_local_exact_thumbnail( server_name=None, file_id=media_id, url_cache=url_cache, - thumbnail=True, - thumbnail_width=t_width, - thumbnail_height=t_height, - thumbnail_method=t_method, - thumbnail_type=t_type, + thumbnail=ThumbnailInfo( + width=t_width, + height=t_height, + method=t_method, + type=t_type, + ), ) output_path = await self.media_storage.store_file( @@ -616,11 +618,12 @@ async def generate_remote_exact_thumbnail( file_info = FileInfo( server_name=server_name, file_id=file_id, - thumbnail=True, - thumbnail_width=t_width, - thumbnail_height=t_height, - thumbnail_method=t_method, - thumbnail_type=t_type, + thumbnail=ThumbnailInfo( + width=t_width, + height=t_height, + method=t_method, + type=t_type, + ), ) output_path = await self.media_storage.store_file( @@ -742,12 +745,13 @@ async def _generate_thumbnails( file_info = FileInfo( server_name=server_name, file_id=file_id, - thumbnail=True, - thumbnail_width=t_width, - thumbnail_height=t_height, - thumbnail_method=t_method, - thumbnail_type=t_type, url_cache=url_cache, + thumbnail=ThumbnailInfo( + width=t_width, + height=t_height, + method=t_method, + type=t_type, + ), ) with self.media_storage.store_into_file(file_info) as (f, fname, finish): diff --git a/synapse/rest/media/v1/media_storage.py b/synapse/rest/media/v1/media_storage.py index 318c04d10d7d..c0bb40c116c1 100644 --- a/synapse/rest/media/v1/media_storage.py +++ b/synapse/rest/media/v1/media_storage.py @@ -172,18 +172,13 @@ async def fetch_media(self, file_info: FileInfo) -> Optional[Responder]: # fallback for remote thumbnails with no method in the filename if file_info.thumbnail and file_info.server_name: - # These must exist if it is a thumbnail. - assert file_info.thumbnail_width is not None - assert file_info.thumbnail_height is not None - assert file_info.thumbnail_type is not None - paths.append( self.filepaths.remote_media_thumbnail_rel_legacy( server_name=file_info.server_name, file_id=file_info.file_id, - width=file_info.thumbnail_width, - height=file_info.thumbnail_height, - content_type=file_info.thumbnail_type, + width=file_info.thumbnail.width, + height=file_info.thumbnail.height, + content_type=file_info.thumbnail.type, ) ) @@ -222,17 +217,12 @@ async def ensure_media_is_in_local_cache(self, file_info: FileInfo) -> str: # Fallback for paths without method names # Should be removed in the future if file_info.thumbnail and file_info.server_name: - # These must exist if it is a thumbnail. - assert file_info.thumbnail_width is not None - assert file_info.thumbnail_height is not None - assert file_info.thumbnail_type is not None - legacy_path = self.filepaths.remote_media_thumbnail_rel_legacy( server_name=file_info.server_name, file_id=file_info.file_id, - width=file_info.thumbnail_width, - height=file_info.thumbnail_height, - content_type=file_info.thumbnail_type, + width=file_info.thumbnail.width, + height=file_info.thumbnail.height, + content_type=file_info.thumbnail.type, ) legacy_local_path = os.path.join(self.local_media_directory, legacy_path) if os.path.exists(legacy_local_path): @@ -263,54 +253,36 @@ def _file_info_to_path(self, file_info: FileInfo) -> str: """ if file_info.url_cache: if file_info.thumbnail: - # These must exist if it is a thumbnail. - assert file_info.thumbnail_width is not None - assert file_info.thumbnail_height is not None - assert file_info.thumbnail_type is not None - assert file_info.thumbnail_method is not None - return self.filepaths.url_cache_thumbnail_rel( media_id=file_info.file_id, - width=file_info.thumbnail_width, - height=file_info.thumbnail_height, - content_type=file_info.thumbnail_type, - method=file_info.thumbnail_method, + width=file_info.thumbnail.width, + height=file_info.thumbnail.height, + content_type=file_info.thumbnail.type, + method=file_info.thumbnail.method, ) return self.filepaths.url_cache_filepath_rel(file_info.file_id) if file_info.server_name: if file_info.thumbnail: - # These must exist if it is a thumbnail. - assert file_info.thumbnail_width is not None - assert file_info.thumbnail_height is not None - assert file_info.thumbnail_type is not None - assert file_info.thumbnail_method is not None - return self.filepaths.remote_media_thumbnail_rel( server_name=file_info.server_name, file_id=file_info.file_id, - width=file_info.thumbnail_width, - height=file_info.thumbnail_height, - content_type=file_info.thumbnail_type, - method=file_info.thumbnail_method, + width=file_info.thumbnail.width, + height=file_info.thumbnail.height, + content_type=file_info.thumbnail.type, + method=file_info.thumbnail.method, ) return self.filepaths.remote_media_filepath_rel( file_info.server_name, file_info.file_id ) if file_info.thumbnail: - # These must exist if it is a thumbnail. - assert file_info.thumbnail_width is not None - assert file_info.thumbnail_height is not None - assert file_info.thumbnail_type is not None - assert file_info.thumbnail_method is not None - return self.filepaths.local_media_thumbnail_rel( media_id=file_info.file_id, - width=file_info.thumbnail_width, - height=file_info.thumbnail_height, - content_type=file_info.thumbnail_type, - method=file_info.thumbnail_method, + width=file_info.thumbnail.width, + height=file_info.thumbnail.height, + content_type=file_info.thumbnail.type, + method=file_info.thumbnail.method, ) return self.filepaths.local_media_filepath_rel(file_info.file_id) diff --git a/synapse/rest/media/v1/thumbnail_resource.py b/synapse/rest/media/v1/thumbnail_resource.py index aa96b0bc6d6b..6f210bd06e65 100644 --- a/synapse/rest/media/v1/thumbnail_resource.py +++ b/synapse/rest/media/v1/thumbnail_resource.py @@ -26,6 +26,7 @@ from ._base import ( FileInfo, + ThumbnailInfo, parse_media_id, respond_404, respond_with_file, @@ -149,11 +150,12 @@ async def _select_or_generate_local_thumbnail( server_name=None, file_id=media_id, url_cache=media_info["url_cache"], - thumbnail=True, - thumbnail_width=info["thumbnail_width"], - thumbnail_height=info["thumbnail_height"], - thumbnail_type=info["thumbnail_type"], - thumbnail_method=info["thumbnail_method"], + thumbnail=ThumbnailInfo( + width=info["thumbnail_width"], + height=info["thumbnail_height"], + type=info["thumbnail_type"], + method=info["thumbnail_method"], + ), ) t_type = file_info.thumbnail_type @@ -210,11 +212,12 @@ async def _select_or_generate_remote_thumbnail( file_info = FileInfo( server_name=server_name, file_id=media_info["filesystem_id"], - thumbnail=True, - thumbnail_width=info["thumbnail_width"], - thumbnail_height=info["thumbnail_height"], - thumbnail_type=info["thumbnail_type"], - thumbnail_method=info["thumbnail_method"], + thumbnail=ThumbnailInfo( + width=info["thumbnail_width"], + height=info["thumbnail_height"], + type=info["thumbnail_type"], + method=info["thumbnail_method"], + ), ) t_type = file_info.thumbnail_type @@ -318,20 +321,16 @@ async def _select_and_respond_with_thumbnail( respond_404(request) return - # These must exist if it is a thumbnail. - assert file_info.thumbnail_width is not None - assert file_info.thumbnail_height is not None - assert file_info.thumbnail_type is not None - assert file_info.thumbnail_method is not None - assert file_info.thumbnail_length is not None + # The thumbnail property must exist. + assert file_info.thumbnail is not None responder = await self.media_storage.fetch_media(file_info) if responder: await respond_with_responder( request, responder, - file_info.thumbnail_type, - file_info.thumbnail_length, + file_info.thumbnail.type, + file_info.thumbnail.length, ) return @@ -358,18 +357,18 @@ async def _select_and_respond_with_thumbnail( server_name, file_id=file_id, media_id=media_id, - t_width=file_info.thumbnail_width, - t_height=file_info.thumbnail_height, - t_method=file_info.thumbnail_method, - t_type=file_info.thumbnail_type, + t_width=file_info.thumbnail.width, + t_height=file_info.thumbnail.height, + t_method=file_info.thumbnail.method, + t_type=file_info.thumbnail.type, ) else: await self.media_repo.generate_local_exact_thumbnail( media_id=media_id, - t_width=file_info.thumbnail_width, - t_height=file_info.thumbnail_height, - t_method=file_info.thumbnail_method, - t_type=file_info.thumbnail_type, + t_width=file_info.thumbnail.width, + t_height=file_info.thumbnail.height, + t_method=file_info.thumbnail.method, + t_type=file_info.thumbnail.type, url_cache=url_cache, ) @@ -377,8 +376,8 @@ async def _select_and_respond_with_thumbnail( await respond_with_responder( request, responder, - file_info.thumbnail_type, - file_info.thumbnail_length, + file_info.thumbnail.type, + file_info.thumbnail.length, ) else: logger.info("Failed to find any generated thumbnails") @@ -502,12 +501,13 @@ def _select_thumbnail( file_id=file_id, url_cache=url_cache, server_name=server_name, - thumbnail=True, - thumbnail_width=thumbnail_info["thumbnail_width"], - thumbnail_height=thumbnail_info["thumbnail_height"], - thumbnail_type=thumbnail_info["thumbnail_type"], - thumbnail_method=thumbnail_info["thumbnail_method"], - thumbnail_length=thumbnail_info["thumbnail_length"], + thumbnail=ThumbnailInfo( + width=thumbnail_info["thumbnail_width"], + height=thumbnail_info["thumbnail_height"], + type=thumbnail_info["thumbnail_type"], + method=thumbnail_info["thumbnail_method"], + length=thumbnail_info["thumbnail_length"], + ), ) # No matching thumbnail was found. From 12552c13f3934530fb600f730a441eba211a7d8b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 10 Sep 2021 08:44:13 -0400 Subject: [PATCH 6/7] Add docstring. --- synapse/rest/media/v1/_base.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/rest/media/v1/_base.py b/synapse/rest/media/v1/_base.py index 2180459e23ed..6fb80b289c92 100644 --- a/synapse/rest/media/v1/_base.py +++ b/synapse/rest/media/v1/_base.py @@ -291,6 +291,8 @@ def __exit__(self, exc_type, exc_val, exc_tb): @attr.s(slots=True, frozen=True, auto_attribs=True) class ThumbnailInfo: + """Details about a generated thumbnail.""" + width: int height: int method: str From 4ac04bbd0ed344c23a77bfdb97e032e8f957737d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 13 Sep 2021 10:41:36 -0400 Subject: [PATCH 7/7] Revert url_cache to a bool and fix-up uses. --- synapse/rest/media/v1/_base.py | 2 +- synapse/rest/media/v1/media_repository.py | 6 +++--- synapse/rest/media/v1/preview_url_resource.py | 6 +++--- synapse/rest/media/v1/thumbnail_resource.py | 14 +++++++------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/synapse/rest/media/v1/_base.py b/synapse/rest/media/v1/_base.py index 6fb80b289c92..814f4309f570 100644 --- a/synapse/rest/media/v1/_base.py +++ b/synapse/rest/media/v1/_base.py @@ -311,7 +311,7 @@ class FileInfo: # The local ID of the file. For local files this is the same as the media_id file_id: str # If the file is for the url preview cache - url_cache: Optional[str] = None + url_cache: bool = False # Whether the file is a thumbnail or not. thumbnail: Optional[ThumbnailInfo] = None diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index 4aefc41cae5c..40ce8d2bc676 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -211,7 +211,7 @@ async def get_local_media( upload_name = name if name else media_info["upload_name"] url_cache = media_info["url_cache"] - file_info = FileInfo(None, media_id, url_cache=url_cache) + file_info = FileInfo(None, media_id, url_cache=bool(url_cache)) responder = await self.media_storage.fetch_media(file_info) await respond_with_responder( @@ -515,7 +515,7 @@ async def generate_local_exact_thumbnail( t_height: int, t_method: str, t_type: str, - url_cache: Optional[str], + url_cache: bool, ) -> Optional[str]: input_path = await self.media_storage.ensure_media_is_in_local_cache( FileInfo(None, media_id, url_cache=url_cache) @@ -658,7 +658,7 @@ async def _generate_thumbnails( media_id: str, file_id: str, media_type: str, - url_cache: Optional[str] = None, + url_cache: bool = False, ) -> Optional[dict]: """Generate and store thumbnails for an image. diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py index ad70d504b675..f108da05db55 100644 --- a/synapse/rest/media/v1/preview_url_resource.py +++ b/synapse/rest/media/v1/preview_url_resource.py @@ -261,7 +261,7 @@ async def _do_preview(self, url: str, user: str, ts: int) -> bytes: if _is_media(media_info.media_type): file_id = media_info.filesystem_id dims = await self.media_repo._generate_thumbnails( - None, file_id, file_id, media_info.media_type, url_cache=url + None, file_id, file_id, media_info.media_type, url_cache=True ) og = { @@ -300,7 +300,7 @@ async def _do_preview(self, url: str, user: str, ts: int) -> bytes: # TODO: make sure we don't choke on white-on-transparent images file_id = image_info.filesystem_id dims = await self.media_repo._generate_thumbnails( - None, file_id, file_id, image_info.media_type, url_cache=url + None, file_id, file_id, image_info.media_type, url_cache=True ) if dims: og["og:image:width"] = dims["width"] @@ -355,7 +355,7 @@ async def _download_url(self, url: str, user: str) -> MediaInfo: file_id = datetime.date.today().isoformat() + "_" + random_string(16) - file_info = FileInfo(server_name=None, file_id=file_id, url_cache=url) + file_info = FileInfo(server_name=None, file_id=file_id, url_cache=True) # If this URL can be accessed via oEmbed, use that instead. url_to_download: Optional[str] = url diff --git a/synapse/rest/media/v1/thumbnail_resource.py b/synapse/rest/media/v1/thumbnail_resource.py index 6f210bd06e65..22f43d85310b 100644 --- a/synapse/rest/media/v1/thumbnail_resource.py +++ b/synapse/rest/media/v1/thumbnail_resource.py @@ -115,7 +115,7 @@ async def _respond_local_thumbnail( thumbnail_infos, media_id, media_id, - url_cache=media_info["url_cache"], + url_cache=bool(media_info["url_cache"]), server_name=None, ) @@ -175,7 +175,7 @@ async def _select_or_generate_local_thumbnail( desired_height, desired_method, desired_type, - url_cache=media_info["url_cache"], + url_cache=bool(media_info["url_cache"]), ) if file_path: @@ -274,7 +274,7 @@ async def _respond_remote_thumbnail( thumbnail_infos, media_id, media_info["filesystem_id"], - url_cache=None, + url_cache=False, server_name=server_name, ) @@ -288,7 +288,7 @@ async def _select_and_respond_with_thumbnail( thumbnail_infos: List[Dict[str, Any]], media_id: str, file_id: str, - url_cache: Optional[str] = None, + url_cache: bool, server_name: Optional[str] = None, ) -> None: """ @@ -302,7 +302,7 @@ async def _select_and_respond_with_thumbnail( desired_type: The desired content-type of the thumbnail. thumbnail_infos: A list of dictionaries of candidate thumbnails. file_id: The ID of the media that a thumbnail is being requested for. - url_cache: The URL cache value. + url_cache: True if this is from a URL cache. server_name: The server name, if this is a remote thumbnail. """ if thumbnail_infos: @@ -391,7 +391,7 @@ def _select_thumbnail( desired_type: str, thumbnail_infos: List[Dict[str, Any]], file_id: str, - url_cache: Optional[str], + url_cache: bool, server_name: Optional[str], ) -> Optional[FileInfo]: """ @@ -404,7 +404,7 @@ def _select_thumbnail( desired_type: The desired content-type of the thumbnail. thumbnail_infos: A list of dictionaries of candidate thumbnails. file_id: The ID of the media that a thumbnail is being requested for. - url_cache: The URL cache value. + url_cache: True if this is from a URL cache. server_name: The server name, if this is a remote thumbnail. Returns: