diff --git a/Gemfile.lock b/Gemfile.lock index 43d4e718b..2f87d5cff 100644 --- a/Gemfile.lock +++ b/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) diff --git a/Gemfile.saas.lock b/Gemfile.saas.lock index 0c8880b08..18f80a22f 100644 --- a/Gemfile.saas.lock +++ b/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) diff --git a/app/models/concerns/attachments.rb b/app/models/concerns/attachments.rb index 1d1c2c4d6..bcc7303ae 100644 --- a/app/models/concerns/attachments.rb +++ b/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 diff --git a/app/models/user/avatar.rb b/app/models/user/avatar.rb index a4b2ae631..c635edddb 100644 --- a/app/models/user/avatar.rb +++ b/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 diff --git a/config/initializers/action_text.rb b/config/initializers/action_text.rb index b2724b1c7..eee87de0d 100644 --- a/config/initializers/action_text.rb +++ b/config/initializers/action_text.rb @@ -7,7 +7,7 @@ module ActionText # 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 diff --git a/test/controllers/users/avatars_controller_test.rb b/test/controllers/users/avatars_controller_test.rb index 3ded884b8..be4532448 100644 --- a/test/controllers/users/avatars_controller_test.rb +++ b/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)) diff --git a/test/mailers/notification/bundle_mailer_test.rb b/test/mailers/notification/bundle_mailer_test.rb index fbc4274c9..63647e68a 100644 --- a/test/mailers/notification/bundle_mailer_test.rb +++ b/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) diff --git a/test/models/comment_test.rb b/test/models/comment_test.rb index c519f8fbd..13a5e36c3 100644 --- a/test/models/comment_test.rb +++ b/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 diff --git a/test/models/user/avatar_test.rb b/test/models/user/avatar_test.rb index 96ea5bac7..58b78969c 100644 --- a/test/models/user/avatar_test.rb +++ b/test/models/user/avatar_test.rb @@ -2,7 +2,8 @@ require "test_helper" 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,22 @@ 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. + # + # Root cause: When ActiveStorage::Record uses `connects_to` for read replica support + # (as in SAAS mode), it creates a separate connection pool from application models. + # Since after_commit callbacks are tracked per connection pool, the callback order + # between User's pool (upload) and Attachment's pool (create_variants) isn't guaranteed. + # In MySQL/SAAS mode, the Attachment callback fires before the file is uploaded. + 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