Merge pull request #23976 from danielrhodes/enhancement/ac-ping-to-message-type

ActionCable: Add a "welcome" and "ping" message type

7 files changed, 24 insertions, 19 deletions

diff --git a/actioncable/app/assets/javascripts/action_cable/connection.coffee b/actioncable/app/assets/javascripts/action_cable/connection.coffee
index 4244322a1e..e8c9ae6bd0 100644
--- a/actioncable/app/assets/javascripts/action_cable/connection.coffee
+++ b/actioncable/app/assets/javascripts/action_cable/connection.coffee
@@ -73,8 +73,11 @@ class ActionCable.Connection
   events:
     message: (event) ->
       {identifier, message, type} = JSON.parse(event.data)
-
       switch type
+        when message_types.welcome
+          @consumer.connectionMonitor.connected()
+        when message_types.ping
+          @consumer.connectionMonitor.ping()
         when message_types.confirmation
           @consumer.subscriptions.notify(identifier, "connected")
         when message_types.rejection
diff --git a/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee b/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee
index 75a6f1fb07..740e86643e 100644
--- a/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee
+++ b/actioncable/app/assets/javascripts/action_cable/connection_monitor.coffee
@@ -7,10 +7,7 @@ class ActionCable.ConnectionMonitor
 
   @staleThreshold: 6 # Server::Connections::BEAT_INTERVAL * 2 (missed two pings)
 
-  identifier: ActionCable.INTERNAL.identifiers.ping
-
   constructor: (@consumer) ->
-    @consumer.subscriptions.add(this)
     @start()
 
   connected: ->
@@ -22,11 +19,12 @@ class ActionCable.ConnectionMonitor
   disconnected: ->
     @disconnectedAt = now()
 
-  received: ->
+  ping: ->
     @pingedAt = now()
 
   reset: ->
     @reconnectAttempts = 0
+    @consumer.connection.isOpen()
 
   start: ->
     @reset()
diff --git a/actioncable/app/assets/javascripts/action_cable/subscriptions.coffee b/actioncable/app/assets/javascripts/action_cable/subscriptions.coffee
index ae041ffa2b..2443bca14a 100644
--- a/actioncable/app/assets/javascripts/action_cable/subscriptions.coffee
+++ b/actioncable/app/assets/javascripts/action_cable/subscriptions.coffee
@@ -58,7 +58,5 @@ class ActionCable.Subscriptions
 
   sendCommand: (subscription, command) ->
     {identifier} = subscription
