Merge branch 'jv-redis-default-url-legacy' into 'master'

Make clear Redis "default_url" is not an example to be followed

See merge request gitlab-org/gitlab!63208

lib/gitlab/redis/cache.rb

@@ -5,10 +5,10 @@ module Gitlab
     class Cache < ::Gitlab::Redis::Wrapper
       CACHE_NAMESPACE = 'cache:gitlab'
 
-      class << self
-        def default_url
-          'redis://localhost:6380'
-        end
+      private
+
+      def raw_config_hash
+        super || { url: 'redis://localhost:6380' }
       end
     end
   end

lib/gitlab/redis/queues.rb

@@ -9,10 +9,10 @@ module Gitlab
       SIDEKIQ_NAMESPACE = 'resque:gitlab'
       MAILROOM_NAMESPACE = 'mail_room:gitlab'
 
-      class << self
-        def default_url
-          'redis://localhost:6381'
-        end
+      private
+
+      def raw_config_hash
+        super || { url: 'redis://localhost:6381' }
       end
     end
   end

lib/gitlab/redis/shared_state.rb

@@ -8,10 +8,10 @@ module Gitlab
       USER_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:user:gitlab'
       IP_SESSIONS_LOOKUP_NAMESPACE = 'session:lookup:ip:gitlab2'
 
-      class << self
-        def default_url
-          'redis://localhost:6382'
-        end
+      private
+
+      def raw_config_hash
+        super || { url: 'redis://localhost:6382' }
       end
     end
   end

lib/gitlab/redis/wrapper.rb

@@ -51,10 +51,6 @@ module Gitlab
             end
         end
 
-        def default_url
-          raise NotImplementedError
-        end
-
         def config_file_path(filename)
           path = File.join(rails_root, 'config', filename)
           return path if File.file?(path)
@@ -137,8 +133,6 @@ module Gitlab
 
         if config_data
           config_data.is_a?(String) ? { url: config_data } : config_data.deep_symbolize_keys
-        else
-          { url: self.class.default_url }
         end
       end
 

spec/lib/gitlab/redis/cache_spec.rb

@@ -5,7 +5,14 @@ require 'spec_helper'
 RSpec.describe Gitlab::Redis::Cache do
   let(:instance_specific_config_file) { "config/redis.cache.yml" }
   let(:environment_config_file_name) { "GITLAB_REDIS_CACHE_CONFIG_FILE" }
-  let(:class_redis_url) { 'redis://localhost:6380' }
 
   include_examples "redis_shared_examples"
+
+  describe '#raw_config_hash' do
+    it 'has a legacy default URL' do
+      expect(subject).to receive(:fetch_config) { false }
+
+      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6380' )
+    end
+  end
 end

spec/lib/gitlab/redis/queues_spec.rb

@@ -5,7 +5,14 @@ require 'spec_helper'
 RSpec.describe Gitlab::Redis::Queues do
   let(:instance_specific_config_file) { "config/redis.queues.yml" }
   let(:environment_config_file_name) { "GITLAB_REDIS_QUEUES_CONFIG_FILE" }
-  let(:class_redis_url) { 'redis://localhost:6381' }
 
   include_examples "redis_shared_examples"
+
+  describe '#raw_config_hash' do
+    it 'has a legacy default URL' do
+      expect(subject).to receive(:fetch_config) { false }
+
+      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6381' )
+    end
+  end
 end

spec/lib/gitlab/redis/shared_state_spec.rb

@@ -5,7 +5,14 @@ require 'spec_helper'
 RSpec.describe Gitlab::Redis::SharedState do
   let(:instance_specific_config_file) { "config/redis.shared_state.yml" }
   let(:environment_config_file_name) { "GITLAB_REDIS_SHARED_STATE_CONFIG_FILE" }
-  let(:class_redis_url) { 'redis://localhost:6382' }
 
   include_examples "redis_shared_examples"
+
+  describe '#raw_config_hash' do
+    it 'has a legacy default URL' do
+      expect(subject).to receive(:fetch_config) { false }
+
+      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6382' )
+    end
+  end
 end

spec/lib/gitlab/redis/wrapper_spec.rb

@@ -8,10 +8,4 @@ RSpec.describe Gitlab::Redis::Wrapper do
       expect { described_class.instrumentation_class }.to raise_error(NameError)
     end
   end
-
-  describe '.default_url' do
-    it 'is not implemented' do
-      expect { described_class.default_url }.to raise_error(NotImplementedError)
-    end
-  end
 end

spec/support/redis/redis_shared_examples.rb

@@ -92,6 +92,7 @@ RSpec.shared_examples "redis_shared_examples" do
     subject { described_class.new(rails_env).params }
 
     let(:rails_env) { 'development' }
+    let(:config_file_name) { config_old_format_socket }
 
     it 'withstands mutation' do
       params1 = described_class.params
@@ -153,6 +154,8 @@ RSpec.shared_examples "redis_shared_examples" do
   end
 
   describe '.url' do
+    let(:config_file_name) { config_old_format_socket }
+
     it 'withstands mutation' do
       url1 = described_class.url
       url2 = described_class.url
@@ -201,6 +204,8 @@ RSpec.shared_examples "redis_shared_examples" do
   end
 
   describe '.with' do
+    let(:config_file_name) { config_old_format_socket }
+
     before do
       clear_pool
     end
@@ -288,12 +293,6 @@ RSpec.shared_examples "redis_shared_examples" do
   end
 
   describe '#raw_config_hash' do
-    it 'returns default redis url when no config file is present' do
-      expect(subject).to receive(:fetch_config) { false }
-
-      expect(subject.send(:raw_config_hash)).to eq(url: class_redis_url )
-    end
-
     it 'returns old-style single url config in a hash' do
       expect(subject).to receive(:fetch_config) { test_redis_url }
       expect(subject.send(:raw_config_hash)).to eq(url: test_redis_url)