Merge pull request rails#50107 from chaadow/fix_proxy_controller_untr…

…acked_variants

[ActiveStorage] Fix Non tracked variants not working with `ActiveStorage::Representations::ProxyController`

activestorage/CHANGELOG.md

@@ -1,3 +1,8 @@
+*   Fix `ActiveStorage::Representations::ProxyController` to proxy untracked
+    variants.
+
+    *Chedli Bourguiba*
+
 *   When using the `preprocessed: true` option, avoid enqueuing transform jobs
     for blobs that are not representable.
 

activestorage/app/models/active_storage/blob.rb

@@ -175,6 +175,7 @@ def validate_global_service_configuration # :nodoc:
   include Analyzable
   include Identifiable
   include Representable
+  include Servable
 
   # Returns a signed ID for this blob that's suitable for reference on the client-side without fear of tampering.
   def signed_id(purpose: :blob_id, expires_in: nil, expires_at: nil)
@@ -245,16 +246,6 @@ def service_headers_for_direct_upload
     service.headers_for_direct_upload key, filename: filename, content_type: content_type, content_length: byte_size, checksum: checksum, custom_metadata: custom_metadata
   end
 
-  def content_type_for_serving # :nodoc:
-    forcibly_serve_as_binary? ? ActiveStorage.binary_content_type : content_type
-  end
-
-  def forced_disposition_for_serving # :nodoc:
-    if forcibly_serve_as_binary? || !allowed_inline?
-      :attachment
-    end
-  end
-
 
   # Uploads the +io+ to the service on the +key+ for this blob. Blobs are intended to be immutable, so you shouldn't be
   # using this method after a file has already been uploaded to fit with a blob. If you want to create a derivative blob,
@@ -373,14 +364,6 @@ def extract_content_type(io)
       Marcel::MimeType.for io, name: filename.to_s, declared_type: content_type
     end
 
-    def forcibly_serve_as_binary?
-      ActiveStorage.content_types_to_serve_as_binary.include?(content_type)
-    end
-
-    def allowed_inline?
-      ActiveStorage.content_types_allowed_inline.include?(content_type)
-    end
-
     def web_image?
       ActiveStorage.web_image_content_types.include?(content_type)
     end

activestorage/app/models/active_storage/blob/servable.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+
+module ActiveStorage::Blob::Servable # :nodoc:
+  def content_type_for_serving
+    forcibly_serve_as_binary? ? ActiveStorage.binary_content_type : content_type
+  end
+
+  def forced_disposition_for_serving
+    if forcibly_serve_as_binary? || !allowed_inline?
+      :attachment
+    end
+  end
+
+  private
+    def forcibly_serve_as_binary?
+      ActiveStorage.content_types_to_serve_as_binary.include?(content_type)
+    end
+
+    def allowed_inline?
+      ActiveStorage.content_types_allowed_inline.include?(content_type)
+    end
+end

activestorage/app/models/active_storage/variant.rb

@@ -53,6 +53,8 @@
 # * {ImageProcessing::Vips}[https://github.com/janko/image_processing/blob/master/doc/vips.md#methods]
 # * {ruby-vips reference}[http://www.rubydoc.info/gems/ruby-vips/Vips/Image]
 class ActiveStorage::Variant
+  include ActiveStorage::Blob::Servable
+
   attr_reader :blob, :variation
   delegate :service, to: :blob
   delegate :content_type, to: :variation

activestorage/test/controllers/representations/proxy_controller_test.rb

@@ -6,19 +6,20 @@
 class ActiveStorage::Representations::ProxyControllerWithVariantsTest < ActionDispatch::IntegrationTest
   setup do
     @blob = create_file_blob filename: "racecar.jpg"
+    @transformations = { resize_to_limit: [100, 100] }
   end
 
   test "showing variant attachment" do
     get rails_blob_representation_proxy_url(
       disposition: :attachment,
       filename: @blob.filename,
       signed_blob_id: @blob.signed_id,
-      variation_key: ActiveStorage::Variation.encode(resize_to_limit: [100, 100]))
+      variation_key: ActiveStorage::Variation.encode(@transformations))
 
     assert_response :ok
     assert_match(/^attachment/, response.headers["Content-Disposition"])
 
-    image = read_image(@blob.variant(resize_to_limit: [100, 100]))
+    image = read_image(@blob.variant(@transformations))
     assert_equal 100, image.width
     assert_equal 67, image.height
   end
@@ -27,21 +28,35 @@ class ActiveStorage::Representations::ProxyControllerWithVariantsTest < ActionDi
     get rails_blob_representation_proxy_url(
       filename: @blob.filename,
       signed_blob_id: @blob.signed_id,
-      variation_key: ActiveStorage::Variation.encode(resize_to_limit: [100, 100]))
+      variation_key: ActiveStorage::Variation.encode(@transformations))
 
     assert_response :ok
     assert_match(/^inline/, response.headers["Content-Disposition"])
 
-    image = read_image(@blob.variant(resize_to_limit: [100, 100]))
+    image = read_image(@blob.variant(@transformations))
     assert_equal 100, image.width
     assert_equal 67, image.height
   end
 
+  test "showing untracked variant" do
+    without_variant_tracking do
+      get rails_blob_representation_proxy_url(
+        disposition: :attachment,
+        filename: @blob.filename,
+        signed_blob_id: @blob.signed_id,
+        variation_key: ActiveStorage::Variation.encode(@transformations))
+
+      assert_response :ok
+      assert_match(/^attachment/, response.headers["Content-Disposition"])
+      assert_equal @blob.representation(@transformations).download, response.body
+    end
+  end
+
   test "showing variant with invalid signed blob ID" do
     get rails_blob_representation_proxy_url(
       filename: @blob.filename,
       signed_blob_id: "invalid",
-      variation_key: ActiveStorage::Variation.encode(resize_to_limit: [100, 100]))
+      variation_key: ActiveStorage::Variation.encode(@transformations))
 
     assert_response :not_found
   end