Skip to content

Commit aa47bde

Browse files
Fix strict loading for Active Storage previews
`ActiveStorage::Preview#url` delegates to the preview image's variant, which in turn delegates to the variant's blob. Thus when the variant has already been processed and strict loading is enabled, the association chain of `preview_image_attachment` => `blob` => `variant_records` => `image_attachment` => `blob` must be fully pre-loaded; otherwise, `ActiveStorage::Preview#url` will raise an `ActiveRecord::StrictLoadingViolationError`.
1 parent 2ea77d6 commit aa47bde

File tree

4 files changed

+14
-20
lines changed

4 files changed

+14
-20
lines changed

activestorage/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
* Prevent `ActiveRecord::StrictLoadingViolationError` when strict loading is
2+
enabled and the variant of an Active Storage preview has already been
3+
processed (for example, by calling `ActiveStorage::Preview#url`).
4+
5+
*Jonathan Hefner*
6+
17
* Fix `preprocessed: true` option for named variants of previewable files.
28

39
*Nico Wenterodt*

activestorage/app/models/active_storage/blob.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,10 @@ def signed_id_verifier # :nodoc:
134134

135135
def scope_for_strict_loading # :nodoc:
136136
if strict_loading_by_default? && ActiveStorage.track_variants
137-
includes(variant_records: { image_attachment: :blob }, preview_image_attachment: :blob)
137+
includes(
138+
variant_records: { image_attachment: :blob },
139+
preview_image_attachment: { blob: { variant_records: { image_attachment: :blob } } }
140+
)
138141
else
139142
all
140143
end

activestorage/test/controllers/representations/redirect_controller_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ class ActiveStorage::Representations::RedirectControllerWithPreviewsTest < Actio
110110
class ActiveStorage::Representations::RedirectControllerWithPreviewsWithStrictLoadingTest < ActionDispatch::IntegrationTest
111111
setup do
112112
@blob = create_file_blob filename: "report.pdf", content_type: "application/pdf"
113-
@blob.preview(resize_to_limit: [100, 100]).processed
113+
@blob.preview(resize_to_limit: [100, 100]).processed.send(:variant).processed
114114
end
115115

116116
test "showing existing preview record inline" do

activestorage/test/models/blob_test.rb

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -328,28 +328,13 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase
328328
end
329329

330330
test "scope_for_strict_loading adds includes only when track_variants and strict_loading_by_default" do
331-
assert_empty(
332-
ActiveStorage::Blob.scope_for_strict_loading.includes_values,
333-
"Expected ActiveStorage::Blob.scope_for_strict_loading have no includes"
334-
)
331+
assert_empty ActiveStorage::Blob.scope_for_strict_loading.includes_values
335332

336333
with_strict_loading_by_default do
337-
includes_values = ActiveStorage::Blob.scope_for_strict_loading.includes_values
338-
339-
assert(
340-
includes_values.any? { |values| values[:variant_records] == { image_attachment: :blob } },
341-
"Expected ActiveStorage::Blob.scope_for_strict_loading to have variant_records included"
342-
)
343-
assert(
344-
includes_values.any? { |values| values[:preview_image_attachment] == :blob },
345-
"Expected ActiveStorage::Blob.scope_for_strict_loading to have preview_image_attachment included"
346-
)
334+
assert_not_empty ActiveStorage::Blob.scope_for_strict_loading.includes_values
347335

348336
without_variant_tracking do
349-
assert_empty(
350-
ActiveStorage::Blob.scope_for_strict_loading.includes_values,
351-
"Expected ActiveStorage::Blob.scope_for_strict_loading have no includes"
352-
)
337+
assert_empty ActiveStorage::Blob.scope_for_strict_loading.includes_values
353338
end
354339
end
355340
end

0 commit comments

Comments
 (0)