mirror of
https://github.com/zulip/zulip.git
synced 2025-11-03 05:23:35 +00:00
thumbnail: Generate a transcoded high-res version of HEIC/TIFF images.
If the content-type of the image is not in INLINE_MIME_TYPES, then we do not expect browsers to be able to display it. This behaviour is particularly confusing because the thumbnail will render properly, since that will be in the more widely-supported WebP format, but the lightbox will show a broken image. In these cases, generate a high-resolution (4032x3024) "thumbnail" which clients can choose to use instead. This thumbnail format is not in the listed in the server's advertised thumbnail size list, because it is not reliably generated for every image. The transcoded thumbnail format is set on the `img` tag if it is generated, and the original content-type is always passed to the client, so it can decide how or if to render the original image. This content-type is as the _original uploader_ specified it, so may be incorrect. The transcoded image is not animated, even if the original was. HEIC files can nominally be animated, but in testing libvips was not able to correctly recognize them as such. TIFF files are parsed as being "animated," with one page per frame; this is of dubious utility, so we merely transcode the first page. Always generating a static transcoded image serves to also limit the computational time spent. THUMBNAIL_OUTPUT_FORMATS is switched to be a tuple to ensure that it is not accidentally mutated.
This commit is contained in:
Alex Vandiver
committed by
Tim Abbott
Tim Abbott
parent
9fa5ab951c
commit
230bae17bb
@@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with.
|
||||
|
||||
## Changes in Zulip 10.0
|
||||
|
||||
**Feature level 336**
|
||||
|
||||
* [Markdown message formatting](/api/message-formatting#image-previews): Added
|
||||
`data-original-content-type` attribute to convey the type of the original
|
||||
image, and optional `data-transcoded-image` attribute for images with formats
|
||||
which are not widely supported by browsers.
|
||||
|
||||
**Feature level 335**
|
||||
|
||||
* [`GET /streams/{stream_id}/email_address`](/api/get-stream-email-address):
|
||||
|
||||
@@ -68,6 +68,7 @@ generate an image preview element with the following format.
|
||||
<div class="message_inline_image">
|
||||
<a href="/user_uploads/path/to/image.png" title="image.png">
|
||||
<img data-original-dimensions="1920x1080"
|
||||
data-original-content-type="image/png"
|
||||
src="/user_uploads/thumbnail/path/to/image.png/840x560.webp">
|
||||
</a>
|
||||
</div>
|
||||
@@ -83,7 +84,10 @@ desired. For example:
|
||||
``` html
|
||||
<div class="message_inline_image">
|
||||
<a href="/user_uploads/path/to/image.png" title="image.png">
|
||||
<img class="image-loading-placeholder" data-original-dimensions="1920x1080" src="/path/to/spinner.png">
|
||||
<img class="image-loading-placeholder"
|
||||
data-original-dimensions="1920x1080"
|
||||
data-original-content-type="image/png"
|
||||
src="/path/to/spinner.png">
|
||||
</a>
|
||||
</div>
|
||||
```
|
||||
@@ -122,6 +126,14 @@ previews:
|
||||
change over time.
|
||||
- Download button type elements should provide the original image
|
||||
(encoded via the `href` of the containing `a` tag).
|
||||
- The content-type of the original image is provided on a
|
||||
`data-original-content-type` attribute, so clients can decide if
|
||||
they are capable of rendering the original image.
|
||||
- For images whose formats which are not widely-accepted by browsers
|
||||
(e.g., HEIC and TIFF), the image may contain a
|
||||
`data-transcoded-image` attribute, which specifies a high-resolution
|
||||
thumbnail format which clients may use instead of the original
|
||||
image.
|
||||
- Lightbox elements for viewing an image should be designed to
|
||||
immediately display any already-downloaded thumbnail while fetching
|
||||
the original-quality image or an appropriate higher-quality
|
||||
@@ -143,6 +155,11 @@ previews:
|
||||
format match what they requested.
|
||||
- No other processing of the URLs is recommended.
|
||||
|
||||
**Changes**: In Zulip 10.0 (feature level 336), added
|
||||
`data-original-content-type` attribute to convey the type of the
|
||||
original image, and optional `data-transcoded-image` attribute for
|
||||
images with formats which are not widely supported by browsers.
|
||||
|
||||
**Changes**: In Zulip 9.2 (feature levels 278-279, and 287+), added
|
||||
`data-original-dimensions` to the `image-loading-placeholder` spinner
|
||||
images, containing the dimensions of the original image.
|
||||
@@ -159,7 +176,7 @@ point to the original image.
|
||||
|
||||
Clients that correctly implement the current API should handle
|
||||
Thumbor-based older thumbnails correctly, as long as they do not
|
||||
assume that `data-original-dimension` is present. Clients should not
|
||||
assume that `data-original-dimensions` is present. Clients should not
|
||||
assume that messages sent prior to the introduction of thumbnailing
|
||||
have been re-rendered to use the new format or have thumbnails
|
||||
available.
|
||||
|
||||
@@ -34,7 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3"
|
||||
# new level means in api_docs/changelog.md, as well as "**Changes**"
|
||||
# entries in the endpoint's documentation in `zulip.yaml`.
|
||||
|
||||
API_FEATURE_LEVEL = 335 # Last bumped for adding sender_id in get channel email address endpoint.
|
||||
API_FEATURE_LEVEL = 336 # Last bumped for data-original-content-type and data-transcoded-image
|
||||
|
||||
# Bump the minor PROVISION_VERSION to indicate that folks should provision
|
||||
# only when going from an old version of the code to a newer version. Bump
|
||||
|
||||
@@ -669,6 +669,11 @@ class InlineInterestingLinkProcessor(markdown.treeprocessors.Treeprocessor):
|
||||
"data-original-dimensions",
|
||||
f"{metadata.original_width_px}x{metadata.original_height_px}",
|
||||
)
|
||||
if metadata.original_content_type:
|
||||
img.set(
|
||||
"data-original-content-type",
|
||||
metadata.original_content_type,
|
||||
)
|
||||
else:
|
||||
img.set("src", image_url)
|
||||
|
||||
|
||||
@@ -9,13 +9,14 @@ from typing import TypeVar
|
||||
import pyvips
|
||||
from bs4 import BeautifulSoup
|
||||
from bs4.formatter import EntitySubstitution, HTMLFormatter
|
||||
from django.db.models import OuterRef, Subquery
|
||||
from django.utils.translation import gettext as _
|
||||
from typing_extensions import override
|
||||
|
||||
from zerver.lib.exceptions import ErrorCode, JsonableError
|
||||
from zerver.lib.mime_types import INLINE_MIME_TYPES
|
||||
from zerver.lib.queue import queue_event_on_commit
|
||||
from zerver.models import ImageAttachment
|
||||
from zerver.models import Attachment, ImageAttachment
|
||||
|
||||
DEFAULT_AVATAR_SIZE = 100
|
||||
MEDIUM_AVATAR_SIZE = 500
|
||||
@@ -80,7 +81,7 @@ class StoredThumbnailFormat(BaseThumbnailFormat):
|
||||
# Formats that we generate; the first animated and non-animated
|
||||
# options on this list are the ones which are written into
|
||||
# rendered_content.
|
||||
THUMBNAIL_OUTPUT_FORMATS = [
|
||||
THUMBNAIL_OUTPUT_FORMATS = (
|
||||
# We generate relatively large default "thumbnails", so that
|
||||
# clients that do not understand the thumbnailing protocol
|
||||
# (e.g. mobile) get something which does not look pixelated. This
|
||||
@@ -88,7 +89,13 @@ THUMBNAIL_OUTPUT_FORMATS = [
|
||||
# upsized thumbnail while loading the full resolution image.
|
||||
ThumbnailFormat("webp", 840, 560, animated=True),
|
||||
ThumbnailFormat("webp", 840, 560, animated=False),
|
||||
]
|
||||
)
|
||||
|
||||
# This format is generated, in addition to THUMBNAIL_OUTPUT_FORMATS,
|
||||
# for images in THUMBNAIL_ACCEPT_IMAGE_TYPES which are not in
|
||||
# INLINE_MIME_TYPES: somewhat-common image types which are not broadly
|
||||
# supported by browsers.
|
||||
TRANSCODED_IMAGE_FORMAT = ThumbnailFormat("webp", 4032, 3024, animated=False)
|
||||
|
||||
|
||||
# These are the image content-types which the server supports parsing
|
||||
@@ -257,18 +264,39 @@ def resize_emoji(
|
||||
return (animated.write_to_buffer(write_file_ext), first_still)
|
||||
|
||||
|
||||
def missing_thumbnails(image_attachment: ImageAttachment) -> list[ThumbnailFormat]:
|
||||
def missing_thumbnails(
|
||||
image_attachment: ImageAttachment, original_content_type: str | None = None
|
||||
) -> list[ThumbnailFormat]:
|
||||
seen_thumbnails: set[StoredThumbnailFormat] = set()
|
||||
for existing_thumbnail in image_attachment.thumbnail_metadata:
|
||||
seen_thumbnails.add(StoredThumbnailFormat(**existing_thumbnail))
|
||||
|
||||
potential_output_formats = list(THUMBNAIL_OUTPUT_FORMATS)
|
||||
if original_content_type is None or original_content_type not in INLINE_MIME_TYPES:
|
||||
if image_attachment.original_width_px >= image_attachment.original_height_px:
|
||||
additional_format = ThumbnailFormat(
|
||||
TRANSCODED_IMAGE_FORMAT.extension,
|
||||
TRANSCODED_IMAGE_FORMAT.max_width,
|
||||
TRANSCODED_IMAGE_FORMAT.max_height,
|
||||
TRANSCODED_IMAGE_FORMAT.animated,
|
||||
)
|
||||
else:
|
||||
additional_format = ThumbnailFormat(
|
||||
TRANSCODED_IMAGE_FORMAT.extension,
|
||||
# Swap width and height to make a portrait-oriented version
|
||||
TRANSCODED_IMAGE_FORMAT.max_height,
|
||||
TRANSCODED_IMAGE_FORMAT.max_width,
|
||||
TRANSCODED_IMAGE_FORMAT.animated,
|
||||
)
|
||||
potential_output_formats.append(additional_format)
|
||||
|
||||
# We use the shared `__eq__` method from BaseThumbnailFormat to
|
||||
# compare between the StoredThumbnailFormat values pulled from the
|
||||
# database, and the ThumbnailFormat values in
|
||||
# THUMBNAIL_OUTPUT_FORMATS.
|
||||
needed_thumbnails = [
|
||||
thumbnail_format
|
||||
for thumbnail_format in THUMBNAIL_OUTPUT_FORMATS
|
||||
for thumbnail_format in potential_output_formats
|
||||
if thumbnail_format not in seen_thumbnails
|
||||
]
|
||||
|
||||
@@ -341,6 +369,8 @@ class MarkdownImageMetadata:
|
||||
is_animated: bool
|
||||
original_width_px: int
|
||||
original_height_px: int
|
||||
original_content_type: str | None
|
||||
transcoded_image: StoredThumbnailFormat | None = None
|
||||
|
||||
|
||||
def get_user_upload_previews(
|
||||
@@ -357,11 +387,17 @@ def get_user_upload_previews(
|
||||
|
||||
upload_preview_data: dict[str, MarkdownImageMetadata] = {}
|
||||
|
||||
image_attachments = ImageAttachment.objects.filter(
|
||||
realm_id=realm_id, path_id__in=path_ids
|
||||
).order_by("id")
|
||||
image_attachments = (
|
||||
ImageAttachment.objects.filter(realm_id=realm_id, path_id__in=path_ids)
|
||||
.annotate(
|
||||
original_content_type=Subquery(
|
||||
Attachment.objects.filter(path_id=OuterRef("path_id")).values("content_type")
|
||||
)
|
||||
)
|
||||
.order_by("id")
|
||||
)
|
||||
if lock:
|
||||
image_attachments = image_attachments.select_for_update()
|
||||
image_attachments = image_attachments.select_for_update(of=("self",))
|
||||
for image_attachment in image_attachments:
|
||||
if image_attachment.thumbnail_metadata == []:
|
||||
# Image exists, and header of it parsed as a valid image,
|
||||
@@ -372,6 +408,7 @@ def get_user_upload_previews(
|
||||
is_animated=False,
|
||||
original_width_px=image_attachment.original_width_px,
|
||||
original_height_px=image_attachment.original_height_px,
|
||||
original_content_type=image_attachment.original_content_type,
|
||||
)
|
||||
|
||||
# We re-queue the row for thumbnailing to make sure that
|
||||
@@ -389,6 +426,10 @@ def get_user_upload_previews(
|
||||
is_animated=is_animated,
|
||||
original_width_px=image_attachment.original_width_px,
|
||||
original_height_px=image_attachment.original_height_px,
|
||||
original_content_type=image_attachment.original_content_type,
|
||||
transcoded_image=get_transcoded_format(
|
||||
image_attachment, image_attachment.original_content_type
|
||||
),
|
||||
)
|
||||
return upload_preview_data
|
||||
|
||||
@@ -414,6 +455,25 @@ def get_default_thumbnail_url(image_attachment: ImageAttachment) -> tuple[str, b
|
||||
)
|
||||
|
||||
|
||||
def get_transcoded_format(
|
||||
image_attachment: ImageAttachment, original_content_type: str | None
|
||||
) -> StoredThumbnailFormat | None:
|
||||
# Returns None if the original content-type is judged to be
|
||||
# renderable inline. Otherwise, we return the largest thumbnail
|
||||
# that we generated. Since formats which are thumbnailable but
|
||||
# not in INLINE_MIME_TYPES get an extra large-resolution thumbnail
|
||||
# added to their list of formats, this is thus either None or a
|
||||
# high-resolution thumbnail.
|
||||
if original_content_type is None or original_content_type in INLINE_MIME_TYPES:
|
||||
return None
|
||||
|
||||
thumbs_by_size = sorted(
|
||||
(StoredThumbnailFormat(**d) for d in image_attachment.thumbnail_metadata),
|
||||
key=lambda t: t.width * t.height,
|
||||
)
|
||||
return thumbs_by_size.pop() if thumbs_by_size else None
|
||||
|
||||
|
||||
# Like HTMLFormatter.REGISTRY["html5"], this formatter avoids producing
|
||||
# self-closing tags, but it differs by avoiding unnecessary escaping with
|
||||
# HTML5-specific entities that cannot be parsed by lxml and libxml2
|
||||
@@ -481,8 +541,11 @@ def rewrite_thumbnailed_images(
|
||||
image_tag["data-original-dimensions"] = (
|
||||
f"{image_data.original_width_px}x{image_data.original_height_px}"
|
||||
)
|
||||
image_tag["data-original-content-type"] = image_data.original_content_type
|
||||
if image_data.is_animated:
|
||||
image_tag["data-animated"] = "true"
|
||||
if image_data.transcoded_image is not None:
|
||||
image_tag["data-transcoded-image"] = str(image_data.transcoded_image)
|
||||
|
||||
if changed:
|
||||
return parsed_message.encode(
|
||||
|
||||
@@ -1273,7 +1273,10 @@ class RealmImportExportTest(ExportFile):
|
||||
expected_rendered_preview = (
|
||||
f'<p>An <a href="/user_uploads/{path_id}">image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
)
|
||||
self.assertEqual(
|
||||
imported_message_with_thumbnail.rendered_content, expected_rendered_preview
|
||||
|
||||
@@ -73,15 +73,24 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
"<p>Test 1<br>\n"
|
||||
f'<a href="/user_uploads/{path_ids[0]}">{image_names[0]}</a> </p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_ids[0]}" title="{image_names[0]}">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{path_ids[0]}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/jpeg"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{path_ids[0]}/840x560.webp"></a></div>'
|
||||
"<p>Next image<br>\n"
|
||||
f'<a href="/user_uploads/{path_ids[1]}">{image_names[1]}</a> </p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_ids[1]}" title="{image_names[1]}">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{path_ids[1]}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{path_ids[1]}/840x560.webp"></a></div>'
|
||||
"<p>Another screenshot<br>\n"
|
||||
f'<a href="/user_uploads/{path_ids[2]}">{image_names[2]}</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_ids[2]}" title="{image_names[2]}">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{path_ids[2]}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/gif"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{path_ids[2]}/840x560.webp"></a></div>'
|
||||
),
|
||||
)
|
||||
|
||||
@@ -114,7 +123,10 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
expected = (
|
||||
f'<p><a href="/user_uploads/{path_id}">image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
'<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
'<img class="image-loading-placeholder"'
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
' src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
)
|
||||
|
||||
message_id = self.send_message_content(content)
|
||||
@@ -124,7 +136,10 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
expected = (
|
||||
f'<p><a href="/user_uploads/{path_id}">image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
)
|
||||
self.assert_message_content_is(message_id, expected)
|
||||
|
||||
@@ -143,7 +158,10 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
expected = (
|
||||
f'<p><a href="/user_uploads/{path_id}">I am 95% ± 5% certain!</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="I am 95% ± 5% certain!">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
)
|
||||
self.assert_message_content_is(message_id, expected)
|
||||
|
||||
@@ -163,7 +181,9 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
expected = (
|
||||
f'<p><a href="/user_uploads/{path_id}">animated_unequal_img.gif</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="animated_unequal_img.gif">'
|
||||
'<img data-animated="true" data-original-dimensions="128x56"'
|
||||
'<img data-animated="true"'
|
||||
' data-original-content-type="image/gif"'
|
||||
' data-original-dimensions="128x56"'
|
||||
f' src="/user_uploads/thumbnail/{path_id}/100x75-anim.webp"></a></div>'
|
||||
)
|
||||
message_id = self.send_message_content(content, do_thumbnail=True)
|
||||
@@ -172,7 +192,7 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
|
||||
# Re-thumbnail with a non-overlapping set of sizes
|
||||
with self.thumbnail_formats(ThumbnailFormat("jpg", 100, 75, animated=False)):
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=path_id))
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=path_id), "image/gif")
|
||||
|
||||
# We generate a new size but leave the old ones
|
||||
self.assert_length(ImageAttachment.objects.get(path_id=path_id).thumbnail_metadata, 3)
|
||||
@@ -194,37 +214,55 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
f'<p><a href="/user_uploads/{first_path_id}">first image</a><br>\n'
|
||||
f'<a href="/user_uploads/{second_path_id}">second image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{first_path_id}" title="first image">'
|
||||
'<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
'<img class="image-loading-placeholder"'
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
' src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{second_path_id}" title="second image">'
|
||||
'<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
'<img class="image-loading-placeholder"'
|
||||
' data-original-content-type="image/jpeg"'
|
||||
' data-original-dimensions="128x128"'
|
||||
' src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
),
|
||||
)
|
||||
|
||||
# Complete thumbnailing the second image first -- replacing only that spinner
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=second_path_id))
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=second_path_id), "image/jpeg")
|
||||
self.assert_message_content_is(
|
||||
message_id,
|
||||
(
|
||||
f'<p><a href="/user_uploads/{first_path_id}">first image</a><br>\n'
|
||||
f'<a href="/user_uploads/{second_path_id}">second image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{first_path_id}" title="first image">'
|
||||
'<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
'<img class="image-loading-placeholder"'
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
' src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{second_path_id}" title="second image">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{second_path_id}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/jpeg"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{second_path_id}/840x560.webp"></a></div>'
|
||||
),
|
||||
)
|
||||
|
||||
# Finish the other thumbnail
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=first_path_id))
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=first_path_id), "image/png")
|
||||
self.assert_message_content_is(
|
||||
message_id,
|
||||
(
|
||||
f'<p><a href="/user_uploads/{first_path_id}">first image</a><br>\n'
|
||||
f'<a href="/user_uploads/{second_path_id}">second image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{first_path_id}" title="first image">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{first_path_id}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{first_path_id}/840x560.webp"></a></div>'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{second_path_id}" title="second image">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{second_path_id}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/jpeg"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{second_path_id}/840x560.webp"></a></div>'
|
||||
),
|
||||
)
|
||||
|
||||
@@ -245,11 +283,14 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
|
||||
# Completing rendering after it is deleted should work, and
|
||||
# update the rendered content in the archived message
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=path_id))
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=path_id), "image/png")
|
||||
expected = (
|
||||
f'<p><a href="/user_uploads/{path_id}">image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
)
|
||||
self.assertEqual(
|
||||
ArchivedMessage.objects.get(id=message_id).rendered_content,
|
||||
@@ -272,7 +313,7 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
) as thumb_mock,
|
||||
self.assertLogs("zerver.worker.thumbnail", "ERROR") as thumbnail_logs,
|
||||
):
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=path_id))
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=path_id), "image/png")
|
||||
thumb_mock.assert_called_once()
|
||||
self.assert_length(thumbnail_logs.output, 1)
|
||||
self.assertTrue(
|
||||
@@ -295,7 +336,10 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
)
|
||||
placeholder = (
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
'<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
'<img class="image-loading-placeholder"'
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
' src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
)
|
||||
self.assert_message_content_is(
|
||||
channel_message_id,
|
||||
@@ -316,14 +360,17 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
ThumbnailFormat("webp", 200, 150, animated=False),
|
||||
),
|
||||
):
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=path_id))
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=path_id), "image/png")
|
||||
|
||||
# Called once per format
|
||||
self.assertEqual(thumb_mock.call_count, 2)
|
||||
|
||||
rendered_thumb = (
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{path_id}/100x75.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{path_id}/100x75.webp"></a></div>'
|
||||
)
|
||||
|
||||
self.assert_message_content_is(
|
||||
@@ -354,13 +401,16 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
expected = (
|
||||
f'<p><a href="/user_uploads/{path_id}">image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
'<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
'<img class="image-loading-placeholder"'
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
' src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
)
|
||||
self.assertEqual(send_request.message.rendered_content, expected)
|
||||
|
||||
# Thumbnail the image. The message does not exist yet, so
|
||||
# nothing is re-written.
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=path_id))
|
||||
ensure_thumbnails(ImageAttachment.objects.get(path_id=path_id), "image/png")
|
||||
|
||||
# Send the message; this should re-check the ImageAttachment
|
||||
# data, find the thumbnails, and update the rendered_content
|
||||
@@ -369,7 +419,10 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
|
||||
rendered_thumb = (
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
)
|
||||
self.assert_message_content_is(
|
||||
message_id, f'<p><a href="/user_uploads/{path_id}">image</a></p>\n{rendered_thumb}'
|
||||
@@ -389,7 +442,10 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
expected = (
|
||||
f'<p><a href="/user_uploads/{path_id}">image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
'<img class="image-loading-placeholder" data-original-dimensions="128x128" src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
'<img class="image-loading-placeholder"'
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
' src="/static/images/loading/loader-black.svg"></a></div>'
|
||||
)
|
||||
|
||||
message_id = self.send_message_content(content)
|
||||
@@ -400,6 +456,25 @@ class MarkdownThumbnailTest(ZulipTestCase):
|
||||
expected = (
|
||||
f'<p><a href="/user_uploads/{path_id}">image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
f'<img data-original-dimensions="128x128" src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
"<img"
|
||||
' data-original-content-type="image/png"'
|
||||
' data-original-dimensions="128x128"'
|
||||
f' src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
)
|
||||
self.assert_message_content_is(message_id, expected)
|
||||
|
||||
def test_thumbnail_transcode(self) -> None:
|
||||
path_id = self.upload_image("img.tif")
|
||||
message_id = self.send_message_content(
|
||||
f"An [image](/user_uploads/{path_id})", do_thumbnail=True
|
||||
)
|
||||
expected = (
|
||||
f'<p>An <a href="/user_uploads/{path_id}">image</a></p>\n'
|
||||
f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
|
||||
"<img"
|
||||
' data-original-content-type="image/tiff"'
|
||||
' data-original-dimensions="128x128"'
|
||||
' data-transcoded-image="4032x3024.webp"'
|
||||
f' src="/user_uploads/thumbnail/{path_id}/840x560.webp"></a></div>'
|
||||
)
|
||||
self.assert_message_content_is(message_id, expected)
|
||||
|
||||
@@ -22,6 +22,7 @@ from zerver.lib.thumbnail import (
|
||||
StoredThumbnailFormat,
|
||||
ThumbnailFormat,
|
||||
get_image_thumbnail_path,
|
||||
get_transcoded_format,
|
||||
missing_thumbnails,
|
||||
resize_emoji,
|
||||
split_thumbnail_path,
|
||||
@@ -363,10 +364,10 @@ class TestStoreThumbnail(ZulipTestCase):
|
||||
self.assertEqual(thumbnailed_image.get_n_pages(), 2)
|
||||
|
||||
with self.thumbnail_formats(ThumbnailFormat("webp", 100, 75, animated=True)):
|
||||
self.assertEqual(ensure_thumbnails(image_attachment), 0)
|
||||
self.assertEqual(ensure_thumbnails(image_attachment, "image/webp"), 0)
|
||||
|
||||
with self.thumbnail_formats(ThumbnailFormat("webp", 150, 100, opts="Q=90", animated=False)):
|
||||
self.assertEqual(ensure_thumbnails(image_attachment), 1)
|
||||
self.assertEqual(ensure_thumbnails(image_attachment, "image/webp"), 1)
|
||||
self.assert_length(image_attachment.thumbnail_metadata, 2)
|
||||
|
||||
bigger_thumbnail = StoredThumbnailFormat(**image_attachment.thumbnail_metadata[1])
|
||||
@@ -507,10 +508,10 @@ class TestStoreThumbnail(ZulipTestCase):
|
||||
thumbnail_metadata=[],
|
||||
)
|
||||
with self.thumbnail_formats(ThumbnailFormat("webp", 100, 75, animated=False)):
|
||||
self.assert_length(missing_thumbnails(image_attachment), 1)
|
||||
self.assert_length(missing_thumbnails(image_attachment, "image/gif"), 1)
|
||||
|
||||
with self.assertLogs("zerver.worker.thumbnail", level="ERROR") as error_log:
|
||||
self.assertEqual(ensure_thumbnails(image_attachment), 0)
|
||||
self.assertEqual(ensure_thumbnails(image_attachment, "image/gif"), 0)
|
||||
|
||||
libvips_version = (pyvips.version(0), pyvips.version(1))
|
||||
# This error message changed
|
||||
@@ -532,21 +533,23 @@ class TestStoreThumbnail(ZulipTestCase):
|
||||
thumbnail_metadata=[],
|
||||
)
|
||||
with self.thumbnail_formats():
|
||||
self.assertEqual(missing_thumbnails(image_attachment), [])
|
||||
self.assertEqual(missing_thumbnails(image_attachment, "image/png"), [])
|
||||
|
||||
still_webp = ThumbnailFormat("webp", 100, 75, animated=False, opts="Q=90")
|
||||
with self.thumbnail_formats(still_webp):
|
||||
self.assertEqual(missing_thumbnails(image_attachment), [still_webp])
|
||||
self.assertEqual(missing_thumbnails(image_attachment, "image/png"), [still_webp])
|
||||
|
||||
anim_webp = ThumbnailFormat("webp", 100, 75, animated=True, opts="Q=90")
|
||||
with self.thumbnail_formats(still_webp, anim_webp):
|
||||
# It's not animated, so the animated format doesn't appear at all
|
||||
self.assertEqual(missing_thumbnails(image_attachment), [still_webp])
|
||||
self.assertEqual(missing_thumbnails(image_attachment, "image/png"), [still_webp])
|
||||
|
||||
still_jpeg = ThumbnailFormat("jpeg", 100, 75, animated=False, opts="Q=90")
|
||||
with self.thumbnail_formats(still_webp, anim_webp, still_jpeg):
|
||||
# But other still formats do
|
||||
self.assertEqual(missing_thumbnails(image_attachment), [still_webp, still_jpeg])
|
||||
self.assertEqual(
|
||||
missing_thumbnails(image_attachment, "image/png"), [still_webp, still_jpeg]
|
||||
)
|
||||
|
||||
# If we have a rendered 150x100.webp, then we're not missing it
|
||||
rendered_still_webp = StoredThumbnailFormat(
|
||||
@@ -561,12 +564,94 @@ class TestStoreThumbnail(ZulipTestCase):
|
||||
)
|
||||
image_attachment.thumbnail_metadata = [asdict(rendered_still_webp)]
|
||||
with self.thumbnail_formats(still_webp, anim_webp, still_jpeg):
|
||||
self.assertEqual(missing_thumbnails(image_attachment), [still_jpeg])
|
||||
self.assertEqual(missing_thumbnails(image_attachment, "image/png"), [still_jpeg])
|
||||
|
||||
# If we have the still, and it's animated, we do still need the animated
|
||||
image_attachment.frames = 10
|
||||
with self.thumbnail_formats(still_webp, anim_webp, still_jpeg):
|
||||
self.assertEqual(missing_thumbnails(image_attachment), [anim_webp, still_jpeg])
|
||||
self.assertEqual(
|
||||
missing_thumbnails(image_attachment, "image/png"), [anim_webp, still_jpeg]
|
||||
)
|
||||
|
||||
def test_transcoded_format(self) -> None:
|
||||
image_attachment = ImageAttachment(
|
||||
path_id="example",
|
||||
original_width_px=150,
|
||||
original_height_px=100,
|
||||
frames=1,
|
||||
thumbnail_metadata=[],
|
||||
)
|
||||
still_webp = ThumbnailFormat("webp", 100, 75, animated=False, opts="Q=90")
|
||||
with self.thumbnail_formats(still_webp):
|
||||
# We add a high-resolution transcoded format if the image isn't in INLINE_MIME_TYPES:
|
||||
transcoded = ThumbnailFormat("webp", 4032, 3024, animated=False)
|
||||
self.assertEqual(
|
||||
missing_thumbnails(image_attachment, "image/tiff"), [still_webp, transcoded]
|
||||
)
|
||||
|
||||
# We flip to being portrait if the image is higher than it is wide
|
||||
transcoded = ThumbnailFormat("webp", 3024, 4032, animated=False)
|
||||
image_attachment.original_height_px = 300
|
||||
self.assertEqual(
|
||||
missing_thumbnails(image_attachment, "image/tiff"), [still_webp, transcoded]
|
||||
)
|
||||
|
||||
# The format is not animated, even if the original was
|
||||
image_attachment.original_height_px = 100
|
||||
image_attachment.frames = 10
|
||||
transcoded = ThumbnailFormat("webp", 4032, 3024, animated=False)
|
||||
self.assertEqual(
|
||||
missing_thumbnails(image_attachment, "image/tiff"), [still_webp, transcoded]
|
||||
)
|
||||
|
||||
# We do not store on the image_attachment if we generated
|
||||
# a transcoded version; it just picks the largest format
|
||||
# if one is called for.
|
||||
self.assertEqual(get_transcoded_format(image_attachment, "image/tiff"), None)
|
||||
image_attachment.thumbnail_metadata = [
|
||||
asdict(
|
||||
StoredThumbnailFormat(
|
||||
"webp",
|
||||
100,
|
||||
75,
|
||||
animated=False,
|
||||
content_type="image/webp",
|
||||
width=100,
|
||||
height=75,
|
||||
byte_size=100,
|
||||
)
|
||||
),
|
||||
asdict(
|
||||
StoredThumbnailFormat(
|
||||
"webp",
|
||||
840,
|
||||
560,
|
||||
animated=False,
|
||||
content_type="image/webp",
|
||||
width=747,
|
||||
height=560,
|
||||
byte_size=800,
|
||||
)
|
||||
),
|
||||
asdict(
|
||||
StoredThumbnailFormat(
|
||||
"webp",
|
||||
4032,
|
||||
3024,
|
||||
animated=False,
|
||||
content_type="image/webp",
|
||||
width=4032,
|
||||
height=3024,
|
||||
byte_size=2000,
|
||||
)
|
||||
),
|
||||
]
|
||||
self.assertEqual(get_transcoded_format(image_attachment, "image/png"), None)
|
||||
self.assertEqual(get_transcoded_format(image_attachment, None), None)
|
||||
self.assertEqual(
|
||||
get_transcoded_format(image_attachment, "image/tiff"),
|
||||
ThumbnailFormat("webp", 4032, 3024, animated=False),
|
||||
)
|
||||
|
||||
def test_maybe_thumbnail_from_stream(self) -> None:
|
||||
# If we put the file in place directly (e.g. simulating a
|
||||
|
||||
@@ -310,14 +310,18 @@ def serve_file(
|
||||
if not thumbnail_format.animated
|
||||
]
|
||||
else:
|
||||
potential_output_formats = THUMBNAIL_OUTPUT_FORMATS
|
||||
potential_output_formats = list(THUMBNAIL_OUTPUT_FORMATS)
|
||||
if requested_format not in potential_output_formats:
|
||||
if rendered_formats == []:
|
||||
# We haven't rendered anything, and they requested
|
||||
# something we don't support.
|
||||
return serve_image_error(404, "images/errors/image-not-exist.png")
|
||||
elif requested_format in rendered_formats:
|
||||
# Not a _current_ format, but we did render it at the time, so fine to serve
|
||||
# Not a _current_ format, but we did render it at the
|
||||
# time, so fine to serve. We also end up here for
|
||||
# TRANSCODED_IMAGE_FORMAT requests, which are not in
|
||||
# the default THUMBNAIL_OUTPUT_FORMATS, but may exist
|
||||
# for some images types not in INLINE_MIME_TYPES.
|
||||
pass
|
||||
else:
|
||||
# Find something "close enough". This will not be a
|
||||
@@ -337,7 +341,8 @@ def serve_file(
|
||||
# currently processing the row.
|
||||
with transaction.atomic(savepoint=False):
|
||||
ensure_thumbnails(
|
||||
ImageAttachment.objects.select_for_update().get(id=image_attachment.id)
|
||||
ImageAttachment.objects.select_for_update().get(id=image_attachment.id),
|
||||
attachment.content_type,
|
||||
)
|
||||
|
||||
# Update the path that we are fetching to be the thumbnail
|
||||
|
||||
@@ -6,6 +6,7 @@ from typing import Any
|
||||
|
||||
import pyvips
|
||||
from django.db import transaction
|
||||
from django.db.models import OuterRef, Subquery
|
||||
from typing_extensions import override
|
||||
|
||||
from zerver.actions.message_edit import do_update_embedded_data
|
||||
@@ -15,11 +16,12 @@ from zerver.lib.thumbnail import (
|
||||
StoredThumbnailFormat,
|
||||
get_default_thumbnail_url,
|
||||
get_image_thumbnail_path,
|
||||
get_transcoded_format,
|
||||
missing_thumbnails,
|
||||
rewrite_thumbnailed_images,
|
||||
)
|
||||
from zerver.lib.upload import save_attachment_contents, upload_backend
|
||||
from zerver.models import ArchivedMessage, ImageAttachment, Message
|
||||
from zerver.models import ArchivedMessage, Attachment, ImageAttachment, Message
|
||||
from zerver.worker.base import QueueProcessingWorker, assign_queue
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -37,11 +39,21 @@ class ThumbnailWorker(QueueProcessingWorker):
|
||||
# directly to a thumbnail URL we have not made yet.
|
||||
# This may mean that we may generate 0 thumbnail
|
||||
# images once we get the lock.
|
||||
row = ImageAttachment.objects.select_for_update().get(id=event["id"])
|
||||
row = (
|
||||
ImageAttachment.objects.select_for_update(of=("self",))
|
||||
.annotate(
|
||||
original_content_type=Subquery(
|
||||
Attachment.objects.filter(path_id=OuterRef("path_id")).values(
|
||||
"content_type"
|
||||
)
|
||||
)
|
||||
)
|
||||
.get(id=event["id"])
|
||||
)
|
||||
except ImageAttachment.DoesNotExist: # nocoverage
|
||||
logger.info("ImageAttachment row %d missing", event["id"])
|
||||
return
|
||||
uploaded_thumbnails = ensure_thumbnails(row)
|
||||
uploaded_thumbnails = ensure_thumbnails(row, row.original_content_type)
|
||||
end = time.time()
|
||||
logger.info(
|
||||
"Processed %d thumbnails (%dms)",
|
||||
@@ -50,8 +62,10 @@ class ThumbnailWorker(QueueProcessingWorker):
|
||||
)
|
||||
|
||||
|
||||
def ensure_thumbnails(image_attachment: ImageAttachment) -> int:
|
||||
needed_thumbnails = missing_thumbnails(image_attachment)
|
||||
def ensure_thumbnails(
|
||||
image_attachment: ImageAttachment, original_content_type: str | None = None
|
||||
) -> int:
|
||||
needed_thumbnails = missing_thumbnails(image_attachment, original_content_type)
|
||||
|
||||
if not needed_thumbnails:
|
||||
return 0
|
||||
@@ -150,6 +164,8 @@ def ensure_thumbnails(image_attachment: ImageAttachment) -> int:
|
||||
is_animated=is_animated,
|
||||
original_width_px=image_attachment.original_width_px,
|
||||
original_height_px=image_attachment.original_height_px,
|
||||
original_content_type=original_content_type,
|
||||
transcoded_image=get_transcoded_format(image_attachment, original_content_type),
|
||||
),
|
||||
)
|
||||
return written_images
|
||||
|
||||
Reference in New Issue
Block a user