Merge pull request #37856 from peterzhu2118/deprecate-service-url

Deprecate `service_url` methods in favour of `url`

activestorage/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   Deprecate `service_url` methods in favour of `url`.
+
+    Deprecate `Variant#service_url` and `Preview#service_url` to instead use
+    `#url` method to be consistent with `Blob`.
+
+    *Peter Zhu*
+
 *   Permanent URLs for public storage blobs.
 
     Services can be configured in `config/storage.yml` with a new key

activestorage/app/controllers/concerns/active_storage/set_current.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 
 # Sets the <tt>ActiveStorage::Current.host</tt> attribute, which the disk service uses to generate URLs.
-# Include this concern in custom controllers that call ActiveStorage::Blob#service_url,
-# ActiveStorage::Variant#service_url, or ActiveStorage::Preview#service_url so the disk service can
+# Include this concern in custom controllers that call ActiveStorage::Blob#url,
+# ActiveStorage::Variant#url, or ActiveStorage::Preview#url so the disk service can
 # generate URLs using the same host, protocol, and base path as the current request.
 module ActiveStorage::SetCurrent
   extend ActiveSupport::Concern

activestorage/app/models/active_storage/blob.rb

@@ -161,9 +161,9 @@ class ActiveStorage::Blob < ActiveRecord::Base
   end
 
   # Returns the URL of the blob on the service. This returns a permanent URL for public files, and returns a
-  # short-lived URL for private files. Private files are for security and not used directly with users, instead,
+  # short-lived URL for private files. Private files are signed, and not for public use. Instead,
   # the URL should only be exposed as a redirect from a stable, possibly authenticated URL. Hiding the
-  # URL behind a redirect also gives you the power to change services without updating all URLs.
+  # URL behind a redirect also allows you to change services without updating all URLs.
   def url(expires_in: ActiveStorage.service_urls_expire_in, disposition: :inline, filename: nil, **options)
     filename = ActiveStorage::Filename.wrap(filename || self.filename)
 

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

@@ -10,7 +10,7 @@ module ActiveStorage::Blob::Representable
   # Returns an ActiveStorage::Variant instance with the set of +transformations+ provided. This is only relevant for image
   # files, and it allows any image to be transformed for size, colors, and the like. Example:
   #
-  #   avatar.variant(resize_to_limit: [100, 100]).processed.service_url
+  #   avatar.variant(resize_to_limit: [100, 100]).processed.url
   #
   # This will create and process a variant of the avatar blob that's constrained to a height and width of 100px.
   # Then it'll upload said variant to the service according to a derivative key of the blob and the transformations.
@@ -43,7 +43,7 @@ module ActiveStorage::Blob::Representable
   # from a non-image blob. Active Storage comes with built-in previewers for videos and PDF documents. The video previewer
   # extracts the first frame from a video and the PDF previewer extracts the first page from a PDF document.
   #
-  #   blob.preview(resize_to_limit: [100, 100]).processed.service_url
+  #   blob.preview(resize_to_limit: [100, 100]).processed.url
   #
   # Avoid processing previews synchronously in views. Instead, link to a controller action that processes them on demand.
   # Active Storage provides one, but you may want to create your own (for example, if you need authentication). Here’s
@@ -69,7 +69,7 @@ module ActiveStorage::Blob::Representable
 
   # Returns an ActiveStorage::Preview for a previewable blob or an ActiveStorage::Variant for a variable image blob.
   #
-  #   blob.representation(resize_to_limit: [100, 100]).processed.service_url
+  #   blob.representation(resize_to_limit: [100, 100]).processed.url
   #
   # Raises ActiveStorage::UnrepresentableError if the receiving blob is neither variable nor previewable. Call
   # ActiveStorage::Blob#representable? to determine whether a blob is representable.

activestorage/app/models/active_storage/preview.rb

@@ -38,7 +38,7 @@ class ActiveStorage::Preview
 
   # Processes the preview if it has not been processed yet. Returns the receiving Preview instance for convenience:
   #
-  #   blob.preview(resize_to_limit: [100, 100]).processed.service_url
+  #   blob.preview(resize_to_limit: [100, 100]).processed.url
   #
   # Processing a preview generates an image from its blob and attaches the preview image to the blob. Because the preview
   # image is stored with the blob, it is only generated once.
