Don't enqueue jobs to process a preview image if no variant requires it

This follows up on https://github.com/rails/rails/pull/51030, that introduces a
behaviour change for previewable attachments that don't specify any variants at all
or no variants that require pre-processing via `TransformJob`.
Before, when no variant required pre-processing, Attachment#transform_variants_later
didn't do anything. Now, a `ActiveStorage::PreviewImageJob` is enqueued with
the attachment's blob and a empty array.
This commit is contained in:
Rosa Gutierrez 2024-03-19 14:23:36 +01:00
parent 3efae44519
commit 6ecb53cf92
6 changed files with 62 additions and 13 deletions

View File

@ -138,7 +138,7 @@ class ActiveStorage::Attachment < ActiveStorage::Record
end
}
if blob.preview_image_needed_before_processing_variants?
if blob.preview_image_needed_before_processing_variants? && preprocessed_variations.any?
blob.create_preview_image_later(preprocessed_variations)
else
preprocessed_variations.each do |transformations|

View File

@ -0,0 +1,23 @@
# frozen_string_literal: true
require "test_helper"
require "database/setup"
class ActiveStorage::PreviewImageJobTest < ActiveJob::TestCase
setup do
@blob = create_file_blob(filename: "report.pdf", content_type: "application/pdf")
@transformation = { resize_to_limit: [ 100, 100 ] }
end
test "creates preview" do
assert_changes -> { @blob.preview_image.attached? }, from: false, to: true do
ActiveStorage::PreviewImageJob.perform_now @blob, [ @transformation ]
end
end
test "enqueues transform variant jobs" do
assert_enqueued_with job: ActiveStorage::TransformJob, args: [ @blob, @transformation ] do
ActiveStorage::PreviewImageJob.perform_now @blob, [ @transformation ]
end
end
end

View File

@ -925,7 +925,7 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase
end
test "transforms variants later conditionally via proc" do
assert_no_enqueued_jobs only: ActiveStorage::TransformJob do
assert_no_enqueued_jobs only: [ ActiveStorage::TransformJob, ActiveStorage::PreviewImageJob ] do
@user.highlights_with_conditional_preprocessed.attach create_file_blob(filename: "racecar.jpg")
end
@ -938,7 +938,7 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase
end
test "transforms variants later conditionally via method" do
assert_no_enqueued_jobs only: ActiveStorage::TransformJob do
assert_no_enqueued_jobs only: [ ActiveStorage::TransformJob, ActiveStorage::PreviewImageJob ] do
@user.highlights_with_conditional_preprocessed.attach create_file_blob(filename: "racecar.jpg")
end
@ -946,14 +946,16 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase
@user.update(name: "transform via method")
assert_enqueued_with job: ActiveStorage::TransformJob, args: [blob, resize_to_limit: [3, 3]] do
@user.highlights_with_conditional_preprocessed.attach blob
assert_no_enqueued_jobs only: ActiveStorage::PreviewImageJob do
@user.highlights_with_conditional_preprocessed.attach blob
end
end
end
test "avoids enqueuing transform later job when blob is not representable" do
test "avoids enqueuing transform later and create preview job job when blob is not representable" do
unrepresentable_blob = create_blob(filename: "hello.txt")
assert_no_enqueued_jobs only: ActiveStorage::TransformJob do
assert_no_enqueued_jobs only: [ ActiveStorage::TransformJob, ActiveStorage::PreviewImageJob ] do
@user.highlights_with_preprocessed.attach unrepresentable_blob
end
end

View File

