Merge branch 'master' into custom-discarded-job-handling

21 files changed, 75 insertions, 177 deletions

diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md
index 560ee89846..38bf842b14 100644
--- a/actioncable/CHANGELOG.md
+++ b/actioncable/CHANGELOG.md
@@ -1,3 +1,18 @@
+## Rails 5.2.0.beta2 (November 28, 2017) ##
+
+*   No changes.
+
+
+## Rails 5.2.0.beta1 (November 27, 2017) ##
+
+*   Removed deprecated evented redis adapter.
+
+    *Rafael Mendonça França*
+
+*   Support redis-rb 4.0.
+
+    *Jeremy Daer*
+
 *   Hash long stream identifiers when using PostgreSQL adapter.
 
     PostgreSQL has a limit on identifiers length (63 chars, [docs](https://www.postgresql.org/docs/current/static/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS)).
@@ -16,11 +31,4 @@
 
     *Marc Rendl Ignacio*
 
-*   Action Cable socket errors are now logged to the console
-
-    Previously any socket errors were ignored and this made it hard to diagnose socket issues (e.g. as discussed in #28362).
-
-    *Edward Poot*
-
-
 Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/actioncable/CHANGELOG.md) for previous changes.
diff --git a/actioncable/README.md b/actioncable/README.md
index a060e8938e..a05ef1dd20 100644
--- a/actioncable/README.md
+++ b/actioncable/README.md
@@ -446,7 +446,7 @@ The WebSocket server doesn't have access to the session, but it has access to th
 
 ## Dependencies
 
-Action Cable provides a subscription adapter interface to process its pubsub internals. By default, asynchronous, inline, PostgreSQL, evented Redis, and non-evented Redis adapters are included. The default adapter in new Rails applications is the asynchronous (`async`) adapter. To create your own adapter, you can look at `ActionCable::SubscriptionAdapter::Base` for all methods that must be implemented, and any of the adapters included within Action Cable as example implementations.
+Action Cable provides a subscription adapter interface to process its pubsub internals. By default, asynchronous, inline, PostgreSQL, and Redis adapters are included. The default adapter in new Rails applications is the asynchronous (`async`) adapter. To create your own adapter, you can look at `ActionCable::SubscriptionAdapter::Base` for all methods that must be implemented, and any of the adapters included within Action Cable as example implementations.
 
 The Ruby side of things is built on top of [websocket-driver](https://github.com/faye/websocket-driver-ruby), [nio4r](https://github.com/celluloid/nio4r), and [concurrent-ruby](https://github.com/ruby-concurrency/concurrent-ruby).
 
@@ -454,9 +454,9 @@ The Ruby side of things is built on top of [websocket-driver](https://github.com
 ## Deployment
 
 Action Cable is powered by a combination of WebSockets and threads. All of the
-connection management is handled internally by utilizing Ruby’s native thread
+connection management is handled internally by utilizing Ruby's native thread
 support, which means you can use all your regular Rails models with no problems
-as long as you haven’t committed any thread-safety sins.
+as long as you haven't committed any thread-safety sins.
 
 The Action Cable server does _not_ need to be a multi-threaded application server.
 This is because Action Cable uses the [Rack socket hijacking API](http://www.rubydoc.info/github/rack/rack/file/SPEC#Hijacking)
@@ -558,7 +558,7 @@ API documentation is at:
 
 * http://api.rubyonrails.org
 
-Bug reports can be filed for the Ruby on Rails project here:
+Bug reports for the Ruby on Rails project can be filed here:
 
 * https://github.com/rails/rails/issues
 
diff --git a/actioncable/lib/action_cable.rb b/actioncable/lib/action_cable.rb
index bd828b2d0f..001a043409 100644
--- a/actioncable/lib/action_cable.rb
+++ b/actioncable/lib/action_cable.rb
@@ -25,7 +25,7 @@
 
 require "active_support"
 require "active_support/rails"
-require_relative "action_cable/version"
+require "action_cable/version"
 
 module ActionCable
   extend ActiveSupport::Autoload
diff --git a/actioncable/lib/action_cable/connection/client_socket.rb b/actioncable/lib/action_cable/connection/client_socket.rb
index ba33c8b982..10289ab55c 100644
--- a/actioncable/lib/action_cable/connection/client_socket.rb
+++ b/actioncable/lib/action_cable/connection/client_socket.rb
@@ -21,7 +21,7 @@ module ActionCable
         return true if env["HTTP_X_FORWARDED_PROTO"] == "https"
         return true if env["rack.url_scheme"] == "https"
 
-        return false
+        false
       end
 
       CONNECTING = 0
diff --git a/actioncable/lib/action_cable/connection/subscriptions.rb b/actioncable/lib/action_cable/connection/subscriptions.rb
index faafd6d0a6..bb8d64e27a 100644
--- a/actioncable/lib/action_cable/connection/subscriptions.rb
+++ b/actioncable/lib/action_cable/connection/subscriptions.rb
@@ -43,7 +43,7 @@ module ActionCable
 
       def remove(data)
         logger.info "Unsubscribing from channel: #{data['identifier']}"
-        remove_subscription subscriptions[data["identifier"]]
+        remove_subscription find(data)
       end
 
       def remove_subscription(subscription)
diff --git a/actioncable/lib/action_cable/engine.rb b/actioncable/lib/action_cable/engine.rb
index c961f47342..53cbb597cd 100644
--- a/actioncable/lib/action_cable/engine.rb
+++ b/actioncable/lib/action_cable/engine.rb
@@ -2,7 +2,7 @@
 
 require "rails"
 require "action_cable"
-require_relative "helpers/action_cable_helper"
+require "action_cable/helpers/action_cable_helper"
 require "active_support/core_ext/hash/indifferent_access"
 
 module ActionCable
diff --git a/actioncable/lib/action_cable/gem_version.rb b/actioncable/lib/action_cable/gem_version.rb
index af8277d06e..d72ba18acd 100644
--- a/actioncable/lib/action_cable/gem_version.rb
+++ b/actioncable/lib/action_cable/gem_version.rb
@@ -10,7 +10,7 @@ module ActionCable
     MAJOR = 5
     MINOR = 2
     TINY  = 0
-    PRE   = "alpha"
+    PRE   = "beta2"
 
     STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".")
   end
diff --git a/actioncable/lib/action_cable/server/configuration.rb b/actioncable/lib/action_cable/server/configuration.rb
index 82fed81a18..26209537df 100644
--- a/actioncable/lib/action_cable/server/configuration.rb
+++ b/actioncable/lib/action_cable/server/configuration.rb
@@ -25,13 +25,26 @@ module ActionCable
       # Also makes sure proper dependencies are required.
       def pubsub_adapter
         adapter = (cable.fetch("adapter") { "redis" })
+
+        # Require the adapter itself and give useful feedback about
+        #   1. Missing adapter gems and
+        #   2. Adapter gems' missing dependencies.
         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
+          # We couldn't require the adapter itself. Raise an exception that
+          # points out config typos and missing gems.
+          if e.path == path_to_adapter
+            # We can assume that a non-builtin adapter was specified, so it's
+            # either misspelled or missing from Gemfile.
+            raise e.class, "Could not load the '#{adapter}' Action Cable pubsub adapter. Ensure that the adapter is spelled correctly in config/cable.yml and that you've added the necessary adapter gem to your Gemfile.", e.backtrace
+
+          # Bubbled up from the adapter require. Prefix the exception message
+          # with some guidance about how to address it and reraise.
+          else
+            raise e.class, "Error loading the '#{adapter}' Action Cable pubsub adapter. Missing a gem it depends on? #{e.message}", e.backtrace
+          end
         end
 
         adapter = adapter.camelize
diff --git a/actioncable/lib/action_cable/subscription_adapter/async.rb b/actioncable/lib/action_cable/subscription_adapter/async.rb
index 96c18c4a2f..c9930299c7 100644
--- a/actioncable/lib/action_cable/subscription_adapter/async.rb
+++ b/actioncable/lib/action_cable/subscription_adapter/async.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require_relative "inline"
+require "action_cable/subscription_adapter/inline"
 
 module ActionCable
   module SubscriptionAdapter
diff --git a/actioncable/lib/action_cable/subscription_adapter/evented_redis.rb b/actioncable/lib/action_cable/subscription_adapter/evented_redis.rb
deleted file mode 100644
index 07774810ce..0000000000
--- a/actioncable/lib/action_cable/subscription_adapter/evented_redis.rb
+++ /dev/null
@@ -1,89 +0,0 @@
-# frozen_string_literal: true
-
-require "thread"
-
-gem "em-hiredis", "~> 0.3.0"
-gem "redis", "~> 3.0"
-require "em-hiredis"
-require "redis"
-
-EventMachine.epoll  if EventMachine.epoll?
-EventMachine.kqueue if EventMachine.kqueue?
-
-module ActionCable
-  module SubscriptionAdapter
-    class EventedRedis < Base # :nodoc:
-      prepend ChannelPrefix
-
-      @@mutex = Mutex.new
-
-      # Overwrite this factory method for EventMachine Redis connections if you want to use a different Redis connection library than EM::Hiredis.
-      # This is needed, for example, when using Makara proxies for distributed Redis.
-      cattr_accessor :em_redis_connector, default: ->(config) { EM::Hiredis.connect(config[:url]) }
-
-      # Overwrite this factory method for Redis connections if you want to use a different Redis connection library than Redis.
-      # This is needed, for example, when using Makara proxies for distributed Redis.
-      cattr_accessor :redis_connector, default: ->(config) { ::Redis.new(url: config[:url]) }
-
-      def initialize(*)
-        ActiveSupport::Deprecation.warn(<<-MSG.squish)
-          The "evented_redis" subscription adapter is deprecated and
-          will be removed in Rails 5.2. Please use the "redis" adapter
-          instead.
-        MSG
-
-        super
-        @redis_connection_for_broadcasts = @redis_connection_for_subscriptions = nil
-      end
-
-      def broadcast(channel, payload)
-        redis_connection_for_broadcasts.publish(channel, payload)
-      end
-
-      def subscribe(channel, message_callback, success_callback = nil)
-        redis_connection_for_subscriptions.pubsub.subscribe(channel, &message_callback).tap do |result|
-          result.callback { |reply| success_callback.call } if success_callback
-        end
-      end
-
-      def unsubscribe(channel, message_callback)
-        redis_connection_for_subscriptions.pubsub.unsubscribe_proc(channel, message_callback)
-      end
-
-      def shutdown
-        redis_connection_for_subscriptions.pubsub.close_connection
-        @redis_connection_for_subscriptions = nil
-      end
-
-      private
-        def redis_connection_for_subscriptions
-          ensure_reactor_running
-          @redis_connection_for_subscriptions || @server.mutex.synchronize do
-            @redis_connection_for_subscriptions ||= self.class.em_redis_connector.call(@server.config.cable).tap do |redis|
-              redis.on(:reconnect_failed) do
-                @logger.error "[ActionCable] Redis reconnect failed."
-              end
-
-              redis.on(:failed) do
-                @logger.error "[ActionCable] Redis connection has failed."
-              end
-            end
-          end
-        end
-
-        def redis_connection_for_broadcasts
-          @redis_connection_for_broadcasts || @server.mutex.synchronize do
-            @redis_connection_for_broadcasts ||= self.class.redis_connector.call(@server.config.cable)
-          end
-        end
-
-        def ensure_reactor_running
-          return if EventMachine.reactor_running? && EventMachine.reactor_thread
-          @@mutex.synchronize do
-            Thread.new { EventMachine.run } unless EventMachine.reactor_running?
-            Thread.pass until EventMachine.reactor_running? && EventMachine.reactor_thread
-          end
-        end
-    end
-  end
-end
diff --git a/actioncable/lib/action_cable/subscription_adapter/redis.rb b/actioncable/lib/action_cable/subscription_adapter/redis.rb
index facea944ff..c28951608f 100644
--- a/actioncable/lib/action_cable/subscription_adapter/redis.rb
+++ b/actioncable/lib/action_cable/subscription_adapter/redis.rb
@@ -2,7 +2,7 @@
 
 require "thread"
 
-gem "redis", "~> 3.0"
+gem "redis", ">= 3", "< 5"
 require "redis"
 
 module ActionCable
@@ -76,7 +76,7 @@ module ActionCable
 
           def listen(conn)
             conn.without_reconnect do
-              original_client = conn.client
+              original_client = conn.respond_to?(:_client) ? conn._client : conn.client
 
               conn.subscribe("_action_cable_internal") do |on|
                 on.subscribe do |chan, count|
diff --git a/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb b/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb.tt
index d672697283..d672697283 100644
--- a/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb
+++ b/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb.tt
diff --git a/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb b/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb.tt
index 0ff5442f47..0ff5442f47 100644
--- a/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb
+++ b/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb.tt
diff --git a/actioncable/lib/rails/generators/channel/templates/assets/cable.js b/actioncable/lib/rails/generators/channel/templates/assets/cable.js.tt
index 739aa5f022..739aa5f022 100644
--- a/actioncable/lib/rails/generators/channel/templates/assets/cable.js
+++ b/actioncable/lib/rails/generators/channel/templates/assets/cable.js.tt
diff --git a/actioncable/lib/rails/generators/channel/templates/assets/channel.coffee b/actioncable/lib/rails/generators/channel/templates/assets/channel.coffee.tt
index 5467811aba..5467811aba 100644
--- a/actioncable/lib/rails/generators/channel/templates/assets/channel.coffee
+++ b/actioncable/lib/rails/generators/channel/templates/assets/channel.coffee.tt
diff --git a/actioncable/lib/rails/generators/channel/templates/assets/channel.js b/actioncable/lib/rails/generators/channel/templates/assets/channel.js.tt
index ab0e68b11a..ab0e68b11a 100644
--- a/actioncable/lib/rails/generators/channel/templates/assets/channel.js
+++ b/actioncable/lib/rails/generators/channel/templates/assets/channel.js.tt
diff --git a/actioncable/lib/rails/generators/channel/templates/channel.rb b/actioncable/lib/rails/generators/channel/templates/channel.rb.tt
index 4bcfb2be4d..4bcfb2be4d 100644
--- a/actioncable/lib/rails/generators/channel/templates/channel.rb
+++ b/actioncable/lib/rails/generators/channel/templates/channel.rb.tt
diff --git a/actioncable/package.json b/actioncable/package.json
index acec1e2e9c..8d7f9ff302 100644
--- a/actioncable/package.json
+++ b/actioncable/package.json
@@ -1,6 +1,6 @@
 {
   "name": "actioncable",
-  "version": "5.2.0-alpha",
+  "version": "5.2.0-beta2",
   "description": "WebSocket framework for Ruby on Rails.",
   "main": "lib/assets/compiled/action_cable.js",
   "files": [
diff --git a/actioncable/test/connection/client_socket_test.rb b/actioncable/test/connection/client_socket_test.rb
index 2051216010..5c31690c8b 100644
--- a/actioncable/test/connection/client_socket_test.rb
+++ b/actioncable/test/connection/client_socket_test.rb
@@ -67,9 +67,9 @@ class ActionCable::Connection::ClientSocketTest < ActionCable::TestCase
       env = Rack::MockRequest.env_for "/test",
         "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket",
         "HTTP_HOST" => "localhost", "HTTP_ORIGIN" => "http://rubyonrails.com"
-      io = \
+      io, client_io = \
         begin
-          Socket.pair(Socket::AF_UNIX, Socket::SOCK_STREAM, 0).first
+          Socket.pair(Socket::AF_UNIX, Socket::SOCK_STREAM, 0)
         rescue
           StringIO.new
         end
@@ -77,6 +77,14 @@ class ActionCable::Connection::ClientSocketTest < ActionCable::TestCase
 
       Connection.new(@server, env).tap do |connection|
         connection.process
+        if client_io
+          # Make sure server returns handshake response
+          Timeout.timeout(1) do
+            loop do
+              break if client_io.readline == "\r\n"
+            end
+          end
+        end
         connection.send :handle_open
         assert connection.connected
       end
diff --git a/actioncable/test/subscription_adapter/evented_redis_test.rb b/actioncable/test/subscription_adapter/evented_redis_test.rb
deleted file mode 100644
index e3e0a0c72a..0000000000
--- a/actioncable/test/subscription_adapter/evented_redis_test.rb
+++ /dev/null
@@ -1,61 +0,0 @@
-# frozen_string_literal: true
-
-require "test_helper"
-require_relative "common"
-require_relative "channel_prefix"
-
-class EventedRedisAdapterTest < ActionCable::TestCase
-  include CommonSubscriptionAdapterTest
-  include ChannelPrefixTest
-
-  def setup
-    assert_deprecated do
-      super
-    end
-
-    # em-hiredis is warning-rich
-    @previous_verbose, $VERBOSE = $VERBOSE, nil
-  end
-
-  def teardown
-    super
-
-    # Ensure EM is shut down before we re-enable warnings
-    EventMachine.reactor_thread.tap do |thread|
-      EventMachine.stop
-      thread.join
-    end
-
-    $VERBOSE = @previous_verbose
-  end
-
-  def test_slow_eventmachine
-    require "eventmachine"
-    require "thread"
-
-    lock = Mutex.new
-
-    EventMachine.singleton_class.class_eval do
-      alias_method :delayed_initialize_event_machine, :initialize_event_machine
-      define_method(:initialize_event_machine) do
-        lock.synchronize do
-          sleep 0.5
-          delayed_initialize_event_machine
-        end
-      end
-    end
-
-    test_basic_broadcast
-  ensure
-    lock.synchronize do
-      EventMachine.singleton_class.class_eval do
-        alias_method :initialize_event_machine, :delayed_initialize_event_machine
-        remove_method :delayed_initialize_event_machine
-      end
-    end
-  end
-
-  def cable_config
-    { adapter: "evented_redis", url: "redis://:password@127.0.0.1:6379/12" }
-  end
-end
diff --git a/actioncable/test/subscription_adapter/redis_test.rb b/actioncable/test/subscription_adapter/redis_test.rb
index 69120d5753..63823d6ef0 100644
--- a/actioncable/test/subscription_adapter/redis_test.rb
+++ b/actioncable/test/subscription_adapter/redis_test.rb
@@ -4,12 +4,15 @@ require "test_helper"
 require_relative "common"
 require_relative "channel_prefix"
 
+require "active_support/testing/method_call_assertions"
+require "action_cable/subscription_adapter/redis"
+
 class RedisAdapterTest < ActionCable::TestCase
   include CommonSubscriptionAdapterTest
   include ChannelPrefixTest
 
   def cable_config
-    { adapter: "redis", driver: "ruby", url: "redis://:password@127.0.0.1:6379/12" }
+    { adapter: "redis", driver: "ruby" }
   end
 end
 
@@ -23,6 +26,22 @@ class RedisAdapterTest::AlternateConfiguration < RedisAdapterTest
   def cable_config
     alt_cable_config = super.dup
     alt_cable_config.delete(:url)
-    alt_cable_config.merge(host: "127.0.0.1", port: 6379, db: 12, password: "password")
+    alt_cable_config.merge(host: "127.0.0.1", port: 6379, db: 12)
+  end
+end
+
+class RedisAdapterTest::Connector < ActiveSupport::TestCase
+  include ActiveSupport::Testing::MethodCallAssertions
+
+  test "slices url, host, port, db, and password from config" do
+    config = { url: 1, host: 2, port: 3, db: 4, password: 5 }
+
+    assert_called_with ::Redis, :new, [ config ] do
+      connect config.merge(other: "unrelated", stuff: "here")
+    end
+  end
+
+  def connect(config)
+    ActionCable::SubscriptionAdapter::Redis.redis_connector.call(config)
   end
 end