Refactor (ruby) redis configuration (#31694)

app/lib/redis_connection.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class RedisConfiguration
+class RedisConnection
   class << self
     def establish_pool(new_pool_size)
       @pool&.shutdown(&:close)
@@ -22,33 +22,24 @@ class RedisConfiguration
     end
   end
 
+  attr_reader :config
+
+  def initialize
+    @config = REDIS_CONFIGURATION.base
+  end
+
   def connection
-    if namespace?
+    namespace = config[:namespace]
+    if namespace.present?
       Redis::Namespace.new(namespace, redis: raw_connection)
     else
       raw_connection
     end
   end
 
-  def namespace?
-    namespace.present?
-  end
-
-  def namespace
-    ENV.fetch('REDIS_NAMESPACE', nil)
-  end
-
-  def url
-    ENV['REDIS_URL']
-  end
-
-  def redis_driver
-    ENV.fetch('REDIS_DRIVER', 'hiredis') == 'ruby' ? :ruby : :hiredis
-  end
-
   private
 
   def raw_connection
-    Redis.new(url: url, driver: redis_driver)
+    Redis.new(**config)
   end
 end

app/models/concerns/redisable.rb

@@ -2,10 +2,10 @@
 
 module Redisable
   def redis
-    Thread.current[:redis] ||= RedisConfiguration.pool.checkout
+    Thread.current[:redis] ||= RedisConnection.pool.checkout
   end
 
   def with_redis(&block)
-    RedisConfiguration.with(&block)
+    RedisConnection.with(&block)
   end
 end

config/application.rb

@@ -101,7 +101,8 @@ module Mastodon
     end
 
     config.before_configuration do
-      require 'mastodon/redis_config'
+      require 'mastodon/redis_configuration'
+      ::REDIS_CONFIGURATION = Mastodon::RedisConfiguration.new
 
       config.x.use_vips = ENV['MASTODON_USE_LIBVIPS'] == 'true'
 

config/environments/development.rb

@@ -25,7 +25,7 @@ Rails.application.configure do
     config.action_controller.perform_caching = true
     config.action_controller.enable_fragment_cache_logging = true
 
-    config.cache_store = :redis_cache_store, REDIS_CACHE_PARAMS
+    config.cache_store = :redis_cache_store, REDIS_CONFIGURATION.cache
     config.public_file_server.headers = {
       'Cache-Control' => "public, max-age=#{2.days.to_i}",
     }

config/environments/production.rb

@@ -57,7 +57,7 @@ Rails.application.configure do
   config.log_tags = [:request_id]
 
   # Use a different cache store in production.
-  config.cache_store = :redis_cache_store, REDIS_CACHE_PARAMS
+  config.cache_store = :redis_cache_store, REDIS_CONFIGURATION.cache
 
   # Use a real queuing backend for Active Job (and separate queues per environment).
   # config.active_job.queue_adapter     = :resque

config/initializers/sidekiq.rb

@@ -3,7 +3,7 @@
 require_relative '../../lib/mastodon/sidekiq_middleware'
 
 Sidekiq.configure_server do |config|
-  config.redis = REDIS_SIDEKIQ_PARAMS
+  config.redis = REDIS_CONFIGURATION.sidekiq
 
   # This is used in Kubernetes setups, to signal that the Sidekiq process has started and will begin processing jobs
   # This comes from https://github.com/sidekiq/sidekiq/wiki/Kubernetes#sidekiq
@@ -51,7 +51,7 @@ Sidekiq.configure_server do |config|
 end
 
 Sidekiq.configure_client do |config|
-  config.redis = REDIS_SIDEKIQ_PARAMS
+  config.redis = REDIS_CONFIGURATION.sidekiq
 
   config.client_middleware do |chain|
     chain.add SidekiqUniqueJobs::Middleware::Client

config/initializers/stoplight.rb

@@ -3,6 +3,6 @@
 require 'stoplight'
 
 Rails.application.reloader.to_prepare do
-  Stoplight.default_data_store = Stoplight::DataStore::Redis.new(RedisConfiguration.new.connection)
+  Stoplight.default_data_store = Stoplight::DataStore::Redis.new(RedisConnection.new.connection)
   Stoplight.default_notifiers  = [Stoplight::Notifier::Logger.new(Rails.logger)]
 end

db/migrate/20170920032311_fix_reblogs_in_feeds.rb

@@ -2,7 +2,7 @@
 
 class FixReblogsInFeeds < ActiveRecord::Migration[5.1]
   def up
-    redis = RedisConfiguration.pool.checkout
+    redis = RedisConnection.pool.checkout
     fm = FeedManager.instance
 
     # Old scheme:

db/migrate/20200407202420_migrate_unavailable_inboxes.rb

@@ -4,7 +4,7 @@ class MigrateUnavailableInboxes < ActiveRecord::Migration[5.2]
   disable_ddl_transaction!
 
   def up
-    redis = RedisConfiguration.pool.checkout
+    redis = RedisConnection.pool.checkout
     urls = redis.smembers('unavailable_inboxes')
 
     hosts = urls.filter_map do |url|

lib/chewy/strategy/mastodon.rb

@@ -14,7 +14,7 @@ module Chewy
       end
 
       def leave
-        RedisConfiguration.with do |redis|
+        RedisConnection.with do |redis|
           redis.pipelined do |pipeline|
             @stash.each do |type, ids|
               pipeline.sadd("chewy:queue:#{type.name}", ids)

lib/mastodon/cli/base.rb

@@ -40,7 +40,7 @@ module Mastodon
             .dup
             .tap { |config| config['pool'] = options[:concurrency] + 1 }
         )
