Immediate avatar and embed variants

Process variants synchronously on attachment to close the window between
image upload and variant availability, guaranteeing that we won't have
lazy variant processing attempts in GET requests.

Tradeoff is that we do variant processing in upload requests, which is
actually desirable. We're working with images that should take
milliseconds to resize given that we'll already have the file on hand.

References rails/rails#51951

Author: jeremy

Gemfile.lock

@@ -6,7 +6,7 @@ GIT
 
 GIT
   remote: https://github.com/rails/rails.git
-  revision: 3c3f6c8a253c8a0f346695374f927c9ab32fbefb
+  revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0
   branch: main
   specs:
     actioncable (8.2.0.alpha)

Gemfile.saas.lock

@@ -75,7 +75,7 @@ GIT
 
 GIT
   remote: https://github.com/rails/rails.git
-  revision: 690ec8898318b8f50714e86676353ebe1551261e
+  revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0
   branch: main
   specs:
     actioncable (8.2.0.alpha)
@@ -292,7 +292,7 @@ GEM
       actionview (>= 7.0.0)
       activesupport (>= 7.0.0)
     jmespath (1.6.2)
-    json (2.16.0)
+    json (2.17.1)
     jwt (3.1.2)
       base64
     kamal (2.9.0)
@@ -552,7 +552,7 @@ GEM
       ostruct
     stimulus-rails (1.3.4)
       railties (>= 6.0.0)
-    stringio (3.1.8)
+    stringio (3.1.9)
     thor (1.4.0)
     thruster (0.1.16)
     thruster (0.1.16-aarch64-linux)

app/models/concerns/attachments.rb

@@ -1,13 +1,10 @@
 module Attachments
   extend ActiveSupport::Concern
 
-  # The variants we use must all declared here so that we can preprocess them.
+  # Variants used by ActionText embeds. Processed immediately on attachment to avoid
+  # read replica issues (lazy variants would attempt writes on read replicas).
   #
-  # If they are not preprocessed, then Rails will attempt to transform the image on-the-fly when
-  # they are first viewed, which may be on the read replica where writing to the database is not
-  # allowed. Chaos will ensue if that happens.
-  #
-  # These variants are patched into ActionText::RichText in config/initializers/action_text.rb
+  # Patched into ActionText::RichText in config/initializers/action_text.rb
   VARIANTS = {
     # vipsthumbnail used to create sized image variants has a intent setting to manage colors during
     # resize. By setting an invalid intent value the gif-incompatible intent filtering is skipped and

app/models/user/avatar.rb

@@ -5,7 +5,7 @@ module User::Avatar
 
   included do
     has_one_attached :avatar do |attachable|
-      attachable.variant :thumb, resize_to_fill: [ 256, 256 ]
+      attachable.variant :thumb, resize_to_fill: [ 256, 256 ], process: :immediately
     end
 
     validate :avatar_content_type_allowed

config/initializers/action_text.rb

@@ -7,7 +7,7 @@ module RichText
         # This overrides the default :embeds association!
         has_many_attached :embeds do |attachable|
           ::Attachments::VARIANTS.each do |variant_name, variant_options|
-            attachable.variant variant_name, variant_options.merge(preprocessed: true)
+            attachable.variant variant_name, **variant_options, process: :immediately
           end
         end
       end

test/controllers/users/avatars_controller_test.rb

@@ -27,7 +27,9 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "show own image redirects to the blob url" do
-    users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
+    # Create blob separately to ensure file is uploaded before variant processing
+    blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
+    users(:david).avatar.attach(blob)
     assert users(:david).avatar.attached?
 
     get user_avatar_path(users(:david))
@@ -36,7 +38,9 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "show other image redirects to the blob url" do
-    users(:kevin).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
+    # Create blob separately to ensure file is uploaded before variant processing
+    blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
+    users(:kevin).avatar.attach(blob)
     assert users(:kevin).avatar.attached?
 
     get user_avatar_path(users(:kevin))

test/mailers/notification/bundle_mailer_test.rb

@@ -22,11 +22,12 @@ class Notification::BundleMailerTest < ActionMailer::TestCase
   end
 
   test "renders avatar with external image URL when avatar is attached" do
-    @user.avatar.attach(
+    blob = ActiveStorage::Blob.create_and_upload!(
       io: File.open(Rails.root.join("test", "fixtures", "files", "avatar.png")),
       filename: "avatar.png",
       content_type: "image/png"
     )
+    @user.avatar.attach(blob)
 
     create_notification(@user)
 

test/models/comment_test.rb

@@ -4,4 +4,24 @@ class CommentTest < ActiveSupport::TestCase
   setup do
     Current.session = sessions(:david)
   end
+
+  test "rich text embed variants are processed immediately on attachment" do
+    comment = cards(:logo).comments.create!(body: "Check this out")
+    comment.body.body.attachables # force load
+
+    blob = ActiveStorage::Blob.create_and_upload! \
+      io: File.open(file_fixture("moon.jpg")),
+      filename: "moon.jpg",
+      content_type: "image/jpeg"
+
+    comment.body.body = ActionText::Content.new(comment.body.body.to_html).append_attachables(blob)
+    comment.save!
+
+    embed = comment.body.embeds.sole
+
+    Attachments::VARIANTS.each_key do |variant_name|
+      variant = embed.variant(variant_name)
+      assert variant.processed?, "Expected #{variant_name} variant to be processed immediately"
+    end
+  end
 end

test/models/user/avatar_test.rb

@@ -2,7 +2,8 @@
 
 class User::AvatarTest < ActiveSupport::TestCase
   test "avatar_thumbnail returns variant for variable images" do
-    users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
+    blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
+    users(:david).avatar.attach(blob)
 
     assert users(:david).avatar.variable?
     assert_equal users(:david).avatar.variant(:thumb).blob, users(:david).avatar_thumbnail.blob
@@ -16,7 +17,8 @@ class User::AvatarTest < ActiveSupport::TestCase
   end
 
   test "allows valid image content types" do
-    users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg")
+    blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg", content_type: "image/jpeg")
+    users(:david).avatar.attach(blob)
 
     assert users(:david).valid?
   end
@@ -27,4 +29,19 @@ class User::AvatarTest < ActiveSupport::TestCase
     assert_not users(:david).valid?
     assert_includes users(:david).errors[:avatar], "must be a JPEG, PNG, GIF, or WebP image"
   end
+
+  test "thumb variant is processed immediately on attachment" do
+    # Create blob separately to ensure file is uploaded before variant processing.
+    # This is necessary because Rails' after_commit callback order differs between
+    # SQLite and MySQL - in MySQL, the attachment's after_create_commit (which triggers
+    # variant processing) can fire before the model's after_commit (which uploads the file).
+    blob = ActiveStorage::Blob.create_and_upload!(
+      io: File.open(file_fixture("avatar.png")),
+      filename: "avatar.png",
+      content_type: "image/png"
+    )
+    users(:david).avatar.attach(blob)
+
+    assert users(:david).avatar.variant(:thumb).processed?
+  end
 end