Blob/Variant#url -> #service_url to emphasize this URL isn't to be public

5 files changed, 10 insertions, 6 deletions

diff --git a/app/controllers/active_storage/blobs_controller.rb b/app/controllers/active_storage/blobs_controller.rb
index 5a527d0a33..cf5c008841 100644
--- a/app/controllers/active_storage/blobs_controller.rb
+++ b/app/controllers/active_storage/blobs_controller.rb
@@ -5,7 +5,7 @@
 class ActiveStorage::BlobsController < ActionController::Base
   def show
     if blob = find_signed_blob
-      redirect_to blob.url(disposition: disposition_param)
+      redirect_to blob.service_url(disposition: disposition_param)
     else
       head :not_found
     end
diff --git a/app/controllers/active_storage/direct_uploads_controller.rb b/app/controllers/active_storage/direct_uploads_controller.rb
index 0d1b806f9f..d42c52913a 100644
--- a/app/controllers/active_storage/direct_uploads_controller.rb
+++ b/app/controllers/active_storage/direct_uploads_controller.rb
@@ -4,7 +4,7 @@
 class ActiveStorage::DirectUploadsController < ActionController::Base
   def create
     blob = ActiveStorage::Blob.create_before_direct_upload!(blob_args)
-    render json: { upload_to_url: blob.url_for_direct_upload, signed_blob_id: blob.signed_id }
+    render json: { upload_to_url: blob.service_url_for_direct_upload, signed_blob_id: blob.signed_id }
   end
 
   private
diff --git a/app/controllers/active_storage/variants_controller.rb b/app/controllers/active_storage/variants_controller.rb
index a65d7d7571..5d5dd1a63c 100644
--- a/app/controllers/active_storage/variants_controller.rb
+++ b/app/controllers/active_storage/variants_controller.rb
@@ -3,7 +3,7 @@ require "active_storage/variant"
 class ActiveStorage::VariantsController < ActionController::Base
   def show
     if blob = find_signed_blob
-      redirect_to ActiveStorage::Variant.new(blob, decoded_variation).processed.url(disposition: disposition_param)
+      redirect_to ActiveStorage::Variant.new(blob, decoded_variation).processed.service_url(disposition: disposition_param)
     else
       head :not_found
     end
diff --git a/app/models/active_storage/blob.rb b/app/models/active_storage/blob.rb
index 3340c88d12..9196692530 100644
--- a/app/models/active_storage/blob.rb
+++ b/app/models/active_storage/blob.rb
@@ -56,11 +56,15 @@ class ActiveStorage::Blob < ActiveRecord::Base
   end
 
 
-  def url(expires_in: 5.minutes, disposition: :inline)
+  # Returns the URL of the blob 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 redirec to the `service_url` to be cached in the view.
+  def service_url(expires_in: 5.minutes, disposition: :inline)
     service.url key, expires_in: expires_in, disposition: disposition, filename: filename, content_type: content_type
   end
 
-  def url_for_direct_upload(expires_in: 5.minutes)
+  def service_url_for_direct_upload(expires_in: 5.minutes)
     service.url_for_direct_upload key, expires_in: expires_in, content_type: content_type, content_length: byte_size
   end
 
diff --git a/app/models/active_storage/variant.rb b/app/models/active_storage/variant.rb
index d0fee3c62c..a45356e9ba 100644
--- a/app/models/active_storage/variant.rb
+++ b/app/models/active_storage/variant.rb
@@ -18,7 +18,7 @@ class ActiveStorage::Variant
     "variants/#{blob.key}/#{variation.key}"
   end
 
-  def url(expires_in: 5.minutes, disposition: :inline)
+  def service_url(expires_in: 5.minutes, disposition: :inline)
     service.url key, expires_in: expires_in, disposition: disposition, filename: blob.filename, content_type: blob.content_type
   end