From e5503399c0bb992ec1fd47b4bc371b8aef679e37 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Sat, 8 Jul 2017 16:55:50 -0700 Subject: Configure services that reference other services * Move service configuration from the Engine to Service * Delegate configuration mechanics to internal Service::Configurator * Delegate service building to the concrete Service classes, allowing them to configure composed services. * Implement for the Mirror service. --- lib/active_storage/engine.rb | 37 +++++++++++++--------------- lib/active_storage/service.rb | 16 ++++++------ lib/active_storage/service/configurator.rb | 31 +++++++++++++++++++++++ lib/active_storage/service/mirror_service.rb | 11 +++++++++ lib/active_storage/storage_services.yml | 2 +- test/service/disk_service_test.rb | 2 +- test/service/gcs_service_test.rb | 2 +- test/service/mirror_service_test.rb | 27 +++++++++++--------- test/service/s3_service_test.rb | 2 +- test/test_helper.rb | 2 +- 10 files changed, 89 insertions(+), 43 deletions(-) create mode 100644 lib/active_storage/service/configurator.rb diff --git a/lib/active_storage/engine.rb b/lib/active_storage/engine.rb index d35d3c16db..d066a689eb 100644 --- a/lib/active_storage/engine.rb +++ b/lib/active_storage/engine.rb @@ -25,31 +25,28 @@ module ActiveStorage end config.after_initialize do |app| - config_choice = app.config.active_storage.service - config_file = Pathname.new(Rails.root.join("config/storage_services.yml")) - - if config_choice + if config_choice = app.config.active_storage.service + config_file = Pathname.new(Rails.root.join("config/storage_services.yml")) raise("Couldn't find Active Storage configuration in #{config_file}") unless config_file.exist? - begin - require "yaml" - require "erb" - configs = YAML.load(ERB.new(config_file.read).result) || {} + require "yaml" + require "erb" - if service_configuration = configs[config_choice.to_s].symbolize_keys - service_name = service_configuration.delete(:service) + configs = + begin + 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.service = ActiveStorage::Service.configure(service_name, service_configuration) - else - raise "Couldn't configure Active Storage as #{config_choice} was not found in #{config_file}" + ActiveStorage::Blob.service = + begin + ActiveStorage::Service.configure config_choice, configs + rescue => e + raise e, "Cannot load `Rails.config.active_storage.service`:\n#{e.message}", e.backtrace end - 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}" - rescue => e - raise e, "Cannot load `Rails.config.active_storage.service`:\n#{e.message}", e.backtrace - end end end end diff --git a/lib/active_storage/service.rb b/lib/active_storage/service.rb index f15958fda9..1a6a55739f 100644 --- a/lib/active_storage/service.rb +++ b/lib/active_storage/service.rb @@ -32,13 +32,15 @@ class ActiveStorage::Service class ActiveStorage::IntegrityError < StandardError; end - def self.configure(service, **options) - begin - require "active_storage/service/#{service.to_s.downcase}_service" - ActiveStorage::Service.const_get(:"#{service}Service").new(**options) - rescue LoadError => e - puts "Couldn't configure service: #{service} (#{e.message})" - end + def self.configure(service_name, configurations) + require 'active_storage/service/configurator' + Configurator.new(service_name, configurations).build + end + + # Override in subclasses that stitch together multiple services and hence + # need to do additional lookups from configurations. See MirrorService. + def self.build(config, configurations) #:nodoc: + new(config) end def upload(key, io, checksum: nil) diff --git a/lib/active_storage/service/configurator.rb b/lib/active_storage/service/configurator.rb new file mode 100644 index 0000000000..5054e07ec7 --- /dev/null +++ b/lib/active_storage/service/configurator.rb @@ -0,0 +1,31 @@ +class ActiveStorage::Service::Configurator #:nodoc: + def initialize(service_name, configurations) + @service_name, @configurations = service_name.to_sym, configurations.symbolize_keys + end + + def build + service_class.build(service_config.except(:service), @configurations) + end + + private + def service_class + resolve service_class_name + end + + def service_class_name + service_config.fetch :service do + raise "Missing Active Storage `service: …` configuration for #{service_config.inspect}" + end + end + + def service_config + @configurations.fetch @service_name do + raise "Missing configuration for the #{@service_name.inspect} Active Storage service. Configurations available for #{@configurations.keys.inspect}" + end + end + + def resolve(service_class_name) + require "active_storage/service/#{service_class_name.to_s.downcase}_service" + ActiveStorage::Service.const_get(:"#{service_class_name}Service") + end +end diff --git a/lib/active_storage/service/mirror_service.rb b/lib/active_storage/service/mirror_service.rb index 7ec166aace..eec1f2af65 100644 --- a/lib/active_storage/service/mirror_service.rb +++ b/lib/active_storage/service/mirror_service.rb @@ -5,6 +5,17 @@ class ActiveStorage::Service::MirrorService < ActiveStorage::Service delegate :download, :exist?, :url, to: :primary + # Stitch together from named configuration. + def self.build(mirror_config, all_configurations) #:nodoc: + primary = ActiveStorage::Service.configure(mirror_config.fetch(:primary), all_configurations) + + mirrors = mirror_config.fetch(:mirrors).collect do |service_name| + ActiveStorage::Service.configure(service_name.to_sym, all_configurations) + end + + new primary: primary, mirrors: mirrors + end + def initialize(primary:, mirrors:) @primary, @mirrors = primary, mirrors end diff --git a/lib/active_storage/storage_services.yml b/lib/active_storage/storage_services.yml index d3f001a27b..a93304d88f 100644 --- a/lib/active_storage/storage_services.yml +++ b/lib/active_storage/storage_services.yml @@ -24,4 +24,4 @@ google: mirror: service: Mirror primary: local - secondaries: [ amazon, google ] + mirrors: [ amazon, google ] diff --git a/test/service/disk_service_test.rb b/test/service/disk_service_test.rb index 5dd7cff303..bd2e68277e 100644 --- a/test/service/disk_service_test.rb +++ b/test/service/disk_service_test.rb @@ -2,7 +2,7 @@ require "tmpdir" require "service/shared_service_tests" class ActiveStorage::Service::DiskServiceTest < ActiveSupport::TestCase - SERVICE = ActiveStorage::Service.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage")) + SERVICE = ActiveStorage::Service.configure(:test, test: { service: "Disk", root: File.join(Dir.tmpdir, "active_storage") }) include ActiveStorage::Service::SharedServiceTests end diff --git a/test/service/gcs_service_test.rb b/test/service/gcs_service_test.rb index 42f9cd3061..7d4700498b 100644 --- a/test/service/gcs_service_test.rb +++ b/test/service/gcs_service_test.rb @@ -2,7 +2,7 @@ require "service/shared_service_tests" if SERVICE_CONFIGURATIONS[:gcs] class ActiveStorage::Service::GCSServiceTest < ActiveSupport::TestCase - SERVICE = ActiveStorage::Service.configure(:GCS, SERVICE_CONFIGURATIONS[:gcs]) + SERVICE = ActiveStorage::Service.configure(:gcs, SERVICE_CONFIGURATIONS) include ActiveStorage::Service::SharedServiceTests diff --git a/test/service/mirror_service_test.rb b/test/service/mirror_service_test.rb index 10af41c0a8..6fee3cadd2 100644 --- a/test/service/mirror_service_test.rb +++ b/test/service/mirror_service_test.rb @@ -2,12 +2,17 @@ require "tmpdir" require "service/shared_service_tests" class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase - PRIMARY_DISK_SERVICE = ActiveStorage::Service.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage")) - MIRROR_SERVICES = (1..3).map do |i| - ActiveStorage::Service.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage_mirror_#{i}")) - end + mirror_config = (1..3).map do |i| + [ "mirror_#{i}", + service: "Disk", + root: File.join(Dir.tmpdir, "active_storage_mirror_#{i}") ] + end.to_h + + config = mirror_config.merge \ + mirror: { service: "Mirror", primary: 'primary', mirrors: mirror_config.keys }, + primary: { service: "Disk", root: File.join(Dir.tmpdir, "active_storage") } - SERVICE = ActiveStorage::Service.configure :Mirror, primary: PRIMARY_DISK_SERVICE, mirrors: MIRROR_SERVICES + SERVICE = ActiveStorage::Service.configure :mirror, config include ActiveStorage::Service::SharedServiceTests @@ -16,8 +21,8 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase data = "Something else entirely!" key = upload(data, to: @service) - assert_equal data, PRIMARY_DISK_SERVICE.download(key) - MIRROR_SERVICES.each do |mirror| + assert_equal data, SERVICE.primary.download(key) + SERVICE.mirrors.each do |mirror| assert_equal data, mirror.download(key) end ensure @@ -27,22 +32,22 @@ class ActiveStorage::Service::MirrorServiceTest < ActiveSupport::TestCase test "downloading from primary service" do data = "Something else entirely!" - key = upload(data, to: PRIMARY_DISK_SERVICE) + key = upload(data, to: SERVICE.primary) assert_equal data, @service.download(key) end test "deleting from all services" do @service.delete FIXTURE_KEY - assert_not PRIMARY_DISK_SERVICE.exist?(FIXTURE_KEY) - MIRROR_SERVICES.each do |mirror| + assert_not SERVICE.primary.exist?(FIXTURE_KEY) + SERVICE.mirrors.each do |mirror| assert_not mirror.exist?(FIXTURE_KEY) end end test "URL generation in primary service" do travel_to Time.now do - assert_equal PRIMARY_DISK_SERVICE.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt"), + assert_equal SERVICE.primary.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt"), @service.url(FIXTURE_KEY, expires_in: 2.minutes, disposition: :inline, filename: "test.txt") end end diff --git a/test/service/s3_service_test.rb b/test/service/s3_service_test.rb index 604dfd6c60..e8cc4cb5f4 100644 --- a/test/service/s3_service_test.rb +++ b/test/service/s3_service_test.rb @@ -2,7 +2,7 @@ require "service/shared_service_tests" if SERVICE_CONFIGURATIONS[:s3] class ActiveStorage::Service::S3ServiceTest < ActiveSupport::TestCase - SERVICE = ActiveStorage::Service.configure(:S3, SERVICE_CONFIGURATIONS[:s3]) + SERVICE = ActiveStorage::Service.configure(:s3, SERVICE_CONFIGURATIONS) include ActiveStorage::Service::SharedServiceTests end diff --git a/test/test_helper.rb b/test/test_helper.rb index dcabe33c18..b374a777dd 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -7,7 +7,7 @@ require "byebug" require "active_storage" require "active_storage/service" -ActiveStorage::Blob.service = ActiveStorage::Service.configure(:Disk, root: File.join(Dir.tmpdir, "active_storage")) +ActiveStorage::Blob.service = ActiveStorage::Service.configure(:test, test: { service: 'Disk', root: File.join(Dir.tmpdir, "active_storage") }) require "active_storage/verified_key_with_expiration" ActiveStorage::VerifiedKeyWithExpiration.verifier = ActiveSupport::MessageVerifier.new("Testing") -- cgit v1.2.3