Merge pull request #3512 from jonasheinrich/fix/take_cache_store_if_present

Don't change cache store if already present

lib/open_project/configuration.rb

@@ -48,7 +48,7 @@ module OpenProject
       'scm_subversion_command'  => nil,
       'disable_browser_cache'   => true,
       # default cache_store is :file_store in production and :memory_store in development
-      'rails_cache_store'       => :file_store,
+      'rails_cache_store'       => nil,
       'cache_expires_in_seconds' => nil,
       'cache_namespace' => nil,
       # use dalli defaults for memcache
@@ -189,15 +189,16 @@ module OpenProject
       end
 
       def configure_cache(application_config)
-        return unless @config['rails_cache_store']
+        return unless override_cache_config? application_config
 
         # rails defaults to :file_store, use :dalli when :memcaches is configured in configuration.yml
-        cache_store = @config['rails_cache_store'].to_sym
+        cache_store = @config['rails_cache_store'].try(:to_sym)
         if cache_store == :memcache
           cache_config = [:dalli_store]
           cache_config << @config['cache_memcache_server'] \
             if @config['cache_memcache_server']
-        elsif cache_store == :file_store
+        # default to :file_store
+        elsif cache_store.nil?
           cache_config = [:file_store, Rails.root.join('tmp/cache')]
         else
           cache_config = [cache_store]
@@ -207,6 +208,13 @@ module OpenProject
         application_config.cache_store = cache_config
       end
 
+      def override_cache_config?(application_config)
+        # override if cache store is not set
+        # or there is something to overwrite it
+        application_config.cache_store.nil? \
+          || @config['rails_cache_store'].present?
+      end
+
       private
 
       def load_config_from_file(filename, env, config)

spec/lib/open_project/configuration_spec.rb

@@ -213,4 +213,67 @@ describe OpenProject::Configuration do
     end
   end
 
+  describe '.configure_cache' do
+    let(:application_config) { Rails::Application::Configuration.new }
+
+    after do
+      # reload configuration to isolate specs
+      OpenProject::Configuration.load
+    end
+
+    context 'with cache store already set' do
+      before do
+        application_config.cache_store = 'foo'
+      end
+
+      context 'with additional cache store configuration' do
+        before do
+          OpenProject::Configuration['rails_cache_store'] = 'bar'
+        end
+
+        it 'changes the cache store' do
+          OpenProject::Configuration.send(:configure_cache, application_config)
+          expect(application_config.cache_store).to eq([:bar])
+        end
+      end
+
+      context 'without additional cache store configuration' do
+        before do
+          OpenProject::Configuration['rails_cache_store'] = nil
+        end
+
+        it 'does not change the cache store' do
+          OpenProject::Configuration.send(:configure_cache, application_config)
+          expect(application_config.cache_store).to eq('foo')
+        end
+      end
+    end
+
+    context 'without cache store already set' do
+      before do
+        application_config.cache_store = nil
+      end
+
+      context 'with additional cache store configuration' do
+        before do
+          OpenProject::Configuration['rails_cache_store'] = 'bar'
+        end
+
+        it 'changes the cache store' do
+          OpenProject::Configuration.send(:configure_cache, application_config)
+          expect(application_config.cache_store).to eq([:bar])
+        end
+      end
+
+      context 'without additional cache store configuration' do
+        before do
+          OpenProject::Configuration['rails_cache_store'] = nil
+        end
+        it 'defaults the cache store to :file_store' do
+          OpenProject::Configuration.send(:configure_cache, application_config)
+          expect(application_config.cache_store.first).to eq(:file_store)
+        end
+      end
+    end
+  end
 end