mirror of https://github.com/zulip/zulip.git
upload: Serve thumbnailed images.
This commit is contained in:
parent
2e38f426f4
commit
d121a80b78
|
@ -1,4 +1,6 @@
|
|||
import re
|
||||
from collections.abc import Iterator
|
||||
from contextlib import contextmanager
|
||||
from dataclasses import asdict
|
||||
from io import StringIO
|
||||
from unittest.mock import patch
|
||||
|
@ -469,3 +471,136 @@ class TestStoreThumbnail(ZulipTestCase):
|
|||
"zerver.lib.thumbnail.THUMBNAIL_OUTPUT_FORMATS", [still_webp, anim_webp, still_jpeg]
|
||||
):
|
||||
self.assertEqual(missing_thumbnails(image_attachment), [anim_webp, still_jpeg])
|
||||
|
||||
|
||||
class TestThumbnailRetrieval(ZulipTestCase):
|
||||
@contextmanager
|
||||
def mock_formats(self, thumbnail_formats: list[ThumbnailFormat]) -> Iterator[None]:
|
||||
with (
|
||||
patch("zerver.lib.thumbnail.THUMBNAIL_OUTPUT_FORMATS", thumbnail_formats),
|
||||
patch("zerver.views.upload.THUMBNAIL_OUTPUT_FORMATS", thumbnail_formats),
|
||||
):
|
||||
yield
|
||||
|
||||
def test_get_thumbnail(self) -> None:
|
||||
assert settings.LOCAL_FILES_DIR
|
||||
hamlet = self.example_user("hamlet")
|
||||
self.login_user(hamlet)
|
||||
|
||||
webp_anim = ThumbnailFormat("webp", 100, 75, animated=True)
|
||||
webp_still = ThumbnailFormat("webp", 100, 75, animated=False)
|
||||
with self.mock_formats([webp_anim, webp_still]):
|
||||
with (
|
||||
self.captureOnCommitCallbacks(execute=True),
|
||||
get_test_image_file("animated_unequal_img.gif") as image_file,
|
||||
):
|
||||
json_response = self.assert_json_success(
|
||||
self.client_post("/json/user_uploads", {"file": image_file})
|
||||
)
|
||||
path_id = re.sub(r"/user_uploads/", "", json_response["url"])
|
||||
|
||||
# Image itself is available immediately
|
||||
response = self.client_get(f"/user_uploads/{path_id}")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertEqual(response.headers["Content-Type"], "image/gif")
|
||||
|
||||
# Exit the block, triggering the thumbnailing worker
|
||||
|
||||
thumbnail_response = self.client_get(f"/user_uploads/thumbnail/{path_id}/{webp_still!s}")
|
||||
self.assertEqual(thumbnail_response.status_code, 200)
|
||||
self.assertEqual(thumbnail_response.headers["Content-Type"], "image/webp")
|
||||
self.assertLess(
|
||||
int(thumbnail_response.headers["Content-Length"]),
|
||||
int(response.headers["Content-Length"]),
|
||||
)
|
||||
|
||||
animated_response = self.client_get(f"/user_uploads/thumbnail/{path_id}/{webp_anim!s}")
|
||||
self.assertEqual(animated_response.status_code, 200)
|
||||
self.assertEqual(animated_response.headers["Content-Type"], "image/webp")
|
||||
self.assertLess(
|
||||
int(thumbnail_response.headers["Content-Length"]),
|
||||
int(animated_response.headers["Content-Length"]),
|
||||
)
|
||||
|
||||
# Invalid thumbnail format
|
||||
response = self.client_get(f"/user_uploads/thumbnail/{path_id}/bogus")
|
||||
self.assertEqual(response.status_code, 404)
|
||||
self.assertEqual(response.headers["Content-Type"], "image/png")
|
||||
|
||||
# Format we don't have
|
||||
response = self.client_get(f"/user_uploads/thumbnail/{path_id}/1x1.png")
|
||||
self.assertEqual(response.status_code, 404)
|
||||
self.assertEqual(response.headers["Content-Type"], "image/png")
|
||||
|
||||
# path_id for a non-image
|
||||
with (
|
||||
self.captureOnCommitCallbacks(execute=True),
|
||||
get_test_image_file("text.txt") as text_file,
|
||||
):
|
||||
json_response = self.assert_json_success(
|
||||
self.client_post("/json/user_uploads", {"file": text_file})
|
||||
)
|
||||
text_path_id = re.sub(r"/user_uploads/", "", json_response["url"])
|
||||
response = self.client_get(f"/user_uploads/thumbnail/{text_path_id}/{webp_still!s}")
|
||||
self.assertEqual(response.status_code, 404)
|
||||
self.assertEqual(response.headers["Content-Type"], "image/png")
|
||||
|
||||
# Shrink the list of formats, and check that we can still get
|
||||
# the thumbnails that were generated at the time
|
||||
with self.mock_formats([webp_still]):
|
||||
response = self.client_get(f"/user_uploads/thumbnail/{path_id}/{webp_still!s}")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
response = self.client_get(f"/user_uploads/thumbnail/{path_id}/{webp_anim!s}")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
|
||||
# Grow the format list, and check that fetching that new
|
||||
# format generates all of the missing formats
|
||||
jpeg_still = ThumbnailFormat("jpg", 100, 75, animated=False)
|
||||
big_jpeg_still = ThumbnailFormat("jpg", 200, 150, animated=False)
|
||||
with (
|
||||
self.mock_formats([webp_still, jpeg_still, big_jpeg_still]),
|
||||
patch.object(
|
||||
pyvips.Image, "thumbnail_buffer", wraps=pyvips.Image.thumbnail_buffer
|
||||
) as thumb_mock,
|
||||
):
|
||||
small_response = self.client_get(f"/user_uploads/thumbnail/{path_id}/{jpeg_still!s}")
|
||||
self.assertEqual(small_response.status_code, 200)
|
||||
self.assertEqual(small_response.headers["Content-Type"], "image/jpeg")
|
||||
# This made two thumbnails
|
||||
self.assertEqual(thumb_mock.call_count, 2)
|
||||
|
||||
thumb_mock.reset_mock()
|
||||
big_response = self.client_get(f"/user_uploads/thumbnail/{path_id}/{big_jpeg_still!s}")
|
||||
self.assertEqual(big_response.status_code, 200)
|
||||
self.assertEqual(big_response.headers["Content-Type"], "image/jpeg")
|
||||
thumb_mock.assert_not_called()
|
||||
|
||||
self.assertLess(
|
||||
int(small_response.headers["Content-Length"]),
|
||||
int(big_response.headers["Content-Length"]),
|
||||
)
|
||||
|
||||
# Upload a static image, and verify that we only generate the still versions
|
||||
with self.mock_formats([webp_anim, webp_still, jpeg_still]):
|
||||
with (
|
||||
self.captureOnCommitCallbacks(execute=True),
|
||||
get_test_image_file("img.tif") as image_file,
|
||||
):
|
||||
json_response = self.assert_json_success(
|
||||
self.client_post("/json/user_uploads", {"file": image_file})
|
||||
)
|
||||
path_id = re.sub(r"/user_uploads/", "", json_response["url"])
|
||||
# Exit the block, triggering the thumbnailing worker
|
||||
|
||||
response = self.client_get(f"/user_uploads/thumbnail/{path_id}/{webp_still!s}")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertEqual(response.headers["Content-Type"], "image/webp")
|
||||
|
||||
response = self.client_get(f"/user_uploads/thumbnail/{path_id}/{webp_anim!s}")
|
||||
self.assertEqual(response.status_code, 404)
|
||||
self.assertEqual(response.headers["Content-Type"], "image/png")
|
||||
|
||||
response = self.client_get(f"/user_uploads/thumbnail/{path_id}/{jpeg_still!s}")
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertEqual(response.headers["Content-Type"], "image/jpeg")
|
||||
|
|
|
@ -8,6 +8,7 @@ from django.conf import settings
|
|||
from django.contrib.auth.models import AnonymousUser
|
||||
from django.core.files.uploadedfile import UploadedFile
|
||||
from django.core.signing import BadSignature, TimestampSigner
|
||||
from django.db import transaction
|
||||
from django.http import (
|
||||
FileResponse,
|
||||
HttpRequest,
|
||||
|
@ -29,15 +30,22 @@ from zerver.lib.exceptions import JsonableError
|
|||
from zerver.lib.mime_types import guess_type
|
||||
from zerver.lib.response import json_success
|
||||
from zerver.lib.storage import static_path
|
||||
from zerver.lib.thumbnail import (
|
||||
THUMBNAIL_OUTPUT_FORMATS,
|
||||
BaseThumbnailFormat,
|
||||
StoredThumbnailFormat,
|
||||
)
|
||||
from zerver.lib.upload import (
|
||||
check_upload_within_quota,
|
||||
get_image_thumbnail_path,
|
||||
get_public_upload_root_url,
|
||||
upload_message_attachment_from_request,
|
||||
)
|
||||
from zerver.lib.upload.base import INLINE_MIME_TYPES
|
||||
from zerver.lib.upload.local import assert_is_local_storage_path
|
||||
from zerver.lib.upload.s3 import get_signed_upload_url
|
||||
from zerver.models import UserProfile
|
||||
from zerver.models import ImageAttachment, UserProfile
|
||||
from zerver.worker.thumbnail import ensure_thumbnails
|
||||
|
||||
|
||||
def patch_disposition_header(response: HttpResponse, url: str, is_attachment: bool) -> None:
|
||||
|
@ -155,8 +163,16 @@ def serve_file_backend(
|
|||
maybe_user_profile: UserProfile | AnonymousUser,
|
||||
realm_id_str: str,
|
||||
filename: str,
|
||||
thumbnail_format: str | None = None,
|
||||
) -> HttpResponseBase:
|
||||
return serve_file(request, maybe_user_profile, realm_id_str, filename, url_only=False)
|
||||
return serve_file(
|
||||
request,
|
||||
maybe_user_profile,
|
||||
realm_id_str,
|
||||
filename,
|
||||
url_only=False,
|
||||
thumbnail_format=thumbnail_format,
|
||||
)
|
||||
|
||||
|
||||
def serve_file_url_backend(
|
||||
|
@ -192,6 +208,7 @@ def serve_file(
|
|||
maybe_user_profile: UserProfile | AnonymousUser,
|
||||
realm_id_str: str,
|
||||
filename: str,
|
||||
thumbnail_format: str | None = None,
|
||||
url_only: bool = False,
|
||||
force_download: bool = False,
|
||||
) -> HttpResponseBase:
|
||||
|
@ -227,6 +244,50 @@ def serve_file(
|
|||
url = generate_unauthed_file_access_url(path_id)
|
||||
return json_success(request, data=dict(url=url))
|
||||
|
||||
if thumbnail_format is not None:
|
||||
# Check if this is something that we thumbnail at all
|
||||
try:
|
||||
image_attachment = ImageAttachment.objects.get(path_id=path_id)
|
||||
except ImageAttachment.DoesNotExist:
|
||||
return serve_image_error(404, "images/errors/image-not-exist.png")
|
||||
|
||||
# Validate that this is a potential thumbnail format
|
||||
requested_format = BaseThumbnailFormat.from_string(thumbnail_format)
|
||||
if requested_format is None:
|
||||
return serve_image_error(404, "images/errors/image-not-exist.png")
|
||||
|
||||
rendered_formats = [StoredThumbnailFormat(**f) for f in image_attachment.thumbnail_metadata]
|
||||
|
||||
# We never generate animated versions if the input was still,
|
||||
# so filter those out
|
||||
if image_attachment.frames == 1:
|
||||
potential_output_formats = [
|
||||
thumbnail_format
|
||||
for thumbnail_format in THUMBNAIL_OUTPUT_FORMATS
|
||||
if not thumbnail_format.animated
|
||||
]
|
||||
else:
|
||||
potential_output_formats = THUMBNAIL_OUTPUT_FORMATS
|
||||
if requested_format not in potential_output_formats:
|
||||
if requested_format in rendered_formats:
|
||||
# Not a _current_ format, but we did render it at the time, so fine to serve
|
||||
pass
|
||||
else:
|
||||
return serve_image_error(404, "images/errors/image-not-exist.png")
|
||||
elif requested_format not in rendered_formats:
|
||||
# They requested a valid format, but one we've not
|
||||
# rendered yet. Take a lock on the row, then render every
|
||||
# missing format, synchronously. The lock prevents us
|
||||
# from doing double-work if the background worker is
|
||||
# currently processing the row.
|
||||
with transaction.atomic(savepoint=False):
|
||||
ensure_thumbnails(
|
||||
ImageAttachment.objects.select_for_update().get(id=image_attachment.id)
|
||||
)
|
||||
|
||||
# Update the path that we are fetching to be the thumbnail
|
||||
path_id = get_image_thumbnail_path(image_attachment, requested_format)
|
||||
|
||||
if settings.LOCAL_UPLOADS_DIR is not None:
|
||||
return serve_local(request, path_id, force_download=force_download)
|
||||
else:
|
||||
|
|
|
@ -24,6 +24,11 @@ class ThumbnailWorker(QueueProcessingWorker):
|
|||
start = time.time()
|
||||
with transaction.atomic(savepoint=False):
|
||||
try:
|
||||
# This lock prevents us from racing with the on-demand
|
||||
# rendering that can be triggered if a request is made
|
||||
# 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"])
|
||||
except ImageAttachment.DoesNotExist: # nocoverage
|
||||
logger.info("ImageAttachment row %d missing", event["id"])
|
||||
|
|
|
@ -657,6 +657,10 @@ urls += [
|
|||
"user_uploads/download/<realm_id_str>/<path:filename>",
|
||||
GET=(serve_file_download_backend, {"override_api_url_scheme", "allow_anonymous_user_web"}),
|
||||
),
|
||||
rest_path(
|
||||
"user_uploads/thumbnail/<realm_id_str>/<path:filename>/<str:thumbnail_format>",
|
||||
GET=(serve_file_backend, {"override_api_url_scheme", "allow_anonymous_user_web"}),
|
||||
),
|
||||
rest_path(
|
||||
"user_uploads/<realm_id_str>/<path:filename>",
|
||||
GET=(serve_file_backend, {"override_api_url_scheme", "allow_anonymous_user_web"}),
|
||||
|
|
Loading…
Reference in New Issue