Make Active Storage services aware of configuration names

activestorage/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Make services aware of configuration names.
+
+    *Gannon McGibbon*
+
 *   The `Content-Type` header is set on image variants when they're uploaded to third-party storage services.
 
     *Kyle Ribordy*

activestorage/app/models/active_storage/blob.rb

@@ -39,7 +39,7 @@ class ActiveStorage::Blob < ActiveRecord::Base
   scope :unattached, -> { left_joins(:attachments).where(ActiveStorage::Attachment.table_name => { blob_id: nil }) }
 
   after_initialize do
-    self.service_name ||= Rails.configuration.active_storage.service
+    self.service_name ||= self.class.service.name
   end
 
   before_destroy(prepend: true) do

activestorage/db/update_migrate/20190112182829_add_service_name_to_active_storage_blobs.rb

@@ -3,7 +3,7 @@ class AddServiceNameToActiveStorageBlobs < ActiveRecord::Migration[6.0]
     unless column_exists?(:active_storage_blobs, :service_name)
       add_column :active_storage_blobs, :service_name, :string
 
-      if configured_service = Rails.configuration.active_storage.service
+      if configured_service = ActiveStorage::Blob.service.name
         ActiveStorage::Blob.unscoped.update_all(service_name: configured_service)
       end
 

activestorage/lib/active_storage/attached/changes/create_one.rb

@@ -61,8 +61,12 @@ module ActiveStorage
             service_name: attachment_service_name
           )
         when Hash
-          args = attachable.reverse_merge(record: record, service_name: attachment_service_name)
-          ActiveStorage::Blob.build_after_unfurling(**args)
+          ActiveStorage::Blob.build_after_unfurling(
+            **attachable.reverse_merge(
+              record: record,
+              service_name: attachment_service_name
+            )
+          )
         when String
           ActiveStorage::Blob.find_signed(attachable, record: record)
         else

activestorage/lib/active_storage/service.rb

@@ -40,6 +40,7 @@ module ActiveStorage
   class Service
     extend ActiveSupport::Autoload
     autoload :Configurator
+    attr_accessor :name
 
     class << self
       # Configure an Active Storage service by name from a set of configurations,
@@ -55,8 +56,10 @@ module ActiveStorage
       # Passes the configurator and all of the service's config as keyword args.
       #
       # See MirrorService for an example.
-      def build(configurator:, service: nil, **service_config) #:nodoc:
-        new(**service_config)
+      def build(configurator:, name:, service: nil, **service_config) #:nodoc:
+        new(**service_config).tap do |service_instance|
+          service_instance.name = name
+        end
       end
     end
 

activestorage/lib/active_storage/service/configurator.rb

@@ -14,7 +14,9 @@ module ActiveStorage
 
     def build(service_name)
       config = config_for(service_name.to_sym)
-      resolve(config.fetch(:service)).build(**config, configurator: self)
+      resolve(config.fetch(:service)).build(
+        **config, configurator: self, name: service_name
+      )
     end
 
     private

activestorage/lib/active_storage/service/mirror_service.rb

@@ -17,10 +17,13 @@ module ActiveStorage
       :url_for_direct_upload, :headers_for_direct_upload, :path_for, to: :primary
 
     # Stitch together from named services.
-    def self.build(primary:, mirrors:, configurator:, **options) #:nodoc:
-      new \
+    def self.build(primary:, mirrors:, name:, configurator:, **options) #:nodoc:
+      new(
         primary: configurator.build(primary),
-        mirrors: mirrors.collect { |name| configurator.build name }
+        mirrors: mirrors.collect { |mirror_name| configurator.build mirror_name }
+      ).tap do |service_instance|
+        service_instance.name = name
+      end
     end
 
     def initialize(primary:, mirrors:)

activestorage/test/controllers/direct_uploads_controller_test.rb

@@ -6,14 +6,12 @@ require "database/setup"
 if SERVICE_CONFIGURATIONS[:s3] && SERVICE_CONFIGURATIONS[:s3][:access_key_id].present?
   class ActiveStorage::S3DirectUploadsControllerTest < ActionDispatch::IntegrationTest
     setup do
-      Rails.configuration.active_storage.service = "s3"
       @old_service = ActiveStorage::Blob.service
       ActiveStorage::Blob.service = ActiveStorage::Service.configure(:s3, SERVICE_CONFIGURATIONS)
     end
 
     teardown do
       ActiveStorage::Blob.service = @old_service
-      Rails.configuration.active_storage.service = "local"
     end
 
     test "creating new direct upload" do
@@ -41,7 +39,6 @@ end
 if SERVICE_CONFIGURATIONS[:gcs]
   class ActiveStorage::GCSDirectUploadsControllerTest < ActionDispatch::IntegrationTest
     setup do
