From ae31da20cd250154c951b67d5625fc71ac27e2f1 Mon Sep 17 00:00:00 2001
From: Jon Moss <me@jonathanmoss.me>
Date: Sat, 16 Jan 2016 10:33:50 -0500
Subject: Fix code review comments
- adapter -> pubsub (re)rename internally
- Change variable names to match method names
- Add EventMachine `~> 1.0` as a runtime dependency of ActionCable
- Refactor dependency loading for adapters
---
actioncable/lib/action_cable/channel/streams.rb | 6 ++--
actioncable/lib/action_cable/connection/base.rb | 2 +-
.../action_cable/connection/internal_channel.rb | 4 +--
actioncable/lib/action_cable/server/base.rb | 4 +--
.../lib/action_cable/server/broadcasting.rb | 2 +-
.../lib/action_cable/server/configuration.rb | 19 +++++++---
.../lib/action_cable/subscription_adapter.rb | 2 --
.../subscription_adapter/postgresql.rb | 8 ++---
.../lib/action_cable/subscription_adapter/redis.rb | 41 +++++++++-------------
9 files changed, 42 insertions(+), 46 deletions(-)
(limited to 'actioncable/lib')
diff --git a/actioncable/lib/action_cable/channel/streams.rb b/actioncable/lib/action_cable/channel/streams.rb
index 89dcbdfa27..589946c3db 100644
--- a/actioncable/lib/action_cable/channel/streams.rb
+++ b/actioncable/lib/action_cable/channel/streams.rb
@@ -76,7 +76,7 @@ module ActionCable
streams << [ broadcasting, callback ]
EM.next_tick do
- adapter.subscribe(broadcasting, callback, lambda do |reply|
+ pubsub.subscribe(broadcasting, callback, lambda do |reply|
transmit_subscription_confirmation
logger.info "#{self.class.name} is streaming from #{broadcasting}"
end)
@@ -92,13 +92,13 @@ module ActionCable
def stop_all_streams
streams.each do |broadcasting, callback|
- adapter.unsubscribe broadcasting, callback
+ pubsub.unsubscribe broadcasting, callback
logger.info "#{self.class.name} stopped streaming from #{broadcasting}"
end.clear
end
private
- delegate :adapter, to: :connection
+ delegate :pubsub, to: :connection
def streams
@_streams ||= []
diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb
index 2d7f99b09a..bb8850aaa0 100644
--- a/actioncable/lib/action_cable/connection/base.rb
+++ b/actioncable/lib/action_cable/connection/base.rb
@@ -49,7 +49,7 @@ module ActionCable
include Authorization
attr_reader :server, :env, :subscriptions, :logger
- delegate :worker_pool, :adapter, to: :server
+ delegate :worker_pool, :pubsub, to: :server
def initialize(server, env)
@server, @env = server, env
diff --git a/actioncable/lib/action_cable/connection/internal_channel.rb b/actioncable/lib/action_cable/connection/internal_channel.rb
index c618e9d087..54ed7672d2 100644
--- a/actioncable/lib/action_cable/connection/internal_channel.rb
+++ b/actioncable/lib/action_cable/connection/internal_channel.rb
@@ -15,14 +15,14 @@ module ActionCable
@_internal_subscriptions ||= []
@_internal_subscriptions << [ internal_channel, callback ]
- EM.next_tick { adapter.subscribe(internal_channel, callback) }
+ EM.next_tick { pubsub.subscribe(internal_channel, callback) }
logger.info "Registered connection (#{connection_identifier})"
end
end
def unsubscribe_from_internal_channel
if @_internal_subscriptions.present?
- @_internal_subscriptions.each { |channel, callback| EM.next_tick { adapter.unsubscribe(channel, callback) } }
+ @_internal_subscriptions.each { |channel, callback| EM.next_tick { pubsub.unsubscribe(channel, callback) } }
end
end
diff --git a/actioncable/lib/action_cable/server/base.rb b/actioncable/lib/action_cable/server/base.rb
index f44d0fdfb7..3385a4c9f3 100644
--- a/actioncable/lib/action_cable/server/base.rb
+++ b/actioncable/lib/action_cable/server/base.rb
@@ -46,8 +46,8 @@ module ActionCable
end
# Adapter used for all streams/broadcasting.
- def adapter
- @adapter ||= config.subscription_adapter.new(self)
+ def pubsub
+ @pubsub ||= config.pubsub_adapter.new(self)
end
# All the identifiers applied to the connection class associated with this server.
diff --git a/actioncable/lib/action_cable/server/broadcasting.rb b/actioncable/lib/action_cable/server/broadcasting.rb
index 021589b82d..4a26ed9269 100644
--- a/actioncable/lib/action_cable/server/broadcasting.rb
+++ b/actioncable/lib/action_cable/server/broadcasting.rb
@@ -39,7 +39,7 @@ module ActionCable
def broadcast(message)
server.logger.info "[ActionCable] Broadcasting to #{broadcasting}: #{message}"
- server.adapter.broadcast broadcasting, ActiveSupport::JSON.encode(message)
+ server.pubsub.broadcast broadcasting, ActiveSupport::JSON.encode(message)
end
end
end
diff --git a/actioncable/lib/action_cable/server/configuration.rb b/actioncable/lib/action_cable/server/configuration.rb
index cdf5e9eb1c..7bd67110a5 100644
--- a/actioncable/lib/action_cable/server/configuration.rb
+++ b/actioncable/lib/action_cable/server/configuration.rb
@@ -30,11 +30,20 @@ module ActionCable
end
end
- # Returns constant of subscription adapter specified in config/cable.yml
- # If the adapter cannot be found, this will default to the Redis adapter
- def subscription_adapter
- # Defaults to redis if no adapter is set
- adapter = cable.fetch('adapter') { 'redis' }
+ # Returns constant of subscription adapter specified in config/cable.yml.
+ # If the adapter cannot be found, this will default to the Redis adapter.
+ # Also makes sure proper dependencies are required.
+ def pubsub_adapter
+ adapter = (cable.fetch('adapter') { 'redis' })
+ path_to_adapter = "action_cable/subscription_adapter/#{adapter}"
+ begin
+ require path_to_adapter
+ rescue Gem::LoadError => e
+ raise Gem::LoadError, "Specified '#{adapter}' for Action Cable pubsub adapter, but the gem is not loaded. Add `gem '#{e.name}'` to your Gemfile (and ensure its version is at the minimum required by Action Cable)."
+ rescue LoadError => e
+ raise LoadError, "Could not load '#{path_to_adapter}'. Make sure that the adapter in config/cable.yml is valid. If you use an adapter other than 'postgresql' or 'redis' add the necessary adapter gem to the Gemfile.", e.backtrace
+ end
+ ###
adapter.camelize
adapter = 'PostgreSQL' if adapter == 'Postgresql'
"ActionCable::SubscriptionAdapter::#{adapter}".constantize
diff --git a/actioncable/lib/action_cable/subscription_adapter.rb b/actioncable/lib/action_cable/subscription_adapter.rb
index 287d2b9611..e770f4fb00 100644
--- a/actioncable/lib/action_cable/subscription_adapter.rb
+++ b/actioncable/lib/action_cable/subscription_adapter.rb
@@ -1,7 +1,5 @@
module ActionCable
module SubscriptionAdapter
autoload :Base, 'action_cable/subscription_adapter/base'
- autoload :PostgreSQL, 'action_cable/subscription_adapter/postgresql'
- autoload :Redis, 'action_cable/subscription_adapter/redis'
end
end
diff --git a/actioncable/lib/action_cable/subscription_adapter/postgresql.rb b/actioncable/lib/action_cable/subscription_adapter/postgresql.rb
index afa99355e8..64c519beed 100644
--- a/actioncable/lib/action_cable/subscription_adapter/postgresql.rb
+++ b/actioncable/lib/action_cable/subscription_adapter/postgresql.rb
@@ -1,11 +1,7 @@
+gem 'pg', '~> 0.18'
+require 'pg'
require 'thread'
-begin
- require 'pg'
-rescue Gem::LoadError => e
- raise Gem::LoadError, "You are trying to use the PostgreSQL ActionCable adapter, but do not have the proper gems installed. Add `gem 'pg'` to your Gemfile (and ensure its version is at the minimum required by ActionCable)."
-end
-
module ActionCable
module SubscriptionAdapter
class PostgreSQL < Base # :nodoc:
diff --git a/actioncable/lib/action_cable/subscription_adapter/redis.rb b/actioncable/lib/action_cable/subscription_adapter/redis.rb
index c6d8371f16..9615430be4 100644
--- a/actioncable/lib/action_cable/subscription_adapter/redis.rb
+++ b/actioncable/lib/action_cable/subscription_adapter/redis.rb
@@ -1,19 +1,18 @@
-begin
- require 'em-hiredis'
- require 'redis'
-rescue Gem::LoadError => e
- raise Gem::LoadError, "You are trying to use the Redis ActionCable adapter, but do not have the proper gems installed. Add `gem 'em-hiredis'` and `gem 'redis'` to your Gemfile (and ensure its version is at the minimum required by ActionCable)."
-end
+gem 'em-hiredis', '~> 0.3.0'
+gem 'redis', '~> 3.0'
+require 'em-hiredis'
+require 'redis'
module ActionCable
module SubscriptionAdapter
- class Redis < Base
+ class Redis < Base # :nodoc:
+ # The redis instance used for broadcasting. Not intended for direct user use.
def broadcast(channel, payload)
- redis_conn.publish(channel, payload)
+ broadcast_redis_connection.publish(channel, payload)
end
def subscribe(channel, message_callback, success_callback = nil)
- hi_redis_conn.pubsub.subscribe(channel, &message_callback).tap do |result|
+ subscription_redis_connection.pubsub.subscribe(channel, &message_callback).tap do |result|
result.callback(&success_callback) if success_callback
end
end
@@ -23,23 +22,17 @@ module ActionCable
end
private
-
- # The redis instance used for broadcasting. Not intended for direct user use.
- def redis_conn
- @broadcast ||= ::Redis.new(@server.config.config_opts)
- end
-
- # The EventMachine Redis instance used by the pubsub adapter.
- def hi_redis_conn
- @hi_redis_conn ||= EM::Hiredis.connect(@server.config.cable[:url]).tap do |redis|
- redis.on(:reconnect_failed) do
- @logger.info "[ActionCable] Redis reconnect failed."
+ def subscription_redis_connection
+ @subscription_redis_connection ||= EM::Hiredis.connect(@server.config.cable[:url]).tap do |redis|
+ redis.on(:reconnect_failed) do
+ @logger.info "[ActionCable] Redis reconnect failed."
+ end
end
end
- end
- def redis_conn
- @redis_conn ||= ::Redis.new(@server.config.cable)
- end
+
+ def broadcast_redis_connection
+ @broadcast_redis_connection ||= ::Redis.new(@server.config.cable)
+ end
end
end
end
--
cgit v1.2.3