Merge pull request #50046 from chaadow/refactor_preview_tests_and_code

[ActiveStorage] Prevent `AS::Preview#processed` to generate a variant for an empty transformation

activestorage/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Prevent `ActiveStorage::Blob#preview` to generate a variant if an empty variation is passed.
+    Calls to `#url`, `#key` or `#download` will now use the original preview
+    image instead of generating a variant with the exact same dimensions.
+
+    *Chedli Bourguiba*
+
 *   Process preview image variant when calling `ActiveStorage::Preview#processed`.
     For example, `attached_pdf.preview(:thumb).processed` will now immediately
     generate the full-sized preview image and the `:thumb` variant of it.

activestorage/app/models/active_storage/preview.rb

@@ -47,7 +47,7 @@ class ActiveStorage::Preview
   # image is stored with the blob, it is only generated once.
   def processed
     process unless processed?
-    variant.processed
+    variant.processed if variant?
     self
   end
 
@@ -63,7 +63,7 @@ class ActiveStorage::Preview
   # a stable URL that redirects to the URL returned by this method.
   def url(**options)
     if processed?
-      variant.url(**options)
+      presentation.url(**options)
     else
       raise UnprocessedError
     end
@@ -72,7 +72,7 @@ class ActiveStorage::Preview
   # Returns a combination key of the blob and the variation that together identifies a specific variant.
   def key
     if processed?
-      variant.key
+      presentation.key
     else
       raise UnprocessedError
     end
@@ -85,7 +85,7 @@ class ActiveStorage::Preview
   # if the preview has not been processed yet.
   def download(&block)
     if processed?
-      variant.download(&block)
+      presentation.download(&block)
     else
       raise UnprocessedError
     end
@@ -105,7 +105,15 @@ class ActiveStorage::Preview
     end
 
     def variant
-      image.variant(variation).processed
+      image.variant(variation)
+    end
+
+    def variant?
+      variation.transformations.present?
+    end
+
+    def presentation
+      variant? ? variant.processed : image
     end
 
 

activestorage/test/models/preview_test.rb

@@ -72,6 +72,27 @@ class ActiveStorage::PreviewTest < ActiveSupport::TestCase
     end
   end
 
+  test "image-related methods raise UnprocessedError when preview is not processed" do
+    blob = create_file_blob(filename: "report.pdf", content_type: "application/pdf")
+    preview = blob.preview(resize_to_limit: [640, 280])
+
+    assert_raises(ActiveStorage::Preview::UnprocessedError) { preview.url }
+    assert_raises(ActiveStorage::Preview::UnprocessedError) { preview.key }
+    assert_raises(ActiveStorage::Preview::UnprocessedError) { preview.download }
+  end
+
+  test "previewing with empty transformations does not generate a variant" do
+    blob = create_file_blob(filename: "report.pdf", content_type: "application/pdf")
+    preview = blob.preview({})
+
+    preview.processed
+
+    freeze_time { assert_equal blob.preview_image.url, preview.url }
+    assert_equal blob.preview_image.key, preview.key
+    assert_equal blob.preview_image.download, preview.download
+    assert_empty preview.image.variant_records
+  end
+
   test "preview of PDF is created on the same service" do
     blob = create_file_blob(filename: "report.pdf", content_type: "application/pdf", service_name: "local_public")
     preview = blob.preview(resize_to_limit: [640, 280]).processed