-      Rails.configuration.active_storage.service = "gcs"
       @config = SERVICE_CONFIGURATIONS[:gcs]
 
       @old_service = ActiveStorage::Blob.service
@@ -50,7 +47,6 @@ if SERVICE_CONFIGURATIONS[:gcs]
 
     teardown do
       ActiveStorage::Blob.service = @old_service
-      Rails.configuration.active_storage.service = "local"
     end
 
     test "creating new direct upload" do
@@ -77,7 +73,6 @@ end
 if SERVICE_CONFIGURATIONS[:azure]
   class ActiveStorage::AzureStorageDirectUploadsControllerTest < ActionDispatch::IntegrationTest
     setup do
-      Rails.configuration.active_storage.service = "azure"
       @config = SERVICE_CONFIGURATIONS[:azure]
 
       @old_service = ActiveStorage::Blob.service
@@ -86,7 +81,6 @@ if SERVICE_CONFIGURATIONS[:azure]
 
     teardown do
       ActiveStorage::Blob.service = @old_service
-      Rails.configuration.active_storage.service = "local"
     end
 
     test "creating new direct upload" do

activestorage/test/service/disk_service_test.rb

@@ -3,10 +3,15 @@
 require "service/shared_service_tests"
 
 class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase
-  SERVICE = ActiveStorage::Service::DiskService.new(root: File.join(Dir.tmpdir, "active_storage"))
+  tmp_config = { tmp: { service: "Disk", root: File.join(Dir.tmpdir, "active_storage") } }
+  SERVICE = ActiveStorage::Service.configure(:tmp, tmp_config)
 
   include ActiveStorage::Service::SharedServiceTests
 
+  test "name" do
+    assert_equal :tmp, @service.name
+  end
+
   test "URL generation" do
     original_url_options = Rails.application.routes.default_url_options.dup
     Rails.application.routes.default_url_options.merge!(protocol: "http", host: "test.example.com", port: 3001)

activestorage/test/service/gcs_service_test.rb

@@ -9,6 +9,10 @@ if SERVICE_CONFIGURATIONS[:gcs]
 
     include ActiveStorage::Service::SharedServiceTests
 
+    test "name" do
+      assert_equal :gcs, @service.name
+    end
+
     test "direct upload" do
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"

activestorage/test/service/mirror_service_test.rb

@@ -17,6 +17,10 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase
 
   include ActiveStorage::Service::SharedServiceTests
 
+  test "name" do
+    assert_equal :mirror, @service.name
+  end
+
   test "uploading to all services" do
     key      = SecureRandom.base58(24)
     data     = "Something else entirely!"

activestorage/test/service/s3_service_test.rb

@@ -10,6 +10,10 @@ if SERVICE_CONFIGURATIONS[:s3]
 
     include ActiveStorage::Service::SharedServiceTests
 
+    test "name" do
+      assert_equal :s3, @service.name
+    end
+
     test "direct upload" do
       key      = SecureRandom.base58(24)
       data     = "Something else entirely!"

activestorage/test/test_helper.rb

@@ -35,13 +35,13 @@ end
 
 require "tmpdir"
 
-Rails.configuration.active_storage.service_configurations = SERVICE_CONFIGURATIONS.deep_stringify_keys.merge(
+Rails.configuration.active_storage.service_configurations = SERVICE_CONFIGURATIONS.merge(
   "local" => { "service" => "Disk", "root" => Dir.mktmpdir("active_storage_tests") },
   "disk_mirror_1" => { "service" => "Disk", "root" => Dir.mktmpdir("active_storage_tests_1") },
   "disk_mirror_2" => { "service" => "Disk", "root" => Dir.mktmpdir("active_storage_tests_2") },
   "disk_mirror_3" => { "service" => "Disk", "root" => Dir.mktmpdir("active_storage_tests_3") },
   "mirror" => { "service" => "Mirror", "primary" => "local", "mirrors" => ["disk_mirror_1", "disk_mirror_2", "disk_mirror_3"] }
-)
+).deep_stringify_keys
 
 Rails.configuration.active_storage.service = "local"
 
@@ -101,14 +101,12 @@ class ActiveSupport::TestCase
 
     def with_service(service_name)
       service = ActiveStorage::ServiceRegistry.fetch(service_name)
-      ActiveStorage::Blob.service, previous_service = service, ActiveStorage::Blob.service
-
-      Rails.configuration.active_storage.service, previous_service_name = service_name, Rails.configuration.active_storage.service
+      previous_service = ActiveStorage::Blob.service
+      ActiveStorage::Blob.service =  service
 
       yield
     ensure
       ActiveStorage::Blob.service = previous_service
-      Rails.configuration.active_storage.service = previous_service_name
     end
 end