Visitar URL original
Backport Redis 5.0 compatibility · rails/rails@a3bd3b5 · GitHub
Skip to content

Commit a3bd3b5

Browse files
byrootbyroot
committed
Backport Redis 5.0 compatibility
Redis 3.0 compatiblity is preserved in Action Cable Redis 4.0 compatiblity is preserved in Active Support
1 parent 67f37ac commit a3bd3b5

7 files changed

Lines changed: 68 additions & 52 deletions

File tree

‎actioncable/CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
* The Redis adapter is now compatible with redis-rb 5.0
2+
3+
Compatibility with redis-rb 3.x was dropped.
4+
5+
*Jean Boussier*
6+
17
* The Action Cable server is now mounted with `anchor: true`.
28

39
This means that routes that also start with `/cable` will no longer clash with Action Cable.

‎actioncable/lib/action_cable/subscription_adapter/redis.rb‎

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
require "thread"
44

5-
gem "redis", ">= 3", "< 5"
5+
gem "redis", ">= 3", "< 6"
66
require "redis"
77

88
require "active_support/core_ext/hash/except"
@@ -56,7 +56,7 @@ def redis_connection_for_broadcasts
5656
end
5757

5858
def redis_connection
59-
self.class.redis_connector.call(@server.config.cable.merge(id: identifier))
59+
self.class.redis_connector.call(@server.config.cable.symbolize_keys.merge(id: identifier))
6060
end
6161

6262
class Listener < SubscriberMap
@@ -69,7 +69,7 @@ def initialize(adapter, event_loop)
6969
@subscribe_callbacks = Hash.new { |h, k| h[k] = [] }
7070
@subscription_lock = Mutex.new
7171

72-
@raw_client = nil
72+
@subscribed_client = nil
7373

7474
@when_connected = []
7575

@@ -78,13 +78,13 @@ def initialize(adapter, event_loop)
7878

7979
def listen(conn)
8080
conn.without_reconnect do
81-
original_client = conn.respond_to?(:_client) ? conn._client : conn.client
81+
original_client = extract_subscribed_client(conn)
8282

8383
conn.subscribe("_action_cable_internal") do |on|
8484
on.subscribe do |chan, count|
8585
@subscription_lock.synchronize do
8686
if count == 1
87-
@raw_client = original_client
87+
@subscribed_client = original_client
8888

8989
until @when_connected.empty?
9090
@when_connected.shift.call
@@ -106,7 +106,7 @@ def listen(conn)
106106
on.unsubscribe do |chan, count|
107107
if count == 0
108108
@subscription_lock.synchronize do
109-
@raw_client = nil
109+
@subscribed_client = nil
110110
end
111111
end
112112
end
@@ -119,8 +119,8 @@ def shutdown
119119
return if @thread.nil?
120120

121121
when_connected do
122-
send_command("unsubscribe")
123-
@raw_client = nil
122+
@subscribed_client.unsubscribe
123+
@subscribed_client = nil
124124
end
125125
end
126126

@@ -131,13 +131,13 @@ def add_channel(channel, on_success)
131131
@subscription_lock.synchronize do
132132
ensure_listener_running
133133
@subscribe_callbacks[channel] << on_success
134-
when_connected { send_command("subscribe", channel) }
134+
when_connected { @subscribed_client.subscribe(channel) }
135135
end
136136
end
137137

138138
def remove_channel(channel)
139139
@subscription_lock.synchronize do
140-
when_connected { send_command("unsubscribe", channel) }
140+
when_connected { @subscribed_client.unsubscribe(channel) }
141141
end
142142
end
143143

@@ -156,22 +156,49 @@ def ensure_listener_running
156156
end
157157

158158
def when_connected(&block)
159-
if @raw_client
159+
if @subscribed_client
160160
block.call
161161
else
162162
@when_connected << block
163163
end
164164
end
165165

166-
def send_command(*command)
167-
@raw_client.write(command)
166+
if ::Redis::VERSION < "5"
167+
class SubscribedClient
168+
def initialize(raw_client)
169+
@raw_client = raw_client
170+
end
171+
172+
def subscribe(*channel)
173+
send_command("subscribe", *channel)
174+
end
175+
176+
def unsubscribe(*channel)
177+
send_command("unsubscribe", *channel)
178+
end
179+
180+
private
181+
def send_command(*command)
182+
@raw_client.write(command)
168183

169-
very_raw_connection =
170-
@raw_client.connection.instance_variable_defined?(:@connection) &&
171-
@raw_client.connection.instance_variable_get(:@connection)
184+
very_raw_connection =
185+
@raw_client.connection.instance_variable_defined?(:@connection) &&
186+
@raw_client.connection.instance_variable_get(:@connection)
172187

173-
if very_raw_connection && very_raw_connection.respond_to?(:flush)
174-
very_raw_connection.flush
188+
if very_raw_connection && very_raw_connection.respond_to?(:flush)
189+
very_raw_connection.flush
190+
end
191+
nil
192+
end
193+
end
194+
195+
def extract_subscribed_client(conn)
196+
raw_client = conn.respond_to?(:_client) ? conn._client : conn.client
197+
SubscribedClient.new(raw_client)
198+
end
199+
else
200+
def extract_subscribed_client(conn)
201+
conn
175202
end
176203
end
177204
end

