diff options
Diffstat (limited to 'actioncable')
107 files changed, 1671 insertions, 1050 deletions
diff --git a/actioncable/CHANGELOG.md b/actioncable/CHANGELOG.md index a767857607..38bf842b14 100644 --- a/actioncable/CHANGELOG.md +++ b/actioncable/CHANGELOG.md @@ -1,2 +1,34 @@ +## Rails 5.2.0.beta2 (November 28, 2017) ## -Please check [5-0-stable](https://github.com/rails/rails/blob/5-0-stable/actioncable/CHANGELOG.md) for previous changes. +* 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)). + Provided fix minifies identifiers longer than 63 chars by hashing them with SHA1. + + Fixes #28751. + + *Vladimir Dementyev* + +* Action Cable's `redis` adapter allows for other common redis-rb options (`host`, `port`, `db`, `password`) in cable.yml. + + Previously, it accepts only a [redis:// url](https://www.iana.org/assignments/uri-schemes/prov/redis) as an option. + While we can add all of these options to the `url` itself, it is not explicitly documented. This alternative setup + is shown as the first example in the [Redis rubygem](https://github.com/redis/redis-rb#getting-started), which + makes this set of options as sensible as using just the `url`. + + *Marc Rendl Ignacio* + +Please check [5-1-stable](https://github.com/rails/rails/blob/5-1-stable/actioncable/CHANGELOG.md) for previous changes. diff --git a/actioncable/MIT-LICENSE b/actioncable/MIT-LICENSE index 27a17cf41b..a42759f024 100644 --- a/actioncable/MIT-LICENSE +++ b/actioncable/MIT-LICENSE @@ -1,4 +1,4 @@ -Copyright (c) 2015-2016 Basecamp, LLC +Copyright (c) 2015-2018 Basecamp, LLC Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the diff --git a/actioncable/README.md b/actioncable/README.md index 8792113664..a05ef1dd20 100644 --- a/actioncable/README.md +++ b/actioncable/README.md @@ -7,7 +7,6 @@ and scalable. It's a full-stack offering that provides both a client-side JavaScript framework and a server-side Ruby framework. You have access to your full domain model written with Active Record or your ORM of choice. - ## Terminology A single Action Cable server can handle multiple connection instances. It has one @@ -52,10 +51,10 @@ module ApplicationCable self.current_user = find_verified_user end - protected + private def find_verified_user - if current_user = User.find_by(id: cookies.signed[:user_id]) - current_user + if verified_user = User.find_by(id: cookies.encrypted[:user_id]) + verified_user else reject_unauthorized_connection end @@ -168,7 +167,7 @@ App.cable.subscriptions.create "AppearanceChannel", buttonSelector = "[data-behavior~=appear_away]" install: -> - $(document).on "page:change.appearance", => + $(document).on "turbolinks:load.appearance", => @appear() $(document).on "click.appearance", buttonSelector, => @@ -300,7 +299,6 @@ The rebroadcast will be received by all connected clients, _including_ the clien See the [rails/actioncable-examples](https://github.com/rails/actioncable-examples) repository for a full example of how to setup Action Cable in a Rails app, and how to add channels. - ## Configuration Action Cable has three required configurations: a subscription adapter, allowed request origins, and the cable server URL (which can optionally be set on the client side). @@ -328,7 +326,10 @@ Rails.application.paths.add "config/cable", with: "somewhere/else/cable.yml" ### Allowed Request Origins -Action Cable will only accept requests from specified origins, which are passed to the server config as an array. The origins can be instances of strings or regular expressions, against which a check for match will be performed. +Action Cable will only accept requests from specific origins. + +By default, only an origin matching the cable server itself will be permitted. +Additional origins can be specified using strings or regular expressions, provided in an array. ```ruby Rails.application.config.action_cable.allowed_request_origins = ['http://rubyonrails.com', /http:\/\/ruby.*/] @@ -336,12 +337,19 @@ Rails.application.config.action_cable.allowed_request_origins = ['http://rubyonr When running in the development environment, this defaults to "http://localhost:3000". -To disable and allow requests from any origin: +To disable protection and allow requests from any origin: ```ruby Rails.application.config.action_cable.disable_request_forgery_protection = true ``` +To disable automatic access for same-origin requests, and strictly allow +only the configured origins: + +```ruby +Rails.application.config.action_cable.allow_same_origin_as_host = false +``` + ### Consumer Configuration Once you have decided how to run your cable server (see below), you must provide the server URL (or path) to your client-side setup. @@ -378,11 +386,11 @@ App.cable = ActionCable.createConsumer() ### Other Configurations -The other common option to configure is the log tags applied to the per-connection logger. Here's close to what we're using in Basecamp: +The other common option to configure is the log tags applied to the per-connection logger. Here's an example that uses the user account id if available, else "no-account" while tagging: ```ruby -Rails.application.config.action_cable.log_tags = [ - -> request { request.env['bc.account_id'] || "no-account" }, +config.action_cable.log_tags = [ + -> request { request.env['user_account_id'] || "no-account" }, :action_cable, -> request { request.uuid } ] @@ -401,14 +409,14 @@ application. The recommended basic setup is as follows: ```ruby # cable/config.ru -require ::File.expand_path('../../config/environment', __FILE__) +require_relative '../config/environment' Rails.application.eager_load! run ActionCable.server ``` Then you start the server using a binstub in bin/cable ala: -``` +```sh #!/bin/bash bundle exec puma -p 28080 cable/config.ru ``` @@ -430,7 +438,7 @@ For every instance of your server you create and for every worker your server sp ### Notes -Beware that currently the cable server will _not_ auto-reload any changes in the framework. As we've discussed, long-running cable connections mean long-running objects. We don't yet have a way of reloading the classes of those objects in a safe manner. So when you change your channels, or the model your channels use, you must restart the cable server. +Beware that currently, the cable server will _not_ auto-reload any changes in the framework. As we've discussed, long-running cable connections mean long-running objects. We don't yet have a way of reloading the classes of those objects in a safe manner. So when you change your channels, or the model your channels use, you must restart the cable server. We'll get all this abstracted properly when the framework is integrated into Rails. @@ -438,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). @@ -446,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) @@ -460,11 +468,88 @@ with all the popular application servers -- Unicorn, Puma and Passenger. Action Cable does not work with WEBrick, because WEBrick does not support the Rack socket hijacking API. +## Frontend assets + +Action Cable's frontend assets are distributed through two channels: the +official gem and npm package, both titled `actioncable`. + +### Gem usage + +Through the `actioncable` gem, Action Cable's frontend assets are +available through the Rails Asset Pipeline. Create a `cable.js` or +`cable.coffee` file (this is automatically done for you with Rails +generators), and then simply require the assets: + +In JavaScript... + +```javascript +//= require action_cable +``` + +... and in CoffeeScript: + +```coffeescript +#= require action_cable +``` + +### npm usage + +In addition to being available through the `actioncable` gem, Action Cable's +frontend JS assets are also bundled in an officially supported npm module, +intended for usage in standalone frontend applications that communicate with a +Rails application. A common use case for this could be if you have a decoupled +frontend application written in React, Ember.js, etc. and want to add real-time +WebSocket functionality. + +### Installation + +``` +npm install actioncable --save +``` + +### Usage + +The `ActionCable` constant is available as a `require`-able module, so +you only have to require the package to gain access to the API that is +provided. + +In JavaScript... + +```javascript +ActionCable = require('actioncable') + +var cable = ActionCable.createConsumer('wss://RAILS-API-PATH.com/cable') + +cable.subscriptions.create('AppearanceChannel', { + // normal channel code goes here... +}); +``` + +and in CoffeeScript... + +```coffeescript +ActionCable = require('actioncable') + +cable = ActionCable.createConsumer('wss://RAILS-API-PATH.com/cable') + +cable.subscriptions.create 'AppearanceChannel', + # normal channel code goes here... +``` + +## Download and Installation + +The latest version of Action Cable can be installed with [RubyGems](#gem-usage), +or with [npm](#npm-usage). + +Source code can be downloaded as part of the Rails project on GitHub + +* https://github.com/rails/rails/tree/master/actioncable + ## License Action Cable is released under the MIT license: -* http://www.opensource.org/licenses/MIT +* https://opensource.org/licenses/MIT ## Support @@ -473,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/Rakefile b/actioncable/Rakefile index 58c18dd457..226d171104 100644 --- a/actioncable/Rakefile +++ b/actioncable/Rakefile @@ -1,19 +1,17 @@ -require 'rake/testtask' -require 'pathname' -require 'sprockets' -require 'coffee-script' -require 'action_cable' +# frozen_string_literal: true -dir = File.dirname(__FILE__) +require "rake/testtask" +require "pathname" +require "open3" +require "action_cable" -task :default => :test +task default: :test -task :package => "assets:compile" -task "package:clean" => "assets:clean" +task package: %w( assets:compile assets:verify ) Rake::TestTask.new do |t| t.libs << "test" - t.test_files = Dir.glob("#{dir}/test/**/*_test.rb") + t.test_files = Dir.glob("#{__dir__}/test/**/*_test.rb") t.warning = true t.verbose = true t.ruby_opts = ["--dev"] if defined?(JRUBY_VERSION) @@ -22,49 +20,57 @@ end namespace :test do task :isolated do Dir.glob("test/**/*_test.rb").all? do |file| - sh(Gem.ruby, '-w', '-Ilib:test', file) - end or raise "Failures" + sh(Gem.ruby, "-w", "-Ilib:test", file) + end || raise("Failures") end - task :javascript do - require 'blade' - Blade.start(interface: :runner) + task :integration do + require "blade" + if ENV["CI"] + Blade.start(interface: :ci) + else + Blade.start(interface: :runner) + end end end namespace :assets do - root_path = Pathname.new(dir) - destination_path = root_path.join("lib/assets/compiled") - - desc "Compile dist/action_cable.js" + desc "Compile Action Cable assets" task :compile do - puts 'Compiling Action Cable assets...' + require "blade" + require "sprockets" + require "sprockets/export" + Blade.build + end - precompile_list = %w(action_cable.js) + desc "Verify compiled Action Cable assets" + task :verify do + file = "lib/assets/compiled/action_cable.js" + pathname = Pathname.new("#{__dir__}/#{file}") - environment = Sprockets::Environment.new - environment.gzip = false - Pathname.glob(root_path.join("app/assets/*/")) do |subdir| - environment.append_path subdir + print "[verify] #{file} exists " + if pathname.exist? + puts "[OK]" + else + $stderr.puts "[FAIL]" + fail end - compile_path = root_path.join("tmp/sprockets") - compile_path.rmtree if compile_path.exist? - compile_path.mkpath - - manifest = Sprockets::Manifest.new(environment.index, compile_path) - manifest.compile(precompile_list) - - destination_path.rmtree if destination_path.exist? - manifest.assets.each do |path, fingerprint_path| - destination_path.join(path).dirname.mkpath - FileUtils.cp(compile_path.join(fingerprint_path), destination_path.join(path)) + print "[verify] #{file} is a UMD module " + if pathname.read =~ /module\.exports.*define\.amd/m + puts "[OK]" + else + $stderr.puts "[FAIL]" + fail end - puts 'Done' - end - - task :clean do - destination_path.rmtree if destination_path.exist? + print "[verify] #{__dir__} can be required as a module " + _, stderr, status = Open3.capture3("node", "--print", "window = {}; require('#{__dir__}');") + if status.success? + puts "[OK]" + else + $stderr.puts "[FAIL]\n#{stderr}" + fail + end end end diff --git a/actioncable/actioncable.gemspec b/actioncable/actioncable.gemspec index f301049d4f..51db4dda3a 100644 --- a/actioncable/actioncable.gemspec +++ b/actioncable/actioncable.gemspec @@ -1,27 +1,32 @@ -version = File.read(File.expand_path('../../RAILS_VERSION', __FILE__)).strip +# frozen_string_literal: true + +version = File.read(File.expand_path("../RAILS_VERSION", __dir__)).strip Gem::Specification.new do |s| s.platform = Gem::Platform::RUBY - s.name = 'actioncable' + s.name = "actioncable" s.version = version - s.summary = 'WebSocket framework for Rails.' - s.description = 'Structure many real-time application concerns into channels over a single WebSocket connection.' + s.summary = "WebSocket framework for Rails." + s.description = "Structure many real-time application concerns into channels over a single WebSocket connection." - s.required_ruby_version = '>= 2.2.2' + s.required_ruby_version = ">= 2.2.2" - s.license = 'MIT' + s.license = "MIT" - s.author = ['Pratik Naik', 'David Heinemeier Hansson'] - s.email = ['pratiknaik@gmail.com', 'david@loudthinking.com'] - s.homepage = 'http://rubyonrails.org' + s.author = ["Pratik Naik", "David Heinemeier Hansson"] + s.email = ["pratiknaik@gmail.com", "david@loudthinking.com"] + s.homepage = "http://rubyonrails.org" - s.files = Dir['CHANGELOG.md', 'MIT-LICENSE', 'README.md', 'lib/**/*'] - s.require_path = 'lib' + s.files = Dir["CHANGELOG.md", "MIT-LICENSE", "README.md", "lib/**/*"] + s.require_path = "lib" - s.add_dependency 'actionpack', version + s.metadata = { + "source_code_uri" => "https://github.com/rails/rails/tree/v#{version}/actioncable", + "changelog_uri" => "https://github.com/rails/rails/blob/v#{version}/actioncable/CHANGELOG.md" + } - s.add_dependency 'nio4r', '~> 1.2' - s.add_dependency 'websocket-driver', '~> 0.6.1' + s.add_dependency "actionpack", version - s.add_development_dependency 'blade', '~> 0.5.1' + s.add_dependency "nio4r", "~> 2.0" + s.add_dependency "websocket-driver", ">= 0.6.1" end diff --git a/actioncable/app/assets/javascripts/action_cable.coffee.erb b/actioncable/app/assets/javascripts/action_cable.coffee.erb index f0422d9d9c..e0758dae72 100644 --- a/actioncable/app/assets/javascripts/action_cable.coffee.erb +++ b/actioncable/app/assets/javascripts/action_cable.coffee.erb @@ -1,8 +1,11 @@ +#= export ActionCable #= require_self #= require ./action_cable/consumer @ActionCable = INTERNAL: <%= ActionCable::INTERNAL.to_json %> + WebSocket: window.WebSocket + logger: window.console createConsumer: (url) -> url ?= @getConfig("url") ? @INTERNAL.default_mount_path @@ -32,4 +35,4 @@ log: (messages...) -> if @debugging messages.push(Date.now()) - console.log("[ActionCable]", messages...) + @logger.log("[ActionCable]", messages...) diff --git a/actioncable/app/assets/javascripts/action_cable/connection.coffee b/actioncable/app/assets/javascripts/action_cable/connection.coffee index d6a6397804..7fd68cad2f 100644 --- a/actioncable/app/assets/javascripts/action_cable/connection.coffee +++ b/actioncable/app/assets/javascripts/action_cable/connection.coffee @@ -23,11 +23,11 @@ class ActionCable.Connection open: => if @isActive() ActionCable.log("Attempted to open WebSocket, but existing socket is #{@getState()}") - throw new Error("Existing connection must be closed before opening") + false else ActionCable.log("Opening WebSocket, current state is #{@getState()}, subprotocols: #{protocols}") @uninstallEventHandlers() if @webSocket? - @webSocket = new WebSocket(@consumer.url, protocols) + @webSocket = new ActionCable.WebSocket(@consumer.url, protocols) @installEventHandlers() @monitor.start() true diff --git a/actioncable/bin/test b/actioncable/bin/test new file mode 100755 index 0000000000..c53377cc97 --- /dev/null +++ b/actioncable/bin/test @@ -0,0 +1,5 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +COMPONENT_ROOT = File.expand_path("..", __dir__) +require_relative "../../tools/test" diff --git a/actioncable/blade.yml b/actioncable/blade.yml index 7980f3d101..e38e9b2f1d 100644 --- a/actioncable/blade.yml +++ b/actioncable/blade.yml @@ -5,3 +5,30 @@ load_paths: logical_paths: - test.js + +build: + logical_paths: + - action_cable.js + path: lib/assets/compiled + clean: true + +plugins: + sauce_labs: + browsers: + Google Chrome: + os: Mac, Windows + version: -1 + Firefox: + os: Mac, Windows + version: -1 + Safari: + platform: Mac + version: -1 + Microsoft Edge: + version: -1 + Internet Explorer: + version: 11 + iPhone: + version: -1 + Motorola Droid 4 Emulator: + version: -1 diff --git a/actioncable/lib/action_cable.rb b/actioncable/lib/action_cable.rb index b6d2842867..e7456e3c1b 100644 --- a/actioncable/lib/action_cable.rb +++ b/actioncable/lib/action_cable.rb @@ -1,5 +1,7 @@ +# frozen_string_literal: true + #-- -# Copyright (c) 2015-2016 Basecamp, LLC +# Copyright (c) 2015-2018 Basecamp, LLC # # Permission is hereby granted, free of charge, to any person obtaining # a copy of this software and associated documentation files (the @@ -21,21 +23,21 @@ # WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. #++ -require 'active_support' -require 'active_support/rails' -require 'action_cable/version' +require "active_support" +require "active_support/rails" +require "action_cable/version" module ActionCable extend ActiveSupport::Autoload INTERNAL = { message_types: { - welcome: 'welcome'.freeze, - ping: 'ping'.freeze, - confirmation: 'confirm_subscription'.freeze, - rejection: 'reject_subscription'.freeze + welcome: "welcome".freeze, + ping: "ping".freeze, + confirmation: "confirm_subscription".freeze, + rejection: "reject_subscription".freeze }, - default_mount_path: '/cable'.freeze, + default_mount_path: "/cable".freeze, protocols: ["actioncable-v1-json".freeze, "actioncable-unsupported".freeze].freeze } diff --git a/actioncable/lib/action_cable/channel.rb b/actioncable/lib/action_cable/channel.rb index 7ae262ce5f..d2f6fbbbc7 100644 --- a/actioncable/lib/action_cable/channel.rb +++ b/actioncable/lib/action_cable/channel.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Channel extend ActiveSupport::Autoload diff --git a/actioncable/lib/action_cable/channel/base.rb b/actioncable/lib/action_cable/channel/base.rb index 845b747fc5..c5ad749bfe 100644 --- a/actioncable/lib/action_cable/channel/base.rb +++ b/actioncable/lib/action_cable/channel/base.rb @@ -1,4 +1,6 @@ -require 'set' +# frozen_string_literal: true + +require "set" module ActionCable module Channel @@ -122,16 +124,16 @@ module ActionCable end end - protected + private # action_methods are cached and there is sometimes need to refresh # them. ::clear_action_methods! allows you to do that, so next time # you run action_methods, they will be recalculated. - def clear_action_methods! + def clear_action_methods! # :doc: @action_methods = nil end # Refresh the cached action_methods when a new action_method is added. - def method_added(name) + def method_added(name) # :doc: super clear_action_methods! end @@ -144,13 +146,14 @@ module ActionCable # When a channel is streaming via pubsub, we want to delay the confirmation # transmission until pubsub subscription is confirmed. - @defer_subscription_confirmation = false + # + # The counter starts at 1 because it's awaiting a call to #subscribe_to_channel + @defer_subscription_confirmation_counter = Concurrent::AtomicFixnum.new(1) @reject_subscription = nil @subscription_confirmation_sent = nil delegate_connection_identifiers - subscribe_to_channel end # Extract the action name from the passed data and process it via the channel. The process will ensure @@ -169,6 +172,17 @@ module ActionCable end end + # This method is called after subscription has been added to the connection + # and confirms or rejects the subscription. + def subscribe_to_channel + run_callbacks :subscribe do + subscribed + end + + reject_subscription if subscription_rejected? + ensure_confirmation_sent + end + # Called by the cable connection when it's cut, so the channel has a chance to cleanup with callbacks. # This method is not intended to be called directly by the user. Instead, overwrite the #unsubscribed callback. def unsubscribe_from_channel # :nodoc: @@ -177,24 +191,25 @@ module ActionCable end end - - protected + private # Called once a consumer has become a subscriber of the channel. Usually the place to setup any streams # you want this channel to be sending to the subscriber. - def subscribed + def subscribed # :doc: # Override in subclasses end # Called once a consumer has cut its cable connection. Can be used for cleaning up connections or marking # users as offline or the like. - def unsubscribed + def unsubscribed # :doc: # Override in subclasses end # Transmit a hash of data to the subscriber. The hash will automatically be wrapped in a JSON envelope with # the proper channel identifier marked as the recipient. - def transmit(data, via: nil) - logger.info "#{self.class.name} transmitting #{data.inspect.truncate(300)}".tap { |m| m << " (via #{via})" if via } + def transmit(data, via: nil) # :doc: + status = "#{self.class.name} transmitting #{data.inspect.truncate(300)}" + status += " (via #{via})" if via + logger.debug(status) payload = { channel_class: self.class.name, data: data, via: via } ActiveSupport::Notifications.instrument("transmit.action_cable", payload) do @@ -202,27 +217,32 @@ module ActionCable end end - def defer_subscription_confirmation! - @defer_subscription_confirmation = true + def ensure_confirmation_sent # :doc: + return if subscription_rejected? + @defer_subscription_confirmation_counter.decrement + transmit_subscription_confirmation unless defer_subscription_confirmation? end - def defer_subscription_confirmation? - @defer_subscription_confirmation + def defer_subscription_confirmation! # :doc: + @defer_subscription_confirmation_counter.increment end - def subscription_confirmation_sent? + def defer_subscription_confirmation? # :doc: + @defer_subscription_confirmation_counter.value > 0 + end + + def subscription_confirmation_sent? # :doc: @subscription_confirmation_sent end - def reject + def reject # :doc: @reject_subscription = true end - def subscription_rejected? + def subscription_rejected? # :doc: @reject_subscription end - private def delegate_connection_identifiers connection.identifiers.each do |identifier| define_singleton_method(identifier) do @@ -231,24 +251,12 @@ module ActionCable end end - def subscribe_to_channel - run_callbacks :subscribe do - subscribed - end - - if subscription_rejected? - reject_subscription - else - transmit_subscription_confirmation unless defer_subscription_confirmation? - end - end - def extract_action(data) - (data['action'].presence || :receive).to_sym + (data["action"].presence || :receive).to_sym end def processable_action?(action) - self.class.action_methods.include?(action.to_s) + self.class.action_methods.include?(action.to_s) unless subscription_rejected? end def dispatch_action(action, data) @@ -262,8 +270,8 @@ module ActionCable end def action_signature(action, data) - "#{self.class.name}##{action}".tap do |signature| - if (arguments = data.except('action')).any? + "#{self.class.name}##{action}".dup.tap do |signature| + if (arguments = data.except("action")).any? signature << "(#{arguments.inspect})" end end diff --git a/actioncable/lib/action_cable/channel/broadcasting.rb b/actioncable/lib/action_cable/channel/broadcasting.rb index afc23d7d1a..acc791817b 100644 --- a/actioncable/lib/action_cable/channel/broadcasting.rb +++ b/actioncable/lib/action_cable/channel/broadcasting.rb @@ -1,4 +1,6 @@ -require 'active_support/core_ext/object/to_param' +# frozen_string_literal: true + +require "active_support/core_ext/object/to_param" module ActionCable module Channel @@ -16,7 +18,7 @@ module ActionCable def broadcasting_for(model) #:nodoc: case when model.is_a?(Array) - model.map { |m| broadcasting_for(m) }.join(':') + model.map { |m| broadcasting_for(m) }.join(":") when model.respond_to?(:to_gid_param) model.to_gid_param else diff --git a/actioncable/lib/action_cable/channel/callbacks.rb b/actioncable/lib/action_cable/channel/callbacks.rb index 295d750e86..4223c0d996 100644 --- a/actioncable/lib/action_cable/channel/callbacks.rb +++ b/actioncable/lib/action_cable/channel/callbacks.rb @@ -1,4 +1,6 @@ -require 'active_support/callbacks' +# frozen_string_literal: true + +require "active_support/callbacks" module ActionCable module Channel diff --git a/actioncable/lib/action_cable/channel/naming.rb b/actioncable/lib/action_cable/channel/naming.rb index 4c9d53b15a..03a5dcd3a0 100644 --- a/actioncable/lib/action_cable/channel/naming.rb +++ b/actioncable/lib/action_cable/channel/naming.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Channel module Naming @@ -10,8 +12,9 @@ module ActionCable # # ChatChannel.channel_name # => 'chat' # Chats::AppearancesChannel.channel_name # => 'chats:appearances' + # FooChats::BarAppearancesChannel.channel_name # => 'foo_chats:bar_appearances' def channel_name - @channel_name ||= name.sub(/Channel$/, '').gsub('::',':').underscore + @channel_name ||= name.sub(/Channel$/, "").gsub("::", ":").underscore end end diff --git a/actioncable/lib/action_cable/channel/periodic_timers.rb b/actioncable/lib/action_cable/channel/periodic_timers.rb index dab604440f..830b3efa3c 100644 --- a/actioncable/lib/action_cable/channel/periodic_timers.rb +++ b/actioncable/lib/action_cable/channel/periodic_timers.rb @@ -1,11 +1,12 @@ +# frozen_string_literal: true + module ActionCable module Channel module PeriodicTimers extend ActiveSupport::Concern included do - class_attribute :periodic_timers, instance_reader: false - self.periodic_timers = [] + class_attribute :periodic_timers, instance_reader: false, default: [] after_subscribe :start_periodic_timers after_unsubscribe :stop_periodic_timers @@ -30,7 +31,7 @@ module ActionCable def periodically(callback_or_method_name = nil, every:, &block) callback = if block_given? - raise ArgumentError, 'Pass a block or provide a callback arg, not both' if callback_or_method_name + raise ArgumentError, "Pass a block or provide a callback arg, not both" if callback_or_method_name block else case callback_or_method_name @@ -64,9 +65,7 @@ module ActionCable def start_periodic_timer(callback, every:) connection.server.event_loop.timer every do - connection.worker_pool.async_invoke connection do - instance_exec(&callback) - end + connection.worker_pool.async_exec self, connection: connection, &callback end end diff --git a/actioncable/lib/action_cable/channel/streams.rb b/actioncable/lib/action_cable/channel/streams.rb index 200c9d053c..81c2c38064 100644 --- a/actioncable/lib/action_cable/channel/streams.rb +++ b/actioncable/lib/action_cable/channel/streams.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Channel # Streams allow channels to route broadcastings to the subscriber. A broadcasting is, as discussed elsewhere, a pubsub queue where any data @@ -19,14 +21,14 @@ module ActionCable # end # # Based on the above example, the subscribers of this channel will get whatever data is put into the, - # let's say, `comments_for_45` broadcasting as soon as it's put there. + # let's say, <tt>comments_for_45</tt> broadcasting as soon as it's put there. # # An example broadcasting for this channel looks like so: # # ActionCable.server.broadcast "comments_for_45", author: 'DHH', content: 'Rails is just swell' # # If you have a stream that is related to a model, then the broadcasting used can be generated from the model and channel. - # The following example would subscribe to a broadcasting like `comments:Z2lkOi8vVGVzdEFwcC9Qb3N0LzE` + # The following example would subscribe to a broadcasting like <tt>comments:Z2lkOi8vVGVzdEFwcC9Qb3N0LzE</tt>. # # class CommentsChannel < ApplicationCable::Channel # def subscribed @@ -69,8 +71,8 @@ module ActionCable # Start streaming from the named <tt>broadcasting</tt> pubsub queue. Optionally, you can pass a <tt>callback</tt> that'll be used # instead of the default of just transmitting the updates straight to the subscriber. - # Pass `coder: ActiveSupport::JSON` to decode messages as JSON before passing to the callback. - # Defaults to `coder: nil` which does no decoding, passes raw messages. + # Pass <tt>coder: ActiveSupport::JSON</tt> to decode messages as JSON before passing to the callback. + # Defaults to <tt>coder: nil</tt> which does no decoding, passes raw messages. def stream_from(broadcasting, callback = nil, coder: nil, &block) broadcasting = String(broadcasting) @@ -84,7 +86,7 @@ module ActionCable connection.server.event_loop.post do pubsub.subscribe(broadcasting, handler, lambda do - transmit_subscription_confirmation + ensure_confirmation_sent logger.info "#{self.class.name} is streaming from #{broadcasting}" end) end @@ -94,8 +96,8 @@ module ActionCable # <tt>callback</tt> that'll be used instead of the default of just transmitting the updates straight # to the subscriber. # - # Pass `coder: ActiveSupport::JSON` to decode messages as JSON before passing to the callback. - # Defaults to `coder: nil` which does no decoding, passes raw messages. + # Pass <tt>coder: ActiveSupport::JSON</tt> to decode messages as JSON before passing to the callback. + # Defaults to <tt>coder: nil</tt> which does no decoding, passes raw messages. def stream_for(model, callback = nil, coder: nil, &block) stream_from(broadcasting_for([ channel_name, model ]), callback || block, coder: coder) end @@ -138,7 +140,7 @@ module ActionCable end # May be overridden to change the default stream handling behavior - # which decodes JSON and transmits to client. + # which decodes JSON and transmits to the client. # # TODO: Tests demonstrating this. # diff --git a/actioncable/lib/action_cable/connection.rb b/actioncable/lib/action_cable/connection.rb index 5f813cf8e0..804b89a707 100644 --- a/actioncable/lib/action_cable/connection.rb +++ b/actioncable/lib/action_cable/connection.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Connection extend ActiveSupport::Autoload @@ -8,8 +10,6 @@ module ActionCable autoload :ClientSocket autoload :Identification autoload :InternalChannel - autoload :FayeClientSocket - autoload :FayeEventLoop autoload :MessageBuffer autoload :Stream autoload :StreamEventLoop diff --git a/actioncable/lib/action_cable/connection/authorization.rb b/actioncable/lib/action_cable/connection/authorization.rb index 070a70e4e2..a22179d988 100644 --- a/actioncable/lib/action_cable/connection/authorization.rb +++ b/actioncable/lib/action_cable/connection/authorization.rb @@ -1,13 +1,15 @@ +# frozen_string_literal: true + module ActionCable module Connection module Authorization class UnauthorizedError < StandardError; end - private - def reject_unauthorized_connection - logger.error "An unauthorized connection attempt was rejected" - raise UnauthorizedError - end + # Closes the \WebSocket connection if it is open and returns a 404 "File not Found" response. + def reject_unauthorized_connection + logger.error "An unauthorized connection attempt was rejected" + raise UnauthorizedError + end end end -end
\ No newline at end of file +end diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb index cc4e0f8c8b..84053db9fd 100644 --- a/actioncable/lib/action_cable/connection/base.rb +++ b/actioncable/lib/action_cable/connection/base.rb @@ -1,8 +1,10 @@ -require 'action_dispatch' +# frozen_string_literal: true + +require "action_dispatch" module ActionCable module Connection - # For every WebSocket the Action Cable server accepts, a Connection object will be instantiated. This instance becomes the parent + # For every WebSocket connection the Action Cable server accepts, a Connection object will be instantiated. This instance becomes the parent # of all of the channel subscriptions that are created from there on. Incoming messages are then routed to these channel subscriptions # based on an identifier sent by the Action Cable consumer. The Connection itself does not deal with any specific application logic beyond # authentication and authorization. @@ -22,13 +24,10 @@ module ActionCable # # Any cleanup work needed when the cable connection is cut. # end # - # protected + # private # def find_verified_user - # if current_user = User.find_by_identity cookies.signed[:identity_id] - # current_user - # else + # User.find_by_identity(cookies.encrypted[:identity_id]) || # reject_unauthorized_connection - # end # end # end # end @@ -57,7 +56,7 @@ module ActionCable @worker_pool = server.worker_pool @logger = new_tagged_logger - @websocket = ActionCable::Connection::WebSocket.new(env, self, event_loop, server.config.client_socket_class) + @websocket = ActionCable::Connection::WebSocket.new(env, self, event_loop) @subscriptions = ActionCable::Connection::Subscriptions.new(self) @message_buffer = ActionCable::Connection::MessageBuffer.new(self) @@ -105,14 +104,14 @@ module ActionCable worker_pool.async_invoke(self, method, *arguments) end - # Return a basic hash of statistics for the connection keyed with `identifier`, `started_at`, and `subscriptions`. + # Return a basic hash of statistics for the connection keyed with <tt>identifier</tt>, <tt>started_at</tt>, <tt>subscriptions</tt>, and <tt>request_id</tt>. # This can be returned by a health check against the connection. def statistics { identifier: connection_identifier, started_at: @started_at, subscriptions: subscriptions.identifiers, - request_id: @env['action_dispatch.request_id'] + request_id: @env["action_dispatch.request_id"] } end @@ -129,16 +128,23 @@ module ActionCable end def on_error(message) # :nodoc: - # ignore + # log errors to make diagnosing socket errors easier + logger.error "WebSocket error occurred: #{message}" end def on_close(reason, code) # :nodoc: send_async :handle_close end + # TODO Change this to private once we've dropped Ruby 2.2 support. + # Workaround for Ruby 2.2 "private attribute?" warning. protected + attr_reader :websocket + attr_reader :message_buffer + + private # The request that initiated the WebSocket connection is available here. This gives access to the environment, cookies, etc. - def request + def request # :doc: @request ||= begin environment = Rails.application.env_config.merge(env) if defined?(Rails.application) && Rails.application ActionDispatch::Request.new(environment || env) @@ -146,14 +152,10 @@ module ActionCable end # The cookies of the request that initiated the WebSocket connection. Useful for performing authorization checks. - def cookies + def cookies # :doc: request.cookie_jar end - attr_reader :websocket - attr_reader :message_buffer - - private def encode(cable_message) @coder.encode cable_message end @@ -195,7 +197,10 @@ module ActionCable def allow_request_origin? return true if server.config.disable_request_forgery_protection - if Array(server.config.allowed_request_origins).any? { |allowed_origin| allowed_origin === env['HTTP_ORIGIN'] } + proto = Rack::Request.new(env).ssl? ? "https" : "http" + if server.config.allow_same_origin_as_host && env["HTTP_ORIGIN"] == "#{proto}://#{env['HTTP_HOST']}" + true + elsif Array(server.config.allowed_request_origins).any? { |allowed_origin| allowed_origin === env["HTTP_ORIGIN"] } true else logger.error("Request origin not allowed: #{env['HTTP_ORIGIN']}") @@ -213,7 +218,7 @@ module ActionCable logger.error invalid_request_message logger.info finished_request_message - [ 404, { 'Content-Type' => 'text/plain' }, [ 'Page not found' ] ] + [ 404, { "Content-Type" => "text/plain" }, [ "Page not found" ] ] end # Tags are declared in the server but computed in the connection. This allows us per-connection tailored tags. @@ -226,7 +231,7 @@ module ActionCable 'Started %s "%s"%s for %s at %s' % [ request.request_method, request.filtered_path, - websocket.possible? ? ' [WebSocket]' : '[non-WebSocket]', + websocket.possible? ? " [WebSocket]" : "[non-WebSocket]", request.ip, Time.now.to_s ] end @@ -234,19 +239,19 @@ module ActionCable def finished_request_message 'Finished "%s"%s for %s at %s' % [ request.filtered_path, - websocket.possible? ? ' [WebSocket]' : '[non-WebSocket]', + websocket.possible? ? " [WebSocket]" : "[non-WebSocket]", request.ip, Time.now.to_s ] end def invalid_request_message - 'Failed to upgrade to WebSocket (REQUEST_METHOD: %s, HTTP_CONNECTION: %s, HTTP_UPGRADE: %s)' % [ + "Failed to upgrade to WebSocket (REQUEST_METHOD: %s, HTTP_CONNECTION: %s, HTTP_UPGRADE: %s)" % [ env["REQUEST_METHOD"], env["HTTP_CONNECTION"], env["HTTP_UPGRADE"] ] end def successful_request_message - 'Successfully upgraded to WebSocket (REQUEST_METHOD: %s, HTTP_CONNECTION: %s, HTTP_UPGRADE: %s)' % [ + "Successfully upgraded to WebSocket (REQUEST_METHOD: %s, HTTP_CONNECTION: %s, HTTP_UPGRADE: %s)" % [ env["REQUEST_METHOD"], env["HTTP_CONNECTION"], env["HTTP_UPGRADE"] ] end diff --git a/actioncable/lib/action_cable/connection/client_socket.rb b/actioncable/lib/action_cable/connection/client_socket.rb index 6f29f32ea9..4b1964c4ae 100644 --- a/actioncable/lib/action_cable/connection/client_socket.rb +++ b/actioncable/lib/action_cable/connection/client_socket.rb @@ -1,4 +1,6 @@ -require 'websocket/driver' +# frozen_string_literal: true + +require "websocket/driver" module ActionCable module Connection @@ -8,18 +10,18 @@ module ActionCable # Copyright (c) 2010-2015 James Coglan class ClientSocket # :nodoc: def self.determine_url(env) - scheme = secure_request?(env) ? 'wss:' : 'ws:' + scheme = secure_request?(env) ? "wss:" : "ws:" "#{ scheme }//#{ env['HTTP_HOST'] }#{ env['REQUEST_URI'] }" end def self.secure_request?(env) - return true if env['HTTPS'] == 'on' - return true if env['HTTP_X_FORWARDED_SSL'] == 'on' - return true if env['HTTP_X_FORWARDED_SCHEME'] == 'https' - return true if env['HTTP_X_FORWARDED_PROTO'] == 'https' - return true if env['rack.url_scheme'] == 'https' + return true if env["HTTPS"] == "on" + return true if env["HTTP_X_FORWARDED_SSL"] == "on" + return true if env["HTTP_X_FORWARDED_SCHEME"] == "https" + return true if env["HTTP_X_FORWARDED_PROTO"] == "https" + return true if env["rack.url_scheme"] == "https" - return false + false end CONNECTING = 0 @@ -37,7 +39,7 @@ module ActionCable @url = ClientSocket.determine_url(@env) @driver = @driver_started = nil - @close_params = ['', 1006] + @close_params = ["", 1006] @ready_state = CONNECTING @@ -56,7 +58,7 @@ module ActionCable return if @driver.nil? || @driver_started @stream.hijack_rack_socket - if callback = @env['async.callback'] + if callback = @env["async.callback"] callback.call([101, {}, @stream]) end @@ -78,20 +80,20 @@ module ActionCable def transmit(message) return false if @ready_state > OPEN case message - when Numeric then @driver.text(message.to_s) - when String then @driver.text(message) - when Array then @driver.binary(message) - else false + when Numeric then @driver.text(message.to_s) + when String then @driver.text(message) + when Array then @driver.binary(message) + else false end end def close(code = nil, reason = nil) code ||= 1000 - reason ||= '' + reason ||= "" - unless code == 1000 or (code >= 3000 and code <= 4999) - raise ArgumentError, "Failed to execute 'close' on WebSocket: " + - "The code must be either 1000, or between 3000 and 4999. " + + unless code == 1000 || (code >= 3000 && code <= 4999) + raise ArgumentError, "Failed to execute 'close' on WebSocket: " \ + "The code must be either 1000, or between 3000 and 4999. " \ "#{code} is neither." end diff --git a/actioncable/lib/action_cable/connection/faye_client_socket.rb b/actioncable/lib/action_cable/connection/faye_client_socket.rb deleted file mode 100644 index a4bfe7db17..0000000000 --- a/actioncable/lib/action_cable/connection/faye_client_socket.rb +++ /dev/null @@ -1,48 +0,0 @@ -require 'faye/websocket' - -module ActionCable - module Connection - class FayeClientSocket - def initialize(env, event_target, stream_event_loop, protocols) - @env = env - @event_target = event_target - @protocols = protocols - - @faye = nil - end - - def alive? - @faye && @faye.ready_state == Faye::WebSocket::API::OPEN - end - - def transmit(data) - connect - @faye.send data - end - - def close - @faye && @faye.close - end - - def protocol - @faye && @faye.protocol - end - - def rack_response - connect - @faye.rack_response - end - - private - def connect - return if @faye - @faye = Faye::WebSocket.new(@env, @protocols) - - @faye.on(:open) { |event| @event_target.on_open } - @faye.on(:message) { |event| @event_target.on_message(event.data) } - @faye.on(:close) { |event| @event_target.on_close(event.reason, event.code) } - @faye.on(:error) { |event| @event_target.on_error(event.message) } - end - end - end -end diff --git a/actioncable/lib/action_cable/connection/faye_event_loop.rb b/actioncable/lib/action_cable/connection/faye_event_loop.rb deleted file mode 100644 index 9c44b38bc3..0000000000 --- a/actioncable/lib/action_cable/connection/faye_event_loop.rb +++ /dev/null @@ -1,44 +0,0 @@ -require 'thread' - -require 'eventmachine' -EventMachine.epoll if EventMachine.epoll? -EventMachine.kqueue if EventMachine.kqueue? - -module ActionCable - module Connection - class FayeEventLoop - @@mutex = Mutex.new - - def timer(interval, &block) - ensure_reactor_running - EMTimer.new(::EM::PeriodicTimer.new(interval, &block)) - end - - def post(task = nil, &block) - task ||= block - - ensure_reactor_running - ::EM.next_tick(&task) - end - - private - def ensure_reactor_running - return if EventMachine.reactor_running? - @@mutex.synchronize do - Thread.new { EventMachine.run } unless EventMachine.reactor_running? - Thread.pass until EventMachine.reactor_running? - end - end - - class EMTimer - def initialize(inner) - @inner = inner - end - - def shutdown - @inner.cancel - end - end - end - end -end diff --git a/actioncable/lib/action_cable/connection/identification.rb b/actioncable/lib/action_cable/connection/identification.rb index 4a54044aff..4b5f9ca115 100644 --- a/actioncable/lib/action_cable/connection/identification.rb +++ b/actioncable/lib/action_cable/connection/identification.rb @@ -1,4 +1,6 @@ -require 'set' +# frozen_string_literal: true + +require "set" module ActionCable module Connection @@ -6,8 +8,7 @@ module ActionCable extend ActiveSupport::Concern included do - class_attribute :identifiers - self.identifiers = Set.new + class_attribute :identifiers, default: Set.new end class_methods do diff --git a/actioncable/lib/action_cable/connection/internal_channel.rb b/actioncable/lib/action_cable/connection/internal_channel.rb index f70d52f99b..f03904137b 100644 --- a/actioncable/lib/action_cable/connection/internal_channel.rb +++ b/actioncable/lib/action_cable/connection/internal_channel.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Connection # Makes it possible for the RemoteConnection to disconnect a specific connection. @@ -27,8 +29,8 @@ module ActionCable end def process_internal_message(message) - case message['type'] - when 'disconnect' + case message["type"] + when "disconnect" logger.info "Removing connection (#{connection_identifier})" websocket.close end diff --git a/actioncable/lib/action_cable/connection/message_buffer.rb b/actioncable/lib/action_cable/connection/message_buffer.rb index 6a80770cae..f151a47072 100644 --- a/actioncable/lib/action_cable/connection/message_buffer.rb +++ b/actioncable/lib/action_cable/connection/message_buffer.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Connection # Allows us to buffer messages received from the WebSocket before the Connection has been fully initialized, and is ready to receive them. @@ -28,6 +30,8 @@ module ActionCable receive_buffered_messages end + # TODO Change this to private once we've dropped Ruby 2.2 support. + # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :connection attr_reader :buffered_messages diff --git a/actioncable/lib/action_cable/connection/stream.rb b/actioncable/lib/action_cable/connection/stream.rb index 0cf59091bc..4873026b71 100644 --- a/actioncable/lib/action_cable/connection/stream.rb +++ b/actioncable/lib/action_cable/connection/stream.rb @@ -1,3 +1,7 @@ +# frozen_string_literal: true + +require "thread" + module ActionCable module Connection #-- @@ -8,9 +12,13 @@ module ActionCable def initialize(event_loop, socket) @event_loop = event_loop @socket_object = socket - @stream_send = socket.env['stream.send'] + @stream_send = socket.env["stream.send"] @rack_hijack_io = nil + @write_lock = Mutex.new + + @write_head = nil + @write_buffer = Queue.new end def each(&callback) @@ -27,21 +35,71 @@ module ActionCable end def write(data) - return @rack_hijack_io.write(data) if @rack_hijack_io - return @stream_send.call(data) if @stream_send + if @stream_send + return @stream_send.call(data) + end + + if @write_lock.try_lock + begin + if @write_head.nil? && @write_buffer.empty? + written = @rack_hijack_io.write_nonblock(data, exception: false) + + case written + when :wait_writable + # proceed below + when data.bytesize + return data.bytesize + else + @write_head = data.byteslice(written, data.bytesize) + @event_loop.writes_pending @rack_hijack_io + + return data.bytesize + end + end + ensure + @write_lock.unlock + end + end + + @write_buffer << data + @event_loop.writes_pending @rack_hijack_io + + data.bytesize rescue EOFError, Errno::ECONNRESET @socket_object.client_gone end + def flush_write_buffer + @write_lock.synchronize do + loop do + if @write_head.nil? + return true if @write_buffer.empty? + @write_head = @write_buffer.pop + end + + written = @rack_hijack_io.write_nonblock(@write_head, exception: false) + case written + when :wait_writable + return false + when @write_head.bytesize + @write_head = nil + else + @write_head = @write_head.byteslice(written, @write_head.bytesize) + return false + end + end + end + end + def receive(data) @socket_object.parse(data) end def hijack_rack_socket - return unless @socket_object.env['rack.hijack'] + return unless @socket_object.env["rack.hijack"] - @socket_object.env['rack.hijack'].call - @rack_hijack_io = @socket_object.env['rack.hijack_io'] + @socket_object.env["rack.hijack"].call + @rack_hijack_io = @socket_object.env["rack.hijack_io"] @event_loop.attach(@rack_hijack_io, self) end diff --git a/actioncable/lib/action_cable/connection/stream_event_loop.rb b/actioncable/lib/action_cable/connection/stream_event_loop.rb index 2abad09c03..d95afc50ba 100644 --- a/actioncable/lib/action_cable/connection/stream_event_loop.rb +++ b/actioncable/lib/action_cable/connection/stream_event_loop.rb @@ -1,11 +1,13 @@ -require 'nio' -require 'thread' +# frozen_string_literal: true + +require "nio" +require "thread" module ActionCable module Connection class StreamEventLoop def initialize - @nio = @thread = nil + @nio = @executor = @thread = nil @map = {} @stopping = false @todo = Queue.new @@ -20,13 +22,14 @@ module ActionCable def post(task = nil, &block) task ||= block - Concurrent.global_io_executor << task + spawn + @executor << task end def attach(io, stream) @todo << lambda do - @map[io] = stream - @nio.register(io, :r) + @map[io] = @nio.register(io, :r) + @map[io].value = stream end wakeup end @@ -35,6 +38,16 @@ module ActionCable @todo << lambda do @nio.deregister io @map.delete io + io.close + end + wakeup + end + + def writes_pending(io) + @todo << lambda do + if monitor = @map[io] + monitor.interests = :rw + end end wakeup end @@ -52,6 +65,13 @@ module ActionCable return if @thread && @thread.status @nio ||= NIO::Selector.new + + @executor ||= Concurrent::ThreadPoolExecutor.new( + min_threads: 1, + max_threads: 10, + max_queue: 0, + ) + @thread = Thread.new { run } return true @@ -77,12 +97,25 @@ module ActionCable monitors.each do |monitor| io = monitor.io - stream = @map[io] + stream = monitor.value begin - stream.receive io.read_nonblock(4096) - rescue IO::WaitReadable - next + if monitor.writable? + if stream.flush_write_buffer + monitor.interests = :r + end + next unless monitor.readable? + end + + incoming = io.read_nonblock(4096, exception: false) + case incoming + when :wait_readable + next + when nil + stream.close + else + stream.receive incoming + end rescue # We expect one of EOFError or Errno::ECONNRESET in # normal operation (when the client goes away). But if diff --git a/actioncable/lib/action_cable/connection/subscriptions.rb b/actioncable/lib/action_cable/connection/subscriptions.rb index 3742f248d1..bb8d64e27a 100644 --- a/actioncable/lib/action_cable/connection/subscriptions.rb +++ b/actioncable/lib/action_cable/connection/subscriptions.rb @@ -1,4 +1,6 @@ -require 'active_support/core_ext/hash/indifferent_access' +# frozen_string_literal: true + +require "active_support/core_ext/hash/indifferent_access" module ActionCable module Connection @@ -11,33 +13,37 @@ module ActionCable end def execute_command(data) - case data['command'] - when 'subscribe' then add data - when 'unsubscribe' then remove data - when 'message' then perform_action data + case data["command"] + when "subscribe" then add data + when "unsubscribe" then remove data + when "message" then perform_action data else logger.error "Received unrecognized command in #{data.inspect}" end rescue Exception => e - logger.error "Could not execute command from #{data.inspect}) [#{e.class} - #{e.message}]: #{e.backtrace.first(5).join(" | ")}" + logger.error "Could not execute command from (#{data.inspect}) [#{e.class} - #{e.message}]: #{e.backtrace.first(5).join(" | ")}" end def add(data) - id_key = data['identifier'] + id_key = data["identifier"] id_options = ActiveSupport::JSON.decode(id_key).with_indifferent_access - subscription_klass = connection.server.channel_classes[id_options[:channel]] + return if subscriptions.key?(id_key) + + subscription_klass = id_options[:channel].safe_constantize - if subscription_klass - subscriptions[id_key] ||= subscription_klass.new(connection, id_key, id_options) + if subscription_klass && ActionCable::Channel::Base >= subscription_klass + subscription = subscription_klass.new(connection, id_key, id_options) + subscriptions[id_key] = subscription + subscription.subscribe_to_channel else - logger.error "Subscription class not found (#{data.inspect})" + logger.error "Subscription class not found: #{id_options[:channel].inspect}" end end def remove(data) logger.info "Unsubscribing from channel: #{data['identifier']}" - remove_subscription subscriptions[data['identifier']] + remove_subscription find(data) end def remove_subscription(subscription) @@ -46,7 +52,7 @@ module ActionCable end def perform_action(data) - find(data).perform_action ActiveSupport::JSON.decode(data['data']) + find(data).perform_action ActiveSupport::JSON.decode(data["data"]) end def identifiers @@ -57,6 +63,8 @@ module ActionCable subscriptions.each { |id, channel| remove_subscription(channel) } end + # TODO Change this to private once we've dropped Ruby 2.2 support. + # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :connection, :subscriptions @@ -64,7 +72,7 @@ module ActionCable delegate :logger, to: :connection def find(data) - if subscription = subscriptions[data['identifier']] + if subscription = subscriptions[data["identifier"]] subscription else raise "Unable to find subscription with identifier: #{data['identifier']}" diff --git a/actioncable/lib/action_cable/connection/tagged_logger_proxy.rb b/actioncable/lib/action_cable/connection/tagged_logger_proxy.rb index 41afa9680a..85831806a9 100644 --- a/actioncable/lib/action_cable/connection/tagged_logger_proxy.rb +++ b/actioncable/lib/action_cable/connection/tagged_logger_proxy.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Connection # Allows the use of per-connection tags against the server logger. This wouldn't work using the traditional @@ -31,8 +33,8 @@ module ActionCable end end - protected - def log(type, message) + private + def log(type, message) # :doc: tag(@logger) { @logger.send type, message } end end diff --git a/actioncable/lib/action_cable/connection/web_socket.rb b/actioncable/lib/action_cable/connection/web_socket.rb index 11f28c37e8..81233ace34 100644 --- a/actioncable/lib/action_cable/connection/web_socket.rb +++ b/actioncable/lib/action_cable/connection/web_socket.rb @@ -1,11 +1,13 @@ -require 'websocket/driver' +# frozen_string_literal: true + +require "websocket/driver" module ActionCable module Connection # Wrap the real socket to minimize the externally-presented API - class WebSocket - def initialize(env, event_target, event_loop, client_socket_class, protocols: ActionCable::INTERNAL[:protocols]) - @websocket = ::WebSocket::Driver.websocket?(env) ? client_socket_class.new(env, event_target, event_loop, protocols) : nil + class WebSocket # :nodoc: + def initialize(env, event_target, event_loop, protocols: ActionCable::INTERNAL[:protocols]) + @websocket = ::WebSocket::Driver.websocket?(env) ? ClientSocket.new(env, event_target, event_loop, protocols) : nil end def possible? @@ -32,6 +34,8 @@ module ActionCable websocket.rack_response end + # TODO Change this to private once we've dropped Ruby 2.2 support. + # Workaround for Ruby 2.2 "private attribute?" warning. protected attr_reader :websocket end diff --git a/actioncable/lib/action_cable/engine.rb b/actioncable/lib/action_cable/engine.rb index 8ce1b24962..53cbb597cd 100644 --- a/actioncable/lib/action_cable/engine.rb +++ b/actioncable/lib/action_cable/engine.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require "rails" require "action_cable" require "action_cable/helpers/action_cable_helper" @@ -22,7 +24,7 @@ module ActionCable initializer "action_cable.set_configs" do |app| options = app.config.action_cable - options.allowed_request_origins ||= "http://localhost:3000" if ::Rails.env.development? + options.allowed_request_origins ||= /https?:\/\/localhost:\d+/ if ::Rails.env.development? app.paths.add "config/cable", with: "config/cable.yml" @@ -31,13 +33,10 @@ module ActionCable self.cable = Rails.application.config_for(config_path).with_indifferent_access end - if 'ApplicationCable::Connection'.safe_constantize - self.connection_class = ApplicationCable::Connection - end - - self.channel_paths = Rails.application.paths['app/channels'].existent + previous_connection_class = connection_class + self.connection_class = -> { "ApplicationCable::Connection".safe_constantize || previous_connection_class.call } - options.each { |k,v| send("#{k}=", v) } + options.each { |k, v| send("#{k}=", v) } end end diff --git a/actioncable/lib/action_cable/gem_version.rb b/actioncable/lib/action_cable/gem_version.rb index 8ba0230d47..d72ba18acd 100644 --- a/actioncable/lib/action_cable/gem_version.rb +++ b/actioncable/lib/action_cable/gem_version.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable # Returns the version of the currently loaded Action Cable as a <tt>Gem::Version</tt>. def self.gem_version @@ -6,9 +8,9 @@ module ActionCable module VERSION MAJOR = 5 - MINOR = 1 + MINOR = 2 TINY = 0 - PRE = "alpha" + PRE = "beta2" STRING = [MAJOR, MINOR, TINY, PRE].compact.join(".") end diff --git a/actioncable/lib/action_cable/helpers/action_cable_helper.rb b/actioncable/lib/action_cable/helpers/action_cable_helper.rb index 2081a37db6..df16c02e83 100644 --- a/actioncable/lib/action_cable/helpers/action_cable_helper.rb +++ b/actioncable/lib/action_cable/helpers/action_cable_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Helpers module ActionCableHelper @@ -6,7 +8,7 @@ module ActionCable # # <head> # <%= action_cable_meta_tag %> - # <%= javascript_include_tag 'application', 'data-turbolinks-track' => true %> + # <%= javascript_include_tag 'application', 'data-turbolinks-track' => 'reload' %> # </head> # # This is then used by Action Cable to determine the URL of your WebSocket server. diff --git a/actioncable/lib/action_cable/remote_connections.rb b/actioncable/lib/action_cable/remote_connections.rb index a528024427..283400d9e7 100644 --- a/actioncable/lib/action_cable/remote_connections.rb +++ b/actioncable/lib/action_cable/remote_connections.rb @@ -1,3 +1,7 @@ +# frozen_string_literal: true + +require "active_support/core_ext/module/redefine_method" + module ActionCable # If you need to disconnect a given connection, you can go through the # RemoteConnections. You can find the connections you're looking for by @@ -41,20 +45,21 @@ module ActionCable # Uses the internal channel to disconnect the connection. def disconnect - server.broadcast internal_channel, type: 'disconnect' + server.broadcast internal_channel, type: "disconnect" end # Returns all the identifiers that were applied to this connection. - def identifiers + redefine_method :identifiers do server.connection_identifiers end - private + protected attr_reader :server + private def set_identifier_instance_vars(ids) raise InvalidIdentifiersError unless valid_identifiers?(ids) - ids.each { |k,v| instance_variable_set("@#{k}", v) } + ids.each { |k, v| instance_variable_set("@#{k}", v) } end def valid_identifiers?(ids) diff --git a/actioncable/lib/action_cable/server.rb b/actioncable/lib/action_cable/server.rb index bd6a3826a3..8d485a44f6 100644 --- a/actioncable/lib/action_cable/server.rb +++ b/actioncable/lib/action_cable/server.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Server extend ActiveSupport::Autoload @@ -9,7 +11,7 @@ module ActionCable autoload :Configuration autoload :Worker - autoload :ActiveRecordConnectionManagement, 'action_cable/server/worker/active_record_connection_management' + autoload :ActiveRecordConnectionManagement, "action_cable/server/worker/active_record_connection_management" end end end diff --git a/actioncable/lib/action_cable/server/base.rb b/actioncable/lib/action_cable/server/base.rb index b1a0e11631..1ee03f6dfc 100644 --- a/actioncable/lib/action_cable/server/base.rb +++ b/actioncable/lib/action_cable/server/base.rb @@ -1,4 +1,6 @@ -require 'monitor' +# frozen_string_literal: true + +require "monitor" module ActionCable module Server @@ -10,7 +12,7 @@ module ActionCable include ActionCable::Server::Broadcasting include ActionCable::Server::Connections - cattr_accessor(:config, instance_accessor: true) { ActionCable::Server::Configuration.new } + cattr_accessor :config, instance_accessor: true, default: ActionCable::Server::Configuration.new def self.logger; config.logger; end delegate :logger, to: :config @@ -19,16 +21,16 @@ module ActionCable def initialize @mutex = Monitor.new - @remote_connections = @event_loop = @worker_pool = @channel_classes = @pubsub = nil + @remote_connections = @event_loop = @worker_pool = @pubsub = nil end # Called by Rack to setup the server. def call(env) setup_heartbeat_timer - config.connection_class.new(self, env).process + config.connection_class.call.new(self, env).process end - # Disconnect all the connections identified by `identifiers` on this server or any others via RemoteConnections. + # Disconnect all the connections identified by +identifiers+ on this server or any others via RemoteConnections. def disconnect(identifiers) remote_connections.where(identifiers).disconnect end @@ -37,9 +39,13 @@ module ActionCable connections.each(&:close) @mutex.synchronize do - worker_pool.halt if @worker_pool - + # Shutdown the worker pool + @worker_pool.halt if @worker_pool @worker_pool = nil + + # Shutdown the pub/sub adapter + @pubsub.shutdown if @pubsub + @pubsub = nil end end @@ -49,34 +55,24 @@ module ActionCable end def event_loop - @event_loop || @mutex.synchronize { @event_loop ||= config.event_loop_class.new } + @event_loop || @mutex.synchronize { @event_loop ||= ActionCable::Connection::StreamEventLoop.new } end # The worker pool is where we run connection callbacks and channel actions. We do as little as possible on the server's main thread. # The worker pool is an executor service that's backed by a pool of threads working from a task queue. The thread pool size maxes out - # at 4 worker threads by default. Tune the size yourself with config.action_cable.worker_pool_size. + # at 4 worker threads by default. Tune the size yourself with <tt>config.action_cable.worker_pool_size</tt>. # # Using Active Record, Redis, etc within your channel actions means you'll get a separate connection from each thread in the worker pool. # Plan your deployment accordingly: 5 servers each running 5 Puma workers each running an 8-thread worker pool means at least 200 database # connections. # # Also, ensure that your database connection pool size is as least as large as your worker pool size. Otherwise, workers may oversubscribe - # the db connection pool and block while they wait for other workers to release their connections. Use a smaller worker pool or a larger - # db connection pool instead. + # the database connection pool and block while they wait for other workers to release their connections. Use a smaller worker pool or a larger + # database connection pool instead. def worker_pool @worker_pool || @mutex.synchronize { @worker_pool ||= ActionCable::Server::Worker.new(max_size: config.worker_pool_size) } end - # Requires and returns a hash of all of the channel class constants, which are keyed by name. - def channel_classes - @channel_classes || @mutex.synchronize do - @channel_classes ||= begin - config.channel_paths.each { |channel_path| require channel_path } - config.channel_class_names.each_with_object({}) { |name, hash| hash[name] = name.constantize } - end - end - end - # Adapter used for all streams/broadcasting. def pubsub @pubsub || @mutex.synchronize { @pubsub ||= config.pubsub_adapter.new(self) } @@ -84,7 +80,7 @@ module ActionCable # All of the identifiers applied to the connection class associated with this server. def connection_identifiers - config.connection_class.identifiers + config.connection_class.call.identifiers end end diff --git a/actioncable/lib/action_cable/server/broadcasting.rb b/actioncable/lib/action_cable/server/broadcasting.rb index 8f93564113..bc54d784b3 100644 --- a/actioncable/lib/action_cable/server/broadcasting.rb +++ b/actioncable/lib/action_cable/server/broadcasting.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Server # Broadcasting is how other parts of your application can send messages to a channel's subscribers. As explained in Channel, most of the time, these @@ -38,9 +40,13 @@ module ActionCable end def broadcast(message) - server.logger.info "[ActionCable] Broadcasting to #{broadcasting}: #{message.inspect}" - encoded = coder ? coder.encode(message) : message - server.pubsub.broadcast broadcasting, encoded + server.logger.debug "[ActionCable] Broadcasting to #{broadcasting}: #{message.inspect}" + + payload = { broadcasting: broadcasting, message: message, coder: coder } + ActiveSupport::Notifications.instrument("broadcast.action_cable", payload) do + encoded = coder ? coder.encode(message) : message + server.pubsub.broadcast broadcasting, encoded + end end end end diff --git a/actioncable/lib/action_cable/server/configuration.rb b/actioncable/lib/action_cable/server/configuration.rb index 0bb378cf03..26209537df 100644 --- a/actioncable/lib/action_cable/server/configuration.rb +++ b/actioncable/lib/action_cable/server/configuration.rb @@ -1,64 +1,56 @@ +# frozen_string_literal: true + module ActionCable module Server # An instance of this configuration object is available via ActionCable.server.config, which allows you to tweak Action Cable configuration # in a Rails config initializer. class Configuration attr_accessor :logger, :log_tags - attr_accessor :use_faye, :connection_class, :worker_pool_size - attr_accessor :disable_request_forgery_protection, :allowed_request_origins + attr_accessor :connection_class, :worker_pool_size + attr_accessor :disable_request_forgery_protection, :allowed_request_origins, :allow_same_origin_as_host attr_accessor :cable, :url, :mount_path - attr_accessor :channel_paths # :nodoc: - def initialize @log_tags = [] - @connection_class = ActionCable::Connection::Base + @connection_class = -> { ActionCable::Connection::Base } @worker_pool_size = 4 @disable_request_forgery_protection = false - end - - def channel_class_names - @channel_class_names ||= channel_paths.collect do |channel_path| - Pathname.new(channel_path).basename.to_s.split('.').first.camelize - end + @allow_same_origin_as_host = true end # 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' }) + 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 - adapter = 'PostgreSQL' if adapter == 'Postgresql' + adapter = "PostgreSQL" if adapter == "Postgresql" "ActionCable::SubscriptionAdapter::#{adapter}".constantize end - - def event_loop_class - if use_faye - ActionCable::Connection::FayeEventLoop - else - ActionCable::Connection::StreamEventLoop - end - end - - def client_socket_class - if use_faye - ActionCable::Connection::FayeClientSocket - else - ActionCable::Connection::ClientSocket - end - end end end end diff --git a/actioncable/lib/action_cable/server/connections.rb b/actioncable/lib/action_cable/server/connections.rb index 5e61b4e335..39557d63a7 100644 --- a/actioncable/lib/action_cable/server/connections.rb +++ b/actioncable/lib/action_cable/server/connections.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Server # Collection class for all the connections that have been established on this specific server. Remember, usually you'll run many Action Cable servers, so diff --git a/actioncable/lib/action_cable/server/worker.rb b/actioncable/lib/action_cable/server/worker.rb index a638ff72e7..c69cc4ac31 100644 --- a/actioncable/lib/action_cable/server/worker.rb +++ b/actioncable/lib/action_cable/server/worker.rb @@ -1,6 +1,8 @@ -require 'active_support/callbacks' -require 'active_support/core_ext/module/attribute_accessors_per_thread' -require 'concurrent' +# frozen_string_literal: true + +require "active_support/callbacks" +require "active_support/core_ext/module/attribute_accessors_per_thread" +require "concurrent" module ActionCable module Server @@ -25,7 +27,7 @@ module ActionCable # Stop processing work: any work that has not already started # running will be discarded from the queue def halt - @executor.kill + @executor.shutdown end def stopping? @@ -42,16 +44,20 @@ module ActionCable self.connection = nil end - def async_invoke(receiver, method, *args, connection: receiver) + def async_exec(receiver, *args, connection:, &block) + async_invoke receiver, :instance_exec, *args, connection: connection, &block + end + + def async_invoke(receiver, method, *args, connection: receiver, &block) @executor.post do - invoke(receiver, method, *args, connection: connection) + invoke(receiver, method, *args, connection: connection, &block) end end - def invoke(receiver, method, *args, connection:) + def invoke(receiver, method, *args, connection:, &block) work(connection) do begin - receiver.send method, *args + receiver.send method, *args, &block rescue Exception => e logger.error "There was an exception - #{e.class}(#{e.message})" logger.error e.backtrace.join("\n") diff --git a/actioncable/lib/action_cable/server/worker/active_record_connection_management.rb b/actioncable/lib/action_cable/server/worker/active_record_connection_management.rb index c1e4aa8103..2e378d4bf3 100644 --- a/actioncable/lib/action_cable/server/worker/active_record_connection_management.rb +++ b/actioncable/lib/action_cable/server/worker/active_record_connection_management.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module Server class Worker diff --git a/actioncable/lib/action_cable/subscription_adapter.rb b/actioncable/lib/action_cable/subscription_adapter.rb index 72e62f3daf..bcece8d33b 100644 --- a/actioncable/lib/action_cable/subscription_adapter.rb +++ b/actioncable/lib/action_cable/subscription_adapter.rb @@ -1,8 +1,11 @@ +# frozen_string_literal: true + module ActionCable module SubscriptionAdapter extend ActiveSupport::Autoload autoload :Base autoload :SubscriberMap + autoload :ChannelPrefix end end diff --git a/actioncable/lib/action_cable/subscription_adapter/async.rb b/actioncable/lib/action_cable/subscription_adapter/async.rb index 10b3ac8cd8..c9930299c7 100644 --- a/actioncable/lib/action_cable/subscription_adapter/async.rb +++ b/actioncable/lib/action_cable/subscription_adapter/async.rb @@ -1,4 +1,6 @@ -require 'action_cable/subscription_adapter/inline' +# frozen_string_literal: true + +require "action_cable/subscription_adapter/inline" module ActionCable module SubscriptionAdapter diff --git a/actioncable/lib/action_cable/subscription_adapter/base.rb b/actioncable/lib/action_cable/subscription_adapter/base.rb index 796db5ffa3..34077707fd 100644 --- a/actioncable/lib/action_cable/subscription_adapter/base.rb +++ b/actioncable/lib/action_cable/subscription_adapter/base.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module SubscriptionAdapter class Base diff --git a/actioncable/lib/action_cable/subscription_adapter/channel_prefix.rb b/actioncable/lib/action_cable/subscription_adapter/channel_prefix.rb new file mode 100644 index 0000000000..df0aa040f5 --- /dev/null +++ b/actioncable/lib/action_cable/subscription_adapter/channel_prefix.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module ActionCable + module SubscriptionAdapter + module ChannelPrefix # :nodoc: + def broadcast(channel, payload) + channel = channel_with_prefix(channel) + super + end + + def subscribe(channel, callback, success_callback = nil) + channel = channel_with_prefix(channel) + super + end + + def unsubscribe(channel, callback) + channel = channel_with_prefix(channel) + super + end + + private + # Returns the channel name, including channel_prefix specified in cable.yml + def channel_with_prefix(channel) + [@server.config.cable[:channel_prefix], channel].compact.join(":") + end + end + end +end 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 4735a4bfa8..0000000000 --- a/actioncable/lib/action_cable/subscription_adapter/evented_redis.rb +++ /dev/null @@ -1,79 +0,0 @@ -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: - @@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) { ->(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) { ->(config) { ::Redis.new(url: config[:url]) } } - - def initialize(*) - 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? - @@mutex.synchronize do - Thread.new { EventMachine.run } unless EventMachine.reactor_running? - Thread.pass until EventMachine.reactor_running? - end - end - end - end -end diff --git a/actioncable/lib/action_cable/subscription_adapter/inline.rb b/actioncable/lib/action_cable/subscription_adapter/inline.rb index 81357faead..d2c85c1c8d 100644 --- a/actioncable/lib/action_cable/subscription_adapter/inline.rb +++ b/actioncable/lib/action_cable/subscription_adapter/inline.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module ActionCable module SubscriptionAdapter class Inline < Base # :nodoc: diff --git a/actioncable/lib/action_cable/subscription_adapter/postgresql.rb b/actioncable/lib/action_cable/subscription_adapter/postgresql.rb index 66c7852f6e..e384ea4afb 100644 --- a/actioncable/lib/action_cable/subscription_adapter/postgresql.rb +++ b/actioncable/lib/action_cable/subscription_adapter/postgresql.rb @@ -1,6 +1,9 @@ -gem 'pg', '~> 0.18' -require 'pg' -require 'thread' +# frozen_string_literal: true + +gem "pg", ">= 0.18", "< 2.0" +require "pg" +require "thread" +require "digest/sha1" module ActionCable module SubscriptionAdapter @@ -12,16 +15,16 @@ module ActionCable def broadcast(channel, payload) with_connection do |pg_conn| - pg_conn.exec("NOTIFY #{pg_conn.escape_identifier(channel)}, '#{pg_conn.escape_string(payload)}'") + pg_conn.exec("NOTIFY #{pg_conn.escape_identifier(channel_identifier(channel))}, '#{pg_conn.escape_string(payload)}'") end end def subscribe(channel, callback, success_callback = nil) - listener.add_subscriber(channel, callback, success_callback) + listener.add_subscriber(channel_identifier(channel), callback, success_callback) end def unsubscribe(channel, callback) - listener.remove_subscriber(channel, callback) + listener.remove_subscriber(channel_identifier(channel), callback) end def shutdown @@ -33,7 +36,7 @@ module ActionCable pg_conn = ar_conn.raw_connection unless pg_conn.is_a?(PG::Connection) - raise 'ActiveRecord database must be Postgres in order to use the Postgres ActionCable storage adapter' + raise "The Active Record database must be PostgreSQL in order to use the PostgreSQL Action Cable storage adapter" end yield pg_conn @@ -41,6 +44,10 @@ module ActionCable end private + def channel_identifier(channel) + channel.size > 63 ? Digest::SHA1.hexdigest(channel) : channel + end + def listener @listener || @server.mutex.synchronize { @listener ||= Listener.new(self, @server.event_loop) } end diff --git a/actioncable/lib/action_cable/subscription_adapter/redis.rb b/actioncable/lib/action_cable/subscription_adapter/redis.rb index 65434f7107..c28951608f 100644 --- a/actioncable/lib/action_cable/subscription_adapter/redis.rb +++ b/actioncable/lib/action_cable/subscription_adapter/redis.rb @@ -1,14 +1,20 @@ -require 'thread' +# frozen_string_literal: true -gem 'redis', '~> 3.0' -require 'redis' +require "thread" + +gem "redis", ">= 3", "< 5" +require "redis" module ActionCable module SubscriptionAdapter class Redis < Base # :nodoc: - # Overwrite this factory method for redis connections if you want to use a different Redis library than Redis. + prepend ChannelPrefix + + # Overwrite this factory method for Redis connections if you want to use a different Redis library than the redis gem. # This is needed, for example, when using Makara proxies for distributed Redis. - cattr_accessor(:redis_connector) { ->(config) { ::Redis.new(url: config[:url]) } } + cattr_accessor :redis_connector, default: ->(config) do + ::Redis.new(config.slice(:url, :host, :port, :db, :password)) + end def initialize(*) super @@ -70,9 +76,9 @@ 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| + conn.subscribe("_action_cable_internal") do |on| on.subscribe do |chan, count| @subscription_lock.synchronize do if count == 1 @@ -111,7 +117,7 @@ module ActionCable return if @thread.nil? when_connected do - send_command('unsubscribe') + send_command("unsubscribe") @raw_client = nil end end @@ -123,13 +129,13 @@ module ActionCable @subscription_lock.synchronize do ensure_listener_running @subscribe_callbacks[channel] << on_success - when_connected { send_command('subscribe', channel) } + when_connected { send_command("subscribe", channel) } end end def remove_channel(channel) @subscription_lock.synchronize do - when_connected { send_command('unsubscribe', channel) } + when_connected { send_command("unsubscribe", channel) } end end diff --git a/actioncable/lib/action_cable/subscription_adapter/subscriber_map.rb b/actioncable/lib/action_cable/subscription_adapter/subscriber_map.rb index 37eed09793..01cdc2dfa1 100644 --- a/actioncable/lib/action_cable/subscription_adapter/subscriber_map.rb +++ b/actioncable/lib/action_cable/subscription_adapter/subscriber_map.rb @@ -1,8 +1,10 @@ +# frozen_string_literal: true + module ActionCable module SubscriptionAdapter class SubscriberMap def initialize - @subscribers = Hash.new { |h,k| h[k] = [] } + @subscribers = Hash.new { |h, k| h[k] = [] } @sync = Mutex.new end @@ -32,7 +34,11 @@ module ActionCable end def broadcast(channel, message) - list = @sync.synchronize { @subscribers[channel].dup } + list = @sync.synchronize do + return if !@subscribers.key?(channel) + @subscribers[channel].dup + end + list.each do |subscriber| invoke_callback(subscriber, message) end diff --git a/actioncable/lib/action_cable/version.rb b/actioncable/lib/action_cable/version.rb index e17877202b..86115c6065 100644 --- a/actioncable/lib/action_cable/version.rb +++ b/actioncable/lib/action_cable/version.rb @@ -1,4 +1,6 @@ -require_relative 'gem_version' +# frozen_string_literal: true + +require_relative "gem_version" module ActionCable # Returns the version of the currently loaded Action Cable as a <tt>Gem::Version</tt> diff --git a/actioncable/lib/rails/generators/channel/USAGE b/actioncable/lib/rails/generators/channel/USAGE index 6249553c22..dd109fda80 100644 --- a/actioncable/lib/rails/generators/channel/USAGE +++ b/actioncable/lib/rails/generators/channel/USAGE @@ -3,7 +3,7 @@ Description: Stubs out a new cable channel for the server (in Ruby) and client (in CoffeeScript). Pass the channel name, either CamelCased or under_scored, and an optional list of channel actions as arguments. - Note: Turn on the cable connection in app/assets/javascript/cable.js after generating any channels. + Note: Turn on the cable connection in app/assets/javascripts/cable.js after generating any channels. Example: ======== @@ -11,4 +11,4 @@ Example: creates a Chat channel class and CoffeeScript asset: Channel: app/channels/chat_channel.rb - Assets: app/assets/javascript/channels/chat.coffee + Assets: app/assets/javascripts/channels/chat.coffee diff --git a/actioncable/lib/rails/generators/channel/channel_generator.rb b/actioncable/lib/rails/generators/channel/channel_generator.rb index 05fd21a954..c3528370c6 100644 --- a/actioncable/lib/rails/generators/channel/channel_generator.rb +++ b/actioncable/lib/rails/generators/channel/channel_generator.rb @@ -1,7 +1,9 @@ +# frozen_string_literal: true + module Rails module Generators class ChannelGenerator < NamedBase - source_root File.expand_path("../templates", __FILE__) + source_root File.expand_path("templates", __dir__) argument :actions, type: :array, default: [], banner: "method method" @@ -10,34 +12,35 @@ module Rails check_class_collision suffix: "Channel" def create_channel_file - template "channel.rb", File.join('app/channels', class_path, "#{file_name}_channel.rb") + template "channel.rb", File.join("app/channels", class_path, "#{file_name}_channel.rb") if options[:assets] - if self.behavior == :invoke + if behavior == :invoke template "assets/cable.js", "app/assets/javascripts/cable.js" end - template "assets/channel.coffee", File.join('app/assets/javascripts/channels', class_path, "#{file_name}.coffee") + + js_template "assets/channel", File.join("app/assets/javascripts/channels", class_path, "#{file_name}") end generate_application_cable_files end - protected + private def file_name - @_file_name ||= super.gsub(/_channel/i, '') + @_file_name ||= super.gsub(/_channel/i, "") end # FIXME: Change these files to symlinks once RubyGems 2.5.0 is required. def generate_application_cable_files - return if self.behavior != :invoke + return if behavior != :invoke files = [ - 'application_cable/channel.rb', - 'application_cable/connection.rb' + "application_cable/channel.rb", + "application_cable/connection.rb" ] files.each do |name| - path = File.join('app/channels/', name) + path = File.join("app/channels/", name) template(name, path) if !File.exist?(path) end end diff --git a/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb b/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb deleted file mode 100644 index d56fa30f4d..0000000000 --- a/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb +++ /dev/null @@ -1,5 +0,0 @@ -# Be sure to restart your server when you modify this file. Action Cable runs in a loop that does not support auto reloading. -module ApplicationCable - class Channel < ActionCable::Channel::Base - end -end diff --git a/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb.tt b/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb.tt new file mode 100644 index 0000000000..d672697283 --- /dev/null +++ b/actioncable/lib/rails/generators/channel/templates/application_cable/channel.rb.tt @@ -0,0 +1,4 @@ +module ApplicationCable + class Channel < ActionCable::Channel::Base + end +end diff --git a/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb b/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb deleted file mode 100644 index b4f41389ad..0000000000 --- a/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb +++ /dev/null @@ -1,5 +0,0 @@ -# Be sure to restart your server when you modify this file. Action Cable runs in a loop that does not support auto reloading. -module ApplicationCable - class Connection < ActionCable::Connection::Base - end -end diff --git a/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb.tt b/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb.tt new file mode 100644 index 0000000000..0ff5442f47 --- /dev/null +++ b/actioncable/lib/rails/generators/channel/templates/application_cable/connection.rb.tt @@ -0,0 +1,4 @@ +module ApplicationCable + class Connection < ActionCable::Connection::Base + end +end diff --git a/actioncable/lib/rails/generators/channel/templates/assets/cable.js b/actioncable/lib/rails/generators/channel/templates/assets/cable.js.tt index 71ee1e66de..739aa5f022 100644 --- a/actioncable/lib/rails/generators/channel/templates/assets/cable.js +++ b/actioncable/lib/rails/generators/channel/templates/assets/cable.js.tt @@ -1,5 +1,5 @@ // Action Cable provides the framework to deal with WebSockets in Rails. -// You can generate new channels where WebSocket features live using the rails generate channel command. +// You can generate new channels where WebSocket features live using the `rails generate channel` command. // //= require action_cable //= require_self 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.tt b/actioncable/lib/rails/generators/channel/templates/assets/channel.js.tt new file mode 100644 index 0000000000..ab0e68b11a --- /dev/null +++ b/actioncable/lib/rails/generators/channel/templates/assets/channel.js.tt @@ -0,0 +1,18 @@ +App.<%= class_name.underscore %> = App.cable.subscriptions.create("<%= class_name %>Channel", { + connected: function() { + // Called when the subscription is ready for use on the server + }, + + disconnected: function() { + // Called when the subscription has been terminated by the server + }, + + received: function(data) { + // Called when there's incoming data on the websocket for this channel + }<%= actions.any? ? ",\n" : '' %> +<% actions.each do |action| -%> + <%=action %>: function() { + return this.perform('<%= action %>'); + }<%= action == actions[-1] ? '' : ",\n" %> +<% end -%> +}); diff --git a/actioncable/lib/rails/generators/channel/templates/channel.rb b/actioncable/lib/rails/generators/channel/templates/channel.rb.tt index 7bff3341c1..4bcfb2be4d 100644 --- a/actioncable/lib/rails/generators/channel/templates/channel.rb +++ b/actioncable/lib/rails/generators/channel/templates/channel.rb.tt @@ -1,4 +1,3 @@ -# Be sure to restart your server when you modify this file. Action Cable runs in a loop that does not support auto reloading. <% module_namespacing do -%> class <%= class_name %>Channel < ApplicationCable::Channel def subscribed diff --git a/actioncable/package.json b/actioncable/package.json new file mode 100644 index 0000000000..8d7f9ff302 --- /dev/null +++ b/actioncable/package.json @@ -0,0 +1,24 @@ +{ + "name": "actioncable", + "version": "5.2.0-beta2", + "description": "WebSocket framework for Ruby on Rails.", + "main": "lib/assets/compiled/action_cable.js", + "files": [ + "lib/assets/compiled/*.js" + ], + "repository": { + "type": "git", + "url": "rails/rails" + }, + "keywords": [ + "websockets", + "actioncable", + "rails" + ], + "author": "David Heinemeier Hansson <david@loudthinking.com>", + "license": "MIT", + "bugs": { + "url": "https://github.com/rails/rails/issues" + }, + "homepage": "http://rubyonrails.org/" +} diff --git a/actioncable/test/channel/base_test.rb b/actioncable/test/channel/base_test.rb index daa782eeb3..866bd7c21b 100644 --- a/actioncable/test/channel/base_test.rb +++ b/actioncable/test/channel/base_test.rb @@ -1,6 +1,8 @@ -require 'test_helper' -require 'stubs/test_connection' -require 'stubs/room' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_connection" +require "stubs/room" class ActionCable::Channel::BaseTest < ActiveSupport::TestCase class ActionCable::Channel::Base @@ -58,7 +60,7 @@ class ActionCable::Channel::BaseTest < ActiveSupport::TestCase end def get_latest - transmit data: 'latest' + transmit data: "latest" end def receive @@ -74,14 +76,16 @@ class ActionCable::Channel::BaseTest < ActiveSupport::TestCase setup do @user = User.new "lifo" @connection = TestConnection.new(@user) - @channel = ChatChannel.new @connection, "{id: 1}", { id: 1 } + @channel = ChatChannel.new @connection, "{id: 1}", id: 1 end - test "should subscribe to a channel on initialize" do + test "should subscribe to a channel" do + @channel.subscribe_to_channel assert_equal 1, @channel.room.id end test "on subscribe callbacks" do + @channel.subscribe_to_channel assert @channel.subscribed end @@ -90,6 +94,8 @@ class ActionCable::Channel::BaseTest < ActiveSupport::TestCase end test "unsubscribing from a channel" do + @channel.subscribe_to_channel + assert @channel.room assert @channel.subscribed? @@ -104,54 +110,59 @@ class ActionCable::Channel::BaseTest < ActiveSupport::TestCase end test "callable action without any argument" do - @channel.perform_action 'action' => :leave + @channel.perform_action "action" => :leave assert_equal [ :leave ], @channel.last_action end test "callable action with arguments" do - data = { 'action' => :speak, 'content' => "Hello World" } + data = { "action" => :speak, "content" => "Hello World" } @channel.perform_action data assert_equal [ :speak, data ], @channel.last_action end test "should not dispatch a private method" do - @channel.perform_action 'action' => :rm_rf + @channel.perform_action "action" => :rm_rf assert_nil @channel.last_action end test "should not dispatch a public method defined on Base" do - @channel.perform_action 'action' => :kick + @channel.perform_action "action" => :kick assert_nil @channel.last_action end test "should dispatch a public method defined on Base and redefined on channel" do - data = { 'action' => :topic, 'content' => "This is Sparta!" } + data = { "action" => :topic, "content" => "This is Sparta!" } @channel.perform_action data assert_equal [ :topic, data ], @channel.last_action end test "should dispatch calling a public method defined in an ancestor" do - @channel.perform_action 'action' => :chatters + @channel.perform_action "action" => :chatters assert_equal [ :chatters ], @channel.last_action end test "should dispatch receive action when perform_action is called with empty action" do - data = { 'content' => 'hello' } + data = { "content" => "hello" } @channel.perform_action data assert_equal [ :receive ], @channel.last_action end test "transmitting data" do - @channel.perform_action 'action' => :get_latest + @channel.perform_action "action" => :get_latest - expected = { "identifier" => "{id: 1}", "message" => { "data" => "latest" }} + expected = { "identifier" => "{id: 1}", "message" => { "data" => "latest" } } assert_equal expected, @connection.last_transmission end - test "subscription confirmation" do + test "do not send subscription confirmation on initialize" do + assert_nil @connection.last_transmission + end + + test "subscription confirmation on subscribe_to_channel" do expected = { "identifier" => "{id: 1}", "type" => "confirm_subscription" } + @channel.subscribe_to_channel assert_equal expected, @connection.last_transmission end @@ -162,7 +173,7 @@ class ActionCable::Channel::BaseTest < ActiveSupport::TestCase test "invalid action on Channel" do assert_logged("Unable to process ActionCable::Channel::BaseTest::ChatChannel#invalid_action") do - @channel.perform_action 'action' => :invalid_action + @channel.perform_action "action" => :invalid_action end end @@ -173,12 +184,12 @@ class ActionCable::Channel::BaseTest < ActiveSupport::TestCase events << ActiveSupport::Notifications::Event.new(*args) end - data = {'action' => :speak, 'content' => 'hello'} + data = { "action" => :speak, "content" => "hello" } @channel.perform_action data assert_equal 1, events.length - assert_equal 'perform_action.action_cable', events[0].name - assert_equal 'ActionCable::Channel::BaseTest::ChatChannel', events[0].payload[:channel_class] + assert_equal "perform_action.action_cable", events[0].name + assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] assert_equal :speak, events[0].payload[:action] assert_equal data, events[0].payload[:data] ensure @@ -189,27 +200,29 @@ class ActionCable::Channel::BaseTest < ActiveSupport::TestCase test "notification for transmit" do begin events = [] - ActiveSupport::Notifications.subscribe 'transmit.action_cable' do |*args| + ActiveSupport::Notifications.subscribe "transmit.action_cable" do |*args| events << ActiveSupport::Notifications::Event.new(*args) end - @channel.perform_action 'action' => :get_latest - expected_data = {data: 'latest'} + @channel.perform_action "action" => :get_latest + expected_data = { data: "latest" } assert_equal 1, events.length - assert_equal 'transmit.action_cable', events[0].name - assert_equal 'ActionCable::Channel::BaseTest::ChatChannel', events[0].payload[:channel_class] + assert_equal "transmit.action_cable", events[0].name + assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] assert_equal expected_data, events[0].payload[:data] assert_nil events[0].payload[:via] ensure - ActiveSupport::Notifications.unsubscribe 'transmit.action_cable' + ActiveSupport::Notifications.unsubscribe "transmit.action_cable" end end test "notification for transmit_subscription_confirmation" do begin + @channel.subscribe_to_channel + events = [] - ActiveSupport::Notifications.subscribe 'transmit_subscription_confirmation.action_cable' do |*args| + ActiveSupport::Notifications.subscribe "transmit_subscription_confirmation.action_cable" do |*args| events << ActiveSupport::Notifications::Event.new(*args) end @@ -217,27 +230,27 @@ class ActionCable::Channel::BaseTest < ActiveSupport::TestCase @channel.send(:transmit_subscription_confirmation) assert_equal 1, events.length - assert_equal 'transmit_subscription_confirmation.action_cable', events[0].name - assert_equal 'ActionCable::Channel::BaseTest::ChatChannel', events[0].payload[:channel_class] + assert_equal "transmit_subscription_confirmation.action_cable", events[0].name + assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] ensure - ActiveSupport::Notifications.unsubscribe 'transmit_subscription_confirmation.action_cable' + ActiveSupport::Notifications.unsubscribe "transmit_subscription_confirmation.action_cable" end end test "notification for transmit_subscription_rejection" do begin events = [] - ActiveSupport::Notifications.subscribe 'transmit_subscription_rejection.action_cable' do |*args| + ActiveSupport::Notifications.subscribe "transmit_subscription_rejection.action_cable" do |*args| events << ActiveSupport::Notifications::Event.new(*args) end @channel.send(:transmit_subscription_rejection) assert_equal 1, events.length - assert_equal 'transmit_subscription_rejection.action_cable', events[0].name - assert_equal 'ActionCable::Channel::BaseTest::ChatChannel', events[0].payload[:channel_class] + assert_equal "transmit_subscription_rejection.action_cable", events[0].name + assert_equal "ActionCable::Channel::BaseTest::ChatChannel", events[0].payload[:channel_class] ensure - ActiveSupport::Notifications.unsubscribe 'transmit_subscription_rejection.action_cable' + ActiveSupport::Notifications.unsubscribe "transmit_subscription_rejection.action_cable" end end diff --git a/actioncable/test/channel/broadcasting_test.rb b/actioncable/test/channel/broadcasting_test.rb index 1de04243e5..ab58f33511 100644 --- a/actioncable/test/channel/broadcasting_test.rb +++ b/actioncable/test/channel/broadcasting_test.rb @@ -1,6 +1,8 @@ -require 'test_helper' -require 'stubs/test_connection' -require 'stubs/room' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_connection" +require "stubs/room" class ActionCable::Channel::BroadcastingTest < ActiveSupport::TestCase class ChatChannel < ActionCable::Channel::Base @@ -11,7 +13,7 @@ class ActionCable::Channel::BroadcastingTest < ActiveSupport::TestCase end test "broadcasts_to" do - ActionCable.stubs(:server).returns mock().tap { |m| m.expects(:broadcast).with('action_cable:channel:broadcasting_test:chat:Room#1-Campfire', "Hello World") } + ActionCable.stubs(:server).returns mock().tap { |m| m.expects(:broadcast).with("action_cable:channel:broadcasting_test:chat:Room#1-Campfire", "Hello World") } ChatChannel.broadcast_to(Room.new(1), "Hello World") end diff --git a/actioncable/test/channel/naming_test.rb b/actioncable/test/channel/naming_test.rb index 89ef6ad8b0..6f094fbb5e 100644 --- a/actioncable/test/channel/naming_test.rb +++ b/actioncable/test/channel/naming_test.rb @@ -1,4 +1,6 @@ -require 'test_helper' +# frozen_string_literal: true + +require "test_helper" class ActionCable::Channel::NamingTest < ActiveSupport::TestCase class ChatChannel < ActionCable::Channel::Base diff --git a/actioncable/test/channel/periodic_timers_test.rb b/actioncable/test/channel/periodic_timers_test.rb index 03464003cf..500b984ca6 100644 --- a/actioncable/test/channel/periodic_timers_test.rb +++ b/actioncable/test/channel/periodic_timers_test.rb @@ -1,7 +1,9 @@ -require 'test_helper' -require 'stubs/test_connection' -require 'stubs/room' -require 'active_support/time' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_connection" +require "stubs/room" +require "active_support/time" class ActionCable::Channel::PeriodicTimersTest < ActiveSupport::TestCase class ChatChannel < ActionCable::Channel::Base @@ -32,36 +34,40 @@ class ActionCable::Channel::PeriodicTimersTest < ActiveSupport::TestCase timers.each_with_index do |timer, i| assert_kind_of Proc, timer[0] - assert_equal i+1, timer[1][:every] + assert_equal i + 1, timer[1][:every] end end - test 'disallow negative and zero periods' do - [ 0, 0.0, 0.seconds, -1, -1.seconds, 'foo', :foo, Object.new ].each do |invalid| - assert_raise ArgumentError, /Expected every:/ do + test "disallow negative and zero periods" do + [ 0, 0.0, 0.seconds, -1, -1.seconds, "foo", :foo, Object.new ].each do |invalid| + e = assert_raise ArgumentError do ChatChannel.periodically :send_updates, every: invalid end + assert_match(/Expected every:/, e.message) end end - test 'disallow block and arg together' do - assert_raise ArgumentError, /not both/ do + test "disallow block and arg together" do + e = assert_raise ArgumentError do ChatChannel.periodically(:send_updates, every: 1) { ping } end + assert_match(/not both/, e.message) end - test 'disallow unknown args' do - [ 'send_updates', Object.new, nil ].each do |invalid| - assert_raise ArgumentError, /Expected a Symbol/ do + test "disallow unknown args" do + [ "send_updates", Object.new, nil ].each do |invalid| + e = assert_raise ArgumentError do ChatChannel.periodically invalid, every: 1 end + assert_match(/Expected a Symbol/, e.message) end end test "timer start and stop" do @connection.server.event_loop.expects(:timer).times(3).returns(stub(shutdown: nil)) - channel = ChatChannel.new @connection, "{id: 1}", { id: 1 } + channel = ChatChannel.new @connection, "{id: 1}", id: 1 + channel.subscribe_to_channel channel.unsubscribe_from_channel assert_equal [], channel.send(:active_periodic_timers) end diff --git a/actioncable/test/channel/rejection_test.rb b/actioncable/test/channel/rejection_test.rb index 15db57d6ba..a6da014a21 100644 --- a/actioncable/test/channel/rejection_test.rb +++ b/actioncable/test/channel/rejection_test.rb @@ -1,12 +1,17 @@ -require 'test_helper' -require 'stubs/test_connection' -require 'stubs/room' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_connection" +require "stubs/room" class ActionCable::Channel::RejectionTest < ActiveSupport::TestCase class SecretChannel < ActionCable::Channel::Base def subscribed reject if params[:id] > 0 end + + def secret_action + end end setup do @@ -16,10 +21,23 @@ class ActionCable::Channel::RejectionTest < ActiveSupport::TestCase test "subscription rejection" do @connection.expects(:subscriptions).returns mock().tap { |m| m.expects(:remove_subscription).with instance_of(SecretChannel) } - @channel = SecretChannel.new @connection, "{id: 1}", { id: 1 } + @channel = SecretChannel.new @connection, "{id: 1}", id: 1 + @channel.subscribe_to_channel expected = { "identifier" => "{id: 1}", "type" => "reject_subscription" } assert_equal expected, @connection.last_transmission end + test "does not execute action if subscription is rejected" do + @connection.expects(:subscriptions).returns mock().tap { |m| m.expects(:remove_subscription).with instance_of(SecretChannel) } + @channel = SecretChannel.new @connection, "{id: 1}", id: 1 + @channel.subscribe_to_channel + + expected = { "identifier" => "{id: 1}", "type" => "reject_subscription" } + assert_equal expected, @connection.last_transmission + assert_equal 1, @connection.transmissions.size + + @channel.perform_action("action" => :secret_action) + assert_equal 1, @connection.transmissions.size + end end diff --git a/actioncable/test/channel/stream_test.rb b/actioncable/test/channel/stream_test.rb index 0b0c72ccf6..eca06fe365 100644 --- a/actioncable/test/channel/stream_test.rb +++ b/actioncable/test/channel/stream_test.rb @@ -1,6 +1,8 @@ -require 'test_helper' -require 'stubs/test_connection' -require 'stubs/room' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_connection" +require "stubs/room" module ActionCable::StreamTests class Connection < ActionCable::Connection::Base @@ -25,11 +27,11 @@ module ActionCable::StreamTests private def pick_coder(coder) case coder - when nil, 'json' + when nil, "json" ActiveSupport::JSON - when 'custom' + when "custom" DummyEncoder - when 'none' + when "none" nil end end @@ -38,7 +40,7 @@ module ActionCable::StreamTests module DummyEncoder extend self def encode(*) '{ "foo": "encoded" }' end - def decode(*) { foo: 'decoded' } end + def decode(*) { foo: "decoded" } end end class SymbolChannel < ActionCable::Channel::Base @@ -52,7 +54,10 @@ module ActionCable::StreamTests run_in_eventmachine do connection = TestConnection.new connection.expects(:pubsub).returns mock().tap { |m| m.expects(:subscribe).with("test_room_1", kind_of(Proc), kind_of(Proc)).returns stub_everything(:pubsub) } - channel = ChatChannel.new connection, "{id: 1}", { id: 1 } + channel = ChatChannel.new connection, "{id: 1}", id: 1 + channel.subscribe_to_channel + + wait_for_async connection.expects(:pubsub).returns mock().tap { |m| m.expects(:unsubscribe) } channel.unsubscribe_from_channel @@ -64,6 +69,9 @@ module ActionCable::StreamTests connection = TestConnection.new connection.expects(:pubsub).returns mock().tap { |m| m.expects(:subscribe).with("channel", kind_of(Proc), kind_of(Proc)).returns stub_everything(:pubsub) } channel = SymbolChannel.new connection, "" + channel.subscribe_to_channel + + wait_for_async connection.expects(:pubsub).returns mock().tap { |m| m.expects(:unsubscribe) } channel.unsubscribe_from_channel @@ -76,6 +84,7 @@ module ActionCable::StreamTests connection.expects(:pubsub).returns mock().tap { |m| m.expects(:subscribe).with("action_cable:stream_tests:chat:Room#1-Campfire", kind_of(Proc), kind_of(Proc)).returns stub_everything(:pubsub) } channel = ChatChannel.new connection, "" + channel.subscribe_to_channel channel.stream_for Room.new(1) end end @@ -84,7 +93,9 @@ module ActionCable::StreamTests run_in_eventmachine do connection = TestConnection.new - ChatChannel.new connection, "{id: 1}", { id: 1 } + channel = ChatChannel.new connection, "{id: 1}", id: 1 + channel.subscribe_to_channel + assert_nil connection.last_transmission wait_for_async @@ -114,7 +125,7 @@ module ActionCable::StreamTests end end - require 'action_cable/subscription_adapter/inline' + require "action_cable/subscription_adapter/async" class UserCallbackChannel < ActionCable::Channel::Base def subscribed @@ -124,23 +135,26 @@ module ActionCable::StreamTests end end - class StreamEncodingTest < ActionCable::TestCase + class MultiChatChannel < ActionCable::Channel::Base + def subscribed + stream_from "main_room" + stream_from "test_all_rooms" + end + end + + class StreamFromTest < ActionCable::TestCase setup do - @server = TestServer.new(subscription_adapter: ActionCable::SubscriptionAdapter::Inline) + @server = TestServer.new(subscription_adapter: ActionCable::SubscriptionAdapter::Async) @server.config.allowed_request_origins = %w( http://rubyonrails.com ) - @server.stubs(:channel_classes).returns( - ChatChannel.name => ChatChannel, - UserCallbackChannel.name => UserCallbackChannel, - ) end - test 'custom encoder' do + test "custom encoder" do run_in_eventmachine do connection = open_connection subscribe_to connection, identifiers: { id: 1 } connection.websocket.expects(:transmit) - @server.broadcast 'test_room_1', { foo: 'bar' }, coder: DummyEncoder + @server.broadcast "test_room_1", { foo: "bar" }, { coder: DummyEncoder } wait_for_async wait_for_executor connection.server.worker_pool.executor end @@ -149,21 +163,32 @@ module ActionCable::StreamTests test "user supplied callbacks are run through the worker pool" do run_in_eventmachine do connection = open_connection - receive(connection, command: 'subscribe', channel: UserCallbackChannel.name, identifiers: { id: 1 }) + receive(connection, command: "subscribe", channel: UserCallbackChannel.name, identifiers: { id: 1 }) - @server.broadcast 'channel', {} + @server.broadcast "channel", {} wait_for_async refute Thread.current[:ran_callback], "User callback was not run through the worker pool" end end + test "subscription confirmation should only be sent out once with muptiple stream_from" do + run_in_eventmachine do + connection = open_connection + expected = { "identifier" => { "channel" => MultiChatChannel.name }.to_json, "type" => "confirm_subscription" } + connection.websocket.expects(:transmit).with(expected.to_json) + receive(connection, command: "subscribe", channel: MultiChatChannel.name, identifiers: {}) + + wait_for_async + end + end + private def subscribe_to(connection, identifiers:) - receive connection, command: 'subscribe', identifiers: identifiers + receive connection, command: "subscribe", identifiers: identifiers end def open_connection - env = Rack::MockRequest.env_for '/test', 'HTTP_HOST' => 'localhost', 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket', 'HTTP_ORIGIN' => 'http://rubyonrails.com' + env = Rack::MockRequest.env_for "/test", "HTTP_HOST" => "localhost", "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket", "HTTP_ORIGIN" => "http://rubyonrails.com" Connection.new(@server, env).tap do |connection| connection.process @@ -174,7 +199,7 @@ module ActionCable::StreamTests end end - def receive(connection, command:, identifiers:, channel: 'ActionCable::StreamTests::ChatChannel') + def receive(connection, command:, identifiers:, channel: "ActionCable::StreamTests::ChatChannel") identifier = JSON.generate(channel: channel, **identifiers) connection.dispatch_websocket_message JSON.generate(command: command, identifier: identifier) wait_for_async diff --git a/actioncable/test/client/echo_channel.rb b/actioncable/test/client/echo_channel.rb deleted file mode 100644 index 5a7bac25c5..0000000000 --- a/actioncable/test/client/echo_channel.rb +++ /dev/null @@ -1,22 +0,0 @@ -class EchoChannel < ActionCable::Channel::Base - def subscribed - stream_from "global" - end - - def unsubscribed - 'Goodbye from EchoChannel!' - end - - def ding(data) - transmit(dong: data['message']) - end - - def delay(data) - sleep 1 - transmit(dong: data['message']) - end - - def bulk(data) - ActionCable.server.broadcast "global", wide: data['message'] - end -end diff --git a/actioncable/test/client_test.rb b/actioncable/test/client_test.rb index fe503fd703..56b3ef143b 100644 --- a/actioncable/test/client_test.rb +++ b/actioncable/test/client_test.rb @@ -1,95 +1,151 @@ -require 'test_helper' -require 'concurrent' +# frozen_string_literal: true -require 'active_support/core_ext/hash/indifferent_access' -require 'pathname' +require "test_helper" +require "concurrent" -require 'faye/websocket' -require 'json' +require "websocket-client-simple" +require "json" + +require "active_support/hash_with_indifferent_access" + +#### +# 😷 Warning suppression 😷 +WebSocket::Frame::Handler::Handler03.prepend Module.new { + def initialize(*) + @application_data_buffer = nil + super + end +} + +WebSocket::Frame::Data.prepend Module.new { + def initialize(*) + @masking_key = nil + super + end +} +# +#### class ClientTest < ActionCable::TestCase - WAIT_WHEN_EXPECTING_EVENT = 8 + WAIT_WHEN_EXPECTING_EVENT = 2 WAIT_WHEN_NOT_EXPECTING_EVENT = 0.5 + class EchoChannel < ActionCable::Channel::Base + def subscribed + stream_from "global" + end + + def unsubscribed + "Goodbye from EchoChannel!" + end + + def ding(data) + transmit(dong: data["message"]) + end + + def delay(data) + sleep 1 + transmit(dong: data["message"]) + end + + def bulk(data) + ActionCable.server.broadcast "global", wide: data["message"] + end + end + def setup ActionCable.instance_variable_set(:@server, nil) server = ActionCable.server server.config.logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } - server.config.cable = { adapter: 'async' }.with_indifferent_access - server.config.use_faye = ENV['FAYE'].present? + server.config.cable = ActiveSupport::HashWithIndifferentAccess.new(adapter: "async") # and now the "real" setup for our test: server.config.disable_request_forgery_protection = true - server.config.channel_paths = [ File.expand_path('client/echo_channel.rb', __dir__) ] - - Thread.new { EventMachine.run } unless EventMachine.reactor_running? - Thread.pass until EventMachine.reactor_running? - - # faye-websocket is warning-rich - @previous_verbose, $VERBOSE = $VERBOSE, nil - end - - def teardown - $VERBOSE = @previous_verbose end def with_puma_server(rack_app = ActionCable.server, port = 3099) server = ::Puma::Server.new(rack_app, ::Puma::Events.strings) - server.add_tcp_listener '127.0.0.1', port + server.add_tcp_listener "127.0.0.1", port server.min_threads = 1 server.max_threads = 4 - t = Thread.new { server.run.join } - yield port + thread = server.run - ensure - server.stop(true) if server - t.join if t + begin + yield port + + ensure + server.stop + + begin + thread.join + + rescue IOError + # Work around https://bugs.ruby-lang.org/issues/13405 + # + # Puma's sometimes raising while shutting down, when it closes + # its internal pipe. We can safely ignore that, but we do need + # to do the step skipped by the exception: + server.binder.close + + rescue RuntimeError => ex + # Work around https://bugs.ruby-lang.org/issues/13239 + raise unless ex.message =~ /can't modify frozen IOError/ + + # Handle this as if it were the IOError: do the same as above. + server.binder.close + end + end end class SyncClient attr_reader :pings def initialize(port) - @ws = Faye::WebSocket::Client.new("ws://127.0.0.1:#{port}/") - @messages = Queue.new - @closed = Concurrent::Event.new - @has_messages = Concurrent::Semaphore.new(0) - @pings = 0 - - open = Concurrent::Event.new - error = nil - - @ws.on(:error) do |event| - if open.set? - @messages << RuntimeError.new(event.message) - else - error = event.message - open.set + messages = @messages = Queue.new + closed = @closed = Concurrent::Event.new + has_messages = @has_messages = Concurrent::Semaphore.new(0) + pings = @pings = Concurrent::AtomicFixnum.new(0) + + open = Concurrent::Promise.new + + @ws = WebSocket::Client::Simple.connect("ws://127.0.0.1:#{port}/") do |ws| + ws.on(:error) do |event| + event = RuntimeError.new(event.message) unless event.is_a?(Exception) + + if open.pending? + open.fail(event) + else + messages << event + has_messages.release + end end - end - @ws.on(:open) do |event| - open.set - end + ws.on(:open) do |event| + open.set(true) + end - @ws.on(:message) do |event| - message = JSON.parse(event.data) - if message['type'] == 'ping' - @pings += 1 - else - @messages << message - @has_messages.release + ws.on(:message) do |event| + if event.type == :close + closed.set + else + message = JSON.parse(event.data) + if message["type"] == "ping" + pings.increment + else + messages << message + has_messages.release + end + end end - end - @ws.on(:close) do |event| - @closed.set + ws.on(:close) do |event| + closed.set + end end - open.wait(WAIT_WHEN_EXPECTING_EVENT) - raise error if error + open.wait!(WAIT_WHEN_EXPECTING_EVENT) end def read_message @@ -140,76 +196,80 @@ class ClientTest < ActionCable::TestCase end end - def faye_client(port) + def websocket_client(port) SyncClient.new(port) end + def concurrently(enum) + enum.map { |*x| Concurrent::Future.execute { yield(*x) } }.map(&:value!) + end + 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.generate(channel: 'EchoChannel') - assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message) - c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello') - assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "message"=>{"dong"=>"hello"}}, c.read_message) + c = websocket_client(port) + assert_equal({ "type" => "welcome" }, c.read_message) # pop the first welcome message off the stack + c.send_message command: "subscribe", identifier: JSON.generate(channel: "ClientTest::EchoChannel") + assert_equal({ "identifier" => "{\"channel\":\"ClientTest::EchoChannel\"}", "type" => "confirm_subscription" }, c.read_message) + c.send_message command: "message", identifier: JSON.generate(channel: "ClientTest::EchoChannel"), data: JSON.generate(action: "ding", message: "hello") + assert_equal({ "identifier" => "{\"channel\":\"ClientTest::EchoChannel\"}", "message" => { "dong" => "hello" } }, c.read_message) c.close end end def test_interacting_clients with_puma_server do |port| - clients = 10.times.map { faye_client(port) } + clients = concurrently(10.times) { websocket_client(port) } barrier_1 = Concurrent::CyclicBarrier.new(clients.size) 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.generate(channel: 'EchoChannel') - assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "type"=>"confirm_subscription"}, c.read_message) - c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello') - assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "message"=>{"dong"=>"hello"}}, c.read_message) + concurrently(clients) do |c| + assert_equal({ "type" => "welcome" }, c.read_message) # pop the first welcome message off the stack + c.send_message command: "subscribe", identifier: JSON.generate(channel: "ClientTest::EchoChannel") + assert_equal({ "identifier" => '{"channel":"ClientTest::EchoChannel"}', "type" => "confirm_subscription" }, c.read_message) + c.send_message command: "message", identifier: JSON.generate(channel: "ClientTest::EchoChannel"), data: JSON.generate(action: "ding", message: "hello") + assert_equal({ "identifier" => '{"channel":"ClientTest::EchoChannel"}', "message" => { "dong" => "hello" } }, c.read_message) barrier_1.wait WAIT_WHEN_EXPECTING_EVENT - c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'bulk', message: 'hello') + c.send_message command: "message", identifier: JSON.generate(channel: "ClientTest::EchoChannel"), data: JSON.generate(action: "bulk", message: "hello") barrier_2.wait WAIT_WHEN_EXPECTING_EVENT assert_equal clients.size, c.read_messages(clients.size).size - } }.each(&:wait!) + end - clients.map {|c| Concurrent::Future.execute { c.close } }.each(&:wait!) + concurrently(clients, &:close) end end def test_many_clients with_puma_server do |port| - 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.generate(channel: 'EchoChannel') - assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "type"=>"confirm_subscription"}, c.read_message) - c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello') - assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "message"=>{"dong"=>"hello"}}, c.read_message) - } }.each(&:wait!) + clients = concurrently(100.times) { websocket_client(port) } + + concurrently(clients) do |c| + assert_equal({ "type" => "welcome" }, c.read_message) # pop the first welcome message off the stack + c.send_message command: "subscribe", identifier: JSON.generate(channel: "ClientTest::EchoChannel") + assert_equal({ "identifier" => '{"channel":"ClientTest::EchoChannel"}', "type" => "confirm_subscription" }, c.read_message) + c.send_message command: "message", identifier: JSON.generate(channel: "ClientTest::EchoChannel"), data: JSON.generate(action: "ding", message: "hello") + assert_equal({ "identifier" => '{"channel":"ClientTest::EchoChannel"}', "message" => { "dong" => "hello" } }, c.read_message) + end - clients.map {|c| Concurrent::Future.execute { c.close } }.each(&:wait!) + concurrently(clients, &:close) end end 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.generate(channel: 'EchoChannel') - assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message) - c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'delay', message: 'hello') + c = websocket_client(port) + assert_equal({ "type" => "welcome" }, c.read_message) # pop the first welcome message off the stack + c.send_message command: "subscribe", identifier: JSON.generate(channel: "ClientTest::EchoChannel") + assert_equal({ "identifier" => "{\"channel\":\"ClientTest::EchoChannel\"}", "type" => "confirm_subscription" }, c.read_message) + c.send_message command: "message", identifier: JSON.generate(channel: "ClientTest::EchoChannel"), data: JSON.generate(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.generate(channel: 'EchoChannel') - assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message) - c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello') - assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "message"=>{"dong"=>"hello"}}, c.read_message) + c = websocket_client(port) + assert_equal({ "type" => "welcome" }, c.read_message) # pop the first welcome message off the stack + c.send_message command: "subscribe", identifier: JSON.generate(channel: "ClientTest::EchoChannel") + assert_equal({ "identifier" => "{\"channel\":\"ClientTest::EchoChannel\"}", "type" => "confirm_subscription" }, c.read_message) + c.send_message command: "message", identifier: JSON.generate(channel: "ClientTest::EchoChannel"), data: JSON.generate(action: "ding", message: "hello") + assert_equal({ "identifier" => '{"channel":"ClientTest::EchoChannel"}', "message" => { "dong" => "hello" } }, c.read_message) c.close # disappear before read end end @@ -217,17 +277,17 @@ class ClientTest < ActionCable::TestCase def test_unsubscribe_client with_puma_server do |port| app = ActionCable.server - identifier = JSON.generate(channel: 'EchoChannel') + identifier = JSON.generate(channel: "ClientTest::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) + c = websocket_client(port) + assert_equal({ "type" => "welcome" }, c.read_message) + c.send_message command: "subscribe", identifier: identifier + assert_equal({ "identifier" => "{\"channel\":\"ClientTest::EchoChannel\"}", "type" => "confirm_subscription" }, c.read_message) assert_equal(1, app.connections.count) assert(app.remote_connections.where(identifier: identifier)) subscriptions = app.connections.first.subscriptions.send(:subscriptions) - assert_not_equal 0, subscriptions.size, 'Missing EchoChannel subscription' + assert_not_equal 0, subscriptions.size, "Missing EchoChannel subscription" channel = subscriptions.first[1] channel.expects(:unsubscribed) c.close @@ -240,10 +300,10 @@ 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.generate(channel: 'EchoChannel') - assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message) + c = websocket_client(port) + assert_equal({ "type" => "welcome" }, c.read_message) + c.send_message command: "subscribe", identifier: JSON.generate(channel: "ClientTest::EchoChannel") + assert_equal({ "identifier" => "{\"channel\":\"ClientTest::EchoChannel\"}", "type" => "confirm_subscription" }, c.read_message) ActionCable.server.restart c.wait_for_close diff --git a/actioncable/test/connection/authorization_test.rb b/actioncable/test/connection/authorization_test.rb index a0506cb9c0..7d039336b8 100644 --- a/actioncable/test/connection/authorization_test.rb +++ b/actioncable/test/connection/authorization_test.rb @@ -1,5 +1,7 @@ -require 'test_helper' -require 'stubs/test_server' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_server" class ActionCable::Connection::AuthorizationTest < ActionCable::TestCase class Connection < ActionCable::Connection::Base @@ -19,8 +21,8 @@ class ActionCable::Connection::AuthorizationTest < ActionCable::TestCase server = TestServer.new server.config.allowed_request_origins = %w( http://rubyonrails.com ) - env = Rack::MockRequest.env_for "/test", 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket', - 'HTTP_HOST' => 'localhost', 'HTTP_ORIGIN' => 'http://rubyonrails.com' + env = Rack::MockRequest.env_for "/test", "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket", + "HTTP_HOST" => "localhost", "HTTP_ORIGIN" => "http://rubyonrails.com" connection = Connection.new(server, env) connection.websocket.expects(:close) diff --git a/actioncable/test/connection/base_test.rb b/actioncable/test/connection/base_test.rb index d7e1041e68..99488e38c8 100644 --- a/actioncable/test/connection/base_test.rb +++ b/actioncable/test/connection/base_test.rb @@ -1,6 +1,8 @@ -require 'test_helper' -require 'stubs/test_server' -require 'active_support/core_ext/object/json' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_server" +require "active_support/core_ext/object/json" class ActionCable::Connection::BaseTest < ActionCable::TestCase class Connection < ActionCable::Connection::Base @@ -113,14 +115,14 @@ class ActionCable::Connection::BaseTest < ActionCable::TestCase run_in_eventmachine do class CallMeMaybe def call(*) - raise 'Do not call me!' + raise "Do not call me!" end end env = Rack::MockRequest.env_for( "/test", - { 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket', - 'HTTP_HOST' => 'localhost', 'HTTP_ORIGIN' => 'http://rubyonrails.org', 'rack.hijack' => CallMeMaybe.new } + "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket", + "HTTP_HOST" => "localhost", "HTTP_ORIGIN" => "http://rubyonrails.org", "rack.hijack" => CallMeMaybe.new ) connection = ActionCable::Connection::Base.new(@server, env) @@ -131,8 +133,8 @@ class ActionCable::Connection::BaseTest < ActionCable::TestCase private def open_connection - env = Rack::MockRequest.env_for "/test", 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket', - 'HTTP_HOST' => 'localhost', 'HTTP_ORIGIN' => 'http://rubyonrails.com' + env = Rack::MockRequest.env_for "/test", "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket", + "HTTP_HOST" => "localhost", "HTTP_ORIGIN" => "http://rubyonrails.com" Connection.new(@server, env) end diff --git a/actioncable/test/connection/client_socket_test.rb b/actioncable/test/connection/client_socket_test.rb index 4af071b4da..5c31690c8b 100644 --- a/actioncable/test/connection/client_socket_test.rb +++ b/actioncable/test/connection/client_socket_test.rb @@ -1,5 +1,7 @@ -require 'test_helper' -require 'stubs/test_server' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_server" class ActionCable::Connection::ClientSocketTest < ActionCable::TestCase class Connection < ActionCable::Connection::Base @@ -32,31 +34,57 @@ class ActionCable::Connection::ClientSocketTest < ActionCable::TestCase @server.config.allowed_request_origins = %w( http://rubyonrails.com ) end - test 'delegate socket errors to on_error handler' do - skip if ENV['FAYE'].present? - + test "delegate socket errors to on_error handler" do run_in_eventmachine do connection = open_connection # Internal hax = :( client = connection.websocket.send(:websocket) - client.instance_variable_get('@stream').expects(:write).raises('foo') + client.instance_variable_get("@stream").expects(:write).raises("foo") client.expects(:client_gone).never - client.write('boo') + client.write("boo") assert_equal %w[ foo ], connection.errors end end + test "closes hijacked i/o socket at shutdown" do + run_in_eventmachine do + connection = open_connection + + client = connection.websocket.send(:websocket) + event = Concurrent::Event.new + client.instance_variable_get("@stream") + .instance_variable_get("@rack_hijack_io") + .define_singleton_method(:close) { event.set } + connection.close + event.wait + end + end + private def open_connection - env = Rack::MockRequest.env_for '/test', - 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket', - 'HTTP_HOST' => 'localhost', 'HTTP_ORIGIN' => 'http://rubyonrails.com' - env['rack.hijack'] = -> { env['rack.hijack_io'] = StringIO.new } + env = Rack::MockRequest.env_for "/test", + "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket", + "HTTP_HOST" => "localhost", "HTTP_ORIGIN" => "http://rubyonrails.com" + io, client_io = \ + begin + Socket.pair(Socket::AF_UNIX, Socket::SOCK_STREAM, 0) + rescue + StringIO.new + end + env["rack.hijack"] = -> { env["rack.hijack_io"] = io } 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/connection/cross_site_forgery_test.rb b/actioncable/test/connection/cross_site_forgery_test.rb index 2d516b0533..3e21138ffc 100644 --- a/actioncable/test/connection/cross_site_forgery_test.rb +++ b/actioncable/test/connection/cross_site_forgery_test.rb @@ -1,8 +1,10 @@ -require 'test_helper' -require 'stubs/test_server' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_server" class ActionCable::Connection::CrossSiteForgeryTest < ActionCable::TestCase - HOST = 'rubyonrails.com' + HOST = "rubyonrails.com" class Connection < ActionCable::Connection::Base def send_async(method, *args) @@ -13,44 +15,53 @@ class ActionCable::Connection::CrossSiteForgeryTest < ActionCable::TestCase setup do @server = TestServer.new @server.config.allowed_request_origins = %w( http://rubyonrails.com ) + @server.config.allow_same_origin_as_host = false end teardown do @server.config.disable_request_forgery_protection = false @server.config.allowed_request_origins = [] + @server.config.allow_same_origin_as_host = true end test "disable forgery protection" do @server.config.disable_request_forgery_protection = true - assert_origin_allowed 'http://rubyonrails.com' - assert_origin_allowed 'http://hax.com' + assert_origin_allowed "http://rubyonrails.com" + assert_origin_allowed "http://hax.com" end test "explicitly specified a single allowed origin" do - @server.config.allowed_request_origins = 'http://hax.com' - assert_origin_not_allowed 'http://rubyonrails.com' - assert_origin_allowed 'http://hax.com' + @server.config.allowed_request_origins = "http://hax.com" + assert_origin_not_allowed "http://rubyonrails.com" + assert_origin_allowed "http://hax.com" end test "explicitly specified multiple allowed origins" do @server.config.allowed_request_origins = %w( http://rubyonrails.com http://www.rubyonrails.com ) - assert_origin_allowed 'http://rubyonrails.com' - assert_origin_allowed 'http://www.rubyonrails.com' - assert_origin_not_allowed 'http://hax.com' + assert_origin_allowed "http://rubyonrails.com" + assert_origin_allowed "http://www.rubyonrails.com" + assert_origin_not_allowed "http://hax.com" end test "explicitly specified a single regexp allowed origin" do @server.config.allowed_request_origins = /.*ha.*/ - assert_origin_not_allowed 'http://rubyonrails.com' - assert_origin_allowed 'http://hax.com' + assert_origin_not_allowed "http://rubyonrails.com" + assert_origin_allowed "http://hax.com" end test "explicitly specified multiple regexp allowed origins" do - @server.config.allowed_request_origins = [/http:\/\/ruby.*/, /.*rai.s.*com/, 'string' ] - assert_origin_allowed 'http://rubyonrails.com' - assert_origin_allowed 'http://www.rubyonrails.com' - assert_origin_not_allowed 'http://hax.com' - assert_origin_not_allowed 'http://rails.co.uk' + @server.config.allowed_request_origins = [/http:\/\/ruby.*/, /.*rai.s.*com/, "string" ] + assert_origin_allowed "http://rubyonrails.com" + assert_origin_allowed "http://www.rubyonrails.com" + assert_origin_not_allowed "http://hax.com" + assert_origin_not_allowed "http://rails.co.uk" + end + + test "allow same origin as host" do + @server.config.allow_same_origin_as_host = true + assert_origin_allowed "http://#{HOST}" + assert_origin_not_allowed "http://hax.com" + assert_origin_not_allowed "http://rails.co.uk" end private @@ -75,7 +86,7 @@ class ActionCable::Connection::CrossSiteForgeryTest < ActionCable::TestCase end def env_for_origin(origin) - Rack::MockRequest.env_for "/test", 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket', 'SERVER_NAME' => HOST, - 'HTTP_HOST' => HOST, 'HTTP_ORIGIN' => origin + Rack::MockRequest.env_for "/test", "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket", "SERVER_NAME" => HOST, + "HTTP_HOST" => HOST, "HTTP_ORIGIN" => origin end end diff --git a/actioncable/test/connection/identifier_test.rb b/actioncable/test/connection/identifier_test.rb index b48d9af809..6b6c8cd31a 100644 --- a/actioncable/test/connection/identifier_test.rb +++ b/actioncable/test/connection/identifier_test.rb @@ -1,6 +1,8 @@ -require 'test_helper' -require 'stubs/test_server' -require 'stubs/user' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_server" +require "stubs/user" class ActionCable::Connection::IdentifierTest < ActionCable::TestCase class Connection < ActionCable::Connection::Base @@ -23,9 +25,9 @@ class ActionCable::Connection::IdentifierTest < ActionCable::TestCase test "should subscribe to internal channel on open and unsubscribe on close" do run_in_eventmachine do - pubsub = mock('pubsub_adapter') - pubsub.expects(:subscribe).with('action_cable/User#lifo', kind_of(Proc)) - pubsub.expects(:unsubscribe).with('action_cable/User#lifo', kind_of(Proc)) + pubsub = mock("pubsub_adapter") + pubsub.expects(:subscribe).with("action_cable/User#lifo", kind_of(Proc)) + pubsub.expects(:unsubscribe).with("action_cable/User#lifo", kind_of(Proc)) server = TestServer.new server.stubs(:pubsub).returns(pubsub) @@ -40,7 +42,7 @@ class ActionCable::Connection::IdentifierTest < ActionCable::TestCase open_connection_with_stubbed_pubsub @connection.websocket.expects(:close) - @connection.process_internal_message 'type' => 'disconnect' + @connection.process_internal_message "type" => "disconnect" end end @@ -49,20 +51,20 @@ class ActionCable::Connection::IdentifierTest < ActionCable::TestCase open_connection_with_stubbed_pubsub @connection.websocket.expects(:close).never - @connection.process_internal_message 'type' => 'unknown' + @connection.process_internal_message "type" => "unknown" end end - protected + private def open_connection_with_stubbed_pubsub server = TestServer.new - server.stubs(:adapter).returns(stub_everything('adapter')) + server.stubs(:adapter).returns(stub_everything("adapter")) open_connection server: server end def open_connection(server:) - env = Rack::MockRequest.env_for "/test", 'HTTP_HOST' => 'localhost', 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket' + env = Rack::MockRequest.env_for "/test", "HTTP_HOST" => "localhost", "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket" @connection = Connection.new(server, env) @connection.process diff --git a/actioncable/test/connection/multiple_identifiers_test.rb b/actioncable/test/connection/multiple_identifiers_test.rb index 484e73bb30..7f90cb3876 100644 --- a/actioncable/test/connection/multiple_identifiers_test.rb +++ b/actioncable/test/connection/multiple_identifiers_test.rb @@ -1,6 +1,8 @@ -require 'test_helper' -require 'stubs/test_server' -require 'stubs/user' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_server" +require "stubs/user" class ActionCable::Connection::MultipleIdentifiersTest < ActionCable::TestCase class Connection < ActionCable::Connection::Base @@ -19,23 +21,19 @@ class ActionCable::Connection::MultipleIdentifiersTest < ActionCable::TestCase end end - protected + private def open_connection_with_stubbed_pubsub server = TestServer.new - server.stubs(:pubsub).returns(stub_everything('pubsub')) + server.stubs(:pubsub).returns(stub_everything("pubsub")) open_connection server: server end def open_connection(server:) - env = Rack::MockRequest.env_for "/test", 'HTTP_HOST' => 'localhost', 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket' + env = Rack::MockRequest.env_for "/test", "HTTP_HOST" => "localhost", "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket" @connection = Connection.new(server, env) @connection.process @connection.send :handle_open end - - def close_connection - @connection.send :handle_close - end end diff --git a/actioncable/test/connection/stream_test.rb b/actioncable/test/connection/stream_test.rb index a7a61d8d6f..b0419b0994 100644 --- a/actioncable/test/connection/stream_test.rb +++ b/actioncable/test/connection/stream_test.rb @@ -1,5 +1,7 @@ -require 'test_helper' -require 'stubs/test_server' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_server" class ActionCable::Connection::StreamTest < ActionCable::TestCase class Connection < ActionCable::Connection::Base @@ -34,17 +36,15 @@ class ActionCable::Connection::StreamTest < ActionCable::TestCase [ EOFError, Errno::ECONNRESET ].each do |closed_exception| test "closes socket on #{closed_exception}" do - skip if ENV['FAYE'].present? - run_in_eventmachine do connection = open_connection # Internal hax = :( client = connection.websocket.send(:websocket) - client.instance_variable_get('@stream').instance_variable_get('@rack_hijack_io').expects(:write).raises(closed_exception, 'foo') + client.instance_variable_get("@stream").instance_variable_get("@rack_hijack_io").expects(:write).raises(closed_exception, "foo") client.expects(:client_gone) - client.write('boo') + client.write("boo") assert_equal [], connection.errors end end @@ -52,10 +52,10 @@ class ActionCable::Connection::StreamTest < ActionCable::TestCase private def open_connection - env = Rack::MockRequest.env_for '/test', - 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket', - 'HTTP_HOST' => 'localhost', 'HTTP_ORIGIN' => 'http://rubyonrails.com' - env['rack.hijack'] = -> { env['rack.hijack_io'] = StringIO.new } + env = Rack::MockRequest.env_for "/test", + "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket", + "HTTP_HOST" => "localhost", "HTTP_ORIGIN" => "http://rubyonrails.com" + env["rack.hijack"] = -> { env["rack.hijack_io"] = StringIO.new } Connection.new(@server, env).tap do |connection| connection.process diff --git a/actioncable/test/connection/string_identifier_test.rb b/actioncable/test/connection/string_identifier_test.rb index eca0c31060..4cb58e7fd0 100644 --- a/actioncable/test/connection/string_identifier_test.rb +++ b/actioncable/test/connection/string_identifier_test.rb @@ -1,5 +1,7 @@ -require 'test_helper' -require 'stubs/test_server' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_server" class ActionCable::Connection::StringIdentifierTest < ActionCable::TestCase class Connection < ActionCable::Connection::Base @@ -21,23 +23,19 @@ class ActionCable::Connection::StringIdentifierTest < ActionCable::TestCase end end - protected + private def open_connection_with_stubbed_pubsub @server = TestServer.new - @server.stubs(:pubsub).returns(stub_everything('pubsub')) + @server.stubs(:pubsub).returns(stub_everything("pubsub")) open_connection end def open_connection - env = Rack::MockRequest.env_for "/test", 'HTTP_HOST' => 'localhost', 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket' + env = Rack::MockRequest.env_for "/test", "HTTP_HOST" => "localhost", "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket" @connection = Connection.new(@server, env) @connection.process @connection.send :on_open end - - def close_connection - @connection.send :on_close - end end diff --git a/actioncable/test/connection/subscriptions_test.rb b/actioncable/test/connection/subscriptions_test.rb index 53e8547245..149a40604a 100644 --- a/actioncable/test/connection/subscriptions_test.rb +++ b/actioncable/test/connection/subscriptions_test.rb @@ -1,4 +1,6 @@ -require 'test_helper' +# frozen_string_literal: true + +require "test_helper" class ActionCable::Connection::SubscriptionsTest < ActionCable::TestCase class Connection < ActionCable::Connection::Base @@ -24,9 +26,8 @@ class ActionCable::Connection::SubscriptionsTest < ActionCable::TestCase setup do @server = TestServer.new - @server.stubs(:channel_classes).returns(ChatChannel.name => ChatChannel) - @chat_identifier = ActiveSupport::JSON.encode(id: 1, channel: 'ActionCable::Connection::SubscriptionsTest::ChatChannel') + @chat_identifier = ActiveSupport::JSON.encode(id: 1, channel: "ActionCable::Connection::SubscriptionsTest::ChatChannel") end test "subscribe command" do @@ -43,7 +44,7 @@ class ActionCable::Connection::SubscriptionsTest < ActionCable::TestCase run_in_eventmachine do setup_connection - @subscriptions.execute_command 'command' => 'subscribe' + @subscriptions.execute_command "command" => "subscribe" assert @subscriptions.identifiers.empty? end end @@ -56,7 +57,7 @@ class ActionCable::Connection::SubscriptionsTest < ActionCable::TestCase channel = subscribe_to_chat_channel channel.expects(:unsubscribe_from_channel) - @subscriptions.execute_command 'command' => 'unsubscribe', 'identifier' => @chat_identifier + @subscriptions.execute_command "command" => "unsubscribe", "identifier" => @chat_identifier assert @subscriptions.identifiers.empty? end end @@ -65,7 +66,7 @@ class ActionCable::Connection::SubscriptionsTest < ActionCable::TestCase run_in_eventmachine do setup_connection - @subscriptions.execute_command 'command' => 'unsubscribe' + @subscriptions.execute_command "command" => "unsubscribe" assert @subscriptions.identifiers.empty? end end @@ -75,8 +76,8 @@ class ActionCable::Connection::SubscriptionsTest < ActionCable::TestCase setup_connection channel = subscribe_to_chat_channel - data = { 'content' => 'Hello World!', 'action' => 'speak' } - @subscriptions.execute_command 'command' => 'message', 'identifier' => @chat_identifier, 'data' => ActiveSupport::JSON.encode(data) + data = { "content" => "Hello World!", "action" => "speak" } + @subscriptions.execute_command "command" => "message", "identifier" => @chat_identifier, "data" => ActiveSupport::JSON.encode(data) assert_equal [ data ], channel.lines end @@ -88,7 +89,7 @@ class ActionCable::Connection::SubscriptionsTest < ActionCable::TestCase channel1 = subscribe_to_chat_channel - channel2_id = ActiveSupport::JSON.encode(id: 2, channel: 'ActionCable::Connection::SubscriptionsTest::ChatChannel') + channel2_id = ActiveSupport::JSON.encode(id: 2, channel: "ActionCable::Connection::SubscriptionsTest::ChatChannel") channel2 = subscribe_to_chat_channel(channel2_id) channel1.expects(:unsubscribe_from_channel) @@ -100,14 +101,14 @@ class ActionCable::Connection::SubscriptionsTest < ActionCable::TestCase private def subscribe_to_chat_channel(identifier = @chat_identifier) - @subscriptions.execute_command 'command' => 'subscribe', 'identifier' => identifier + @subscriptions.execute_command "command" => "subscribe", "identifier" => identifier assert_equal identifier, @subscriptions.identifiers.last - @subscriptions.send :find, 'identifier' => identifier + @subscriptions.send :find, "identifier" => identifier end def setup_connection - env = Rack::MockRequest.env_for "/test", 'HTTP_HOST' => 'localhost', 'HTTP_CONNECTION' => 'upgrade', 'HTTP_UPGRADE' => 'websocket' + env = Rack::MockRequest.env_for "/test", "HTTP_HOST" => "localhost", "HTTP_CONNECTION" => "upgrade", "HTTP_UPGRADE" => "websocket" @connection = Connection.new(@server, env) @subscriptions = ActionCable::Connection::Subscriptions.new(@connection) diff --git a/actioncable/test/javascript/src/test.coffee b/actioncable/test/javascript/src/test.coffee index 3ce88c7789..eb95fb2604 100644 --- a/actioncable/test/javascript/src/test.coffee +++ b/actioncable/test/javascript/src/test.coffee @@ -1,3 +1,3 @@ #= require action_cable -#= require_tree ./test_helpers +#= require ./test_helpers #= require_tree ./unit diff --git a/actioncable/test/javascript/src/test_helpers/consumer_test_helper.coffee b/actioncable/test/javascript/src/test_helpers/consumer_test_helper.coffee new file mode 100644 index 0000000000..a9e95c37f0 --- /dev/null +++ b/actioncable/test/javascript/src/test_helpers/consumer_test_helper.coffee @@ -0,0 +1,47 @@ +#= require mock-socket + +{TestHelpers} = ActionCable + +TestHelpers.consumerTest = (name, options = {}, callback) -> + unless callback? + callback = options + options = {} + + options.url ?= TestHelpers.testURL + + QUnit.test name, (assert) -> + doneAsync = assert.async() + + ActionCable.WebSocket = MockWebSocket + server = new MockServer options.url + consumer = ActionCable.createConsumer(options.url) + + server.on "connection", -> + clients = server.clients() + assert.equal clients.length, 1 + assert.equal clients[0].readyState, WebSocket.OPEN + + server.broadcastTo = (subscription, data = {}, callback) -> + data.identifier = subscription.identifier + + if data.message_type + data.type = ActionCable.INTERNAL.message_types[data.message_type] + delete data.message_type + + server.send(JSON.stringify(data)) + TestHelpers.defer(callback) + + done = -> + consumer.disconnect() + server.close() + doneAsync() + + testData = {assert, consumer, server, done} + + if options.connect is false + callback(testData) + else + server.on "connection", -> + testData.client = server.clients()[0] + callback(testData) + consumer.connect() diff --git a/actioncable/test/javascript/src/test_helpers/index.coffee b/actioncable/test/javascript/src/test_helpers/index.coffee index e0d1e412cd..c84cbbcb2c 100644 --- a/actioncable/test/javascript/src/test_helpers/index.coffee +++ b/actioncable/test/javascript/src/test_helpers/index.coffee @@ -3,3 +3,9 @@ ActionCable.TestHelpers = testURL: "ws://cable.example.com/" + + defer: (callback) -> + setTimeout(callback, 1) + +originalWebSocket = ActionCable.WebSocket +QUnit.testDone -> ActionCable.WebSocket = originalWebSocket diff --git a/actioncable/test/javascript/src/test_helpers/mock_websocket.coffee b/actioncable/test/javascript/src/test_helpers/mock_websocket.coffee deleted file mode 100644 index b7f86f18f6..0000000000 --- a/actioncable/test/javascript/src/test_helpers/mock_websocket.coffee +++ /dev/null @@ -1,21 +0,0 @@ -#= require mock-socket - -NativeWebSocket = window.WebSocket - -server = null -consumer = null - -ActionCable.TestHelpers.createConsumer = (url, callback) -> - window.WebSocket = MockWebSocket - server = new MockServer url - consumer = ActionCable.createConsumer(url) - callback(consumer, server) - -QUnit.testDone -> - if consumer? - consumer.disconnect() - - if server? - server.clients().forEach (client) -> client.close() - server.close() - window.WebSocket = NativeWebSocket diff --git a/actioncable/test/javascript/src/unit/action_cable_test.coffee b/actioncable/test/javascript/src/unit/action_cable_test.coffee index f9eff64769..3944f3a7f6 100644 --- a/actioncable/test/javascript/src/unit/action_cable_test.coffee +++ b/actioncable/test/javascript/src/unit/action_cable_test.coffee @@ -2,6 +2,23 @@ {testURL} = ActionCable.TestHelpers module "ActionCable", -> + module "Adapters", -> + module "WebSocket", -> + test "default is window.WebSocket", (assert) -> + assert.equal ActionCable.WebSocket, window.WebSocket + + test "configurable", (assert) -> + ActionCable.WebSocket = "" + assert.equal ActionCable.WebSocket, "" + + module "logger", -> + test "default is window.console", (assert) -> + assert.equal ActionCable.logger, window.console + + test "configurable", (assert) -> + ActionCable.logger = "" + assert.equal ActionCable.logger, "" + module "#createConsumer", -> test "uses specified URL", (assert) -> consumer = ActionCable.createConsumer(testURL) diff --git a/actioncable/test/javascript/src/unit/consumer_test.coffee b/actioncable/test/javascript/src/unit/consumer_test.coffee index d8b1450ad8..41445274eb 100644 --- a/actioncable/test/javascript/src/unit/consumer_test.coffee +++ b/actioncable/test/javascript/src/unit/consumer_test.coffee @@ -1,31 +1,14 @@ {module, test} = QUnit -{testURL, createConsumer} = ActionCable.TestHelpers +{consumerTest} = ActionCable.TestHelpers module "ActionCable.Consumer", -> - test "#connect", (assert) -> - done = assert.async() + consumerTest "#connect", connect: false, ({consumer, server, assert, done}) -> + server.on "connection", -> + assert.equal consumer.connect(), false + done() - createConsumer testURL, (consumer, server) -> - server.on "connection", -> - clients = server.clients() - assert.equal clients.length, 1 - assert.equal clients[0].readyState, WebSocket.OPEN - done() + consumer.connect() - consumer.connect() - - test "#disconnect", (assert) -> - done = assert.async() - - createConsumer testURL, (consumer, server) -> - server.on "connection", -> - clients = server.clients() - assert.equal clients.length, 1 - - clients[0].addEventListener "close", (event) -> - assert.equal event.type, "close" - done() - - consumer.disconnect() - - consumer.connect() + consumerTest "#disconnect", ({consumer, client, done}) -> + client.addEventListener("close", done) + consumer.disconnect() diff --git a/actioncable/test/javascript/src/unit/subscription_test.coffee b/actioncable/test/javascript/src/unit/subscription_test.coffee new file mode 100644 index 0000000000..07027ed170 --- /dev/null +++ b/actioncable/test/javascript/src/unit/subscription_test.coffee @@ -0,0 +1,40 @@ +{module, test} = QUnit +{consumerTest} = ActionCable.TestHelpers + +module "ActionCable.Subscription", -> + consumerTest "#initialized callback", ({server, consumer, assert, done}) -> + consumer.subscriptions.create "chat", + initialized: -> + assert.ok true + done() + + consumerTest "#connected callback", ({server, consumer, assert, done}) -> + subscription = consumer.subscriptions.create "chat", + connected: -> + assert.ok true + done() + + server.broadcastTo(subscription, message_type: "confirmation") + + consumerTest "#disconnected callback", ({server, consumer, assert, done}) -> + subscription = consumer.subscriptions.create "chat", + disconnected: -> + assert.ok true + done() + + server.broadcastTo subscription, message_type: "confirmation", -> + server.close() + + consumerTest "#perform", ({consumer, server, assert, done}) -> + subscription = consumer.subscriptions.create "chat", + connected: -> + @perform(publish: "hi") + + server.on "message", (message) -> + data = JSON.parse(message) + assert.equal data.identifier, subscription.identifier + assert.equal data.command, "message" + assert.deepEqual data.data, JSON.stringify(action: { publish: "hi" }) + done() + + server.broadcastTo(subscription, message_type: "confirmation") diff --git a/actioncable/test/javascript/src/unit/subscriptions_test.coffee b/actioncable/test/javascript/src/unit/subscriptions_test.coffee new file mode 100644 index 0000000000..170b370e4a --- /dev/null +++ b/actioncable/test/javascript/src/unit/subscriptions_test.coffee @@ -0,0 +1,25 @@ +{module, test} = QUnit +{consumerTest} = ActionCable.TestHelpers + +module "ActionCable.Subscriptions", -> + consumerTest "create subscription with channel string", ({consumer, server, assert, done}) -> + channel = "chat" + + server.on "message", (message) -> + data = JSON.parse(message) + assert.equal data.command, "subscribe" + assert.equal data.identifier, JSON.stringify({channel}) + done() + + consumer.subscriptions.create(channel) + + consumerTest "create subscription with channel object", ({consumer, server, assert, done}) -> + channel = channel: "chat", room: "action" + + server.on "message", (message) -> + data = JSON.parse(message) + assert.equal data.command, "subscribe" + assert.equal data.identifier, JSON.stringify(channel) + done() + + consumer.subscriptions.create(channel) diff --git a/actioncable/test/server/base_test.rb b/actioncable/test/server/base_test.rb new file mode 100644 index 0000000000..1312e45f49 --- /dev/null +++ b/actioncable/test/server/base_test.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_server" +require "active_support/core_ext/hash/indifferent_access" + +class BaseTest < ActiveSupport::TestCase + def setup + @server = ActionCable::Server::Base.new + @server.config.cable = { adapter: "async" }.with_indifferent_access + end + + class FakeConnection + def close + end + end + + test "#restart closes all open connections" do + conn = FakeConnection.new + @server.add_connection(conn) + + conn.expects(:close) + @server.restart + end + + test "#restart shuts down worker pool" do + @server.worker_pool.expects(:halt) + @server.restart + end + + test "#restart shuts down pub/sub adapter" do + @server.pubsub.expects(:shutdown) + @server.restart + end +end diff --git a/actioncable/test/server/broadcasting_test.rb b/actioncable/test/server/broadcasting_test.rb index 3b4a7eaf90..72cec26234 100644 --- a/actioncable/test/server/broadcasting_test.rb +++ b/actioncable/test/server/broadcasting_test.rb @@ -1,10 +1,9 @@ +# frozen_string_literal: true + require "test_helper" +require "stubs/test_server" class BroadcastingTest < ActiveSupport::TestCase - class TestServer - include ActionCable::Server::Broadcasting - end - test "fetching a broadcaster converts the broadcasting queue to a string" do broadcasting = :test_queue server = TestServer.new @@ -12,4 +11,52 @@ class BroadcastingTest < ActiveSupport::TestCase assert_equal "test_queue", broadcaster.broadcasting end + + test "broadcast generates notification" do + begin + server = TestServer.new + + events = [] + ActiveSupport::Notifications.subscribe "broadcast.action_cable" do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end + + broadcasting = "test_queue" + message = { body: "test message" } + server.broadcast(broadcasting, message) + + assert_equal 1, events.length + assert_equal "broadcast.action_cable", events[0].name + assert_equal broadcasting, events[0].payload[:broadcasting] + assert_equal message, events[0].payload[:message] + assert_equal ActiveSupport::JSON, events[0].payload[:coder] + ensure + ActiveSupport::Notifications.unsubscribe "broadcast.action_cable" + end + end + + test "broadcaster from broadcaster_for generates notification" do + begin + server = TestServer.new + + events = [] + ActiveSupport::Notifications.subscribe "broadcast.action_cable" do |*args| + events << ActiveSupport::Notifications::Event.new(*args) + end + + broadcasting = "test_queue" + message = { body: "test message" } + + broadcaster = server.broadcaster_for(broadcasting) + broadcaster.broadcast(message) + + assert_equal 1, events.length + assert_equal "broadcast.action_cable", events[0].name + assert_equal broadcasting, events[0].payload[:broadcasting] + assert_equal message, events[0].payload[:message] + assert_equal ActiveSupport::JSON, events[0].payload[:coder] + ensure + ActiveSupport::Notifications.unsubscribe "broadcast.action_cable" + end + end end diff --git a/actioncable/test/stubs/global_id.rb b/actioncable/test/stubs/global_id.rb index 334f0d03e8..15fab6b8a7 100644 --- a/actioncable/test/stubs/global_id.rb +++ b/actioncable/test/stubs/global_id.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class GlobalID attr_reader :uri delegate :to_param, :to_s, to: :uri diff --git a/actioncable/test/stubs/room.rb b/actioncable/test/stubs/room.rb index cd66a0b687..df7236f408 100644 --- a/actioncable/test/stubs/room.rb +++ b/actioncable/test/stubs/room.rb @@ -1,7 +1,9 @@ +# frozen_string_literal: true + class Room attr_reader :id, :name - def initialize(id, name='Campfire') + def initialize(id, name = "Campfire") @id = id @name = name end diff --git a/actioncable/test/stubs/test_adapter.rb b/actioncable/test/stubs/test_adapter.rb index bbd142b287..c481046973 100644 --- a/actioncable/test/stubs/test_adapter.rb +++ b/actioncable/test/stubs/test_adapter.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class SuccessAdapter < ActionCable::SubscriptionAdapter::Base def broadcast(channel, payload) end diff --git a/actioncable/test/stubs/test_connection.rb b/actioncable/test/stubs/test_connection.rb index 885450dda6..fdddd1159e 100644 --- a/actioncable/test/stubs/test_connection.rb +++ b/actioncable/test/stubs/test_connection.rb @@ -1,4 +1,6 @@ -require 'stubs/user' +# frozen_string_literal: true + +require "stubs/user" class TestConnection attr_reader :identifiers, :logger, :current_user, :server, :transmissions diff --git a/actioncable/test/stubs/test_server.rb b/actioncable/test/stubs/test_server.rb index b86f422a13..0bc4625e28 100644 --- a/actioncable/test/stubs/test_server.rb +++ b/actioncable/test/stubs/test_server.rb @@ -1,4 +1,6 @@ -require 'ostruct' +# frozen_string_literal: true + +require "ostruct" class TestServer include ActionCable::Server::Connections @@ -10,14 +12,8 @@ class TestServer @logger = ActiveSupport::TaggedLogging.new ActiveSupport::Logger.new(StringIO.new) @config = OpenStruct.new(log_tags: [], subscription_adapter: subscription_adapter) - @config.use_faye = ENV['FAYE'].present? - @config.client_socket_class = if @config.use_faye - ActionCable::Connection::FayeClientSocket - else - ActionCable::Connection::ClientSocket - end - - @mutex = Monitor.new + + @mutex = Monitor.new end def pubsub @@ -25,11 +21,9 @@ class TestServer end def event_loop - @event_loop ||= if @config.use_faye - ActionCable::Connection::FayeEventLoop.new - else - ActionCable::Connection::StreamEventLoop.new - end + @event_loop ||= ActionCable::Connection::StreamEventLoop.new.tap do |loop| + loop.instance_variable_set(:@executor, Concurrent.global_io_executor) + end end def worker_pool diff --git a/actioncable/test/stubs/user.rb b/actioncable/test/stubs/user.rb index a66b4f87d5..7894d1d9ae 100644 --- a/actioncable/test/stubs/user.rb +++ b/actioncable/test/stubs/user.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class User attr_reader :name diff --git a/actioncable/test/subscription_adapter/async_test.rb b/actioncable/test/subscription_adapter/async_test.rb index 8f413f14c2..6e038259b5 100644 --- a/actioncable/test/subscription_adapter/async_test.rb +++ b/actioncable/test/subscription_adapter/async_test.rb @@ -1,5 +1,7 @@ -require 'test_helper' -require_relative './common' +# frozen_string_literal: true + +require "test_helper" +require_relative "common" class AsyncAdapterTest < ActionCable::TestCase include CommonSubscriptionAdapterTest @@ -12,6 +14,6 @@ class AsyncAdapterTest < ActionCable::TestCase end def cable_config - { adapter: 'async' } + { adapter: "async" } end end diff --git a/actioncable/test/subscription_adapter/base_test.rb b/actioncable/test/subscription_adapter/base_test.rb index 256dce673f..999dc0cba1 100644 --- a/actioncable/test/subscription_adapter/base_test.rb +++ b/actioncable/test/subscription_adapter/base_test.rb @@ -1,5 +1,7 @@ -require 'test_helper' -require 'stubs/test_server' +# frozen_string_literal: true + +require "test_helper" +require "stubs/test_server" class ActionCable::SubscriptionAdapter::BaseTest < ActionCable::TestCase ## TEST THAT ERRORS ARE RETURNED FOR INHERITORS THAT DON'T OVERRIDE METHODS @@ -15,59 +17,49 @@ class ActionCable::SubscriptionAdapter::BaseTest < ActionCable::TestCase test "#broadcast returns NotImplementedError by default" do assert_raises NotImplementedError do - BrokenAdapter.new(@server).broadcast('channel', 'payload') + BrokenAdapter.new(@server).broadcast("channel", "payload") end end test "#subscribe returns NotImplementedError by default" do - callback = lambda { puts 'callback' } - success_callback = lambda { puts 'success' } + callback = lambda { puts "callback" } + success_callback = lambda { puts "success" } assert_raises NotImplementedError do - BrokenAdapter.new(@server).subscribe('channel', callback, success_callback) + BrokenAdapter.new(@server).subscribe("channel", callback, success_callback) end end test "#unsubscribe returns NotImplementedError by default" do - callback = lambda { puts 'callback' } + callback = lambda { puts "callback" } assert_raises NotImplementedError do - BrokenAdapter.new(@server).unsubscribe('channel', callback) + BrokenAdapter.new(@server).unsubscribe("channel", callback) end end # TEST METHODS THAT ARE REQUIRED OF THE ADAPTER'S BACKEND STORAGE OBJECT test "#broadcast is implemented" do - broadcast = SuccessAdapter.new(@server).broadcast('channel', 'payload') - - assert_respond_to(SuccessAdapter.new(@server), :broadcast) - assert_nothing_raised do - broadcast + SuccessAdapter.new(@server).broadcast("channel", "payload") end end test "#subscribe is implemented" do - callback = lambda { puts 'callback' } - success_callback = lambda { puts 'success' } - subscribe = SuccessAdapter.new(@server).subscribe('channel', callback, success_callback) - - assert_respond_to(SuccessAdapter.new(@server), :subscribe) + callback = lambda { puts "callback" } + success_callback = lambda { puts "success" } assert_nothing_raised do - subscribe + SuccessAdapter.new(@server).subscribe("channel", callback, success_callback) end end test "#unsubscribe is implemented" do - callback = lambda { puts 'callback' } - unsubscribe = SuccessAdapter.new(@server).unsubscribe('channel', callback) - - assert_respond_to(SuccessAdapter.new(@server), :unsubscribe) + callback = lambda { puts "callback" } assert_nothing_raised do - unsubscribe + SuccessAdapter.new(@server).unsubscribe("channel", callback) end end end diff --git a/actioncable/test/subscription_adapter/channel_prefix.rb b/actioncable/test/subscription_adapter/channel_prefix.rb new file mode 100644 index 0000000000..3071facd9d --- /dev/null +++ b/actioncable/test/subscription_adapter/channel_prefix.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "test_helper" + +class ActionCable::Server::WithIndependentConfig < ActionCable::Server::Base + # ActionCable::Server::Base defines config as a class variable. + # Need config to be an instance variable here as we're testing 2 separate configs + def config + @config ||= ActionCable::Server::Configuration.new + end +end + +module ChannelPrefixTest + def test_channel_prefix + server2 = ActionCable::Server::WithIndependentConfig.new + server2.config.cable = alt_cable_config + server2.config.logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } + + adapter_klass = server2.config.pubsub_adapter + + rx_adapter2 = adapter_klass.new(server2) + tx_adapter2 = adapter_klass.new(server2) + + subscribe_as_queue("channel") do |queue| + subscribe_as_queue("channel", rx_adapter2) do |queue2| + @tx_adapter.broadcast("channel", "hello world") + tx_adapter2.broadcast("channel", "hello world 2") + + assert_equal "hello world", queue.pop + assert_equal "hello world 2", queue2.pop + end + end + end + + def alt_cable_config + cable_config.merge(channel_prefix: "foo") + end +end diff --git a/actioncable/test/subscription_adapter/common.rb b/actioncable/test/subscription_adapter/common.rb index 285c690df0..c533a9f3eb 100644 --- a/actioncable/test/subscription_adapter/common.rb +++ b/actioncable/test/subscription_adapter/common.rb @@ -1,8 +1,10 @@ -require 'test_helper' -require 'concurrent' +# frozen_string_literal: true -require 'active_support/core_ext/hash/indifferent_access' -require 'pathname' +require "test_helper" +require "concurrent" + +require "active_support/core_ext/hash/indifferent_access" +require "pathname" module CommonSubscriptionAdapterTest WAIT_WHEN_EXPECTING_EVENT = 3 @@ -11,7 +13,7 @@ module CommonSubscriptionAdapterTest def setup server = ActionCable::Server::Base.new server.config.cable = cable_config.with_indifferent_access - server.config.use_faye = ENV['FAYE'].present? + server.config.logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } adapter_klass = server.config.pubsub_adapter @@ -20,10 +22,9 @@ module CommonSubscriptionAdapterTest end def teardown - [@rx_adapter, @tx_adapter].uniq.each(&:shutdown) + [@rx_adapter, @tx_adapter].uniq.compact.each(&:shutdown) end - def subscribe_as_queue(channel, adapter = @rx_adapter) queue = Queue.new @@ -41,77 +42,90 @@ module CommonSubscriptionAdapterTest adapter.unsubscribe(channel, callback) if subscribed.set? end - def test_subscribe_and_unsubscribe - subscribe_as_queue('channel') do |queue| + subscribe_as_queue("channel") do |queue| end end def test_basic_broadcast - subscribe_as_queue('channel') do |queue| - @tx_adapter.broadcast('channel', 'hello world') + subscribe_as_queue("channel") do |queue| + @tx_adapter.broadcast("channel", "hello world") - assert_equal 'hello world', queue.pop + assert_equal "hello world", queue.pop end end def test_broadcast_after_unsubscribe keep_queue = nil - subscribe_as_queue('channel') do |queue| + subscribe_as_queue("channel") do |queue| keep_queue = queue - @tx_adapter.broadcast('channel', 'hello world') + @tx_adapter.broadcast("channel", "hello world") - assert_equal 'hello world', queue.pop + assert_equal "hello world", queue.pop end - @tx_adapter.broadcast('channel', 'hello void') + @tx_adapter.broadcast("channel", "hello void") sleep WAIT_WHEN_NOT_EXPECTING_EVENT assert_empty keep_queue end def test_multiple_broadcast - subscribe_as_queue('channel') do |queue| - @tx_adapter.broadcast('channel', 'bananas') - @tx_adapter.broadcast('channel', 'apples') + subscribe_as_queue("channel") do |queue| + @tx_adapter.broadcast("channel", "bananas") + @tx_adapter.broadcast("channel", "apples") received = [] 2.times { received << queue.pop } - assert_equal ['apples', 'bananas'], received.sort + assert_equal ["apples", "bananas"], received.sort end end def test_identical_subscriptions - subscribe_as_queue('channel') do |queue| - subscribe_as_queue('channel') do |queue_2| - @tx_adapter.broadcast('channel', 'hello') + subscribe_as_queue("channel") do |queue| + subscribe_as_queue("channel") do |queue_2| + @tx_adapter.broadcast("channel", "hello") - assert_equal 'hello', queue_2.pop + assert_equal "hello", queue_2.pop end - assert_equal 'hello', queue.pop + assert_equal "hello", queue.pop end end def test_simultaneous_subscriptions - subscribe_as_queue('channel') do |queue| - subscribe_as_queue('other channel') do |queue_2| - @tx_adapter.broadcast('channel', 'apples') - @tx_adapter.broadcast('other channel', 'oranges') + subscribe_as_queue("channel") do |queue| + subscribe_as_queue("other channel") do |queue_2| + @tx_adapter.broadcast("channel", "apples") + @tx_adapter.broadcast("other channel", "oranges") - assert_equal 'apples', queue.pop - assert_equal 'oranges', queue_2.pop + assert_equal "apples", queue.pop + assert_equal "oranges", queue_2.pop end end end def test_channel_filtered_broadcast - subscribe_as_queue('channel') do |queue| - @tx_adapter.broadcast('other channel', 'one') - @tx_adapter.broadcast('channel', 'two') + subscribe_as_queue("channel") do |queue| + @tx_adapter.broadcast("other channel", "one") + @tx_adapter.broadcast("channel", "two") + + assert_equal "two", queue.pop + end + end - assert_equal 'two', queue.pop + def test_long_identifiers + channel_1 = "a" * 100 + "1" + channel_2 = "a" * 100 + "2" + subscribe_as_queue(channel_1) do |queue| + subscribe_as_queue(channel_2) do |queue_2| + @tx_adapter.broadcast(channel_1, "apples") + @tx_adapter.broadcast(channel_2, "oranges") + + assert_equal "apples", queue.pop + assert_equal "oranges", queue_2.pop + end end end 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 6d20e6ed78..0000000000 --- a/actioncable/test/subscription_adapter/evented_redis_test.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'test_helper' -require_relative './common' - -class EventedRedisAdapterTest < ActionCable::TestCase - include CommonSubscriptionAdapterTest - - def setup - super - - # em-hiredis is warning-rich - @previous_verbose, $VERBOSE = $VERBOSE, nil - end - - def teardown - $VERBOSE = @previous_verbose - end - - def cable_config - { adapter: 'evented_redis', url: 'redis://127.0.0.1:6379/12' } - end -end diff --git a/actioncable/test/subscription_adapter/inline_test.rb b/actioncable/test/subscription_adapter/inline_test.rb index 75ea51e6b3..6305626b2b 100644 --- a/actioncable/test/subscription_adapter/inline_test.rb +++ b/actioncable/test/subscription_adapter/inline_test.rb @@ -1,5 +1,7 @@ -require 'test_helper' -require_relative './common' +# frozen_string_literal: true + +require "test_helper" +require_relative "common" class InlineAdapterTest < ActionCable::TestCase include CommonSubscriptionAdapterTest @@ -12,6 +14,6 @@ class InlineAdapterTest < ActionCable::TestCase end def cable_config - { adapter: 'inline' } + { adapter: "inline" } end end diff --git a/actioncable/test/subscription_adapter/postgresql_test.rb b/actioncable/test/subscription_adapter/postgresql_test.rb index 214352a0b2..1c375188ba 100644 --- a/actioncable/test/subscription_adapter/postgresql_test.rb +++ b/actioncable/test/subscription_adapter/postgresql_test.rb @@ -1,18 +1,20 @@ -require 'test_helper' -require_relative './common' +# frozen_string_literal: true -require 'active_record' +require "test_helper" +require_relative "common" + +require "active_record" class PostgresqlAdapterTest < ActionCable::TestCase include CommonSubscriptionAdapterTest def setup - database_config = { 'adapter' => 'postgresql', 'database' => 'activerecord_unittest' } - ar_tests = File.expand_path('../../../activerecord/test', __dir__) + database_config = { "adapter" => "postgresql", "database" => "activerecord_unittest" } + ar_tests = File.expand_path("../../../activerecord/test", __dir__) if Dir.exist?(ar_tests) - require File.join(ar_tests, 'config') - require File.join(ar_tests, 'support/config') - local_config = ARTest.config['arunit'] + require File.join(ar_tests, "config") + require File.join(ar_tests, "support/config") + local_config = ARTest.config["connections"]["postgresql"]["arunit"] database_config.update local_config if local_config end @@ -35,6 +37,6 @@ class PostgresqlAdapterTest < ActionCable::TestCase end def cable_config - { adapter: 'postgresql' } + { adapter: "postgresql" } end end diff --git a/actioncable/test/subscription_adapter/redis_test.rb b/actioncable/test/subscription_adapter/redis_test.rb index 4f34dd86c9..63823d6ef0 100644 --- a/actioncable/test/subscription_adapter/redis_test.rb +++ b/actioncable/test/subscription_adapter/redis_test.rb @@ -1,16 +1,47 @@ -require 'test_helper' -require_relative './common' +# frozen_string_literal: true + +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://127.0.0.1:6379/12' } + { adapter: "redis", driver: "ruby" } end end class RedisAdapterTest::Hiredis < RedisAdapterTest def cable_config - super.merge(driver: 'hiredis') + super.merge(driver: "hiredis") + end +end + +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) + 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 diff --git a/actioncable/test/subscription_adapter/subscriber_map_test.rb b/actioncable/test/subscription_adapter/subscriber_map_test.rb new file mode 100644 index 0000000000..ed81099cbc --- /dev/null +++ b/actioncable/test/subscription_adapter/subscriber_map_test.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require "test_helper" + +class SubscriberMapTest < ActionCable::TestCase + test "broadcast should not change subscribers" do + setup_subscription_map + origin = @subscription_map.instance_variable_get(:@subscribers).dup + + @subscription_map.broadcast("not_exist_channel", "") + + assert_equal origin, @subscription_map.instance_variable_get(:@subscribers) + end + + private + def setup_subscription_map + @subscription_map = ActionCable::SubscriptionAdapter::SubscriberMap.new + end +end diff --git a/actioncable/test/test_helper.rb b/actioncable/test/test_helper.rb index 0a9ee7ce77..2a4611fb37 100644 --- a/actioncable/test/test_helper.rb +++ b/actioncable/test/test_helper.rb @@ -1,53 +1,21 @@ -require 'action_cable' -require 'active_support/testing/autorun' +# frozen_string_literal: true -require 'puma' -require 'mocha/setup' -require 'rack/mock' +require "action_cable" +require "active_support/testing/autorun" + +require "puma" +require "mocha/setup" +require "rack/mock" begin - require 'byebug' + require "byebug" rescue LoadError end # Require all the stubs and models -Dir[File.dirname(__FILE__) + '/stubs/*.rb'].each {|file| require file } - -if ENV['FAYE'].present? - require 'faye/websocket' - class << Faye::WebSocket - remove_method :ensure_reactor_running - - # We don't want Faye to start the EM reactor in tests because it makes testing much harder. - # We want to be able to start and stop EM loop in tests to make things simpler. - def ensure_reactor_running - # no-op - end - end -end +Dir[File.expand_path("stubs/*.rb", __dir__)].each { |file| require file } -module EventMachineConcurrencyHelpers - def wait_for_async - EM.run_deferred_callbacks - end - - def run_in_eventmachine - failure = nil - EM.run do - begin - yield - rescue => ex - failure = ex - ensure - wait_for_async - EM.stop if EM.reactor_running? - end - end - raise failure if failure - end -end - -module ConcurrentRubyConcurrencyHelpers +class ActionCable::TestCase < ActiveSupport::TestCase def wait_for_async wait_for_executor Concurrent.global_io_executor end @@ -56,18 +24,14 @@ module ConcurrentRubyConcurrencyHelpers yield wait_for_async end -end - -class ActionCable::TestCase < ActiveSupport::TestCase - if ENV['FAYE'].present? - include EventMachineConcurrencyHelpers - else - include ConcurrentRubyConcurrencyHelpers - end def wait_for_executor(executor) + # do not wait forever, wait 2s + timeout = 2 until executor.completed_task_count == executor.scheduled_task_count sleep 0.1 + timeout -= 0.1 + raise "Executor could not complete all tasks in 2 seconds" unless timeout > 0 end end end diff --git a/actioncable/test/worker_test.rb b/actioncable/test/worker_test.rb index e2c81fe312..bc1f3e415a 100644 --- a/actioncable/test/worker_test.rb +++ b/actioncable/test/worker_test.rb @@ -1,4 +1,6 @@ -require 'test_helper' +# frozen_string_literal: true + +require "test_helper" class WorkerTest < ActiveSupport::TestCase class Receiver @@ -9,7 +11,7 @@ class WorkerTest < ActiveSupport::TestCase end def process(message) - @last_action = [ :process, message ] + @last_action = [ :process, message ] end def connection |