Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Regenerate exact thumbnails if missing #9438

Merged
merged 4 commits into from
Feb 19, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/9438.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for regenerating thumbnails if they have been deleted but the original image is still stored.
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/media_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ async def generate_local_exact_thumbnail(
t_height: int,
t_method: str,
t_type: str,
url_cache: str,
url_cache: Optional[str],
) -> Optional[str]:
input_path = await self.media_storage.ensure_media_is_in_local_cache(
FileInfo(None, media_id, url_cache=url_cache)
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/media_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ async def ensure_media_is_in_local_cache(self, file_info: FileInfo) -> str:
await consumer.wait()
return local_path

raise Exception("file could not be found")
raise NotFoundError()

def _file_info_to_path(self, file_info: FileInfo) -> str:
"""Converts file_info into a relative path.
Expand Down
56 changes: 55 additions & 1 deletion synapse/rest/media/v1/thumbnail_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ async def _respond_local_thumbnail(
m_type,
thumbnail_infos,
media_id,
media_id,
url_cache=media_info["url_cache"],
server_name=None,
)
Expand Down Expand Up @@ -269,6 +270,7 @@ async def _respond_remote_thumbnail(
method,
m_type,
thumbnail_infos,
media_id,
media_info["filesystem_id"],
url_cache=None,
server_name=server_name,
Expand All @@ -282,6 +284,7 @@ async def _select_and_respond_with_thumbnail(
desired_method: str,
desired_type: str,
thumbnail_infos: List[Dict[str, Any]],
media_id: str,
file_id: str,
url_cache: Optional[str] = None,
server_name: Optional[str] = None,
Expand Down Expand Up @@ -316,9 +319,60 @@ async def _select_and_respond_with_thumbnail(
respond_404(request)
return

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,
)
return

# If we can't find the thumbnail we regenerate it. This can happen
# if e.g. we've deleted the thumbnails but still have the original
# image somewhere.
clokep marked this conversation as resolved.
Show resolved Hide resolved
#
# Since we have an entry for the thumbnail in the DB we a) know we
# have have successfully generated the thumbnail in the past (so we
# don't need to worry about repeatedly failing to generate
# thumbnails), and b) have already calculated that appropriate
# width/height/method so we can just call the "generate exact"
# methods.

# First let's check that we do actually have the original image
# still. This will throw a 404 if we don't.
# TODO: We should refetch the thumbnails for remote media.
await self.media_storage.ensure_media_is_in_local_cache(
FileInfo(server_name, file_id, url_cache=url_cache)
)
Comment on lines +343 to +348
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this is actually the first line of generate_remote_exact_thumbnail and generate_local_exact_thumbnail 🤦 so maybe we don't need to check this again?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second time it gets called is fast (just a check if the path exists), so I think its better to be explicit by keeping this here (and I think its good to check again in the functions to avoid foot guns)


if server_name:
await self.media_repo.generate_remote_exact_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,
)
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,
url_cache=url_cache,
)

responder = await self.media_storage.fetch_media(file_info)
await respond_with_responder(
request, responder, file_info.thumbnail_type, file_info.thumbnail_length
request,
responder,
file_info.thumbnail_type,
file_info.thumbnail_length,
)
else:
logger.info("Failed to find any generated thumbnails")
Expand Down
18 changes: 9 additions & 9 deletions synapse/storage/databases/main/media_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,16 +344,16 @@ async def store_local_thumbnail(
thumbnail_method,
thumbnail_length,
):
await self.db_pool.simple_insert(
"local_media_repository_thumbnails",
{
await self.db_pool.simple_upsert(
table="local_media_repository_thumbnails",
keyvalues={
"media_id": media_id,
"thumbnail_width": thumbnail_width,
"thumbnail_height": thumbnail_height,
"thumbnail_method": thumbnail_method,
"thumbnail_type": thumbnail_type,
"thumbnail_length": thumbnail_length,
},
values={"thumbnail_length": thumbnail_length},
desc="store_local_thumbnail",
)

Expand Down Expand Up @@ -498,18 +498,18 @@ async def store_remote_media_thumbnail(
thumbnail_method,
thumbnail_length,
):
await self.db_pool.simple_insert(
"remote_media_cache_thumbnails",
{
await self.db_pool.simple_upsert(
table="remote_media_cache_thumbnails",
keyvalues={
"media_origin": origin,
"media_id": media_id,
"thumbnail_width": thumbnail_width,
"thumbnail_height": thumbnail_height,
"thumbnail_method": thumbnail_method,
"thumbnail_type": thumbnail_type,
"thumbnail_length": thumbnail_length,
"filesystem_id": filesystem_id,
},
values={"thumbnail_length": thumbnail_length},
insertion_values={"filesystem_id": filesystem_id},
desc="store_remote_media_thumbnail",
)

Expand Down