diff --git a/config/mail_room.yml b/config/mail_room.yml index d93795bbd94..febd5f7c1f1 100644 --- a/config/mail_room.yml +++ b/config/mail_room.yml @@ -3,7 +3,7 @@ require_relative "lib/gitlab/mail_room" unless defined?(Gitlab::MailRoom) config = Gitlab::MailRoom.config - if Gitlab::Mailroom.enabled? + if Gitlab::MailRoom.enabled? %> - :host: <%= config[:host].to_json %> diff --git a/lib/gitlab/mail_room.rb b/lib/gitlab/mail_room.rb index 745cc79a10e..781c89579b6 100644 --- a/lib/gitlab/mail_room.rb +++ b/lib/gitlab/mail_room.rb @@ -1,12 +1,11 @@ require 'yaml' require 'json' -require_relative 'lib/gitlab/redis' unless defined?(Gitlab::Redis) +require_relative 'redis' unless defined?(Gitlab::Redis) module Gitlab module MailRoom class << self - def enabled? config[:enabled] && config[:address] end @@ -18,7 +17,7 @@ module Gitlab private def fetch_config - return nil unless File.exists?(config_file) + return {} unless File.exist?(config_file) rails_env = ENV['RAILS_ENV'] || ENV['RACK_ENV'] || 'development' all_config = YAML.load_file(config_file)[rails_env].deep_symbolize_keys @@ -33,11 +32,12 @@ module Gitlab if config[:enabled] && config[:address] config[:redis_url] = Gitlab::Redis.new(rails_env).url end + + config end def config_file - file = ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] || 'config/gitlab.yml' - File.expand_path("../../../#{file}", __FILE__) + ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] || File.expand_path('../../../config/gitlab.yml', __FILE__) end end end diff --git a/lib/gitlab/redis.rb b/lib/gitlab/redis.rb index 8c277d97f1a..9c4b01bfe1a 100644 --- a/lib/gitlab/redis.rb +++ b/lib/gitlab/redis.rb @@ -19,7 +19,7 @@ module Gitlab class << self def params - @params || PARAMS_MUTEX.synchronize { @params = new.params } + PARAMS_MUTEX.synchronize { new.params } end # @deprecated Use .params instead to get sentinel support @@ -38,7 +38,7 @@ module Gitlab end def initialize(rails_env=nil) - @rails_env = rails_env || Rails.env + @rails_env = rails_env || ::Rails.env end def params @@ -55,7 +55,7 @@ module Gitlab # Redis::Store does not handle Unix sockets well, so let's do it for them config[:path] = redis_uri.path else - redis_hash = ::Redis::Store::Factory.extract_host_options_from_uri(redis_uri) + redis_hash = ::Redis::Store::Factory.extract_host_options_from_uri(config[:url]) config.merge!(redis_hash) end @@ -74,7 +74,8 @@ module Gitlab end def fetch_config - File.exists?(config_file) ? YAML.load_file(config_file)[@rails_env] : false + file = config_file + File.exist?(file) ? YAML.load_file(file)[@rails_env] : false end def config_file diff --git a/spec/lib/gitlab/redis_config_spec.rb b/spec/lib/gitlab/redis_spec.rb similarity index 62% rename from spec/lib/gitlab/redis_config_spec.rb rename to spec/lib/gitlab/redis_spec.rb index cb98dd6d994..04257f89627 100644 --- a/spec/lib/gitlab/redis_config_spec.rb +++ b/spec/lib/gitlab/redis_spec.rb @@ -1,18 +1,18 @@ require 'spec_helper' -describe Gitlab::RedisConfig do - let(:redis_config) { Rails.root.join('config', 'resque.yml') } +describe Gitlab::Redis do + let(:redis_config) { Rails.root.join('config', 'resque.yml').to_s } describe '.params' do subject { described_class.params } context 'when url contains unix socket reference' do - let(:config_old) { Rails.root.join('spec/fixtures/config/redis_old_format_socket.yml') } - let(:config_new) { Rails.root.join('spec/fixtures/config/redis_new_format_socket.yml') } + let(:config_old) { Rails.root.join('spec/fixtures/config/redis_old_format_socket.yml').to_s } + let(:config_new) { Rails.root.join('spec/fixtures/config/redis_new_format_socket.yml').to_s } context 'with old format' do it 'returns path key instead' do - allow(Gitlab::RedisConfig).to receive(:config_file) { config_old } + expect_any_instance_of(described_class).to receive(:config_file) { config_old } is_expected.to include(path: '/path/to/redis.sock') is_expected.not_to have_key(:url) @@ -21,7 +21,7 @@ describe Gitlab::RedisConfig do context 'with new format' do it 'returns path key instead' do - allow(Gitlab::RedisConfig).to receive(:config_file) { config_new } + expect_any_instance_of(described_class).to receive(:config_file) { config_new } is_expected.to include(path: '/path/to/redis.sock') is_expected.not_to have_key(:url) @@ -35,7 +35,7 @@ describe Gitlab::RedisConfig do context 'with old format' do it 'returns hash with host, port, db, and password' do - allow(Gitlab::RedisConfig).to receive(:config_file) { config_old } + expect_any_instance_of(described_class).to receive(:config_file) { config_old } is_expected.to include(host: 'localhost', password: 'mypassword', port: 6379, db: 99) is_expected.not_to have_key(:url) @@ -44,7 +44,7 @@ describe Gitlab::RedisConfig do context 'with new format' do it 'returns hash with host, port, db, and password' do - allow(Gitlab::RedisConfig).to receive(:config_file) { config_new } + expect_any_instance_of(described_class).to receive(:config_file) { config_new } is_expected.to include(host: 'localhost', password: 'mypassword', port: 6379, db: 99) is_expected.not_to have_key(:url) @@ -53,29 +53,25 @@ describe Gitlab::RedisConfig do end end - describe '.raw_params' do - subject { described_class.send(:raw_params) } - + describe '#raw_config_hash' do it 'returns default redis url when no config file is present' do - expect(Gitlab::RedisConfig).to receive(:fetch_config) { false } + expect(subject).to receive(:fetch_config) { false } - is_expected.to eq(url: Gitlab::RedisConfig::DEFAULT_REDIS_URL) + expect(subject.send(:raw_config_hash)).to eq(url: Gitlab::Redis::DEFAULT_REDIS_URL) end it 'returns old-style single url config in a hash' do - expect(Gitlab::RedisConfig).to receive(:fetch_config) { 'redis://myredis:6379' } - is_expected.to eq(url: 'redis://myredis:6379') + expect(subject).to receive(:fetch_config) { 'redis://myredis:6379' } + expect(subject.send(:raw_config_hash)).to eq(url: 'redis://myredis:6379') end end - describe '.fetch_config' do - subject { described_class.send(:fetch_config) } - + describe '#fetch_config' do it 'returns false when no config file is present' do - allow(File).to receive(:exists?).with(redis_config) { false } + allow(File).to receive(:exist?).with(redis_config) { false } - is_expected.to be_falsey + expect(subject.send(:fetch_config)).to be_falsey end end end