diff options
34 files changed, 506 insertions, 126 deletions
@@ -63,6 +63,9 @@ end # Action Cable group :cable do gem 'puma', require: false + + gem 'em-hiredis', require: false + gem 'redis', require: false end # Add your own local bundler stuff. diff --git a/Gemfile.lock b/Gemfile.lock index 6b29d2c44b..0789c59905 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -139,6 +139,9 @@ GEM delayed_job_active_record (4.1.0) activerecord (>= 3.0, < 5) delayed_job (>= 3.0, < 5) + em-hiredis (0.3.0) + eventmachine (~> 1.0) + hiredis (~> 0.5.0) erubis (2.7.0) eventmachine (1.0.9.1) execjs (2.6.0) @@ -150,6 +153,7 @@ GEM ffi (1.9.10-x86-mingw32) globalid (0.3.6) activesupport (>= 4.1.0) + hiredis (0.5.2) hitimes (1.2.3) hitimes (1.2.3-x86-mingw32) i18n (0.7.0) @@ -302,6 +306,7 @@ DEPENDENCIES dalli (>= 2.2.1) delayed_job delayed_job_active_record + em-hiredis jquery-rails json kindlerb (= 0.1.1) @@ -322,6 +327,7 @@ DEPENDENCIES rails! rake (>= 10.3) redcarpet (~> 3.2.3) + redis resque resque-scheduler sass! diff --git a/actioncable/lib/action_cable/channel/streams.rb b/actioncable/lib/action_cable/channel/streams.rb index 589946c3db..e2876ef6fa 100644 --- a/actioncable/lib/action_cable/channel/streams.rb +++ b/actioncable/lib/action_cable/channel/streams.rb @@ -76,7 +76,7 @@ module ActionCable streams << [ broadcasting, callback ] EM.next_tick do - pubsub.subscribe(broadcasting, callback, lambda do |reply| + pubsub.subscribe(broadcasting, callback, lambda do transmit_subscription_confirmation logger.info "#{self.class.name} is streaming from #{broadcasting}" end) diff --git a/actioncable/lib/action_cable/subscription_adapter.rb b/actioncable/lib/action_cable/subscription_adapter.rb index e770f4fb00..72e62f3daf 100644 --- a/actioncable/lib/action_cable/subscription_adapter.rb +++ b/actioncable/lib/action_cable/subscription_adapter.rb @@ -1,5 +1,8 @@ module ActionCable module SubscriptionAdapter - autoload :Base, 'action_cable/subscription_adapter/base' + extend ActiveSupport::Autoload + + autoload :Base + autoload :SubscriberMap end end diff --git a/actioncable/lib/action_cable/subscription_adapter/async.rb b/actioncable/lib/action_cable/subscription_adapter/async.rb new file mode 100644 index 0000000000..85d4892e4c --- /dev/null +++ b/actioncable/lib/action_cable/subscription_adapter/async.rb @@ -0,0 +1,22 @@ +require 'action_cable/subscription_adapter/inline' + +module ActionCable + module SubscriptionAdapter + class Async < Inline # :nodoc: + private + def subscriber_map + @subscriber_map ||= AsyncSubscriberMap.new + end + + class AsyncSubscriberMap < SubscriberMap + def add_subscriber(*) + ::EM.next_tick { super } + end + + def invoke_callback(*) + ::EM.next_tick { super } + end + end + end + end +end diff --git a/actioncable/lib/action_cable/subscription_adapter/base.rb b/actioncable/lib/action_cable/subscription_adapter/base.rb index 11910803e8..796db5ffa3 100644 --- a/actioncable/lib/action_cable/subscription_adapter/base.rb +++ b/actioncable/lib/action_cable/subscription_adapter/base.rb @@ -19,6 +19,10 @@ module ActionCable def unsubscribe(channel, message_callback) raise NotImplementedError end + + def shutdown + raise NotImplementedError + end end end end diff --git a/actioncable/lib/action_cable/subscription_adapter/inline.rb b/actioncable/lib/action_cable/subscription_adapter/inline.rb new file mode 100644 index 0000000000..4a2a8d23a2 --- /dev/null +++ b/actioncable/lib/action_cable/subscription_adapter/inline.rb @@ -0,0 +1,26 @@ +module ActionCable + module SubscriptionAdapter + class Inline < Base # :nodoc: + def broadcast(channel, payload) + subscriber_map.broadcast(channel, payload) + end + + def subscribe(channel, callback, success_callback = nil) + subscriber_map.add_subscriber(channel, callback, success_callback) + end + + def unsubscribe(channel, callback) + subscriber_map.remove_subscriber(channel, callback) + end + + def shutdown + # nothing to do + end + + private + def subscriber_map + @subscriber_map ||= SubscriberMap.new + end + end + end +end diff --git a/actioncable/lib/action_cable/subscription_adapter/postgresql.rb b/actioncable/lib/action_cable/subscription_adapter/postgresql.rb index 6465663c97..78f8aeb599 100644 --- a/actioncable/lib/action_cable/subscription_adapter/postgresql.rb +++ b/actioncable/lib/action_cable/subscription_adapter/postgresql.rb @@ -12,11 +12,15 @@ module ActionCable end def subscribe(channel, callback, success_callback = nil) - listener.subscribe_to(channel, callback, success_callback) + listener.add_subscriber(channel, callback, success_callback) end def unsubscribe(channel, callback) - listener.unsubscribe_from(channel, callback) + listener.remove_subscriber(channel, callback) + end + + def shutdown + listener.shutdown end def with_connection(&block) # :nodoc: @@ -36,14 +40,14 @@ module ActionCable @listener ||= Listener.new(self) end - class Listener + class Listener < SubscriberMap def initialize(adapter) + super() + @adapter = adapter - @subscribers = Hash.new { |h,k| h[k] = [] } - @sync = Mutex.new @queue = Queue.new - Thread.new do + @thread = Thread.new do Thread.current.abort_on_exception = true listen end @@ -51,46 +55,45 @@ module ActionCable def listen @adapter.with_connection do |pg_conn| - loop do - until @queue.empty? - action, channel, callback = @queue.pop(true) - escaped_channel = pg_conn.escape_identifier(channel) - - if action == :listen - pg_conn.exec("LISTEN #{escaped_channel}") - ::EM.next_tick(&callback) if callback - elsif action == :unlisten - pg_conn.exec("UNLISTEN #{escaped_channel}") + catch :shutdown do + loop do + until @queue.empty? + action, channel, callback = @queue.pop(true) + + case action + when :listen + pg_conn.exec("LISTEN #{pg_conn.escape_identifier channel}") + ::EM.next_tick(&callback) if callback + when :unlisten + pg_conn.exec("UNLISTEN #{pg_conn.escape_identifier channel}") + when :shutdown + throw :shutdown + end end - end - pg_conn.wait_for_notify(1) do |chan, pid, message| - @subscribers[chan].each do |callback| - ::EM.next_tick { callback.call(message) } + pg_conn.wait_for_notify(1) do |chan, pid, message| + broadcast(chan, message) end end end end end - def subscribe_to(channel, callback, success_callback) - @sync.synchronize do - if @subscribers[channel].empty? - @queue.push([:listen, channel, success_callback]) - end + def shutdown + @queue.push([:shutdown]) + Thread.pass while @thread.alive? + end - @subscribers[channel] << callback - end + def add_channel(channel, on_success) + @queue.push([:listen, channel, on_success]) end - def unsubscribe_from(channel, callback) - @sync.synchronize do - @subscribers[channel].delete(callback) + def remove_channel(channel) + @queue.push([:unlisten, channel]) + end - if @subscribers[channel].empty? - @queue.push([:unlisten, channel]) - end - end + def invoke_callback(*) + ::EM.next_tick { super } end end end diff --git a/actioncable/lib/action_cable/subscription_adapter/redis.rb b/actioncable/lib/action_cable/subscription_adapter/redis.rb index e42ab2a03f..3b86354621 100644 --- a/actioncable/lib/action_cable/subscription_adapter/redis.rb +++ b/actioncable/lib/action_cable/subscription_adapter/redis.rb @@ -12,7 +12,7 @@ module ActionCable def subscribe(channel, message_callback, success_callback = nil) redis_connection_for_subscriptions.pubsub.subscribe(channel, &message_callback).tap do |result| - result.callback(&success_callback) if success_callback + result.callback { |reply| success_callback.call } if success_callback end end @@ -20,6 +20,11 @@ module ActionCable 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 @redis_connection_for_subscriptions ||= EM::Hiredis.connect(@server.config.cable[:url]).tap do |redis| diff --git a/actioncable/lib/action_cable/subscription_adapter/subscriber_map.rb b/actioncable/lib/action_cable/subscription_adapter/subscriber_map.rb new file mode 100644 index 0000000000..37eed09793 --- /dev/null +++ b/actioncable/lib/action_cable/subscription_adapter/subscriber_map.rb @@ -0,0 +1,53 @@ +module ActionCable + module SubscriptionAdapter + class SubscriberMap + def initialize + @subscribers = Hash.new { |h,k| h[k] = [] } + @sync = Mutex.new + end + + def add_subscriber(channel, subscriber, on_success) + @sync.synchronize do + new_channel = !@subscribers.key?(channel) + + @subscribers[channel] << subscriber + + if new_channel + add_channel channel, on_success + elsif on_success + on_success.call + end + end + end + + def remove_subscriber(channel, subscriber) + @sync.synchronize do + @subscribers[channel].delete(subscriber) + + if @subscribers[channel].empty? + @subscribers.delete channel + remove_channel channel + end + end + end + + def broadcast(channel, message) + list = @sync.synchronize { @subscribers[channel].dup } + list.each do |subscriber| + invoke_callback(subscriber, message) + end + end + + def add_channel(channel, on_success) + on_success.call if on_success + end + + def remove_channel(channel) + end + + def invoke_callback(callback, message) + callback.call message + end + end + end +end diff --git a/actioncable/test/subscription_adapter/async_test.rb b/actioncable/test/subscription_adapter/async_test.rb new file mode 100644 index 0000000000..8f413f14c2 --- /dev/null +++ b/actioncable/test/subscription_adapter/async_test.rb @@ -0,0 +1,17 @@ +require 'test_helper' +require_relative './common' + +class AsyncAdapterTest < ActionCable::TestCase + include CommonSubscriptionAdapterTest + + def setup + super + + @tx_adapter.shutdown + @tx_adapter = @rx_adapter + end + + def cable_config + { adapter: 'async' } + end +end diff --git a/actioncable/test/subscription_adapter/common.rb b/actioncable/test/subscription_adapter/common.rb new file mode 100644 index 0000000000..d4a13be889 --- /dev/null +++ b/actioncable/test/subscription_adapter/common.rb @@ -0,0 +1,142 @@ +require 'test_helper' +require 'concurrent' + +require 'action_cable/process/logging' +require 'active_support/core_ext/hash/indifferent_access' +require 'pathname' + +module CommonSubscriptionAdapterTest + WAIT_WHEN_EXPECTING_EVENT = 3 + WAIT_WHEN_NOT_EXPECTING_EVENT = 0.2 + + def setup + # TODO: ActionCable requires a *lot* of setup at the moment... + ::Object.const_set(:ApplicationCable, Module.new) + ::ApplicationCable.const_set(:Connection, Class.new(ActionCable::Connection::Base)) + + ::Object.const_set(:Rails, Module.new) + ::Rails.singleton_class.send(:define_method, :root) { Pathname.new(__dir__) } + + server = ActionCable::Server::Base.new + server.config = ActionCable::Server::Configuration.new + inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } + server.config.logger = ActionCable::Connection::TaggedLoggerProxy.new(inner_logger, tags: []) + + + # and now the "real" setup for our test: + spawn_eventmachine + + server.config.cable = cable_config.with_indifferent_access + + adapter_klass = server.config.pubsub_adapter + + @rx_adapter = adapter_klass.new(server) + @tx_adapter = adapter_klass.new(server) + end + + def teardown + @tx_adapter.shutdown if @tx_adapter && @tx_adapter != @rx_adapter + @rx_adapter.shutdown if @rx_adapter + + begin + ::Object.send(:remove_const, :ApplicationCable) + rescue NameError + end + begin + ::Object.send(:remove_const, :Rails) + rescue NameError + end + end + + + def subscribe_as_queue(channel, adapter = @rx_adapter) + queue = Queue.new + + callback = -> data { queue << data } + subscribed = Concurrent::Event.new + adapter.subscribe(channel, callback, Proc.new { subscribed.set }) + subscribed.wait(WAIT_WHEN_EXPECTING_EVENT) + assert subscribed.set? + + yield queue + + sleep WAIT_WHEN_NOT_EXPECTING_EVENT + assert_empty queue + ensure + adapter.unsubscribe(channel, callback) if subscribed.set? + end + + + def test_subscribe_and_unsubscribe + subscribe_as_queue('channel') do |queue| + end + end + + def test_basic_broadcast + subscribe_as_queue('channel') do |queue| + @tx_adapter.broadcast('channel', 'hello world') + + assert_equal 'hello world', queue.pop + end + end + + def test_broadcast_after_unsubscribe + keep_queue = nil + subscribe_as_queue('channel') do |queue| + keep_queue = queue + + @tx_adapter.broadcast('channel', 'hello world') + + assert_equal 'hello world', queue.pop + end + + @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') + + received = [] + 2.times { received << queue.pop } + 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') + + assert_equal 'hello', queue_2.pop + end + + 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') + + 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') + + assert_equal 'two', queue.pop + end + end +end diff --git a/actioncable/test/subscription_adapter/inline_test.rb b/actioncable/test/subscription_adapter/inline_test.rb new file mode 100644 index 0000000000..75ea51e6b3 --- /dev/null +++ b/actioncable/test/subscription_adapter/inline_test.rb @@ -0,0 +1,17 @@ +require 'test_helper' +require_relative './common' + +class InlineAdapterTest < ActionCable::TestCase + include CommonSubscriptionAdapterTest + + def setup + super + + @tx_adapter.shutdown + @tx_adapter = @rx_adapter + end + + def cable_config + { adapter: 'inline' } + end +end diff --git a/actioncable/test/subscription_adapter/postgresql_test.rb b/actioncable/test/subscription_adapter/postgresql_test.rb new file mode 100644 index 0000000000..64c632b0cd --- /dev/null +++ b/actioncable/test/subscription_adapter/postgresql_test.rb @@ -0,0 +1,32 @@ +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__) + if Dir.exist?(ar_tests) + require File.join(ar_tests, 'config') + require File.join(ar_tests, 'support/config') + local_config = ARTest.config['arunit'] + database_config.update local_config if local_config + end + ActiveRecord::Base.establish_connection database_config + + super + end + + def teardown + super + + ActiveRecord::Base.clear_all_connections! + end + + def cable_config + { adapter: 'postgresql' } + end +end diff --git a/actioncable/test/subscription_adapter/redis_test.rb b/actioncable/test/subscription_adapter/redis_test.rb new file mode 100644 index 0000000000..8d52832c87 --- /dev/null +++ b/actioncable/test/subscription_adapter/redis_test.rb @@ -0,0 +1,10 @@ +require 'test_helper' +require_relative './common' + +class RedisAdapterTest < ActionCable::TestCase + include CommonSubscriptionAdapterTest + + def cable_config + { adapter: 'redis', url: 'redis://127.0.0.1:6379/12' } + end +end diff --git a/actioncable/test/test_helper.rb b/actioncable/test/test_helper.rb index 65b45e0c89..6636ce078b 100644 --- a/actioncable/test/test_helper.rb +++ b/actioncable/test/test_helper.rb @@ -33,4 +33,8 @@ class ActionCable::TestCase < ActiveSupport::TestCase EM.stop end end + + def spawn_eventmachine + Thread.new { EventMachine.run } unless EventMachine.reactor_running? + end end diff --git a/actioncable/test/worker_test.rb b/actioncable/test/worker_test.rb index 9911a3b98b..4a699cde27 100644 --- a/actioncable/test/worker_test.rb +++ b/actioncable/test/worker_test.rb @@ -13,6 +13,11 @@ class WorkerTest < ActiveSupport::TestCase end def connection + self + end + + def logger + ActionCable.server.logger end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 81799b65d6..f3dc26ddb3 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Using `references` or `belongs_to` in migrations will always add index + for the referenced column by default, without adding `index: true` option + to generated migration file. Users can opt out of this by passing + `index: false`. + + Fixes #18146. + + *Matthew Draper*, *Prathamesh Sonpatki* + * Run `type` attributes through attributes API type-casting before instantiating the corresponding subclass. This makes it possible to define custom STI mappings. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index 1cda23dc1d..690e0ba957 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -69,7 +69,7 @@ module ActiveRecord def initialize( name, polymorphic: false, - index: false, + index: true, foreign_key: false, type: :integer, **options diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index 1b94573870..5d742b523b 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -5,6 +5,12 @@ module ActiveRecord module FourTwoShared module TableDefinition + def references(*, **options) + options[:index] ||= false + super + end + alias :belongs_to :references + def timestamps(*, **options) options[:null] = true if options[:null].nil? super @@ -24,6 +30,12 @@ module ActiveRecord end end + def add_reference(*, **options) + options[:index] ||= false + super + end + alias :add_belongs_to :add_reference + def add_timestamps(*, **options) options[:null] = true if options[:null].nil? super diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index d81d6b54b3..69a7838001 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -302,7 +302,7 @@ db_namespace = namespace :db do end desc "Recreates the databases from the structure.sql file" - task :load => [:load_config] do + task :load => [:environment, :load_config] do ActiveRecord::Tasks::DatabaseTasks.load_schema_current(:sql, ENV['SCHEMA']) end diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index b1e1d72944..6a9cdd9d29 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -53,6 +53,24 @@ module ActiveRecord ActiveRecord::Migrator.new(:up, [migration]).migrate assert_not connection.index_exists?(:testings, :bar) end + + def test_references_does_not_add_index_by_default + migration = Class.new(ActiveRecord::Migration) { + def migrate(x) + create_table :more_testings do |t| + t.references :foo + t.belongs_to :bar, index: false + end + end + }.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + + assert_not connection.index_exists?(:more_testings, :foo_id) + assert_not connection.index_exists?(:more_testings, :bar_id) + ensure + connection.drop_table :more_testings rescue nil + end end end end diff --git a/activerecord/test/cases/migration/references_foreign_key_test.rb b/activerecord/test/cases/migration/references_foreign_key_test.rb index edbc8abe4d..b01415afb2 100644 --- a/activerecord/test/cases/migration/references_foreign_key_test.rb +++ b/activerecord/test/cases/migration/references_foreign_key_test.rb @@ -32,10 +32,10 @@ module ActiveRecord assert_equal [], @connection.foreign_keys("testings") end - test "foreign keys can be created in one query" do + test "foreign keys can be created in one query when index is not added" do assert_queries(1) do @connection.create_table :testings do |t| - t.references :testing_parent, foreign_key: true + t.references :testing_parent, foreign_key: true, index: false end end end diff --git a/activerecord/test/cases/migration/references_index_test.rb b/activerecord/test/cases/migration/references_index_test.rb index ad6b828d0b..a9a7f0f4c4 100644 --- a/activerecord/test/cases/migration/references_index_test.rb +++ b/activerecord/test/cases/migration/references_index_test.rb @@ -23,12 +23,12 @@ module ActiveRecord assert connection.index_exists?(table_name, :foo_id, :name => :index_testings_on_foo_id) end - def test_does_not_create_index + def test_creates_index_by_default_even_if_index_option_is_not_passed connection.create_table table_name do |t| t.references :foo end - assert_not connection.index_exists?(table_name, :foo_id, :name => :index_testings_on_foo_id) + assert connection.index_exists?(table_name, :foo_id, :name => :index_testings_on_foo_id) end def test_does_not_create_index_explicit @@ -68,13 +68,13 @@ module ActiveRecord assert connection.index_exists?(table_name, :foo_id, :name => :index_testings_on_foo_id) end - def test_does_not_create_index_for_existing_table + def test_creates_index_for_existing_table_even_if_index_option_is_not_passed connection.create_table table_name connection.change_table table_name do |t| t.references :foo end - assert_not connection.index_exists?(table_name, :foo_id, :name => :index_testings_on_foo_id) + assert connection.index_exists?(table_name, :foo_id, :name => :index_testings_on_foo_id) end def test_does_not_create_index_for_existing_table_explicit diff --git a/activerecord/test/cases/migration/references_statements_test.rb b/activerecord/test/cases/migration/references_statements_test.rb index f613fd66c3..b9ce6bbc55 100644 --- a/activerecord/test/cases/migration/references_statements_test.rb +++ b/activerecord/test/cases/migration/references_statements_test.rb @@ -30,14 +30,14 @@ module ActiveRecord assert column_exists?(table_name, :taggable_type, :string) end - def test_creates_reference_id_index - add_reference table_name, :user, index: true - assert index_exists?(table_name, :user_id) + def test_does_not_create_reference_id_index_if_index_is_false + add_reference table_name, :user, index: false + assert_not index_exists?(table_name, :user_id) end - def test_does_not_create_reference_id_index + def test_create_reference_id_index_even_if_index_option_is_passed add_reference table_name, :user - assert_not index_exists?(table_name, :user_id) + assert index_exists?(table_name, :user_id) end def test_creates_polymorphic_index diff --git a/activesupport/test/multibyte_conformance_test.rb b/activesupport/test/multibyte_conformance_test.rb index d493a48fe4..5df8f32e46 100644 --- a/activesupport/test/multibyte_conformance_test.rb +++ b/activesupport/test/multibyte_conformance_test.rb @@ -5,25 +5,25 @@ require 'fileutils' require 'open-uri' require 'tmpdir' -class Downloader - def self.download(from, to) - unless File.exist?(to) - unless File.exist?(File.dirname(to)) - system "mkdir -p #{File.dirname(to)}" - end - open(from) do |source| - File.open(to, 'w') do |target| - source.each_line do |l| - target.write l +class MultibyteConformanceTest < ActiveSupport::TestCase + class Downloader + def self.download(from, to) + unless File.exist?(to) + unless File.exist?(File.dirname(to)) + system "mkdir -p #{File.dirname(to)}" + end + open(from) do |source| + File.open(to, 'w') do |target| + source.each_line do |l| + target.write l + end end end end + true end - true end -end -class MultibyteConformanceTest < ActiveSupport::TestCase include MultibyteTestHelpers UNIDATA_URL = "http://www.unicode.org/Public/#{ActiveSupport::Multibyte::Unicode::UNICODE_VERSION}/ucd" diff --git a/activesupport/test/multibyte_grapheme_break_conformance.rb b/activesupport/test/multibyte_grapheme_break_conformance_test.rb index 7d185e2cae..229f24990e 100644 --- a/activesupport/test/multibyte_grapheme_break_conformance.rb +++ b/activesupport/test/multibyte_grapheme_break_conformance_test.rb @@ -6,25 +6,25 @@ require 'fileutils' require 'open-uri' require 'tmpdir' -class Downloader - def self.download(from, to) - unless File.exist?(to) - $stderr.puts "Downloading #{from} to #{to}" - unless File.exist?(File.dirname(to)) - system "mkdir -p #{File.dirname(to)}" - end - open(from) do |source| - File.open(to, 'w') do |target| - source.each_line do |l| - target.write l +class MultibyteGraphemeBreakConformanceTest < ActiveSupport::TestCase + class Downloader + def self.download(from, to) + unless File.exist?(to) + $stderr.puts "Downloading #{from} to #{to}" + unless File.exist?(File.dirname(to)) + system "mkdir -p #{File.dirname(to)}" + end + open(from) do |source| + File.open(to, 'w') do |target| + source.each_line do |l| + target.write l + end end end - end - end + end + end end -end -class MultibyteGraphemeBreakConformanceTest < ActiveSupport::TestCase TEST_DATA_URL = "http://www.unicode.org/Public/#{ActiveSupport::Multibyte::Unicode::UNICODE_VERSION}/ucd/auxiliary" TEST_DATA_FILE = '/GraphemeBreakTest.txt' CACHE_DIR = File.join(Dir.tmpdir, 'cache') diff --git a/activesupport/test/multibyte_normalization_conformance.rb b/activesupport/test/multibyte_normalization_conformance_test.rb index 839aec7fa8..8bc91ef708 100644 --- a/activesupport/test/multibyte_normalization_conformance.rb +++ b/activesupport/test/multibyte_normalization_conformance_test.rb @@ -7,25 +7,25 @@ require 'fileutils' require 'open-uri' require 'tmpdir' -class Downloader - def self.download(from, to) - unless File.exist?(to) - $stderr.puts "Downloading #{from} to #{to}" - unless File.exist?(File.dirname(to)) - system "mkdir -p #{File.dirname(to)}" - end - open(from) do |source| - File.open(to, 'w') do |target| - source.each_line do |l| - target.write l +class MultibyteNormalizationConformanceTest < ActiveSupport::TestCase + class Downloader + def self.download(from, to) + unless File.exist?(to) + $stderr.puts "Downloading #{from} to #{to}" + unless File.exist?(File.dirname(to)) + system "mkdir -p #{File.dirname(to)}" + end + open(from) do |source| + File.open(to, 'w') do |target| + source.each_line do |l| + target.write l + end end end - end - end + end + end end -end -class MultibyteNormalizationConformanceTest < ActiveSupport::TestCase include MultibyteTestHelpers UNIDATA_URL = "http://www.unicode.org/Public/#{ActiveSupport::Multibyte::Unicode::UNICODE_VERSION}/ucd" diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index cd83175da8..9ca731347a 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -20,7 +20,7 @@ module Rails # Set the message to be shown in logs. Uses the git repo if one is given, # otherwise use name (version). - parts, message = [ quote(name) ], name + parts, message = [ quote(name) ], name.dup if version ||= options.delete(:version) parts << quote(version) message << " (#{version})" diff --git a/railties/lib/rails/generators/generated_attribute.rb b/railties/lib/rails/generators/generated_attribute.rb index 8145a26e22..7e437e7344 100644 --- a/railties/lib/rails/generators/generated_attribute.rb +++ b/railties/lib/rails/generators/generated_attribute.rb @@ -23,8 +23,9 @@ module Rails type = type.to_sym if type if type && reference?(type) - references_index = UNIQ_INDEX_OPTIONS.include?(has_index) ? { unique: true } : true - attr_options[:index] = references_index + if UNIQ_INDEX_OPTIONS.include?(has_index) + attr_options[:index] = { unique: true } + end end new(name, type, has_index, attr_options) diff --git a/railties/test/generators/actions_test.rb b/railties/test/generators/actions_test.rb index a1a17d90d8..3300850604 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -113,6 +113,14 @@ class ActionsTest < Rails::Generators::TestCase assert_file 'Gemfile', /^gem 'rspec', ">=2\.0'0"$/ end + def test_gem_works_even_if_frozen_string_is_passed_as_argument + run_generator + + action :gem, "frozen_gem".freeze, "1.0.0".freeze + + assert_file 'Gemfile', /^gem 'frozen_gem', '1.0.0'$/ + end + def test_gem_group_should_wrap_gems_in_a_group run_generator diff --git a/railties/test/generators/migration_generator_test.rb b/railties/test/generators/migration_generator_test.rb index 80f284674d..46154b7db2 100644 --- a/railties/test/generators/migration_generator_test.rb +++ b/railties/test/generators/migration_generator_test.rb @@ -79,8 +79,8 @@ class MigrationGeneratorTest < Rails::Generators::TestCase assert_migration "db/migrate/#{migration}.rb" do |content| assert_method :change, content do |change| - assert_match(/remove_reference :books, :author, index: true/, change) - assert_match(/remove_reference :books, :distributor, polymorphic: true, index: true/, change) + assert_match(/remove_reference :books, :author/, change) + assert_match(/remove_reference :books, :distributor, polymorphic: true/, change) end end end @@ -166,8 +166,8 @@ class MigrationGeneratorTest < Rails::Generators::TestCase assert_migration "db/migrate/#{migration}.rb" do |content| assert_method :change, content do |change| - assert_match(/add_reference :books, :author, index: true/, change) - assert_match(/add_reference :books, :distributor, polymorphic: true, index: true/, change) + assert_match(/add_reference :books, :author/, change) + assert_match(/add_reference :books, :distributor, polymorphic: true/, change) end end end @@ -178,8 +178,8 @@ class MigrationGeneratorTest < Rails::Generators::TestCase assert_migration "db/migrate/#{migration}.rb" do |content| assert_method :change, content do |change| - assert_match(/add_reference :books, :author, index: true, null: false/, change) - assert_match(/add_reference :books, :distributor, polymorphic: true, index: true, null: false/, change) + assert_match(/add_reference :books, :author, null: false/, change) + assert_match(/add_reference :books, :distributor, polymorphic: true, null: false/, change) end end end diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index fb502ec0c5..814f4c050e 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -345,26 +345,6 @@ class ModelGeneratorTest < Rails::Generators::TestCase assert_match(/The name 'Object' is either already used in your application or reserved/, content) end - def test_index_is_added_for_belongs_to_association - run_generator ["account", "supplier:belongs_to"] - - assert_migration "db/migrate/create_accounts.rb" do |m| - assert_method :change, m do |up| - assert_match(/index: true/, up) - end - end - end - - def test_index_is_added_for_references_association - run_generator ["account", "supplier:references"] - - assert_migration "db/migrate/create_accounts.rb" do |m| - assert_method :change, m do |up| - assert_match(/index: true/, up) - end - end - end - def test_index_is_skipped_for_belongs_to_association run_generator ["account", "supplier:belongs_to", "--no-indexes"] diff --git a/railties/test/generators/scaffold_generator_test.rb b/railties/test/generators/scaffold_generator_test.rb index eb81ea3d0e..6f7a83cae0 100644 --- a/railties/test/generators/scaffold_generator_test.rb +++ b/railties/test/generators/scaffold_generator_test.rb @@ -14,8 +14,8 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase assert_file "app/models/product_line.rb", /class ProductLine < ActiveRecord::Base/ assert_file "test/models/product_line_test.rb", /class ProductLineTest < ActiveSupport::TestCase/ assert_file "test/fixtures/product_lines.yml" - assert_migration "db/migrate/create_product_lines.rb", /belongs_to :product, index: true/ - assert_migration "db/migrate/create_product_lines.rb", /references :user, index: true/ + assert_migration "db/migrate/create_product_lines.rb", /belongs_to :product/ + assert_migration "db/migrate/create_product_lines.rb", /references :user/ # Route assert_file "config/routes.rb" do |route| @@ -94,8 +94,8 @@ class ScaffoldGeneratorTest < Rails::Generators::TestCase assert_file "app/models/product_line.rb", /class ProductLine < ActiveRecord::Base/ assert_file "test/models/product_line_test.rb", /class ProductLineTest < ActiveSupport::TestCase/ assert_file "test/fixtures/product_lines.yml" - assert_migration "db/migrate/create_product_lines.rb", /belongs_to :product, index: true/ - assert_migration "db/migrate/create_product_lines.rb", /references :user, index: true/ + assert_migration "db/migrate/create_product_lines.rb", /belongs_to :product/ + assert_migration "db/migrate/create_product_lines.rb", /references :user/ # Route assert_file "config/routes.rb" do |route| |