-    if identifier is ActionCable.INTERNAL.identifiers.ping
-      @consumer.connection.isOpen()
-    else
-      @consumer.send({command, identifier})
+    @consumer.send({command, identifier})
+      
diff --git a/actioncable/lib/action_cable.rb b/actioncable/lib/action_cable.rb
index 1dc66ef3ad..a8e4d1cb25 100644
--- a/actioncable/lib/action_cable.rb
+++ b/actioncable/lib/action_cable.rb
@@ -29,10 +29,9 @@ module ActionCable
   extend ActiveSupport::Autoload
 
   INTERNAL = {
-    identifiers: {
-      ping: '_ping'.freeze
-    },
     message_types: {
+      welcome: 'welcome'.freeze,
+      ping: 'ping'.freeze,
       confirmation: 'confirm_subscription'.freeze,
       rejection: 'reject_subscription'.freeze
     }
diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb
index afe0d958d7..f34f5eb109 100644
--- a/actioncable/lib/action_cable/connection/base.rb
+++ b/actioncable/lib/action_cable/connection/base.rb
@@ -115,7 +115,7 @@ module ActionCable
       end
 
       def beat
-        transmit ActiveSupport::JSON.encode(identifier: ActionCable::INTERNAL[:identifiers][:ping], message: Time.now.to_i)
+        transmit ActiveSupport::JSON.encode(type: ActionCable::INTERNAL[:message_types][:ping], message: Time.now.to_i)
       end
 
       def on_open # :nodoc:
@@ -155,7 +155,7 @@ module ActionCable
         def handle_open
           connect if respond_to?(:connect)
           subscribe_to_internal_channel
-          confirm_connection_monitor_subscription
+          send_welcome_message
 
           message_buffer.process!
           server.add_connection(self)
@@ -174,11 +174,11 @@ module ActionCable
           disconnect if respond_to?(:disconnect)
         end
 
-        def confirm_connection_monitor_subscription
-          # Send confirmation message to the internal connection monitor channel.
+        def send_welcome_message
+          # Send welcome message to the internal connection monitor channel.
           # This ensures the connection monitor state is reset after a successful
           # websocket connection.
-          transmit ActiveSupport::JSON.encode(identifier: ActionCable::INTERNAL[:identifiers][:ping], type: ActionCable::INTERNAL[:message_types][:confirmation])
+          transmit ActiveSupport::JSON.encode(type: ActionCable::INTERNAL[:message_types][:welcome])
         end
 
         def allow_request_origin?
diff --git a/actioncable/test/client_test.rb b/actioncable/test/client_test.rb
index a6619d3bd2..75545993da 100644
--- a/actioncable/test/client_test.rb
+++ b/actioncable/test/client_test.rb
@@ -75,7 +75,7 @@ class ClientTest < ActionCable::TestCase
 
       @ws.on(:message) do |event|
         hash = JSON.parse(event.data)
-        if hash['identifier'] == '_ping'
+        if hash['type'] == 'ping'
           @pings += 1
         else
           @messages << hash
@@ -146,6 +146,7 @@ class ClientTest < ActionCable::TestCase
   def test_single_client
     with_puma_server do |port|
       c = faye_client(port)
+      assert_equal({"type" => "welcome"}, c.read_message)  # pop the first welcome message off the stack
       c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
       assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
       c.send_message command: 'message', identifier: JSON.dump(channel: 'EchoChannel'), data: JSON.dump(action: 'ding', message: 'hello')
@@ -162,6 +163,7 @@ class ClientTest < ActionCable::TestCase
       barrier_2 = Concurrent::CyclicBarrier.new(clients.size)
 
       clients.map {|c| Concurrent::Future.execute {
+        assert_equal({"type" => "welcome"}, c.read_message)  # pop the first welcome message off the stack
         c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
         assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "type"=>"confirm_subscription"}, c.read_message)
         c.send_message command: 'message', identifier: JSON.dump(channel: 'EchoChannel'), data: JSON.dump(action: 'ding', message: 'hello')
@@ -181,6 +183,7 @@ class ClientTest < ActionCable::TestCase
       clients = 100.times.map { faye_client(port) }
 
       clients.map {|c| Concurrent::Future.execute {
+        assert_equal({"type" => "welcome"}, c.read_message)  # pop the first welcome message off the stack
         c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
         assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "type"=>"confirm_subscription"}, c.read_message)
         c.send_message command: 'message', identifier: JSON.dump(channel: 'EchoChannel'), data: JSON.dump(action: 'ding', message: 'hello')
@@ -194,12 +197,14 @@ class ClientTest < ActionCable::TestCase
   def test_disappearing_client
     with_puma_server do |port|
       c = faye_client(port)
+      assert_equal({"type" => "welcome"}, c.read_message)  # pop the first welcome message off the stack
       c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
       assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
       c.send_message command: 'message', identifier: JSON.dump(channel: 'EchoChannel'), data: JSON.dump(action: 'delay', message: 'hello')
       c.close # disappear before write
 
       c = faye_client(port)
+      assert_equal({"type" => "welcome"}, c.read_message) # pop the first welcome message off the stack
       c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
       assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
       c.send_message command: 'message', identifier: JSON.dump(channel: 'EchoChannel'), data: JSON.dump(action: 'ding', message: 'hello')
@@ -214,6 +219,7 @@ class ClientTest < ActionCable::TestCase
       identifier = JSON.dump(channel: 'EchoChannel')
 
       c = faye_client(port)
+      assert_equal({"type" => "welcome"}, c.read_message)
       c.send_message command: 'subscribe', identifier: identifier
       assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
       assert_equal(1, app.connections.count)
@@ -232,6 +238,7 @@ class ClientTest < ActionCable::TestCase
   def test_server_restart
     with_puma_server do |port|
       c = faye_client(port)
+      assert_equal({"type" => "welcome"}, c.read_message)
       c.send_message command: 'subscribe', identifier: JSON.dump(channel: 'EchoChannel')
       assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
 
diff --git a/actioncable/test/connection/base_test.rb b/actioncable/test/connection/base_test.rb
index fb11f9be64..fb83bd7d77 100644
--- a/actioncable/test/connection/base_test.rb
+++ b/actioncable/test/connection/base_test.rb
@@ -56,7 +56,7 @@ class ActionCable::Connection::BaseTest < ActionCable::TestCase
     run_in_eventmachine do
       connection = open_connection
 
-      connection.websocket.expects(:transmit).with({ identifier: "_ping", type: "confirm_subscription" }.to_json)
+      connection.websocket.expects(:transmit).with({ type: "welcome" }.to_json)
       connection.message_buffer.expects(:process!)
 
       connection.process