From 709cee9c9a01b1c567b377f961d2e336f9443467 Mon Sep 17 00:00:00 2001 From: George Claghorn Date: Fri, 8 Nov 2019 15:03:42 -0500 Subject: [PATCH] Defer Active Storage service configuration until use --- .../app/models/active_storage/blob.rb | 7 ++-- activestorage/lib/active_storage.rb | 1 - .../lib/active_storage/attached/model.rb | 8 ++--- activestorage/lib/active_storage/engine.rb | 23 +++++++++++-- .../lib/active_storage/service/registry.rb | 32 +++++++++++++++++ .../lib/active_storage/service_registry.rb | 34 ------------------- activestorage/test/test_helper.rb | 3 +- 7 files changed, 61 insertions(+), 47 deletions(-) create mode 100644 activestorage/lib/active_storage/service/registry.rb delete mode 100644 activestorage/lib/active_storage/service_registry.rb diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 8b513a9fc2225..88a9c56bc7c1d 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -32,6 +32,7 @@ class ActiveStorage::Blob < ActiveRecord::Base has_secure_token :key, length: MINIMUM_TOKEN_LENGTH store :metadata, accessors: [ :analyzed, :identified ], coder: ActiveRecord::Coders::JSON + class_attribute :services, default: {} class_attribute :service has_many :attachments @@ -49,8 +50,8 @@ class ActiveStorage::Blob < ActiveRecord::Base validates :service_name, presence: true validate do - if service_name_changed? && service_name - ActiveStorage::ServiceRegistry.fetch(service_name) do + if service_name_changed? && service_name.present? + services.fetch(service_name) do errors.add(:service_name, :invalid) end end @@ -266,7 +267,7 @@ def purge_later # Returns an instance of service, which can be configured globally or per attachment def service - ActiveStorage::ServiceRegistry.fetch(service_name) + services.fetch(service_name) end private diff --git a/activestorage/lib/active_storage.rb b/activestorage/lib/active_storage.rb index 88b78f400eb2b..c35a9920d6e9e 100644 --- a/activestorage/lib/active_storage.rb +++ b/activestorage/lib/active_storage.rb @@ -38,7 +38,6 @@ module ActiveStorage autoload :Attached autoload :Service - autoload :ServiceRegistry autoload :Previewer autoload :Analyzer diff --git a/activestorage/lib/active_storage/attached/model.rb b/activestorage/lib/active_storage/attached/model.rb index f5fed48582b43..02018ae868660 100644 --- a/activestorage/lib/active_storage/attached/model.rb +++ b/activestorage/lib/active_storage/attached/model.rb @@ -165,10 +165,10 @@ def purge_later private def validate_service_configuration(association_name, service) - return unless service - - ServiceRegistry.fetch(service) do - raise ArgumentError, "Cannot configure service :#{service} for #{name}##{association_name}" + if service.present? + ActiveStorage::Blob.services.fetch(service) do + raise ArgumentError, "Cannot configure service :#{service} for #{name}##{association_name}" + end end end end diff --git a/activestorage/lib/active_storage/engine.rb b/activestorage/lib/active_storage/engine.rb index 4a1ce0fc0c449..8cd8f45c98ec3 100644 --- a/activestorage/lib/active_storage/engine.rb +++ b/activestorage/lib/active_storage/engine.rb @@ -14,6 +14,8 @@ require "active_storage/analyzer/image_analyzer" require "active_storage/analyzer/video_analyzer" +require "active_storage/service/registry" + require "active_storage/reflection" module ActiveStorage @@ -101,10 +103,25 @@ class Engine < Rails::Engine # :nodoc: initializer "active_storage.services" do ActiveSupport.on_load(:active_storage_blob) do - if config_choice = Rails.configuration.active_storage.service - ActiveStorage::Blob.service = ActiveStorage::ServiceRegistry.fetch(config_choice) do - raise ArgumentError, "Cannot load `Rails.application.config.active_storage.service`:\n#{config_choice}" + configs = Rails.configuration.active_storage.service_configurations ||= + begin + config_file = Pathname.new(Rails.root.join("config/storage.yml")) + raise("Couldn't find Active Storage configuration in #{config_file}") unless config_file.exist? + + require "yaml" + require "erb" + + YAML.load(ERB.new(config_file.read).result) || {} + rescue Psych::SyntaxError => e + raise "YAML syntax error occurred while parsing #{config_file}. " \ + "Please note that YAML must be consistently indented using spaces. Tabs are not allowed. " \ + "Error: #{e.message}" end + + ActiveStorage::Blob.services = ActiveStorage::Service::Registry.new(configs) + + if config_choice = Rails.configuration.active_storage.service + ActiveStorage::Blob.service = ActiveStorage::Blob.services.fetch(config_choice) end end end diff --git a/activestorage/lib/active_storage/service/registry.rb b/activestorage/lib/active_storage/service/registry.rb new file mode 100644 index 0000000000000..166200503ab0f --- /dev/null +++ b/activestorage/lib/active_storage/service/registry.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module ActiveStorage + class Service::Registry #:nodoc: + def initialize(configurations) + @configurations = configurations.deep_symbolize_keys + @services = {} + end + + def fetch(name) + services.fetch(name.to_sym) do |key| + if configurations.include?(key) + services[key] = configurator.build(key) + else + if block_given? + yield key + else + raise KeyError, "Missing configuration for the #{key} Active Storage service. " \ + "Configurations available for the #{configurations.keys.to_sentence} services." + end + end + end + end + + private + attr_reader :configurations, :services + + def configurator + @configurator ||= ActiveStorage::Service::Configurator.new(configurations) + end + end +end diff --git a/activestorage/lib/active_storage/service_registry.rb b/activestorage/lib/active_storage/service_registry.rb deleted file mode 100644 index b08b6ec19454a..0000000000000 --- a/activestorage/lib/active_storage/service_registry.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -module ActiveStorage - class ServiceRegistry #:nodoc: - class << self - def fetch(service_name, &block) - services.fetch(service_name.to_s, &block) - end - - private - def services - @services ||= configs.keys.each_with_object({}) do |service_name, hash| - hash[service_name] = ActiveStorage::Service.configure(service_name, configs) - end - end - - def configs - Rails.configuration.active_storage.service_configurations ||= begin - config_file = Pathname.new(Rails.root.join("config/storage.yml")) - raise("Couldn't find Active Storage configuration in #{config_file}") unless config_file.exist? - - require "yaml" - require "erb" - - YAML.load(ERB.new(config_file.read).result) || {} - rescue Psych::SyntaxError => e - raise "YAML syntax error occurred while parsing #{config_file}. " \ - "Please note that YAML must be consistently indented using spaces. Tabs are not allowed. " \ - "Error: #{e.message}" - end - end - end - end -end diff --git a/activestorage/test/test_helper.rb b/activestorage/test/test_helper.rb index e5c711baa9526..978410afdafdf 100644 --- a/activestorage/test/test_helper.rb +++ b/activestorage/test/test_helper.rb @@ -103,9 +103,8 @@ def fixture_file_upload(filename) end def with_service(service_name) - service = ActiveStorage::ServiceRegistry.fetch(service_name) previous_service = ActiveStorage::Blob.service - ActiveStorage::Blob.service = service + ActiveStorage::Blob.service = ActiveStorage::Blob.services.fetch(service_name) yield ensure