Skip to content

Commit

Permalink
Refactor file upader to process derivatives in separate jobs
Browse files Browse the repository at this point in the history
This avoids an issue where if e.g. an image derivative is too large for
the serverless image handler, only that single derivative will fail
to process.

This issue was happening when uploading large PNG files. Now, thumb,
small, and medium derivatives should still continue to process, while
large sizes may fail.

Shrine: Creating derivatives concurrently
https://shrinerb.com/docs/processing#c-creating-derivatives-concurrently

Large files cause "body size too long" error
aws-solutions/serverless-image-handler#35
  • Loading branch information
dylanfisher committed Jun 16, 2021
1 parent 7433ffe commit 26a5393
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 44 deletions.
11 changes: 7 additions & 4 deletions app/jobs/attachment_derivative_job.rb
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
class AttachmentDerivativeJob < ApplicationJob
# https://shrinerb.com/docs/plugins/backgrounding

def perform(attacher_class, record_class, record_id, name, file_data)
def perform(attacher_class, record_class, record_id, name, file_data, derivative_name)
attacher_class = Object.const_get(attacher_class)
record = Object.const_get(record_class).find(record_id) # if using Active Record

attacher = attacher_class.retrieve(model: record, name: name, file: file_data)
attacher.create_derivatives # calls derivatives processor
attacher.atomic_promote
attacher.create_derivatives(:image, name: derivative_name) # calls derivatives processor
attacher.atomic_persist do |reloaded_attacher|
# make sure we don't override derivatives created in other jobs
attacher.merge_derivatives(reloaded_attacher.derivatives)
end
rescue Shrine::AttachmentChanged, ActiveRecord::RecordNotFound
# attachment has changed or the record has been deleted, nothing to do
attacher.derivatives[derivative_name].delete # delete now orphaned derivative
end
end
4 changes: 3 additions & 1 deletion app/models/concerns/base_media_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ def reprocess_missing!

next unless attacher.stored? && media_item.image? && attacher.derivatives.blank?

AttachmentDerivativeJob.perform_later(attacher.class.name, attacher.record.class.name, attacher.record.id, attacher.name, attacher.file_data)
FileUploader::IMAGE_DERIVATIVES.each_key do |derivative_name|
AttachmentDerivativeJob.perform_later(attacher.class.name, attacher.record.class.name, attacher.record.id, attacher.name, attacher.file_data, derivative_name)
end
end
end

Expand Down
65 changes: 26 additions & 39 deletions app/uploaders/file_uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@ class FileUploader < Shrine
read_timeout: 120,
open_timeout: 120
}
IMAGE_DERIVATIVES = {
thumb: {
resize: { width: 200, height: 200, fit: 'cover' }.reverse_merge(DEFAULT_EDITS)
},
small: {
resize: { width: 600, height: 600, fit: 'inside' }.reverse_merge(DEFAULT_EDITS)
},
medium: {
resize: { width: 1200, height: 1200, fit: 'inside' }.reverse_merge(DEFAULT_EDITS)
},
large: {
resize: { width: 2200, height: 2200, fit: 'inside' }.reverse_merge(DEFAULT_EDITS)
}
}

# Active Record - Overriding callbacks
# https://shrinerb.com/docs/plugins/activerecord#overriding-callbacks
Expand All @@ -31,7 +45,9 @@ def activerecord_after_save
super

if file.image? && derivatives.blank?
AttachmentDerivativeJob.perform_later(self.class.name, self.record.class.name, self.record.id, self.name, self.file_data)
IMAGE_DERIVATIVES.each_key do |derivative_name|
AttachmentDerivativeJob.perform_later(self.class.name, self.record.class.name, self.record.id, self.name, self.file_data, derivative_name)
end
end
end
end
Expand All @@ -45,6 +61,14 @@ def activerecord_after_save
# The derivatives plugin allows storing processed files ("derivatives") alongside the main attached file
# https://shrinerb.com/docs/plugins/derivatives
Attacher.derivatives do |original|
if file.image?
{}
else
process_derivatives(:file, original)
end
end

Attacher.derivatives :image do |original, name:|
def serverless_image_request(edits = {})
request_path = Base64.strict_encode64({
bucket: Shrine.storages[:store].bucket.name,
Expand All @@ -54,44 +78,7 @@ def serverless_image_request(edits = {})
"#{SERVERLESS_IMAGE_HOST}/#{request_path}"
end

if file.image?
process_derivatives(:image, original)
else
process_derivatives(:file, original)
end
end

Attacher.derivatives :image do |original|
{
thumb: Shrine.remote_url( serverless_image_request({
resize: {
width: 200,
height: 200,
fit: 'cover'
}.reverse_merge(DEFAULT_EDITS)
}), **DOWNLOADER_OPTIONS),
small: Shrine.remote_url( serverless_image_request({
resize: {
width: 600,
height: 600,
fit: 'inside'
}.reverse_merge(DEFAULT_EDITS)
}), **DOWNLOADER_OPTIONS),
medium: Shrine.remote_url( serverless_image_request({
resize: {
width: 1200,
height: 1200,
fit: 'inside'
}.reverse_merge(DEFAULT_EDITS)
}), **DOWNLOADER_OPTIONS),
large: Shrine.remote_url( serverless_image_request({
resize: {
width: 2200,
height: 2200,
fit: 'inside'
}.reverse_merge(DEFAULT_EDITS)
}), **DOWNLOADER_OPTIONS)
}
{ name => Shrine.remote_url( serverless_image_request(IMAGE_DERIVATIVES.fetch(name)) ) }
end

Attacher.derivatives :file do |original|
Expand Down

0 comments on commit 26a5393

Please sign in to comment.