@@ -56,15 +56,18 @@ class ActiveStorage::Preview
   # preview has not been processed yet.
   #
   # This method synchronously processes a variant of the preview image, so do not call it in views. Instead, generate
-  # a stable URL that redirects to the short-lived URL returned by this method.
-  def service_url(**options)
+  # a stable URL that redirects to the URL returned by this method.
+  def url(**options)
     if processed?
-      variant.service_url(**options)
+      variant.url(**options)
     else
       raise UnprocessedError
     end
   end
 
+  alias_method :service_url, :url
+  deprecate service_url: :url
+
   private
     def processed?
       image.attached?

activestorage/app/models/active_storage/variant.rb

@@ -36,7 +36,7 @@ require "ostruct"
 # has already been processed and uploaded to the service, and, if so, just return that. Otherwise it will perform
 # the transformations, upload the variant to the service, and return itself again. Example:
 #
-#   avatar.variant(resize_to_limit: [100, 100]).processed.service_url
+#   avatar.variant(resize_to_limit: [100, 100]).processed.url
 #
 # This will create and process a variant of the avatar blob that's constrained to a height and width of 100.
 # Then it'll upload said variant to the service according to a derivative key of the blob and the transformations.
@@ -73,18 +73,18 @@ class ActiveStorage::Variant
     "variants/#{blob.key}/#{Digest::SHA256.hexdigest(variation.key)}"
   end
 
-  # Returns the URL of the variant on the service. This URL is intended to be short-lived for security and not used directly
-  # with users. Instead, the +service_url+ should only be exposed as a redirect from a stable, possibly authenticated URL.
-  # Hiding the +service_url+ behind a redirect also gives you the power to change services without updating all URLs. And
-  # it allows permanent URLs that redirect to the +service_url+ to be cached in the view.
+  # Returns the URL of the blob variant on the service. See {ActiveStorage::Blob#url} for details.
   #
   # Use <tt>url_for(variant)</tt> (or the implied form, like +link_to variant+ or +redirect_to variant+) to get the stable URL
   # for a variant that points to the ActiveStorage::RepresentationsController, which in turn will use this +service_call+ method
   # for its redirection.
-  def service_url(expires_in: ActiveStorage.service_urls_expire_in, disposition: :inline)
+  def url(expires_in: ActiveStorage.service_urls_expire_in, disposition: :inline)
     service.url key, expires_in: expires_in, disposition: disposition, filename: filename, content_type: content_type
   end
 
+  alias_method :service_url, :url
+  deprecate service_url: :url
+
   # Returns the receiving variant. Allows ActiveStorage::Variant and ActiveStorage::Preview instances to be used interchangeably.
   def image
     self

activestorage/lib/active_storage/service.rb

@@ -105,9 +105,10 @@ module ActiveStorage
       raise NotImplementedError
     end
 
-    # Returns a signed, temporary URL for the file at the +key+. The URL will be valid for the amount
-    # of seconds specified in +expires_in+. You must also provide the +disposition+ (+:inline+ or +:attachment+),
-    # +filename+, and +content_type+ that you wish the file to be served with on request.
+    # Returns the URL for the file at the +key+. This returns a permanent URL for public files, and returns a
+    # short-lived URL for private files. You must provide the +disposition+ (+:inline+ or +:attachment+),
+    # +filename+, and +content_type+ that you wish the file to be served with on request. In addition, for
+    # private files, you must also provide the amount of seconds the URL will be valid for, specified in +expires_in+.
     def url(key, **options)
       instrument :url, key: key do |payload|
         generated_url =

activestorage/test/models/variant_test.rb

@@ -16,7 +16,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   test "resized variation of JPEG blob" do
     blob = create_file_blob(filename: "racecar.jpg")
     variant = blob.variant(resize: "100x100").processed
-    assert_match(/racecar\.jpg/, variant.service_url)
+    assert_match(/racecar\.jpg/, variant.url)
 
     image = read_image(variant)
     assert_equal 100, image.width
@@ -26,7 +26,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   test "resized and monochrome variation of JPEG blob" do
     blob = create_file_blob(filename: "racecar.jpg")
     variant = blob.variant(resize: "100x100", monochrome: true).processed
-    assert_match(/racecar\.jpg/, variant.service_url)
+    assert_match(/racecar\.jpg/, variant.url)
 
     image = read_image(variant)
     assert_equal 100, image.width