‎actioncable/test/subscription_adapter/redis_test.rb‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,6 @@ def cable_config
1919
end
2020
end
2121

22-
class RedisAdapterTest::Hiredis < RedisAdapterTest
23-
def cable_config
24-
super.merge(driver: "hiredis")
25-
end
26-
end
27-
2822
class RedisAdapterTest::AlternateConfiguration < RedisAdapterTest
2923
def cable_config
3024
alt_cable_config = super.dup
@@ -56,7 +50,7 @@ def expected_connection
5650
end
5751

5852
test "sets connection id for connection" do
59-
assert_called_with ::Redis, :new, [ expected_connection.stringify_keys ] do
53+
assert_called_with ::Redis, :new, [ expected_connection.symbolize_keys ] do
6054
@adapter.send(:redis_connection)
6155
end
6256
end

‎activesupport/CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
* Redis cache store is now compatible with redis-rb 5.0.
2+
3+
*Jean Boussier*
4+
15
* Fix `NoMethodError` on custom `ActiveSupport::Deprecation` behavior.
26

37
`ActiveSupport::Deprecation.behavior=` was supposed to accept any object

‎activesupport/Rakefile‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,10 @@ Rake::TestTask.new do |t|
1313
t.ruby_opts = ["--dev"] if defined?(JRUBY_VERSION)
1414
end
1515

16-
Rake::Task[:test].enhance do
17-
Rake::Task["test:cache_stores:redis:ruby"].invoke
18-
end
19-
2016
namespace :test do
2117
task :isolated do
2218
Dir.glob("test/**/*_test.rb").all? do |file|
2319
sh(Gem.ruby, "-w", file)
2420
end || raise("Failures")
2521
end
26-
27-
namespace :cache_stores do
28-
namespace :redis do
29-
%w[ ruby hiredis ].each do |driver|
30-
task("env:#{driver}") { ENV["REDIS_DRIVER"] = driver }
31-
32-
Rake::TestTask.new(driver => "env:#{driver}") do |t|
33-
t.test_files = ["test/cache/stores/redis_cache_store_test.rb"]
34-
t.warning = true
35-
t.verbose = true
36-
t.ruby_opts = ["--dev"] if defined?(JRUBY_VERSION)
37-
end
38-
end
39-
end
40-
end
4122
end

‎activesupport/lib/active_support/cache/redis_cache_store.rb‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,17 @@
55
require "redis"
66
require "redis/distributed"
77
rescue LoadError
8-
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\"`"
8+
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\"`"
99
raise
1010
end
1111

1212
# Prefer the hiredis driver but don't require it.
1313
begin
14-
require "redis/connection/hiredis"
14+
if ::Redis::VERSION < "5"
15+
require "redis/connection/hiredis"
16+
else
17+
require "hiredis-client"
18+
end
1519
rescue LoadError
1620
end
1721

@@ -92,9 +96,11 @@ def build_redis(redis: nil, url: nil, **redis_options) # :nodoc:
9296
elsif redis
9397
redis
9498
elsif urls.size > 1
95-
build_redis_distributed_client urls: urls, **redis_options
99+
build_redis_distributed_client(urls: urls, **redis_options)
100+
elsif urls.empty?
101+
build_redis_client(**redis_options)
96102
else
97-
build_redis_client url: urls.first, **redis_options
103+
build_redis_client(url: urls.first, **redis_options)
98104
end
99105
end
100106

@@ -105,8 +111,8 @@ def build_redis_distributed_client(urls:, **redis_options)
105111
end
106112
end
107113

108-
def build_redis_client(url:, **redis_options)
109-
::Redis.new DEFAULT_REDIS_OPTIONS.merge(redis_options.merge(url: url))
114+
def build_redis_client(**redis_options)
115+
::Redis.new(DEFAULT_REDIS_OPTIONS.merge(redis_options))
110116
end
111117
end
112118

‎activesupport/test/cache/stores/redis_cache_store_test.rb‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ class LookupTest < ActiveSupport::TestCase
5151
class InitializationTest < ActiveSupport::TestCase
5252
test "omitted URL uses Redis client with default settings" do
5353
assert_called_with Redis, :new, [
54-
url: nil,
5554
connect_timeout: 20, read_timeout: 1, write_timeout: 1,
5655
reconnect_attempts: 0, driver: DRIVER
5756
] do
@@ -61,7 +60,6 @@ class InitializationTest < ActiveSupport::TestCase
6160

6261
test "no URLs uses Redis client with default settings" do
6362
assert_called_with Redis, :new, [
64-
url: nil,
6563
connect_timeout: 20, read_timeout: 1, write_timeout: 1,
6664
reconnect_attempts: 0, driver: DRIVER
6765
] do

0 commit comments

Comments
 (0)