From 687176cf494f118ebb4e4aae944f72c15bf81692 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Mon, 30 Oct 2023 15:29:34 +1100
Subject: [PATCH] Remove trouble-some hiredis

The hiredis client was praised as being faster parsing bulk responses
but it seems to have multiple issues now:

- The redis release 5.0 moved hiredis support to another gem.
- I tried the hiredis-client gem and it raised errors.
- There are claims of worse performance of hiredis [1].
- Maintenance responsiveness has been questioned [2].

Using the default redis driver seems to work fine though.

[1]: https://discuss.rubyonrails.org/t/hiredis-does-not-support-ssl-action-cable/75945
[2]: https://github.com/redis/hiredis/issues/655
---
 Gemfile                                                   | 3 +--
 Gemfile.lock                                              | 4 +---
 app/controllers/spree/admin/payment_methods_controller.rb | 4 +++-
 config/environments/development.rb                        | 1 -
 config/environments/production.rb                         | 1 -
 config/environments/staging.rb                            | 1 -
 config/environments/test.rb                               | 1 -
 7 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/Gemfile b/Gemfile
index d9e5029d2e..d8ad95f6f4 100644
--- a/Gemfile
+++ b/Gemfile
@@ -98,9 +98,8 @@ gem 'rack-rewrite'
 gem 'rack-timeout'
 gem 'roadie-rails'
 
-gem 'hiredis'
 gem 'puma'
-gem 'redis', '>= 4.0', require: ['redis', 'redis/connection/hiredis']
+gem 'redis'
 gem 'sidekiq'
 gem 'sidekiq-scheduler'
 
diff --git a/Gemfile.lock b/Gemfile.lock
index 00401c7e9b..331904f52c 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -331,7 +331,6 @@ GEM
     hashery (2.1.2)
     hashie (5.0.0)
     highline (2.0.3)
-    hiredis (0.6.3)
     htmlentities (4.3.4)
     httpclient (2.8.3)
     i18n (1.14.1)
@@ -853,7 +852,6 @@ DEPENDENCIES
   good_migrations
   haml
   highline (= 2.0.3)
-  hiredis
   i18n
   i18n-js (~> 3.9.0)
   image_processing
@@ -897,7 +895,7 @@ DEPENDENCIES
   rails_safe_tasks (~> 1.0)
   ransack (~> 4.1.0)
   redcarpet
-  redis (>= 4.0)
+  redis
   responders
   rexml
   roadie-rails
diff --git a/app/controllers/spree/admin/payment_methods_controller.rb b/app/controllers/spree/admin/payment_methods_controller.rb
index 37c8529f3d..204de6936f 100644
--- a/app/controllers/spree/admin/payment_methods_controller.rb
+++ b/app/controllers/spree/admin/payment_methods_controller.rb
@@ -193,7 +193,9 @@ module Spree
 
       def clear_preference_cache
         @payment_method.calculator.preferences.each_key do |key|
-          Rails.cache.delete(@payment_method.calculator.preference_cache_key(key))
+          # Only persisted models have cache keys.
+          cache_key = @payment_method.calculator.preference_cache_key(key)
+          Rails.cache.delete(cache_key) if cache_key
         end
       end
 
diff --git a/config/environments/development.rb b/config/environments/development.rb
index 84dc8c9958..3dcbe29076 100644
--- a/config/environments/development.rb
+++ b/config/environments/development.rb
@@ -16,7 +16,6 @@ Openfoodnetwork::Application.configure do
   # :file_store is used by default when no cache store is specifically configured.
   if !!ENV["PROFILE"] || !!ENV["DEV_CACHING"]
     config.cache_store = :redis_cache_store, {
-      driver: :hiredis,
       url: ENV.fetch("OFN_REDIS_URL", "redis://localhost:6379/1"),
       expires_in: 90.minutes
     }
diff --git a/config/environments/production.rb b/config/environments/production.rb
index 6fde13c85e..d6877e67e9 100644
--- a/config/environments/production.rb
+++ b/config/environments/production.rb
@@ -43,7 +43,6 @@ Openfoodnetwork::Application.configure do
 
   # Use a different cache store in production
   config.cache_store = :redis_cache_store, {
-    driver: :hiredis,
     url: ENV.fetch("OFN_REDIS_URL", "redis://localhost:6380/0"),
     reconnect_attempts: 1
   }
diff --git a/config/environments/staging.rb b/config/environments/staging.rb
index 6fde13c85e..d6877e67e9 100644
--- a/config/environments/staging.rb
+++ b/config/environments/staging.rb
@@ -43,7 +43,6 @@ Openfoodnetwork::Application.configure do
 
   # Use a different cache store in production
   config.cache_store = :redis_cache_store, {
-    driver: :hiredis,
     url: ENV.fetch("OFN_REDIS_URL", "redis://localhost:6380/0"),
     reconnect_attempts: 1
   }
diff --git a/config/environments/test.rb b/config/environments/test.rb
index e5cc306c18..cfba7d0307 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -15,7 +15,6 @@ Openfoodnetwork::Application.configure do
 
   # Separate cache stores when running in parallel
   config.cache_store = :redis_cache_store, {
-    driver: :hiredis,
      # Unique database number to avoid conflict with others
     url: ENV.fetch("OFN_REDIS_TEST_URL", "redis://localhost:6379/3"),
     reconnect_attempts: 1