Skip to content

Commit cb0e9b9

Browse files
Immediate avatar and embed variants (#2002)
Reverts #2001 Restores #1955
1 parent f8a812f commit cb0e9b9

File tree

10 files changed

+62
-19
lines changed

10 files changed

+62
-19
lines changed

Gemfile.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ GIT
66

77
GIT
88
remote: https://github.com/rails/rails.git
9-
revision: 3c3f6c8a253c8a0f346695374f927c9ab32fbefb
9+
revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0
1010
branch: main
1111
specs:
1212
actioncable (8.2.0.alpha)

Gemfile.saas.lock

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ GIT
7575

7676
GIT
7777
remote: https://github.com/rails/rails.git
78-
revision: 690ec8898318b8f50714e86676353ebe1551261e
78+
revision: 3d105fc346fbb3121bbcf6340f2b19104bf326f0
7979
branch: main
8080
specs:
8181
actioncable (8.2.0.alpha)
@@ -292,7 +292,7 @@ GEM
292292
actionview (>= 7.0.0)
293293
activesupport (>= 7.0.0)
294294
jmespath (1.6.2)
295-
json (2.16.0)
295+
json (2.17.1)
296296
jwt (3.1.2)
297297
base64
298298
kamal (2.9.0)
@@ -552,7 +552,7 @@ GEM
552552
ostruct
553553
stimulus-rails (1.3.4)
554554
railties (>= 6.0.0)
555-
stringio (3.1.8)
555+
stringio (3.1.9)
556556
thor (1.4.0)
557557
thruster (0.1.16)
558558
thruster (0.1.16-aarch64-linux)

app/models/concerns/attachments.rb

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,10 @@
11
module Attachments
22
extend ActiveSupport::Concern
33

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

app/models/user/avatar.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ module User::Avatar
55

66
included do
77
has_one_attached :avatar do |attachable|
8-
attachable.variant :thumb, resize_to_fill: [ 256, 256 ]
8+
attachable.variant :thumb, resize_to_fill: [ 256, 256 ], process: :immediately
99
end
1010

1111
validate :avatar_content_type_allowed

config/initializers/action_text.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ module RichText
77
# This overrides the default :embeds association!
88
has_many_attached :embeds do |attachable|
99
::Attachments::VARIANTS.each do |variant_name, variant_options|
10-
attachable.variant variant_name, variant_options.merge(preprocessed: true)
10+
attachable.variant variant_name, **variant_options, process: :immediately
1111
end
1212
end
1313
end

test/controllers/users/avatars_controller_test.rb

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest
2727
end
2828

2929
test "show own image redirects to the blob url" do
30-
users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
30+
# Create blob separately to ensure file is uploaded before variant processing
31+
blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
32+
users(:david).avatar.attach(blob)
3133
assert users(:david).avatar.attached?
3234

3335
get user_avatar_path(users(:david))
@@ -36,7 +38,9 @@ class Users::AvatarsControllerTest < ActionDispatch::IntegrationTest
3638
end
3739

3840
test "show other image redirects to the blob url" do
39-
users(:kevin).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
41+
# Create blob separately to ensure file is uploaded before variant processing
42+
blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
43+
users(:kevin).avatar.attach(blob)
4044
assert users(:kevin).avatar.attached?
4145

4246
get user_avatar_path(users(:kevin))

test/controllers/users_controller_test.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,10 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
7979
test "update with valid avatar" do
8080
sign_in_as :kevin
8181

82-
png_file = fixture_file_upload("avatar.png", "image/png")
82+
# Create blob separately to ensure file is uploaded before variant processing
83+
blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("avatar.png")), filename: "avatar.png", content_type: "image/png")
8384

84-
put user_path(users(:kevin)), params: { user: { avatar: png_file } }
85+
put user_path(users(:kevin)), params: { user: { avatar: blob.signed_id } }
8586
assert_redirected_to user_path(users(:kevin))
8687
assert users(:kevin).reload.avatar.attached?
8788
assert_equal "image/png", users(:kevin).avatar.content_type

