Improve Fog initialization. Closes #2395

- Make Fog configuration order-free - Eager-loading of Fog only occurs in Rails production. Refs #2100 - config.fog_provider is deprecated, because Fog dependencies added to app's Gemfile are to be required by Bundler
carrierwaveuploader · Jun 22, 2019 · ca201ee · ca201ee
1 parent 67800fd
commit ca201ee
Show file tree

Hide file tree

Showing 9 changed files with 57 additions and 30 deletions.
diff --git a/README.md b/README.md
@@ -678,7 +678,6 @@ If you want to use fog you must add in your CarrierWave initializer the
 following lines
 
 ```ruby
-config.fog_provider = 'fog' # 'fog/aws' etc. Defaults to 'fog'
 config.fog_credentials = { ... } # Provider specific credentials
 ```
 
@@ -696,7 +695,6 @@ You can also pass in additional options, as documented fully in lib/carrierwave/
 
 ```ruby
 CarrierWave.configure do |config|
-  config.fog_provider = 'fog/aws'                        # required
   config.fog_credentials = {
     provider:              'AWS',                        # required
     aws_access_key_id:     'xxx',                        # required unless using use_iam_profile
@@ -745,7 +743,6 @@ Using a US-based account:
 
 ```ruby
 CarrierWave.configure do |config|
