Fix error when selecting between thumbnails with the same quality (#10684)

Fixes #10318
This commit is contained in:
Sean 2021-08-25 10:51:08 +01:00 committed by GitHub
parent cd22fb568a
commit 7367473f96
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 56 additions and 10 deletions

1
changelog.d/10684.bugfix Normal file
View File

@ -0,0 +1 @@
Fix long-standing issue which caused an error when a thumbnail is requested and there are multiple thumbnails with the same quality rating.

View File

@ -15,7 +15,7 @@
import logging import logging
from typing import TYPE_CHECKING, Any, Dict, List, Optional from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple
from twisted.web.server import Request from twisted.web.server import Request
@ -414,9 +414,9 @@ class ThumbnailResource(DirectServeJsonResource):
if desired_method == "crop": if desired_method == "crop":
# Thumbnails that match equal or larger sizes of desired width/height. # Thumbnails that match equal or larger sizes of desired width/height.
crop_info_list = [] crop_info_list: List[Tuple[int, int, int, bool, int, Dict[str, Any]]] = []
# Other thumbnails. # Other thumbnails.
crop_info_list2 = [] crop_info_list2: List[Tuple[int, int, int, bool, int, Dict[str, Any]]] = []
for info in thumbnail_infos: for info in thumbnail_infos:
# Skip thumbnails generated with different methods. # Skip thumbnails generated with different methods.
if info["thumbnail_method"] != "crop": if info["thumbnail_method"] != "crop":
@ -451,15 +451,19 @@ class ThumbnailResource(DirectServeJsonResource):
info, info,
) )
) )
# Pick the most appropriate thumbnail. Some values of `desired_width` and
# `desired_height` may result in a tie, in which case we avoid comparing on
# the thumbnail info dictionary and pick the thumbnail that appears earlier
# in the list of candidates.
if crop_info_list: if crop_info_list:
thumbnail_info = min(crop_info_list)[-1] thumbnail_info = min(crop_info_list, key=lambda t: t[:-1])[-1]
elif crop_info_list2: elif crop_info_list2:
thumbnail_info = min(crop_info_list2)[-1] thumbnail_info = min(crop_info_list2, key=lambda t: t[:-1])[-1]
elif desired_method == "scale": elif desired_method == "scale":
# Thumbnails that match equal or larger sizes of desired width/height. # Thumbnails that match equal or larger sizes of desired width/height.
info_list = [] info_list: List[Tuple[int, bool, int, Dict[str, Any]]] = []
# Other thumbnails. # Other thumbnails.
info_list2 = [] info_list2: List[Tuple[int, bool, int, Dict[str, Any]]] = []
for info in thumbnail_infos: for info in thumbnail_infos:
# Skip thumbnails generated with different methods. # Skip thumbnails generated with different methods.
@ -477,10 +481,14 @@ class ThumbnailResource(DirectServeJsonResource):
info_list2.append( info_list2.append(
(size_quality, type_quality, length_quality, info) (size_quality, type_quality, length_quality, info)
) )
# Pick the most appropriate thumbnail. Some values of `desired_width` and
# `desired_height` may result in a tie, in which case we avoid comparing on
# the thumbnail info dictionary and pick the thumbnail that appears earlier
# in the list of candidates.
if info_list: if info_list:
thumbnail_info = min(info_list)[-1] thumbnail_info = min(info_list, key=lambda t: t[:-1])[-1]
elif info_list2: elif info_list2:
thumbnail_info = min(info_list2)[-1] thumbnail_info = min(info_list2, key=lambda t: t[:-1])[-1]
if thumbnail_info: if thumbnail_info:
return FileInfo( return FileInfo(

View File

@ -21,7 +21,7 @@ from unittest.mock import Mock
from urllib import parse from urllib import parse
import attr import attr
from parameterized import parameterized_class from parameterized import parameterized, parameterized_class
from PIL import Image as Image from PIL import Image as Image
from twisted.internet import defer from twisted.internet import defer
@ -473,6 +473,43 @@ class MediaRepoTests(unittest.HomeserverTestCase):
}, },
) )
@parameterized.expand([("crop", 16), ("crop", 64), ("scale", 16), ("scale", 64)])
def test_same_quality(self, method, desired_size):
"""Test that choosing between thumbnails with the same quality rating succeeds.
We are not particular about which thumbnail is chosen."""
self.assertIsNotNone(
self.thumbnail_resource._select_thumbnail(
desired_width=desired_size,
desired_height=desired_size,
desired_method=method,
desired_type=self.test_image.content_type,
# Provide two identical thumbnails which are guaranteed to have the same
# quality rating.
thumbnail_infos=[
{
"thumbnail_width": 32,
"thumbnail_height": 32,
"thumbnail_method": method,
"thumbnail_type": self.test_image.content_type,
"thumbnail_length": 256,
"filesystem_id": f"thumbnail1{self.test_image.extension}",
},
{
"thumbnail_width": 32,
"thumbnail_height": 32,
"thumbnail_method": method,
"thumbnail_type": self.test_image.content_type,
"thumbnail_length": 256,
"filesystem_id": f"thumbnail2{self.test_image.extension}",
},
],
file_id=f"image{self.test_image.extension}",
url_cache=None,
server_name=None,
)
)
def test_x_robots_tag_header(self): def test_x_robots_tag_header(self):
""" """
Tests that the `X-Robots-Tag` header is present, which informs web crawlers Tests that the `X-Robots-Tag` header is present, which informs web crawlers