@ -867,7 +867,7 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase
end
test "transforms variants later conditionally via proc" do
assert_no_enqueued_jobs only: ActiveStorage::TransformJob do
assert_no_enqueued_jobs only: [ ActiveStorage::TransformJob, ActiveStorage::PreviewImageJob ] do
@user.avatar_with_conditional_preprocessed.attach create_file_blob(filename: "racecar.jpg")
end
@ -880,7 +880,7 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase
end
test "transforms variants later conditionally via method" do
assert_no_enqueued_jobs only: ActiveStorage::TransformJob do
assert_no_enqueued_jobs only: [ ActiveStorage::TransformJob, ActiveStorage::PreviewImageJob ] do
@user.avatar_with_conditional_preprocessed.attach create_file_blob(filename: "racecar.jpg")
end
@ -892,11 +892,29 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase
end
end
test "avoids enqueuing transform later job when blob is not representable" do
test "avoids enqueuing transform later job or preview image job when blob is not representable" do
unrepresentable_blob = create_blob(filename: "hello.txt")
assert_no_enqueued_jobs only: ActiveStorage::TransformJob do
assert_no_enqueued_jobs only: [ ActiveStorage::TransformJob, ActiveStorage::PreviewImageJob ] do
@user.avatar_with_preprocessed.attach unrepresentable_blob
end
end
test "avoids enqueuing transform later job or preview later job if there aren't any variants to preprocess" do
blob = create_file_blob(filename: "report.pdf")
assert_no_enqueued_jobs only: [ ActiveStorage::TransformJob, ActiveStorage::PreviewImageJob ] do
@user.resume.attach blob
end
end
test "creates preview later without transforming variants if required and there are variants to preprocess" do
blob = create_file_blob(filename: "report.pdf")
assert_enqueued_with job: ActiveStorage::PreviewImageJob, args: [blob, [resize_to_fill: [400, 400]]] do
assert_no_enqueued_jobs only: ActiveStorage::TransformJob do
@user.resume_with_preprocessing.attach blob
end
end
end
end

View File

@ -39,8 +39,8 @@ class ActiveStorage::ReflectionTest < ActiveSupport::TestCase
test "reflecting on all attachments" do
reflections = User.reflect_on_all_attachments.sort_by(&:name)
assert_equal [ User ], reflections.collect(&:active_record).uniq
assert_equal %i[ avatar avatar_with_conditional_preprocessed avatar_with_preprocessed avatar_with_variants cover_photo highlights highlights_with_conditional_preprocessed highlights_with_preprocessed highlights_with_variants intro_video name_pronunciation_audio vlogs ], reflections.collect(&:name)
assert_equal %i[ has_one_attached has_one_attached has_one_attached has_one_attached has_one_attached has_many_attached has_many_attached has_many_attached has_many_attached has_one_attached has_one_attached has_many_attached ], reflections.collect(&:macro)
assert_equal [ :purge_later, :purge_later, :purge_later, :purge_later, false, :purge_later, :purge_later, :purge_later, :purge_later, :purge_later, :purge_later, false ], reflections.collect { |reflection| reflection.options[:dependent] }
assert_equal %i[ avatar avatar_with_conditional_preprocessed avatar_with_preprocessed avatar_with_variants cover_photo highlights highlights_with_conditional_preprocessed highlights_with_preprocessed highlights_with_variants intro_video name_pronunciation_audio resume resume_with_preprocessing vlogs ], reflections.collect(&:name)
assert_equal %i[ has_one_attached has_one_attached has_one_attached has_one_attached has_one_attached has_many_attached has_many_attached has_many_attached has_many_attached has_one_attached has_one_attached has_one_attached has_one_attached has_many_attached ], reflections.collect(&:macro)
assert_equal [ :purge_later, :purge_later, :purge_later, :purge_later, false, :purge_later, :purge_later, :purge_later, :purge_later, :purge_later, :purge_later, :purge_later, :purge_later, false ], reflections.collect { |reflection| reflection.options[:dependent] }
end
end

View File

@ -160,6 +160,12 @@ class User < ActiveRecord::Base
attachable.variant :method, resize_to_limit: [3, 3],
preprocessed: :should_preprocessed?
end
has_one_attached :resume do |attachable|
attachable.variant :preview, resize_to_fill: [400, 400]
end
has_one_attached :resume_with_preprocessing do |attachable|
attachable.variant :preview, resize_to_fill: [400, 400], preprocessed: true
end
accepts_nested_attributes_for :highlights_attachments, allow_destroy: true