Skip to content

Commit

Permalink
FIX: Add Redis 5 support
Browse files Browse the repository at this point in the history
Redis 5 is picky about the params you send it, in 4 and below you could ship
it arbitrary params and it would simply ignore. In 5 it requires specific ones.


The change also removes a non relevant test case, Redis::Client::Connector
is no longer a thing Redis gem version 5 and up.

Redis 5 gem also duplicates incoming params so it is inherently more safe.
  • Loading branch information
SamSaffron committed Oct 27, 2022
1 parent d9c28d6 commit b61581c
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 9 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
FUTURE

- FIX: Add redis version 5 support

22-02-2022

- Version 4.2.0
Expand Down
24 changes: 23 additions & 1 deletion lib/message_bus/backends/redis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,29 @@ def global_subscribe(last_id = nil, &blk)
private

def new_redis_connection
::Redis.new(@redis_config.dup)
config = @redis_config.filter do |k, v|
# This is not ideal, required for Redis gem version 5
# redis-client no longer accepts arbitrary params
# anything unknown will error out.
# https://github.com/redis-rb/redis-client/blob/master/lib/redis_client/config.rb#L21-L39
#
#
# We should be doing the opposite and allowlisting params
# or splitting the object up. Starting with the smallest change that is backwards compatible
![
:backend,
:logger,
:long_polling_enabled,
:backend_options,
:base_route,
:client_message_filters,
:site_id_lookup,
:group_id_lookup,
:user_id_lookup,
:transport_codec
].include?(k)
end
::Redis.new(config)
end

# redis connection used for publishing messages
Expand Down
2 changes: 1 addition & 1 deletion message_bus.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Gem::Specification.new do |gem|
gem.add_runtime_dependency 'rack', '>= 1.1.3'

# Optional runtime dependencies
gem.add_development_dependency 'redis', '< 5.0'
gem.add_development_dependency 'redis'
gem.add_development_dependency 'pg'
gem.add_development_dependency 'concurrent-ruby' # for distributed-cache

Expand Down
7 changes: 0 additions & 7 deletions spec/lib/message_bus/backend_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -392,11 +392,4 @@
got.map { |m| m.data }.must_equal ["12"]
end

it 'should not lose redis config' do
test_only :redis
redis_config = { connector: Redis::Client::Connector }
@bus.instance_variable_set(:@redis_config, redis_config)
@bus.send(:new_redis_connection)
expect(@bus.instance_variable_get(:@redis_config)[:connector]).must_equal Redis::Client::Connector
end
end

0 comments on commit b61581c

Please sign in to comment.