mirrors
/
rails
mirror of https://github.com/rails/rails
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Merge pull request #42981 from ghiculescu/as-eager-more-methods-3 

Active Storage eager loading: support more methods
This commit is contained in:
Jeremy Daer 2022-05-16 12:42:23 -07:00 committed by GitHub
parent 712ecf7749 f24500b3ab
commit fe8d41eda1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 106 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
activestorage/app/models/active_storage/attachment.rb
View File

@ -27,7 +27,7 @@ class ActiveStorage::Attachment < ActiveStorage::Record
after_create_commit :mirror_blob_later, :analyze_blob_later
after_destroy_commit :purge_dependent_blob_later
scope :with_all_variant_records, -> { includes(blob: :variant_records) }
scope :with_all_variant_records, -> { includes(blob: { variant_records: { image_attachment: :blob } }) }
# Synchronously deletes the attachment and {purges the blob}[rdoc-ref:ActiveStorage::Blob#purge].
def purge

2
activestorage/lib/active_storage/attached/model.rb
View File

@ -188,7 +188,7 @@ module ActiveStorage
scope :"with_attached_#{name}", -> {
if ActiveStorage.track_variants
includes("#{name}_attachments": { blob: :variant_records })
includes("#{name}_attachments": { blob: { variant_records: { image_attachment: :blob } } })
else
includes("#{name}_attachments": :blob)
end

132
activestorage/test/models/variant_with_record_test.rb
View File

@ -57,7 +57,7 @@ class ActiveStorage::VariantWithRecordTest < ActiveSupport::TestCase
assert_equal "local_public", variant.image.blob.service_name
end
test "eager loading is supported" do
test "eager loading" do
user = User.create!(name: "Josh")
blob1 = directly_upload_file_blob(filename: "racecar.jpg")
@ -77,14 +77,56 @@ class ActiveStorage::VariantWithRecordTest < ActiveSupport::TestCase
user.reload
assert_no_difference -> { ActiveStorage::VariantRecord.count } do
assert_queries(9) do
# 9 queries:
# attachments (vlogs) x 1
# blob x 2
# variant record x 1 per blob
# attachment x 1 per variant record
# variant record x 1 per variant record attachment
user.vlogs.each do |vlog|
rep = vlog.representation(resize_to_limit: [100, 100])
rep.processed
rep.key
rep.url
end
end
end
user.reload
assert_no_difference -> { ActiveStorage::VariantRecord.count } do
assert_queries(7) do
# 7 queries:
# attachments (vlogs) x 1
# blob x 1
# variant record x 1
# attachment -> blob x 1 per variant record (so 2)
user.vlogs.includes(blob: :variant_records).each do |vlog|
rep = vlog.representation(resize_to_limit: [100, 100])
rep.processed
rep.key
rep.url
end
end
end
user.reload
assert_no_difference -> { ActiveStorage::VariantRecord.count } do
assert_queries(5) do
# 5 queries:
# attachments (vlogs) x 1
# blob x 2
# variant record x 2
user.vlogs.map do |vlog|
vlog.representation(resize_to_limit: [100, 100]).processed
# blobs for the vlogs x 1
# variant records for the blobs x 1
# attachments for the variant records x 1
# blobs for the attachments for the variant records x 1
user.vlogs.includes(blob: { variant_records: { image_attachment: :blob } }).each do |vlog|
rep = vlog.representation(resize_to_limit: [100, 100])
rep.processed
rep.key
rep.url
end
end
end
@ -92,13 +134,18 @@ class ActiveStorage::VariantWithRecordTest < ActiveSupport::TestCase
user.reload
assert_no_difference -> { ActiveStorage::VariantRecord.count } do
assert_queries(3) do
# 3 queries:
assert_queries(5) do
# 5 queries:
# attachments (vlogs) x 1
# blob x 1
# variant record x 1
user.vlogs.includes(blob: :variant_records).map do |vlog|
vlog.representation(resize_to_limit: [100, 100]).processed
# blobs for the vlogs x 1
# variant records for the blobs x 1
# attachments for the variant records x 1
# blobs for the attachments for the variant records x 1
user.vlogs.with_all_variant_records.each do |vlog|
rep = vlog.representation(resize_to_limit: [100, 100])
rep.processed
rep.key
rep.url
end
end
end
@ -106,32 +153,55 @@ class ActiveStorage::VariantWithRecordTest < ActiveSupport::TestCase
user.reload
assert_no_difference -> { ActiveStorage::VariantRecord.count } do
assert_queries(3) do
# 3 queries:
# attachments (vlogs) x 1
# blob x 1
# variant record x 1
user.vlogs.with_all_variant_records.map do |vlog|
vlog.representation(resize_to_limit: [100, 100]).processed
end
end
end
user.reload
assert_no_difference -> { ActiveStorage::VariantRecord.count } do
assert_queries(4) do
# 4 queries:
assert_queries(6) do
# 6 queries:
# user x 1
# attachments (vlogs) x 1
# blob x 1
# variant record x 1
User.where(id: user.id).with_attached_vlogs.map do |u|
# blobs for the vlogs x 1
# variant records for the blobs x 1
# attachments for the variant records x 1
# blobs for the attachments for the variant records x 1
User.where(id: user.id).with_attached_vlogs.each do |u|
u.vlogs.map do |vlog|
vlog.representation(resize_to_limit: [100, 100]).processed
rep = vlog.representation(resize_to_limit: [100, 100])
rep.processed
rep.key
rep.url
end
end
end
end
user.reload
assert_difference -> { ActiveStorage::VariantRecord.count }, +2 do
# More queries here because we are creating a different variant.
# The second time we load this variant, we are back down to just 3 queries.
assert_queries(9, matcher: /SELECT/) do
# 9 queries:
# attachments (vlogs) initial load x 1
# blob x 1 (gets both records)
# variant record x 1 (gets both records)
# 2x get blob, attachment, variant records again, this happens when loading the new blob inside `VariantWithRecord#key`
user.vlogs.with_all_variant_records.each do |vlog|
rep = vlog.representation(resize_to_limit: [200, 200])
rep.processed
rep.key
rep.url
end
end
user.reload
assert_queries(5) do
user.vlogs.with_all_variant_records.each do |vlog|
rep = vlog.representation(resize_to_limit: [200, 200])
rep.processed
rep.key
rep.url
end
end
end
end
end

6
activestorage/test/test_helper.rb
View File

@ -61,16 +61,16 @@ class ActiveSupport::TestCase
ActiveStorage::Current.reset
end
def assert_queries(expected_count, &block)
def assert_queries(expected_count, matcher: nil, &block)
ActiveRecord::Base.connection.materialize_transactions
queries = []
ActiveSupport::Notifications.subscribe("sql.active_record") do |*, payload|
queries << payload[:sql] unless %w[ SCHEMA TRANSACTION ].include?(payload[:name])
queries << payload[:sql] if %w[ SCHEMA TRANSACTION ].exclude?(payload[:name]) && (matcher.nil? || payload[:sql].match(matcher))
end
result = _assert_nothing_raised_or_warn("assert_queries", &block)
assert_equal expected_count, queries.size, "#{queries.size} instead of #{expected_count} queries were executed. #{queries.inspect}"
assert_equal expected_count, queries.size, "#{queries.size} instead of #{expected_count} queries were executed. Queries: #{queries.join("\n\n")}"
result
end