stefanwille / crystal-redis

Full featured Redis client for Crystal
MIT License
380 stars 61 forks source link

Pooled Client uses subscribe on psubscribe method #84

Closed KaulSe closed 5 years ago

KaulSe commented 5 years ago

Bug The pooled Redis client is using the wrong method on the psubscribe method which results in a wrong subscription. Here a code example:

REDIS = Redis::PooledClient.new(host=ENV["REDIS_HOST"], port=ENV["REDIS_PORT"].to_i, pool_size: 5)
REDIS.psubscribe("a*") do |on|
    on.pmessage do |channel|
        puts "Channel: #{channel}"
    end
end

Which results in the following subscription on the Redis server:

➜ redis-cli monitor
[...]
1561661772.372462 [0 127.0.0.1:65063] "SUBSCRIBE" "a*"
[...]

If we now try to publish, for example, the following message, redis-cli publish 'ab' '{"id":199}', the message will not arrive the server as it will only listen for the a* key.

Therefore, we can fix this by changing the following line https://github.com/stefanwille/crystal-redis/blob/571ea1c0643a21ddf8a9aa69eba4b21ed6c4d5ba/src/redis/pooled_client.cr#L60 to

    with_pool_connection &.psubscribe(*channel_patterns) { |s| callback_setup_block.call(s) }

Note the change; &.subscribe to &.psubscribe. This will now result in a proper listener:

➜ redis-cli monitor
[...]
1561662810.158611 [0 127.0.0.1:65144] "PSUBSCRIBE" "a*"
[...]

Code reference

  def subscribe(*channels, &callback_setup_block : Redis::Subscription ->)
    with_pool_connection &.subscribe(*channels) { |s| callback_setup_block.call(s) }
  end

  def psubscribe(*channel_patterns, &callback_setup_block : Redis::Subscription ->)
    with_pool_connection &.subscribe(*channel_patterns) { |s| callback_setup_block.call(s) }
  end
stefanwille commented 5 years ago

Fixed by #85