Skip to content

Commit

Permalink
Merge pull request #2433 from tvdeyen/write-thumbs-on-writing-db
Browse files Browse the repository at this point in the history
Fix thumbnail writing for multi-concurrent and multi-db setups
  • Loading branch information
tvdeyen authored Feb 27, 2023
2 parents 0e13375 + d47d2ef commit 11b0505
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 12 deletions.
21 changes: 15 additions & 6 deletions app/models/alchemy/picture/url.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module Alchemy
class Picture < BaseRecord
class Url
attr_reader :variant
attr_reader :variant, :thumb

# @param [Alchemy::PictureVariant]
#
Expand Down Expand Up @@ -31,14 +31,23 @@ def processible_image?

def uid
signature = PictureThumb::Signature.call(variant)
thumb = variant.picture.thumbs.detect { |t| t.signature == signature }
if thumb
uid = thumb.uid
if find_thumb_by(signature)
thumb.uid
else
uid = PictureThumb::Uid.call(signature, variant)
PictureThumb.generator_class.call(variant, signature, uid)
ActiveRecord::Base.connected_to(role: ActiveRecord::Base.writing_role) do
PictureThumb.generator_class.call(variant, signature, uid)
end
uid
end
uid
end

def find_thumb_by(signature)
@thumb = if variant.picture.thumbs.loaded?
variant.picture.thumbs.find { |t| t.signature == signature }
else
variant.picture.thumbs.find_by(signature: signature)
end
end
end
end
Expand Down
11 changes: 5 additions & 6 deletions app/models/alchemy/picture_thumb/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@ def call(variant, signature, uid)

# create the thumb before storing
# to prevent db race conditions
thumb = Alchemy::PictureThumb.create!(
picture: variant.picture,
signature: signature,
uid: uid,
)
@thumb = Alchemy::PictureThumb.create_or_find_by!(signature: signature) do |thumb|
thumb.picture = variant.picture
thumb.uid = uid
end
begin
# process the image
image = variant.image
Expand All @@ -32,7 +31,7 @@ def call(variant, signature, uid)
rescue RuntimeError => e
ErrorTracking.notification_handler.call(e)
# destroy the thumb if processing or storing fails
thumb&.destroy
@thumb&.destroy
end
end

Expand Down
6 changes: 6 additions & 0 deletions spec/models/alchemy/picture/url_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,11 @@
it "returns the url to the thumbnail" do
is_expected.to match(/\/pictures\/\d+\/.+\/image\.png/)
end

it "connects to writing database" do
writing_role = ActiveRecord::Base.writing_role
expect(ActiveRecord::Base).to receive(:connected_to).with(role: writing_role)
subject
end
end
end
14 changes: 14 additions & 0 deletions spec/models/alchemy/picture_thumb/create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@
expect { create }.to change { variant.picture.thumbs.reload.length }.by(1)
end

context "with a thumb already existing" do
let!(:thumb) do
Alchemy::PictureThumb.create!(
picture: picture,
signature: "1234",
uid: "/pictures/#{picture.id}/1234/image.png",
)
end

it "does not create a new thumb" do
expect { create }.to_not change { picture.thumbs.reload.length }
end
end

context "with an invalid picture" do
let(:picture) { FactoryBot.build(:alchemy_picture) }

Expand Down

0 comments on commit 11b0505

Please sign in to comment.