-        RedisConfiguration.establish_pool(options[:concurrency])
+        RedisConnection.establish_pool(options[:concurrency])
       end
     end
   end

lib/mastodon/cli/progress_helper.rb

@@ -51,7 +51,7 @@ module Mastodon::CLI
               result = ActiveRecord::Base.connection_pool.with_connection do
                 yield(item)
               ensure
-                RedisConfiguration.pool.checkin if Thread.current[:redis]
+                RedisConnection.pool.checkin if Thread.current[:redis]
                 Thread.current[:redis] = nil
               end
 

lib/mastodon/rack_middleware.rb

@@ -19,7 +19,7 @@ class Mastodon::RackMiddleware
   end
 
   def clean_up_redis_socket!
-    RedisConfiguration.pool.checkin if Thread.current[:redis]
+    RedisConnection.pool.checkin if Thread.current[:redis]
     Thread.current[:redis] = nil
   end
 

lib/mastodon/redis_config.rb

@@ -1,53 +0,0 @@
-# frozen_string_literal: true
-
-def setup_redis_env_url(prefix = nil, defaults = true)
-  prefix = "#{prefix.to_s.upcase}_" unless prefix.nil?
-  prefix = '' if prefix.nil?
-
-  return if ENV["#{prefix}REDIS_URL"].present?
-
-  password = ENV.fetch("#{prefix}REDIS_PASSWORD") { '' if defaults }
-  host     = ENV.fetch("#{prefix}REDIS_HOST") { 'localhost' if defaults }
-  port     = ENV.fetch("#{prefix}REDIS_PORT") { 6379 if defaults }
-  db       = ENV.fetch("#{prefix}REDIS_DB") { 0 if defaults }
-
-  ENV["#{prefix}REDIS_URL"] = begin
-    if [password, host, port, db].all?(&:nil?)
-      ENV['REDIS_URL']
-    else
-      Addressable::URI.parse("redis://#{host}:#{port}/#{db}").tap do |uri|
-        uri.password = password if password.present?
-      end.normalize.to_str
-    end
-  end
-end
-
-setup_redis_env_url
-setup_redis_env_url(:cache, false)
-setup_redis_env_url(:sidekiq, false)
-
-namespace         = ENV.fetch('REDIS_NAMESPACE', nil)
-cache_namespace   = namespace ? "#{namespace}_cache" : 'cache'
-sidekiq_namespace = namespace
-
-redis_driver = ENV.fetch('REDIS_DRIVER', 'hiredis') == 'ruby' ? :ruby : :hiredis
-
-REDIS_CACHE_PARAMS = {
-  driver: redis_driver,
-  url: ENV['CACHE_REDIS_URL'],
-  expires_in: 10.minutes,
-  namespace: "#{cache_namespace}:7.1",
-  connect_timeout: 5,
-  pool: {
-    size: Sidekiq.server? ? Sidekiq[:concurrency] : Integer(ENV['MAX_THREADS'] || 5),
-    timeout: 5,
-  },
-}.freeze
-
-REDIS_SIDEKIQ_PARAMS = {
-  driver: redis_driver,
-  url: ENV['SIDEKIQ_REDIS_URL'],
-  namespace: sidekiq_namespace,
-}.freeze
-
-ENV['REDIS_NAMESPACE'] = "mastodon_test#{ENV['TEST_ENV_NUMBER']}" if Rails.env.test?

lib/mastodon/redis_configuration.rb

