From e01fdb5f00603d4ac42eef204c7f37ec0f4f88dd Mon Sep 17 00:00:00 2001
From: Sony Mathew <ynos1234@gmail.com>
Date: Fri, 9 Oct 2020 12:37:42 +0530
Subject: [PATCH] chore: refactored redis config (sentinel related) (#1327)

Refactored the redis config module which had redis sentinel related
configs. Also changed the specs accordingly.
---
 lib/redis/config.rb           | 53 +++++++++++++++++------------------
 spec/lib/redis/config_spec.rb |  6 ++--
 2 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/lib/redis/config.rb b/lib/redis/config.rb
index 756b4bbee..888ae89c3 100644
--- a/lib/redis/config.rb
+++ b/lib/redis/config.rb
@@ -4,45 +4,42 @@ module Redis::Config
 
   class << self
     def app
-      return BASE_CONFIG if const_defined? 'BASE_CONFIG'
-
-      reload_config
+      config
     end
 
     def sidekiq
-      config = begin
-        if const_defined? 'BASE_CONFIG'
-          BASE_CONFIG
-        else
-          reload_config
-        end
-      end
-      config.merge(size: SIDEKIQ_SIZE)
+      app.merge(size: SIDEKIQ_SIZE)
     end
 
-    def reload_config
-      config = {
+    def config
+      @config ||= sentinel? ? sentinel_config : base_config
+    end
+
+    def base_config
+      {
         url: ENV.fetch('REDIS_URL', 'redis://127.0.0.1:6379'),
         password: ENV.fetch('REDIS_PASSWORD', nil).presence
       }
+    end
 
-      sentinel_string = ENV.fetch('REDIS_SENTINELS', nil)
-      if sentinel_string.presence
-        # expected format for REDIS_SENTINELS url string is host1:port1, host2:port2
-        sentinels = sentinel_string.split(',').map do |sentinel_url|
-          host, port = sentinel_url.split(':').map(&:strip)
-          { host: host, port: port || DEFAULT_SENTINEL_PORT, password: config[:password] }
-        end
+    def sentinel?
+      ENV.fetch('REDIS_SENTINELS', nil).presence
+    end
 
-        master_name = ENV.fetch('REDIS_SENTINEL_MASTER_NAME', 'mymaster')
-        # over-write redis url as redis://:<your-redis-password>@<master-name>/ when using sentinel
-        # more at https://github.com/redis/redis-rb/issues/531#issuecomment-263501322
-        config[:url] = "redis://#{master_name}"
-        config[:sentinels] = sentinels
+    def sentinel_config
+      redis_sentinels = ENV.fetch('REDIS_SENTINELS', nil)
+
+      # expected format for REDIS_SENTINELS url string is host1:port1, host2:port2
+      sentinels = redis_sentinels.split(',').map do |sentinel_url|
+        host, port = sentinel_url.split(':').map(&:strip)
+        { host: host, port: port.presence || DEFAULT_SENTINEL_PORT, password: base_config[:password] }
       end
-      send(:remove_const, 'BASE_CONFIG') if const_defined? 'BASE_CONFIG'
-      const_set('BASE_CONFIG', config)
-      BASE_CONFIG
+
+      # over-write redis url as redis://:<your-redis-password>@<master-name>/ when using sentinel
+      # more at https://github.com/redis/redis-rb/issues/531#issuecomment-263501322
+      master = "redis://#{ENV.fetch('REDIS_SENTINEL_MASTER_NAME', 'mymaster')}"
+
+      base_config.merge({ url: master, sentinels: sentinels })
     end
   end
 end
diff --git a/spec/lib/redis/config_spec.rb b/spec/lib/redis/config_spec.rb
index d586c1bc3..fa4f4c74c 100644
--- a/spec/lib/redis/config_spec.rb
+++ b/spec/lib/redis/config_spec.rb
@@ -6,11 +6,12 @@ describe ::Redis::Config do
     let(:redis_pasword) { 'some-strong-password' }
 
     before do
+      described_class.instance_variable_set(:@config, nil)
       allow(ENV).to receive(:fetch).with('REDIS_URL', 'redis://127.0.0.1:6379').and_return(redis_url)
       allow(ENV).to receive(:fetch).with('REDIS_PASSWORD', nil).and_return(redis_pasword)
       allow(ENV).to receive(:fetch).with('REDIS_SENTINELS', nil).and_return('')
       allow(ENV).to receive(:fetch).with('REDIS_SENTINEL_MASTER_NAME', 'mymaster').and_return('')
-      described_class.reload_config
+      described_class.config
     end
 
     it 'checks for app redis config' do
@@ -44,11 +45,12 @@ describe ::Redis::Config do
     end
 
     before do
+      described_class.instance_variable_set(:@config, nil)
       allow(ENV).to receive(:fetch).with('REDIS_URL', 'redis://127.0.0.1:6379').and_return(redis_url)
       allow(ENV).to receive(:fetch).with('REDIS_PASSWORD', nil).and_return(redis_pasword)
       allow(ENV).to receive(:fetch).with('REDIS_SENTINELS', nil).and_return(redis_sentinels)
       allow(ENV).to receive(:fetch).with('REDIS_SENTINEL_MASTER_NAME', 'mymaster').and_return(redis_master_name)
-      described_class.reload_config
+      described_class.config
     end
 
     it 'checks for app redis config' do