@@ -48,7 +48,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   test "disabled variation of JPEG blob" do
     blob = create_file_blob(filename: "racecar.jpg")
     variant = blob.variant(resize: "100x100", monochrome: false).processed
-    assert_match(/racecar\.jpg/, variant.service_url)
+    assert_match(/racecar\.jpg/, variant.url)
 
     image = read_image(variant)
     assert_equal 100, image.width
@@ -64,7 +64,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
         monochrome: false
       }).processed
     end
-    assert_match(/racecar\.jpg/, variant.service_url)
+    assert_match(/racecar\.jpg/, variant.url)
 
     image = read_image(variant)
     assert_equal 100, image.width
@@ -81,7 +81,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
         monochrome: false
       }).processed
     end
-    assert_match(/racecar\.jpg/, variant.service_url)
+    assert_match(/racecar\.jpg/, variant.url)
 
     image = read_image(variant)
     assert_equal 100, image.width
@@ -101,7 +101,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
         crop: "100x100+0+0",
       }).processed
     end
-    assert_match(/racecar\.jpg/, variant.service_url)
+    assert_match(/racecar\.jpg/, variant.url)
 
     image = read_image(variant)
     assert_equal 100, image.width
@@ -113,7 +113,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   test "center-weighted crop of JPEG blob using :resize_to_fill" do
     blob = create_file_blob(filename: "racecar.jpg")
     variant = blob.variant(resize_to_fill: [100, 100]).processed
-    assert_match(/racecar\.jpg/, variant.service_url)
+    assert_match(/racecar\.jpg/, variant.url)
 
     image = read_image(variant)
     assert_equal 100, image.width
@@ -123,7 +123,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   test "resized variation of PSD blob" do
     blob = create_file_blob(filename: "icon.psd", content_type: "image/vnd.adobe.photoshop")
     variant = blob.variant(resize: "20x20").processed
-    assert_match(/icon\.png/, variant.service_url)
+    assert_match(/icon\.png/, variant.url)
 
     image = read_image(variant)
     assert_equal "PNG", image.type
@@ -134,7 +134,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   test "resized variation of ICO blob" do
     blob = create_file_blob(filename: "favicon.ico", content_type: "image/vnd.microsoft.icon")
     variant = blob.variant(resize: "20x20").processed
-    assert_match(/icon\.png/, variant.service_url)
+    assert_match(/icon\.png/, variant.url)
 
     image = read_image(variant)
     assert_equal "PNG", image.type
@@ -145,7 +145,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   test "resized variation of TIFF blob" do
     blob = create_file_blob(filename: "racecar.tif")
     variant = blob.variant(resize: "50x50").processed
-    assert_match(/racecar\.png/, variant.service_url)
+    assert_match(/racecar\.png/, variant.url)
 
     image = read_image(variant)
     assert_equal "PNG", image.type
@@ -156,7 +156,7 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
   test "resized variation of BMP blob" do
     blob = create_file_blob(filename: "colors.bmp")
     variant = blob.variant(resize: "15x15").processed
-    assert_match(/colors\.bmp/, variant.service_url)
+    assert_match(/colors\.bmp/, variant.url)
 
     image = read_image(variant)
     assert_equal "BMP", image.type
@@ -178,10 +178,10 @@ class ActiveStorage::VariantTest < ActiveSupport::TestCase
     end
   end
 
-  test "service_url doesn't grow in length despite long variant options" do
+  test "url doesn't grow in length despite long variant options" do
     blob = create_file_blob(filename: "racecar.jpg")
     variant = blob.variant(font: "a" * 10_000).processed
-    assert_operator variant.service_url.length, :<, 730
+    assert_operator variant.url.length, :<, 730
   end
 
   test "works for vips processor" do

guides/source/configuring.md

@@ -873,9 +873,9 @@ text/javascript image/svg+xml application/postscript application/x-shockwave-fla
   ```
 
 * `config.active_storage.service_urls_expire_in` determines the default expiry of URLs generated by:
-  * `ActiveStorage::Blob#service_url`
+  * `ActiveStorage::Blob#url`
   * `ActiveStorage::Blob#service_url_for_direct_upload`
-  * `ActiveStorage::Variant#service_url`
+  * `ActiveStorage::Variant#url`
 
   The default is 5 minutes.