Skip to content

Commit

Permalink
Get rid of @original_client
Browse files Browse the repository at this point in the history
Now that we no longer swap the client during pipelining and such,
we can get rid of that mess.
  • Loading branch information
byroot committed Aug 13, 2022
1 parent 08a2100 commit 5daaf35
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 34 deletions.
34 changes: 19 additions & 15 deletions lib/redis.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,25 +64,27 @@ def initialize(options = {})
inherit_socket = @options.delete(:inherit_socket)
@cluster_mode = options.key?(:cluster)
client = @cluster_mode ? Cluster : Client
@original_client = @client = client.new(@options)
@subscription_client = nil
@client = client.new(@options)
@client.inherit_socket! if inherit_socket
@queue = Hash.new { |h, k| h[k] = [] }
@monitor = Monitor.new
end

# Run code without the client reconnecting
def without_reconnect(&block)
@original_client.disable_reconnection(&block)
@client.disable_reconnection(&block)
end

# Test whether or not the client is connected
def connected?
@original_client.connected?
@client.connected? || @subscription_client&.connected?
end

# Disconnect the client as quickly and silently as possible.
def close
@original_client.close
@client.close
@subscription_client&.close
end
alias disconnect! close

Expand All @@ -103,7 +105,7 @@ def pipelined
end

def id
@original_client.config.id || @original_client.config.server_url
@client.config.id || @client.config.server_url
end

def inspect
Expand All @@ -115,14 +117,14 @@ def dup
end

def connection
return @original_client.connection_info if @cluster_mode
return @client.connection_info if @cluster_mode

{
host: @original_client.host,
port: @original_client.port,
db: @original_client.db,
host: @client.host,
port: @client.port,
db: @client.db,
id: id,
location: "#{@original_client.host}:#{@original_client.port}"
location: "#{@client.host}:#{@client.port}"
}
end

Expand All @@ -145,17 +147,19 @@ def send_blocking_command(command, timeout, &block)
end

def _subscription(method, timeout, channels, block)
return @client.call([method] + channels) if subscribed?
if @subscription_client
return @subscription_client.call([method] + channels)
end

begin
original, @client = @client, SubscribedClient.new(@client.pubsub)
@subscription_client = SubscribedClient.new(@client.pubsub)
if timeout > 0
@client.send(method, timeout, *channels, &block)
@subscription_client.send(method, timeout, *channels, &block)
else
@client.send(method, *channels, &block)
@subscription_client.send(method, *channels, &block)
end
ensure
@client = original
@subscription_client = nil
end
end
end
Expand Down
14 changes: 5 additions & 9 deletions lib/redis/commands/pubsub.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ def publish(channel, message)
end

def subscribed?
synchronize do |client|
client.is_a? SubscribedClient
end
!@subscription_client.nil?
end

# Listen for messages published to the given channels.
Expand All @@ -31,10 +29,9 @@ def subscribe_with_timeout(timeout, *channels, &block)

# Stop listening for messages posted to the given channels.
def unsubscribe(*channels)
raise "Can't unsubscribe if not subscribed." unless subscribed?
synchronize do |client|
raise "Can't unsubscribe if not subscribed." unless subscribed?

client.unsubscribe(*channels)
_subscription(:unsubscribe, 0, channels, nil)
end
end

Expand All @@ -55,10 +52,9 @@ def psubscribe_with_timeout(timeout, *channels, &block)

# Stop listening for messages posted to channels matching the given patterns.
def punsubscribe(*channels)
raise "Can't unsubscribe if not subscribed." unless subscribed?
synchronize do |client|
raise "Can't unsubscribe if not subscribed." unless subscribed?

client.punsubscribe(*channels)
_subscription(:punsubscribe, 0, channels, nil)
end
end

Expand Down
9 changes: 4 additions & 5 deletions test/distributed/publish_subscribe_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,10 @@ def test_subscribe_within_subscribe
end

def test_other_commands_within_a_subscribe
assert_raises Redis::CommandError do
r.subscribe("foo") do |on|
on.subscribe do |_channel, _total|
r.set("bar", "s2")
end
r.subscribe("foo") do |on|
on.subscribe do |_channel, _total|
r.set("bar", "s2")
r.unsubscribe("foo")
end
end
end
Expand Down
9 changes: 4 additions & 5 deletions test/redis/publish_subscribe_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,10 @@ def test_subscribe_within_subscribe
end

def test_other_commands_within_a_subscribe
assert_raises Redis::CommandError do
r.subscribe("foo") do |on|
on.subscribe do |_channel, _total|
r.set("bar", "s2")
end
r.subscribe("foo") do |on|
on.subscribe do |_channel, _total|
r.set("bar", "s2")
r.unsubscribe("foo")
end
end
end
Expand Down

0 comments on commit 5daaf35

Please sign in to comment.