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

Commit 179c095

Browse files
authored
Regenerate exact thumbnails if missing (#9438)
2 parents a1901ab + 3a2fe50 commit 179c095

File tree

6 files changed

+133
-15
lines changed

6 files changed

+133
-15
lines changed

changelog.d/9438.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add support for regenerating thumbnails if they have been deleted but the original image is still stored.

synapse/rest/media/v1/media_repository.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ async def generate_local_exact_thumbnail(
509509
t_height: int,
510510
t_method: str,
511511
t_type: str,
512-
url_cache: str,
512+
url_cache: Optional[str],
513513
) -> Optional[str]:
514514
input_path = await self.media_storage.ensure_media_is_in_local_cache(
515515
FileInfo(None, media_id, url_cache=url_cache)

synapse/rest/media/v1/media_storage.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ async def ensure_media_is_in_local_cache(self, file_info: FileInfo) -> str:
244244
await consumer.wait()
245245
return local_path
246246

247-
raise Exception("file could not be found")
247+
raise NotFoundError()
248248

249249
def _file_info_to_path(self, file_info: FileInfo) -> str:
250250
"""Converts file_info into a relative path.

synapse/rest/media/v1/thumbnail_resource.py

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ async def _respond_local_thumbnail(
114114
m_type,
115115
thumbnail_infos,
116116
media_id,
117+
media_id,
117118
url_cache=media_info["url_cache"],
118119
server_name=None,
119120
)
@@ -269,6 +270,7 @@ async def _respond_remote_thumbnail(
269270
method,
270271
m_type,
271272
thumbnail_infos,
273+
media_id,
272274
media_info["filesystem_id"],
273275
url_cache=None,
274276
server_name=server_name,
@@ -282,6 +284,7 @@ async def _select_and_respond_with_thumbnail(
282284
desired_method: str,
283285
desired_type: str,
284286
thumbnail_infos: List[Dict[str, Any]],
287+
media_id: str,
285288
file_id: str,
286289
url_cache: Optional[str] = None,
287290
server_name: Optional[str] = None,
@@ -316,9 +319,60 @@ async def _select_and_respond_with_thumbnail(
316319
respond_404(request)
317320
return
318321

322+
responder = await self.media_storage.fetch_media(file_info)
323+
if responder:
324+
await respond_with_responder(
325+
request,
326+
responder,
327+
file_info.thumbnail_type,
328+
file_info.thumbnail_length,
329+
)
330+
return
331+
332+
# If we can't find the thumbnail we regenerate it. This can happen
333+
# if e.g. we've deleted the thumbnails but still have the original
334+
# image somewhere.
335+
#
336+
# Since we have an entry for the thumbnail in the DB we a) know we
337+
# have have successfully generated the thumbnail in the past (so we
338+
# don't need to worry about repeatedly failing to generate
339+
# thumbnails), and b) have already calculated that appropriate
340+
# width/height/method so we can just call the "generate exact"
341+
# methods.
342+
343+
# First let's check that we do actually have the original image
344+
# still. This will throw a 404 if we don't.
345+
# TODO: We should refetch the thumbnails for remote media.
346+
await self.media_storage.ensure_media_is_in_local_cache(
347+
FileInfo(server_name, file_id, url_cache=url_cache)
348+
)
349+
350+
if server_name:
351+
await self.media_repo.generate_remote_exact_thumbnail(
352+
server_name,
353+
file_id=file_id,
354+
media_id=media_id,
355+
t_width=file_info.thumbnail_width,
356+
t_height=file_info.thumbnail_height,
357+
t_method=file_info.thumbnail_method,
358+
t_type=file_info.thumbnail_type,
359+
)
360+
else:
361+
await self.media_repo.generate_local_exact_thumbnail(
362+
media_id=media_id,
363+
t_width=file_info.thumbnail_width,
364+
t_height=file_info.thumbnail_height,
365+
t_method=file_info.thumbnail_method,
366+
t_type=file_info.thumbnail_type,
367+
url_cache=url_cache,
368+
)
369+
319370
responder = await self.media_storage.fetch_media(file_info)
320371
await respond_with_responder(
321-
request, responder, file_info.thumbnail_type, file_info.thumbnail_length
372+
request,
373+
responder,
374+
file_info.thumbnail_type,
375+
file_info.thumbnail_length,
322376
)
323377
else:
324378
logger.info("Failed to find any generated thumbnails")

synapse/storage/databases/main/media_repository.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -344,16 +344,16 @@ async def store_local_thumbnail(
344344
thumbnail_method,
345345
thumbnail_length,
346346
):
347-
await self.db_pool.simple_insert(
348-
"local_media_repository_thumbnails",
349-
{
347+
await self.db_pool.simple_upsert(
348+
table="local_media_repository_thumbnails",
349+
keyvalues={
350350
"media_id": media_id,
351351
"thumbnail_width": thumbnail_width,
352352
"thumbnail_height": thumbnail_height,
353353
"thumbnail_method": thumbnail_method,
354354
"thumbnail_type": thumbnail_type,
355-
"thumbnail_length": thumbnail_length,
356355
},
356+
values={"thumbnail_length": thumbnail_length},
357357
desc="store_local_thumbnail",
358358
)
359359

@@ -498,18 +498,18 @@ async def store_remote_media_thumbnail(
498498
thumbnail_method,
499499
thumbnail_length,
500500
):
501-
await self.db_pool.simple_insert(
502-
"remote_media_cache_thumbnails",
503-
{
501+
await self.db_pool.simple_upsert(
502+
table="remote_media_cache_thumbnails",
503+
keyvalues={
504504
"media_origin": origin,
505505
"media_id": media_id,
506506
"thumbnail_width": thumbnail_width,
507507
"thumbnail_height": thumbnail_height,
508508
"thumbnail_method": thumbnail_method,
509509
"thumbnail_type": thumbnail_type,
510-
"thumbnail_length": thumbnail_length,
511-
"filesystem_id": filesystem_id,
512510
},
511+
values={"thumbnail_length": thumbnail_length},
512+
insertion_values={"filesystem_id": filesystem_id},
513513
desc="store_remote_media_thumbnail",
514514
)
515515

tests/rest/media/v1/test_media_storage.py

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -231,9 +231,11 @@ def write_to(r):
231231

232232
def prepare(self, reactor, clock, hs):
233233

234-
self.media_repo = hs.get_media_repository_resource()
235-
self.download_resource = self.media_repo.children[b"download"]
236-
self.thumbnail_resource = self.media_repo.children[b"thumbnail"]
234+
media_resource = hs.get_media_repository_resource()
235+
self.download_resource = media_resource.children[b"download"]
236+
self.thumbnail_resource = media_resource.children[b"thumbnail"]
237+
self.store = hs.get_datastore()
238+
self.media_repo = hs.get_media_repository()
237239

238240
self.media_id = "example.com/12345"
239241

@@ -357,6 +359,67 @@ def test_no_thumbnail_scale(self):
357359
"""
358360
self._test_thumbnail("scale", None, False)
359361

362+
def test_thumbnail_repeated_thumbnail(self):
363+
"""Test that fetching the same thumbnail works, and deleting the on disk
364+
thumbnail regenerates it.
365+
"""
366+
self._test_thumbnail(
367+
"scale", self.test_image.expected_scaled, self.test_image.expected_found
368+
)
369+
370+
if not self.test_image.expected_found:
371+
return
372+
373+
# Fetching again should work, without re-requesting the image from the
374+
# remote.
375+
params = "?width=32&height=32&method=scale"
376+
channel = make_request(
377+
self.reactor,
378+
FakeSite(self.thumbnail_resource),
379+
"GET",
380+
self.media_id + params,
381+
shorthand=False,
382+
await_result=False,
383+
)
384+
self.pump()
385+
386+
self.assertEqual(channel.code, 200)
387+
if self.test_image.expected_scaled:
388+
self.assertEqual(
389+
channel.result["body"],
390+
self.test_image.expected_scaled,
391+
channel.result["body"],
392+
)
393+
394+
# Deleting the thumbnail on disk then re-requesting it should work as
395+
# Synapse should regenerate missing thumbnails.
396+
origin, media_id = self.media_id.split("/")
397+
info = self.get_success(self.store.get_cached_remote_media(origin, media_id))
398+
file_id = info["filesystem_id"]
399+
400+
thumbnail_dir = self.media_repo.filepaths.remote_media_thumbnail_dir(
401+
origin, file_id
402+
)
403+
shutil.rmtree(thumbnail_dir, ignore_errors=True)
404+
405+
channel = make_request(
406+
self.reactor,
407+
FakeSite(self.thumbnail_resource),
408+
"GET",
409+
self.media_id + params,
410+
shorthand=False,
411+
await_result=False,
412+
)
413+
self.pump()
414+
415+
self.assertEqual(channel.code, 200)
416+
if self.test_image.expected_scaled:
417+
self.assertEqual(
418+
channel.result["body"],
419+
self.test_image.expected_scaled,
420+
channel.result["body"],
421+
)
422+
360423
def _test_thumbnail(self, method, expected_body, expected_found):
361424
params = "?width=32&height=32&method=" + method
362425
channel = make_request(

0 commit comments

Comments
 (0)