-  config.fog_provider = "fog/rackspace/storage"   # optional, defaults to "fog"
   config.fog_credentials = {
     provider:           'Rackspace',
     rackspace_username: 'xxxxxx',
@@ -760,7 +757,6 @@ Using a UK-based account:
 
 ```ruby
 CarrierWave.configure do |config|
-  config.fog_provider = "fog/rackspace/storage"   # optional, defaults to "fog"
   config.fog_credentials = {
     provider:           'Rackspace',
     rackspace_username: 'xxxxxx',
@@ -809,7 +805,6 @@ Please read the [fog-google README](https://github.com/fog/fog-google/blob/maste
 
 ```ruby
 CarrierWave.configure do |config|
-  config.fog_provider = 'fog/google'                        # required
   config.fog_credentials = {
     provider:                         'Google',
     google_storage_access_key_id:     'xxxxxx',

diff --git a/lib/carrierwave.rb b/lib/carrierwave.rb
@@ -72,6 +72,10 @@ class Railtie < Rails::Railtie
           require 'carrierwave/orm/activerecord'
         end
       end
+
+      config.before_eager_load do
+        CarrierWave::Storage::Fog.eager_load
+      end
     end
   end
 

diff --git a/lib/carrierwave/storage.rb b/lib/carrierwave/storage.rb
@@ -1,2 +1,3 @@
 require "carrierwave/storage/abstract"
 require "carrierwave/storage/file"
+require "carrierwave/storage/fog"
diff --git a/lib/carrierwave/storage/fog.rb b/lib/carrierwave/storage/fog.rb
@@ -60,6 +60,14 @@ class << self
         def connection_cache
           @connection_cache ||= {}
         end
+
+        def eager_load
+          # see #1198. This will hopefully no longer be necessary in future release of fog
+          fog_credentials = CarrierWave::Uploader::Base.fog_credentials
+          if fog_credentials.present?
+            CarrierWave::Storage::Fog.connection_cache[fog_credentials] ||= ::Fog::Storage.new(fog_credentials)
+          end
+        end
       end
 
       ##

diff --git a/lib/carrierwave/uploader/configuration.rb b/lib/carrierwave/uploader/configuration.rb
@@ -26,7 +26,7 @@ module Configuration
         add_config :downloader
 
         # fog
-        add_config :fog_provider
+        add_deprecated_config :fog_provider
         add_config :fog_attributes
         add_config :fog_credentials
         add_config :fog_directory
@@ -122,31 +122,19 @@ def add_config(name)
           class_eval <<-RUBY, __FILE__, __LINE__ + 1
             @#{name} = nil
 
-            def self.eager_load_fog(fog_credentials)
-              # see #1198. This will hopefully no longer be necessary after fog 2.0
-              require self.fog_provider
-              require 'carrierwave/storage/fog'
-              if fog_credentials.present?
-                CarrierWave::Storage::Fog.connection_cache[fog_credentials] ||= Fog::Storage.new(fog_credentials)
-              end
-            end unless defined? eager_load_fog
-
             def self.#{name}(value=nil)
               @#{name} = value if value
-              eager_load_fog(value) if value && '#{name}' == 'fog_credentials'
               return @#{name} if self.object_id == #{self.object_id} || defined?(@#{name})
               name = superclass.#{name}
               return nil if name.nil? && !instance_variable_defined?(:@#{name})
               @#{name} = name && !name.is_a?(Module) && !name.is_a?(Symbol) && !name.is_a?(Numeric) && !name.is_a?(TrueClass) && !name.is_a?(FalseClass) ? name.dup : name
             end
 
             def self.#{name}=(value)
-              eager_load_fog(value) if '#{name}' == 'fog_credentials' && value.present?
               @#{name} = value
             end
 
             def #{name}=(value)
-              self.class.eager_load_fog(value) if '#{name}' == 'fog_credentials' && value.present?
               @#{name} = value
             end
 
@@ -162,6 +150,26 @@ def #{name}
           RUBY
         end
 
+        def add_deprecated_config(name)
+          class_eval <<-RUBY, __FILE__, __LINE__ + 1
+            def self.#{name}(value=nil)
+              ActiveSupport::Deprecation.warn "##{name} is deprecated and has no effect"
+            end
+
+            def self.#{name}=(value)
+              ActiveSupport::Deprecation.warn "##{name} is deprecated and has no effect"
+            end
+
+            def #{name}=(value)
+              ActiveSupport::Deprecation.warn "##{name} is deprecated and has no effect"
+            end
+
+            def #{name}
+              ActiveSupport::Deprecation.warn "##{name} is deprecated and has no effect"
+            end
+          RUBY
+        end
+
         def configure
           yield self
         end
@@ -179,7 +187,6 @@ def reset_config
             }
             config.storage = :file
             config.cache_storage = :file
-            config.fog_provider = 'fog'
             config.fog_attributes = {}
             config.fog_credentials = {}
             config.fog_public = true

diff --git a/spec/storage/fog_helper.rb b/spec/storage/fog_helper.rb
@@ -6,7 +6,6 @@ def fog_tests(fog_credentials)
         WebMock.disable! unless Fog.mocking?
         CarrierWave.configure do |config|
           config.reset_config
-          config.fog_provider = "fog/#{fog_credentials[:provider].downcase}"
           config.fog_attributes = {}
           config.fog_credentials = fog_credentials
           config.fog_directory = CARRIERWAVE_DIRECTORY

diff --git a/spec/storage/fog_spec.rb b/spec/storage/fog_spec.rb
@@ -3,7 +3,6 @@
 require 'fog/google'
 require 'fog/local'
 require 'fog/rackspace'
-require 'carrierwave/storage/fog'
 
 unless ENV['REMOTE'] == 'true'
   Fog.mock!
@@ -16,6 +15,29 @@
   fog_tests(credential)
 end
 
+describe CarrierWave::Storage::Fog do
+  describe '.eager_load' do
+    after do
+      CarrierWave::Storage::Fog.connection_cache.clear
+      CarrierWave::Uploader::Base.fog_credentials = nil
+    end
+
+    it "caches Fog::Storage instance" do
+      CarrierWave::Uploader::Base.fog_credentials = {
+        provider: 'AWS', aws_access_key_id: 'foo', aws_secret_access_key: 'bar'
+      }
+      expect { CarrierWave::Storage::Fog.eager_load }.
+        to change { CarrierWave::Storage::Fog.connection_cache }
+    end
+
+    it "does nothing when fog_credentials is empty" do
+      CarrierWave::Uploader::Base.fog_credentials = {}
+      expect { CarrierWave::Storage::Fog.eager_load }.
+        not_to change { CarrierWave::Storage::Fog.connection_cache }
+    end
+  end
+end
+
 describe CarrierWave::Storage::Fog::File do
   subject(:file) { CarrierWave::Storage::Fog::File.new(nil, nil, nil) }
 

diff --git a/spec/uploader/configuration_spec.rb b/spec/uploader/configuration_spec.rb
@@ -158,12 +158,4 @@
       end
     end
   end
-
-  describe '.eager_load_fog' do
-    before { uploader_class.fog_provider = 'fog/aws' }
-    it "caches Fog::Storage instance" do
-      expect { uploader_class.eager_load_fog(provider: 'AWS', aws_access_key_id: 'foo', aws_secret_access_key: 'bar') }.
-        to change { CarrierWave::Storage::Fog.connection_cache }
-    end
-  end
 end
diff --git a/spec/uploader/overrides_spec.rb b/spec/uploader/overrides_spec.rb
@@ -5,7 +5,6 @@
     Class.new(CarrierWave::Uploader::Base).tap do |uc|
       uc.configure do |config|
 
-        config.fog_provider = 'fog/aws'
         config.fog_credentials = {
           :provider               => 'AWS',         # required
           :aws_access_key_id      => 'XXXX',        # required