From f2ab66da735f9382a923471e27aca12f07ec05cd Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 19 Aug 2022 10:34:25 +0200 Subject: [PATCH] Stop testing hiredis The driver being used is an implementation detail of `redis-rb`. If somehow something break on one driver but not the other, it should be reported to redis-rb and fixed there. Also `redis-rb` `5.0` has a totally new client and hiredis binding so all this code no longer works with redis-rb 5.0. --- Gemfile | 3 +-- Gemfile.lock | 6 ++--- .../test/subscription_adapter/redis_test.rb | 6 ----- activesupport/Rakefile | 19 --------------- .../active_support/cache/redis_cache_store.rb | 8 +------ .../cache/stores/redis_cache_store_test.rb | 23 ++++++------------- guides/source/caching_with_rails.md | 10 -------- 7 files changed, 11 insertions(+), 64 deletions(-) diff --git a/Gemfile b/Gemfile index 48019c8b62..8861643bf6 100644 --- a/Gemfile +++ b/Gemfile @@ -85,8 +85,7 @@ end group :cable do gem "puma", require: false - gem "hiredis", require: false - gem "redis", "~> 4.0", require: false + gem "redis", ">= 4.0.1", require: false gem "redis-namespace" diff --git a/Gemfile.lock b/Gemfile.lock index a3d83f0180..1217c89977 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -284,7 +284,6 @@ GEM os (>= 0.9, < 2.0) signet (>= 0.16, < 2.a) hashdiff (1.0.1) - hiredis (0.6.3) http_parser.rb (0.8.0) httpclient (2.8.3) i18n (1.8.11) @@ -400,7 +399,7 @@ GEM rbtree (0.4.5) rdoc (6.3.3) redcarpet (3.2.3) - redis (4.5.1) + redis (4.7.1) redis-namespace (1.8.1) redis (>= 3.0.4) regexp_parser (2.2.1) @@ -574,7 +573,6 @@ DEPENDENCIES delayed_job delayed_job_active_record google-cloud-storage (~> 1.11) - hiredis image_processing (~> 1.2) importmap-rails jsbundling-rails @@ -598,7 +596,7 @@ DEPENDENCIES rails! rake (>= 11.1) redcarpet (~> 3.2.3) - redis (~> 4.0) + redis (>= 4.0.1) redis-namespace resque resque-scheduler diff --git a/actioncable/test/subscription_adapter/redis_test.rb b/actioncable/test/subscription_adapter/redis_test.rb index b078e6bb01..2372ada062 100644 --- a/actioncable/test/subscription_adapter/redis_test.rb +++ b/actioncable/test/subscription_adapter/redis_test.rb @@ -17,12 +17,6 @@ class RedisAdapterTest < ActionCable::TestCase end end -class RedisAdapterTest::Hiredis < RedisAdapterTest - def cable_config - super.merge(driver: "hiredis") - end -end - class RedisAdapterTest::AlternateConfiguration < RedisAdapterTest def cable_config alt_cable_config = super.dup diff --git a/activesupport/Rakefile b/activesupport/Rakefile index f48e3ac364..fd60a14155 100644 --- a/activesupport/Rakefile +++ b/activesupport/Rakefile @@ -13,29 +13,10 @@ Rake::TestTask.new do |t| t.ruby_opts = ["--dev"] if defined?(JRUBY_VERSION) end -Rake::Task[:test].enhance do - Rake::Task["test:cache_stores:redis:ruby"].invoke -end - namespace :test do task :isolated do Dir.glob("test/**/*_test.rb").all? do |file| sh(Gem.ruby, "-w", file) end || raise("Failures") end - - namespace :cache_stores do - namespace :redis do - %w[ ruby hiredis ].each do |driver| - task("env:#{driver}") { ENV["REDIS_DRIVER"] = driver } - - Rake::TestTask.new(driver => "env:#{driver}") do |t| - t.test_files = ["test/cache/stores/redis_cache_store_test.rb"] - t.warning = true - t.verbose = true - t.ruby_opts = ["--dev"] if defined?(JRUBY_VERSION) - end - end - end - end end diff --git a/activesupport/lib/active_support/cache/redis_cache_store.rb b/activesupport/lib/active_support/cache/redis_cache_store.rb index 62851938e3..2ff5f1fb54 100644 --- a/activesupport/lib/active_support/cache/redis_cache_store.rb +++ b/activesupport/lib/active_support/cache/redis_cache_store.rb @@ -5,16 +5,10 @@ begin require "redis" require "redis/distributed" rescue LoadError - warn "The Redis cache store requires the redis gem, version 4.0.1 or later. Please add it to your Gemfile: `gem \"redis\", \"~> 4.0\"`" + warn "The Redis cache store requires the redis gem, version 4.0.1 or later. Please add it to your Gemfile: `gem \"redis\", \">= 4.0.1\"`" raise end -# Prefer the hiredis driver but don't require it. -begin - require "redis/connection/hiredis" -rescue LoadError -end - require "connection_pool" require "active_support/core_ext/numeric/time" require "active_support/digest" diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb index 374723fe71..c08b65839f 100644 --- a/activesupport/test/cache/stores/redis_cache_store_test.rb +++ b/activesupport/test/cache/stores/redis_cache_store_test.rb @@ -5,12 +5,6 @@ require "active_support/cache" require "active_support/cache/redis_cache_store" require_relative "../behaviors" -driver_name = %w[ ruby hiredis ].include?(ENV["REDIS_DRIVER"]) ? ENV["REDIS_DRIVER"] : "hiredis" -driver = Object.const_get("Redis::Connection::#{driver_name.camelize}") - -Redis::Connection.drivers.clear -Redis::Connection.drivers.append(driver) - # Emulates a latency on Redis's back-end for the key latency to facilitate # connection pool testing. class SlowRedis < Redis @@ -42,8 +36,6 @@ module ActiveSupport::Cache::RedisCacheStoreTests end end - DRIVER = %w[ ruby hiredis ].include?(ENV["REDIS_DRIVER"]) ? ENV["REDIS_DRIVER"] : "hiredis" - class LookupTest < ActiveSupport::TestCase test "may be looked up as :redis_cache_store" do assert_kind_of ActiveSupport::Cache::RedisCacheStore, @@ -56,7 +48,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests assert_called_with Redis, :new, [ url: nil, connect_timeout: 20, read_timeout: 1, write_timeout: 1, - reconnect_attempts: 0, driver: DRIVER + reconnect_attempts: 0 ] do build end @@ -66,7 +58,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests assert_called_with Redis, :new, [ url: nil, connect_timeout: 20, read_timeout: 1, write_timeout: 1, - reconnect_attempts: 0, driver: DRIVER + reconnect_attempts: 0 ] do build url: [] end @@ -76,7 +68,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests assert_called_with Redis, :new, [ url: REDIS_URL, connect_timeout: 20, read_timeout: 1, write_timeout: 1, - reconnect_attempts: 0, driver: DRIVER + reconnect_attempts: 0 ] do build url: REDIS_URL end @@ -86,7 +78,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests assert_called_with Redis, :new, [ url: REDIS_URL, connect_timeout: 20, read_timeout: 1, write_timeout: 1, - reconnect_attempts: 0, driver: DRIVER + reconnect_attempts: 0 ] do build url: [ REDIS_URL ] end @@ -98,7 +90,6 @@ module ActiveSupport::Cache::RedisCacheStoreTests read_timeout: 1, write_timeout: 1, reconnect_attempts: 0, - driver: DRIVER } mock = Minitest::Mock.new @@ -128,7 +119,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests private def build(**kwargs) - ActiveSupport::Cache::RedisCacheStore.new(driver: DRIVER, **kwargs.merge(pool: false)).tap(&:redis) + ActiveSupport::Cache::RedisCacheStore.new(**kwargs.merge(pool: false)).tap(&:redis) end end @@ -146,7 +137,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests end def lookup_store(options = {}) - ActiveSupport::Cache.lookup_store(:redis_cache_store, { timeout: 0.1, namespace: @namespace, driver: DRIVER, pool: false }.merge(options)) + ActiveSupport::Cache.lookup_store(:redis_cache_store, { timeout: 0.1, namespace: @namespace, pool: false }.merge(options)) end teardown do @@ -452,7 +443,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests test "clear all cache key with Redis::Distributed" do cache = ActiveSupport::Cache::RedisCacheStore.new( url: REDIS_URLS, - timeout: 0.1, namespace: @namespace, expires_in: 60, driver: DRIVER) + timeout: 0.1, namespace: @namespace, expires_in: 60) cache.write("foo", "bar") cache.write("fu", "baz") cache.clear diff --git a/guides/source/caching_with_rails.md b/guides/source/caching_with_rails.md index bc5ac660c8..612e9e7130 100644 --- a/guides/source/caching_with_rails.md +++ b/guides/source/caching_with_rails.md @@ -517,16 +517,6 @@ To get started, add the redis gem to your Gemfile: gem 'redis' ``` -You can enable support for the faster [hiredis](https://github.com/redis/hiredis) -connection library by additionally adding its ruby wrapper to your Gemfile: - -```ruby -gem 'hiredis' -``` - -Redis cache store will automatically require and use hiredis if available. No further -configuration is needed. - Finally, add the configuration in the relevant `config/environments/*.rb` file: ```ruby