thumbnail: Enqueue thumbnails when we render a spinner.

Thumbnails are usually enqueued in the worker when the image is
uploaded.  However, for images which were uploaded before the
existence of the thumbnailing worker, and whose metadata was
backfilled (see previous commit) this leaves a permanent spinner,
since nothing triggers the thumbnail worker for them.

Enqueue a thumbnail worker for every spinner which we render into
Markdown.  This ensures that _something_ is attempting to resolve the
spinner which the user sees.  In the case of freshly-uploaded images
which are still in the queue, this results in a duplicate entry in the
thumbnailing queue -- this is harmless, since the worker determines
that all of the thumbnails we need have already been generated, and it
does no further work.  However, in the case of historical uploads, it
properly kicks off the thumbnailing process and results in a
subsequent message update to include the freshly-generated thumbnail.

While specifically useful for backfilled uploads, this is also
generally a good safety step for a good user experience, as it also
prevents dropped events in the queue from unknown causes from leaving
perpetual spinners in the message feed.

Because `get_user_upload_previews` is potentially called twice for
every message with spinners (see 6f20c15ae9), we add an additional
flag to `get_user_upload_previews` to suppress a _second_ event from
being enqueued for every spinner generated.

zerver/actions/message_send.py

@@ -879,6 +879,7 @@ def do_send_messages(
                     send_request.message.realm_id,
                     send_request.message.content,
                     lock=True,
+                    enqueue=False,
                     path_ids=list(send_request.rendering_result.thumbnail_spinners),
                 )
                 new_rendered_content = rewrite_thumbnailed_images(

zerver/lib/thumbnail.py

@@ -336,6 +336,7 @@ def get_user_upload_previews(
     realm_id: int,
     content: str,
     lock: bool = False,
+    enqueue: bool = True,
     path_ids: list[str] | None = None,
 ) -> dict[str, MarkdownImageMetadata | None]:
     if path_ids is None:
@@ -354,6 +355,15 @@ def get_user_upload_previews(
             # but has not been thumbnailed yet; we will render a
             # spinner.
             upload_preview_data[image_attachment.path_id] = None
+
+            # We re-queue the row for thumbnailing to make sure that
+            # we do eventually thumbnail it (e.g. if this is a
+            # historical upload from before this system, which we
+            # backfilled ImageAttachment rows for); this is a no-op in
+            # the worker if all of the currently-configured thumbnail
+            # formats have already been generated.
+            if enqueue:
+                queue_event_on_commit("thumbnail", {"id": image_attachment.id})
         else:
             url, is_animated = get_default_thumbnail_url(image_attachment)
             upload_preview_data[image_attachment.path_id] = MarkdownImageMetadata(

zerver/tests/test_events.py

@@ -1038,7 +1038,9 @@ class NormalActionsTest(BaseAction):
             "img.png", "image/png", read_test_image_file("img.png"), self.example_user("iago")
         )
         path_id = url[len("/user_upload/") + 1 :]
-        self.send_stream_message(iago, "Verona", f"[img.png]({url})")
+        self.send_stream_message(
+            iago, "Verona", f"[img.png]({url})", skip_capture_on_commit_callbacks=True
+        )
 
         # Generating a thumbnail for an image sends a message update event
         with self.verify_action(state_change_expected=False) as events:

zerver/tests/test_markdown_thumbnail.py

@@ -300,6 +300,7 @@ class MarkdownThumbnailTest(ZulipTestCase):
             from_user=sender_user_profile,
             to_user=self.example_user("hamlet"),
             content=f"This [image](/user_uploads/{path_id}) is private",
+            skip_capture_on_commit_callbacks=True,
         )
         placeholder = (
             f'<div class="message_inline_image"><a href="/user_uploads/{path_id}" title="image">'
@@ -382,3 +383,32 @@ class MarkdownThumbnailTest(ZulipTestCase):
         self.assert_message_content_is(
             message_id, f'<p><a href="/user_uploads/{path_id}">image</a></p>\n{rendered_thumb}'
         )
+
+    def test_thumbnail_historical_image(self) -> None:
+        # Note that this is outside the captureOnCommitCallbacks, so
+        # we don't actually run thumbnailing for it.  This results in
+        # a ImageAttachment row but no thumbnails, which matches the
+        # state of backfilled previously-uploaded images.
+        path_id = self.upload_image("img.png")
+
+        with self.captureOnCommitCallbacks(execute=True):
+            message_id = self.send_message_content(f"An [image](/user_uploads/{path_id})")
+
+            content = f"[image](/user_uploads/{path_id})"
+            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" src="/static/images/loading/loader-black.svg"></a></div>'
+            )
+
+            message_id = self.send_message_content(content)
+            self.assert_message_content_is(message_id, expected)
+
+        # Exiting the block should have run the thumbnailing that was
+        # enqueued when rendering the message.
+        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>'
+        )
+        self.assert_message_content_is(message_id, expected)