@@ -0,0 +1,96 @@
+# frozen_string_literal: true
+
+class Mastodon::RedisConfiguration
+  def base
+    @base ||= {
+      url: setup_base_redis_url,
+      driver: driver,
+      namespace: base_namespace,
+    }
+  end
+
+  def sidekiq
+    @sidekiq ||= {
+      url: setup_prefixed_redis_url(:sidekiq),
+      driver: driver,
+      namespace: sidekiq_namespace,
+    }
+  end
+
+  def cache
+    @cache ||= {
+      url: setup_prefixed_redis_url(:cache),
+      driver: driver,
+      namespace: cache_namespace,
+      expires_in: 10.minutes,
+      connect_timeout: 5,
+      pool: {
+        size: Sidekiq.server? ? Sidekiq[:concurrency] : Integer(ENV['MAX_THREADS'] || 5),
+        timeout: 5,
+      },
+    }
+  end
+
+  private
+
+  def driver
+    ENV['REDIS_DRIVER'] == 'ruby' ? :ruby : :hiredis
+  end
+
+  def namespace
+    @namespace ||= ENV.fetch('REDIS_NAMESPACE', nil)
+  end
+
+  def base_namespace
+    return "mastodon_test#{ENV.fetch('TEST_ENV_NUMBER', nil)}" if Rails.env.test?
+
+    namespace
+  end
+
+  def sidekiq_namespace
+    namespace
+  end
+
+  def cache_namespace
+    namespace ? "#{namespace}_cache" : 'cache'
+  end
+
+  def setup_base_redis_url
+    url = ENV.fetch('REDIS_URL', nil)
+    return url if url.present?
+
+    user     = ENV.fetch('REDIS_USER', '')
+    password = ENV.fetch('REDIS_PASSWORD', '')
+    host     = ENV.fetch('REDIS_HOST', 'localhost')
+    port     = ENV.fetch('REDIS_PORT', 6379)
+    db       = ENV.fetch('REDIS_DB', 0)
+
+    construct_uri(host, port, db, user, password)
+  end
+
+  def setup_prefixed_redis_url(prefix)
+    prefix = "#{prefix.to_s.upcase}_"
+    url = ENV.fetch("#{prefix}REDIS_URL", nil)
+
+    return url if url.present?
+
+    user     = ENV.fetch("#{prefix}REDIS_USER", nil)
+    password = ENV.fetch("#{prefix}REDIS_PASSWORD", nil)
+    host     = ENV.fetch("#{prefix}REDIS_HOST", nil)
+    port     = ENV.fetch("#{prefix}REDIS_PORT", nil)
+    db       = ENV.fetch("#{prefix}REDIS_DB", nil)
+
+    if host.nil?
+      base[:url]
+    else
+      construct_uri(host, port, db, user, password)
+    end
+  end
+
+  def construct_uri(host, port, db, user, password)
+    Addressable::URI.parse("redis://#{host}:#{port}/#{db}").tap do |uri|
+      uri.user = user if user.present?
+      uri.password = password if password.present?
+    end.normalize.to_str
+  end
+end

lib/mastodon/sidekiq_middleware.rb

@@ -53,7 +53,7 @@ class Mastodon::SidekiqMiddleware
   end
 
   def clean_up_redis_socket!
-    RedisConfiguration.pool.checkin if Thread.current[:redis]
+    RedisConnection.pool.checkin if Thread.current[:redis]
     Thread.current[:redis] = nil
   end
 

spec/lib/mastodon/redis_configuration_spec.rb

