Skip to content

Commit

Permalink
Document ActiveStorage::Current
Browse files Browse the repository at this point in the history
because it does not feel friendly that the name of the class is
mentioned in one of error messages, while its documentation is rendered private.

Also edit the error message in order to navigate those who read
first to the higher-level module, `ActiveStorage::SetCurrent`.

Closes rails#52219
  • Loading branch information
sato11 committed Jul 21, 2024
1 parent e0a9ef1 commit c4b388d
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
# frozen_string_literal: true

# Sets the <tt>ActiveStorage::Current.url_options</tt> attribute, which the disk service uses to generate URLs.
# Sets the ActiveStorage::Current.url_options attribute, which the disk service uses to generate URLs.
# 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 port as the current request.
#
# See also ActiveStorage::Current for details.
module ActiveStorage::SetCurrent
extend ActiveSupport::Concern

Expand Down
35 changes: 34 additions & 1 deletion activestorage/app/models/active_storage/current.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,38 @@
# frozen_string_literal: true

class ActiveStorage::Current < ActiveSupport::CurrentAttributes # :nodoc:
# = Active Storage \Current
#
# Disk service for development and testing is a special case
# as it requires protocol, host and port specified additionally
# in order to generate url, which non-disk services can usually dispense with.
#
# Use ActiveStorage::SetCurrent to effectively test the custom controllers' behavior
# to generate url no matter what type of service underlies the storage.
#
# class UsersController < ApplicationController
# include ActiveStorage::SetCurrent
#
# def show
# @user = User.find(params[:id])
# @url = @user.profile.url
# end
# end
#
# If including the module like above does not help because, for example,
# controllers do not participate in generating url, use Current.url_options= but with caution.
# Knowing the nature of ActiveSupport::CurrentAttributes would be of help.
class ActiveStorage::Current < ActiveSupport::CurrentAttributes
##
# :singleton-method: url_options
#
# Returns the options for the current request.

##
# :singleton-method: url_options=
# :call-seq: url_options=(options)
#
# Sets the options for the current request.
# For the supported options, see ActionDispatch::Routing::UrlFor#url_for.

attribute :url_options
end
2 changes: 1 addition & 1 deletion activestorage/lib/active_storage/service/disk_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def generate_url(key, expires_in:, filename:, content_type:, disposition:)
)

if url_options.blank?
raise ArgumentError, "Cannot generate URL for #{filename} using Disk service, please set ActiveStorage::Current.url_options."
raise ArgumentError, "Cannot generate URL for #{filename} using Disk service. Consider using ActiveStorage::SetCurrent."
end

url_helpers.rails_disk_service_url(verified_key_with_expiration, filename: filename, **url_options)
Expand Down
2 changes: 1 addition & 1 deletion activestorage/test/service/disk_service_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase
@service.url(@key, expires_in: 5.minutes, disposition: :inline, filename: ActiveStorage::Filename.new("avatar.png"), content_type: "image/png")
end

assert_equal("Cannot generate URL for avatar.png using Disk service, please set ActiveStorage::Current.url_options.", error.message)
assert_equal("Cannot generate URL for avatar.png using Disk service. Consider using ActiveStorage::SetCurrent.", error.message)
end

test "URL generation keeps working with ActiveStorage::Current.host set" do
Expand Down

0 comments on commit c4b388d

Please sign in to comment.