test/mailers/notification/bundle_mailer_test.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,12 @@ class Notification::BundleMailerTest < ActionMailer::TestCase
2222
end
2323

2424
test "renders avatar with external image URL when avatar is attached" do
25-
@user.avatar.attach(
25+
blob = ActiveStorage::Blob.create_and_upload!(
2626
io: File.open(Rails.root.join("test", "fixtures", "files", "avatar.png")),
2727
filename: "avatar.png",
2828
content_type: "image/png"
2929
)
30+
@user.avatar.attach(blob)
3031

3132
create_notification(@user)
3233

test/models/comment_test.rb

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,24 @@ class CommentTest < ActiveSupport::TestCase
44
setup do
55
Current.session = sessions(:david)
66
end
7+
8+
test "rich text embed variants are processed immediately on attachment" do
9+
comment = cards(:logo).comments.create!(body: "Check this out")
10+
comment.body.body.attachables # force load
11+
12+
blob = ActiveStorage::Blob.create_and_upload! \
13+
io: File.open(file_fixture("moon.jpg")),
14+
filename: "moon.jpg",
15+
content_type: "image/jpeg"
16+
17+
comment.body.body = ActionText::Content.new(comment.body.body.to_html).append_attachables(blob)
18+
comment.save!
19+
20+
embed = comment.body.embeds.sole
21+
22+
Attachments::VARIANTS.each_key do |variant_name|
23+
variant = embed.variant(variant_name)
24+
assert variant.processed?, "Expected #{variant_name} variant to be processed immediately"
25+
end
26+
end
727
end

test/models/user/avatar_test.rb

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22

33
class User::AvatarTest < ActiveSupport::TestCase
44
test "avatar_thumbnail returns variant for variable images" do
5-
users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
5+
blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "moon.jpg", content_type: "image/jpeg")
6+
users(:david).avatar.attach(blob)
67

78
assert users(:david).avatar.variable?
89
assert_equal users(:david).avatar.variant(:thumb).blob, users(:david).avatar_thumbnail.blob
@@ -16,7 +17,8 @@ class User::AvatarTest < ActiveSupport::TestCase
1617
end
1718

1819
test "allows valid image content types" do
19-
users(:david).avatar.attach(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg")
20+
blob = ActiveStorage::Blob.create_and_upload!(io: File.open(file_fixture("moon.jpg")), filename: "test.jpg", content_type: "image/jpeg")
21+
users(:david).avatar.attach(blob)
2022

2123
assert users(:david).valid?
2224
end
@@ -27,4 +29,22 @@ class User::AvatarTest < ActiveSupport::TestCase
2729
assert_not users(:david).valid?
2830
assert_includes users(:david).errors[:avatar], "must be a JPEG, PNG, GIF, or WebP image"
2931
end
32+
33+
test "thumb variant is processed immediately on attachment" do
34+
# Create blob separately to ensure file is uploaded before variant processing.
35+
#
36+
# Root cause: When ActiveStorage::Record uses `connects_to` for read replica support
37+
# (as in SAAS mode), it creates a separate connection pool from application models.
38+
# Since after_commit callbacks are tracked per connection pool, the callback order
39+
# between User's pool (upload) and Attachment's pool (create_variants) isn't guaranteed.
40+
# In MySQL/SAAS mode, the Attachment callback fires before the file is uploaded.
41+
blob = ActiveStorage::Blob.create_and_upload!(
42+
io: File.open(file_fixture("avatar.png")),
43+
filename: "avatar.png",
44+
content_type: "image/png"
45+
)
46+
users(:david).avatar.attach(blob)
47+
48+
assert users(:david).avatar.variant(:thumb).processed?
49+
end
3050
end

0 commit comments

Comments
 (0)