@@ -0,0 +1,170 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Mastodon::RedisConfiguration do
+  let(:redis_environment) { described_class.new }
+
+  before do
+    # We use one numbered namespace per parallel test runner
+    # in the test env. This here should test the non-test
+    # behavior, so we disable it temporarily.
+    allow(Rails.env).to receive(:test?).and_return(false)
+  end
+
+  shared_examples 'setting a different driver' do
+    context 'when setting the `REDIS_DRIVER` variable to `ruby`' do
+      around do |example|
+        ClimateControl.modify REDIS_DRIVER: 'ruby' do
+          example.run
+        end
+      end
+
+      it 'sets the driver accordingly' do
+        expect(subject[:driver]).to eq :ruby
+      end
+    end
+  end
+
+  shared_examples 'setting a namespace' do
+    context 'when setting the `REDIS_NAMESPACE` variable' do
+      around do |example|
+        ClimateControl.modify REDIS_NAMESPACE: 'testns' do
+          example.run
+        end
+      end
+
+      it 'uses the value for the namespace' do
+        expect(subject[:namespace]).to eq 'testns'
+      end
+    end
+  end
+
+  shared_examples 'secondary configuration' do |prefix|
+    context "when no `#{prefix}_REDIS_` environment variables are present" do
+      it 'uses the url from the base config' do
+        expect(subject[:url]).to eq 'redis://localhost:6379/0'
+      end
+    end
+
+    context "when the `#{prefix}_REDIS_URL` environment variable is present" do
+      around do |example|
+        ClimateControl.modify "#{prefix}_REDIS_URL": 'redis::/user@other.example.com/4' do
+          example.run
+        end
+      end
+
+      it 'uses the provided URL' do
+        expect(subject[:url]).to eq 'redis::/user@other.example.com/4'
+      end
+    end
+
+    context 'when giving separate environment variables' do
+      around do |example|
+        ClimateControl.modify "#{prefix}_REDIS_PASSWORD": 'testpass1', "#{prefix}_REDIS_HOST": 'redis2.example.com', "#{prefix}_REDIS_PORT": '3322', "#{prefix}_REDIS_DB": '8' do
+          example.run
+        end
+      end
+
+      it 'constructs the url from them' do
+        expect(subject[:url]).to eq 'redis://:testpass1@redis2.example.com:3322/8'
+      end
+    end
+  end
+
+  describe '#base' do
+    subject { redis_environment.base }
+
+    context 'when no `REDIS_` environment variables are present' do
+      it 'uses defaults' do
+        expect(subject).to eq({
+          url: 'redis://localhost:6379/0',
+          driver: :hiredis,
+          namespace: nil,
+        })
+      end
+    end
+
+    context 'when the `REDIS_URL` environment variable is present' do
+      around do |example|
+        ClimateControl.modify REDIS_URL: 'redis::/user@example.com/2' do
+          example.run
+        end
+      end
+
+      it 'uses the provided URL' do
+        expect(subject).to eq({
+          url: 'redis::/user@example.com/2',
+          driver: :hiredis,
+          namespace: nil,
+        })
+      end
+    end
+
+    context 'when giving separate environment variables' do
+      around do |example|
+        ClimateControl.modify REDIS_PASSWORD: 'testpass', REDIS_HOST: 'redis.example.com', REDIS_PORT: '3333', REDIS_DB: '3' do
+          example.run
+        end
+      end
+
+      it 'constructs the url from them' do
+        expect(subject).to eq({
+          url: 'redis://:testpass@redis.example.com:3333/3',
+          driver: :hiredis,
+          namespace: nil,
+        })
+      end
+    end
+
+    include_examples 'setting a different driver'
+    include_examples 'setting a namespace'
+  end
+
+  describe '#sidekiq' do
+    subject { redis_environment.sidekiq }
+
+    include_examples 'secondary configuration', 'SIDEKIQ'
+    include_examples 'setting a different driver'
+    include_examples 'setting a namespace'
+  end
+
+  describe '#cache' do
+    subject { redis_environment.cache }
+
+    it 'includes extra configuration' do
+      expect(subject).to eq({
+        url: 'redis://localhost:6379/0',
+        driver: :hiredis,
+        namespace: 'cache',
+        expires_in: 10.minutes,
+        connect_timeout: 5,
+        pool: {
+          size: 5,
+          timeout: 5,
+        },
+      })
+    end
+
+    context 'when `REDIS_NAMESPACE` is not set' do
+      it 'uses the `cache` namespace' do
+        expect(subject[:namespace]).to eq 'cache'
+      end
+    end
+
+    context 'when setting the `REDIS_NAMESPACE` variable' do
+      around do |example|
+        ClimateControl.modify REDIS_NAMESPACE: 'testns' do
+          example.run
+        end
+      end
+
+      it 'attaches the `_cache` postfix to the namespace' do
+        expect(subject[:namespace]).to eq 'testns_cache'
+      end
+    end
+
+    include_examples 'secondary configuration', 'CACHE'
+    include_examples 'setting a different driver'
+  end
+end

spec/rails_helper.rb

@@ -176,5 +176,5 @@ def stub_reset_connection_pools
   # TODO: Is there a better way to correctly run specs without stubbing this?
   # (Avoids reset_connection_pools! in test env)
   allow(ActiveRecord::Base).to receive(:establish_connection)
-  allow(RedisConfiguration).to receive(:establish_pool)
+  allow(RedisConnection).to receive(:establish_pool)
 end

spec/services/resolve_account_service_spec.rb

@@ -228,7 +228,7 @@ RSpec.describe ResolveAccountService do
       rescue ActiveRecord::RecordNotUnique
         fail_occurred = true
       ensure
-        RedisConfiguration.pool.checkin if Thread.current[:redis]
+        RedisConnection.pool.checkin if Thread.current[:redis]
       end
     end
 

spec/support/streaming_server_manager.rb

@@ -17,7 +17,7 @@ class StreamingServerManager
     @running_thread = Thread.new do
       Open3.popen2e(
         {
-          'REDIS_NAMESPACE' => ENV.fetch('REDIS_NAMESPACE'),
+          'REDIS_NAMESPACE' => REDIS_CONFIGURATION.base[:namespace],
           'DB_NAME' => "#{ENV.fetch('DB_NAME', 'mastodon')}_test#{ENV.fetch('TEST_ENV_NUMBER', '')}",
           'RAILS_ENV' => ENV.fetch('RAILS_ENV', 'test'),
           'NODE_ENV' => ENV.fetch('STREAMING_NODE_ENV', 'development'),