Merge branch 'master' into adequaterecord

* master:
  Fix eager load of Serializers on Active Model
  Consistence in the block style
  Remove warnings on Ruby 2.1
  Only some dynamic finders are deprecated.
  clean up security guide: his => their [ci skip]
  Remove warning
  Extract a method to simplify setup code
  Use minitest's skip rather than conditionals + early returns
  Move AR test classes inside the test case
  Do not set up a variable used only in two tests
  No need for instance vars on single tests
  Cleanup duplicated setup of callbacks in transactions tests
  No need to use #send with public methods
  Update deprecation warning to give more information about caller
  Update changelog
  Avoid scanning multiple render calls as a single match.
  Improve ERB dependency detection.

2 files changed, 153 insertions, 176 deletions

diff --git a/activerecord/test/cases/associations/eager_singularization_test.rb b/activerecord/test/cases/associations/eager_singularization_test.rb
index 634f6b63ba..669569a774 100644
--- a/activerecord/test/cases/associations/eager_singularization_test.rb
+++ b/activerecord/test/cases/associations/eager_singularization_test.rb
@@ -1,128 +1,133 @@
 require "cases/helper"
 
-class Virus < ActiveRecord::Base
-  belongs_to :octopus
-end
-class Octopus < ActiveRecord::Base
-  has_one :virus
-end
-class Pass < ActiveRecord::Base
-  belongs_to :bus
-end
-class Bus < ActiveRecord::Base
-  has_many :passes
-end
-class Mess < ActiveRecord::Base
-  has_and_belongs_to_many :crises
-end
-class Crisis < ActiveRecord::Base
-  has_and_belongs_to_many :messes
-  has_many :analyses, :dependent => :destroy
-  has_many :successes, :through => :analyses
-  has_many :dresses, :dependent => :destroy
-  has_many :compresses, :through => :dresses
-end
-class Analysis < ActiveRecord::Base
-  belongs_to :crisis
-  belongs_to :success
-end
-class Success < ActiveRecord::Base
-  has_many :analyses, :dependent => :destroy
-  has_many :crises, :through => :analyses
-end
-class Dress < ActiveRecord::Base
-  belongs_to :crisis
-  has_many :compresses
-end
-class Compress < ActiveRecord::Base
-  belongs_to :dress
-end
-
 
 class EagerSingularizationTest < ActiveRecord::TestCase
+  class Virus < ActiveRecord::Base
+    belongs_to :octopus
+  end
+
+  class Octopus < ActiveRecord::Base
+    has_one :virus
+  end
+
+  class Pass < ActiveRecord::Base
+    belongs_to :bus
+  end
+
+  class Bus < ActiveRecord::Base
+    has_many :passes
+  end
+
+  class Mess < ActiveRecord::Base
+    has_and_belongs_to_many :crises
+  end
+
+  class Crisis < ActiveRecord::Base
+    has_and_belongs_to_many :messes
+    has_many :analyses, :dependent => :destroy
+    has_many :successes, :through => :analyses
+    has_many :dresses, :dependent => :destroy
+    has_many :compresses, :through => :dresses
+  end
+
+  class Analysis < ActiveRecord::Base
+    belongs_to :crisis
+    belongs_to :success
+  end
+
+  class Success < ActiveRecord::Base
+    has_many :analyses, :dependent => :destroy
+    has_many :crises, :through => :analyses
+  end
+
+  class Dress < ActiveRecord::Base
+    belongs_to :crisis
+    has_many :compresses
+  end
+
+  class Compress < ActiveRecord::Base
+    belongs_to :dress
+  end
 
   def setup
-    if ActiveRecord::Base.connection.supports_migrations?
-      ActiveRecord::Base.connection.create_table :viri do |t|
-        t.column :octopus_id, :integer
-        t.column :species, :string
-      end
-      ActiveRecord::Base.connection.create_table :octopi do |t|
-        t.column :species, :string
-      end
-      ActiveRecord::Base.connection.create_table :passes do |t|
-        t.column :bus_id, :integer
-        t.column :rides, :integer
-      end
-      ActiveRecord::Base.connection.create_table :buses do |t|
-        t.column :name, :string
-      end
-      ActiveRecord::Base.connection.create_table :crises_messes, :id => false do |t|
-        t.column :crisis_id, :integer
-        t.column :mess_id, :integer
-      end
-      ActiveRecord::Base.connection.create_table :messes do |t|
-        t.column :name, :string
-      end
-      ActiveRecord::Base.connection.create_table :crises do |t|
-        t.column :name, :string
-      end
-      ActiveRecord::Base.connection.create_table :successes do |t|
-        t.column :name, :string
-      end
-      ActiveRecord::Base.connection.create_table :analyses do |t|
-        t.column :crisis_id, :integer
-        t.column :success_id, :integer
-      end
-      ActiveRecord::Base.connection.create_table :dresses do |t|
-        t.column :crisis_id, :integer
-      end
-      ActiveRecord::Base.connection.create_table :compresses do |t|
-        t.column :dress_id, :integer
-      end
-      @have_tables = true
-    else
-      @have_tables = false
+    skip 'Does not support migrations' unless connection.supports_migrations?
+
+    connection.create_table :viri do |t|
+      t.column :octopus_id, :integer
+      t.column :species, :string
+    end
+    connection.create_table :octopi do |t|
+      t.column :species, :string
+    end
+    connection.create_table :passes do |t|
+      t.column :bus_id, :integer
+      t.column :rides, :integer
+    end
+    connection.create_table :buses do |t|
+      t.column :name, :string
+    end
+    connection.create_table :crises_messes, :id => false do |t|
+      t.column :crisis_id, :integer
+      t.column :mess_id, :integer
+    end
+    connection.create_table :messes do |t|
+      t.column :name, :string
+    end
+    connection.create_table :crises do |t|
+      t.column :name, :string
+    end
+    connection.create_table :successes do |t|
+      t.column :name, :string
+    end
+    connection.create_table :analyses do |t|
+      t.column :crisis_id, :integer
+      t.column :success_id, :integer
+    end
+    connection.create_table :dresses do |t|
+      t.column :crisis_id, :integer
+    end
+    connection.create_table :compresses do |t|
+      t.column :dress_id, :integer
     end
   end
 
   def teardown
-    ActiveRecord::Base.connection.drop_table :viri
-    ActiveRecord::Base.connection.drop_table :octopi
-    ActiveRecord::Base.connection.drop_table :passes
-    ActiveRecord::Base.connection.drop_table :buses
-    ActiveRecord::Base.connection.drop_table :crises_messes
-    ActiveRecord::Base.connection.drop_table :messes
-    ActiveRecord::Base.connection.drop_table :crises
-    ActiveRecord::Base.connection.drop_table :successes
-    ActiveRecord::Base.connection.drop_table :analyses
-    ActiveRecord::Base.connection.drop_table :dresses
-    ActiveRecord::Base.connection.drop_table :compresses
+    connection.drop_table :viri
+    connection.drop_table :octopi
+    connection.drop_table :passes
+    connection.drop_table :buses
+    connection.drop_table :crises_messes
+    connection.drop_table :messes
+    connection.drop_table :crises
+    connection.drop_table :successes
+    connection.drop_table :analyses
+    connection.drop_table :dresses
+    connection.drop_table :compresses
+  end
+
+  def connection
+    ActiveRecord::Base.connection
   end
 
   def test_eager_no_extra_singularization_belongs_to
-    return unless @have_tables
     assert_nothing_raised do
       Virus.all.merge!(:includes => :octopus).to_a
     end
   end
 
   def test_eager_no_extra_singularization_has_one
-    return unless @have_tables
     assert_nothing_raised do
       Octopus.all.merge!(:includes => :virus).to_a
     end
   end
 
   def test_eager_no_extra_singularization_has_many
-    return unless @have_tables
     assert_nothing_raised do
       Bus.all.merge!(:includes => :passes).to_a
     end
   end
 
   def test_eager_no_extra_singularization_has_and_belongs_to_many
-    return unless @have_tables
     assert_nothing_raised do
       Crisis.all.merge!(:includes => :messes).to_a
       Mess.all.merge!(:includes => :crises).to_a
@@ -130,14 +135,12 @@ class EagerSingularizationTest < ActiveRecord::TestCase
   end
 
   def test_eager_no_extra_singularization_has_many_through_belongs_to
-    return unless @have_tables
     assert_nothing_raised do
       Crisis.all.merge!(:includes => :successes).to_a
     end
   end
 
   def test_eager_no_extra_singularization_has_many_through_has_many
-    return unless @have_tables
     assert_nothing_raised do
       Crisis.all.merge!(:includes => :compresses).to_a
     end
diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb
index fe0781b7ec..7e7d95841b 100644
--- a/activerecord/test/cases/transaction_callbacks_test.rb
+++ b/activerecord/test/cases/transaction_callbacks_test.rb
@@ -30,14 +30,14 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
 
     has_many :replies, class_name: "ReplyWithCallbacks", foreign_key: "parent_id"
 
-    after_commit{|record| record.send(:do_after_commit, nil)}
-    after_commit(:on => :create){|record| record.send(:do_after_commit, :create)}
-    after_commit(:on => :update){|record| record.send(:do_after_commit, :update)}
-    after_commit(:on => :destroy){|record| record.send(:do_after_commit, :destroy)}
-    after_rollback{|record| record.send(:do_after_rollback, nil)}
-    after_rollback(:on => :create){|record| record.send(:do_after_rollback, :create)}
-    after_rollback(:on => :update){|record| record.send(:do_after_rollback, :update)}
-    after_rollback(:on => :destroy){|record| record.send(:do_after_rollback, :destroy)}
+    after_commit { |record| record.do_after_commit(nil) }
+    after_commit(on: :create) { |record| record.do_after_commit(:create) }
+    after_commit(on: :update) { |record| record.do_after_commit(:update) }
+    after_commit(on: :destroy) { |record| record.do_after_commit(:destroy) }
+    after_rollback { |record| record.do_after_rollback(nil) }
+    after_rollback(on: :create) { |record| record.do_after_rollback(:create) }
+    after_rollback(on: :update) { |record| record.do_after_rollback(:update) }
+    after_rollback(on: :destroy) { |record| record.do_after_rollback(:destroy) }
 
     def history
       @history ||= []
@@ -67,7 +67,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def setup
-    @first, @second = TopicWithCallbacks.find(1, 3).sort_by { |t| t.id }
+    @first = TopicWithCallbacks.find(1)
   end
 
   def test_call_after_commit_after_transaction_commits
@@ -79,40 +79,25 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     @first.save!
     assert_equal [:commit_on_update], @first.history
   end
 
   def test_only_call_after_commit_on_destroy_after_transaction_commits_for_destroyed_record
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     @first.destroy
     assert_equal [:commit_on_destroy], @first.history
   end
 
   def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record
-    @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today)
-    @new_record.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @new_record.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @new_record.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @new_record.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @new_record.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @new_record.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
-
-    @new_record.save!
-    assert_equal [:commit_on_create], @new_record.history
+    new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today)
+    add_transaction_execution_blocks new_record
+
+    new_record.save!
+    assert_equal [:commit_on_create], new_record.history
   end
 
   def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record_if_create_succeeds_creating_through_association
@@ -123,12 +108,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record_on_touch
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     @first.touch
     assert_equal [:commit_on_update], @first.history
@@ -147,12 +127,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     Topic.transaction do
       @first.save!
@@ -163,12 +138,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record_on_touch
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     Topic.transaction do
       @first.touch
@@ -179,12 +149,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_rollback_on_destroy_after_transaction_rollsback_for_destroyed_record
-    @first.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @first.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @first.after_commit_block(:destroy){|r| r.history << :commit_on_update}
-    @first.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @first.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    add_transaction_execution_blocks @first
 
     Topic.transaction do
       @first.destroy
@@ -195,20 +160,15 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
   end
 
   def test_only_call_after_rollback_on_create_after_transaction_rollsback_for_new_record
-    @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today)
-    @new_record.after_commit_block(:create){|r| r.history << :commit_on_create}
-    @new_record.after_commit_block(:update){|r| r.history << :commit_on_update}
-    @new_record.after_commit_block(:destroy){|r| r.history << :commit_on_destroy}
-    @new_record.after_rollback_block(:create){|r| r.history << :rollback_on_create}
-    @new_record.after_rollback_block(:update){|r| r.history << :rollback_on_update}
-    @new_record.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy}
+    new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today)
+    add_transaction_execution_blocks new_record
 
     Topic.transaction do
-      @new_record.save!
+      new_record.save!
       raise ActiveRecord::Rollback
     end
 
-    assert_equal [:rollback_on_create], @new_record.history
+    assert_equal [:rollback_on_create], new_record.history
   end
 
   def test_call_after_rollback_when_commit_fails
@@ -235,23 +195,24 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
     @first.after_rollback_block{|r| r.rollbacks(1)}
     @first.after_commit_block{|r| r.commits(1)}
 
-    def @second.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end
-    def @second.commits(i=0); @commits ||= 0; @commits += i if i; end
-    @second.after_rollback_block{|r| r.rollbacks(1)}
-    @second.after_commit_block{|r| r.commits(1)}
+    second = TopicWithCallbacks.find(3)
+    def second.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end
+    def second.commits(i=0); @commits ||= 0; @commits += i if i; end
+    second.after_rollback_block{|r| r.rollbacks(1)}
+    second.after_commit_block{|r| r.commits(1)}
 
     Topic.transaction do
       @first.save!
       Topic.transaction(:requires_new => true) do
-        @second.save!
+        second.save!
         raise ActiveRecord::Rollback
       end
     end
 
     assert_equal 1, @first.commits
     assert_equal 0, @first.rollbacks
-    assert_equal 0, @second.commits
-    assert_equal 1, @second.rollbacks
+    assert_equal 0, second.commits
+    assert_equal 1, second.rollbacks
   end
 
   def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint_when_release_savepoint_fails
@@ -282,32 +243,34 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
     def @first.last_after_transaction_error; @last_transaction_error; end
     @first.after_commit_block{|r| r.last_after_transaction_error = :commit; raise "fail!";}
     @first.after_rollback_block{|r| r.last_after_transaction_error = :rollback; raise "fail!";}
-    @second.after_commit_block{|r| r.history << :after_commit}
-    @second.after_rollback_block{|r| r.history << :after_rollback}
+
+    second = TopicWithCallbacks.find(3)
+    second.after_commit_block{|r| r.history << :after_commit}
+    second.after_rollback_block{|r| r.history << :after_rollback}
 
     Topic.transaction do
       @first.save!
-      @second.save!
+      second.save!
     end
     assert_equal :commit, @first.last_after_transaction_error
-    assert_equal [:after_commit], @second.history
+    assert_equal [:after_commit], second.history
 
-    @second.history.clear
+    second.history.clear
     Topic.transaction do
       @first.save!
-      @second.save!
+      second.save!
       raise ActiveRecord::Rollback
     end
     assert_equal :rollback, @first.last_after_transaction_error
-    assert_equal [:after_rollback], @second.history
+    assert_equal [:after_rollback], second.history
   end
 
   def test_after_rollback_callbacks_should_validate_on_condition
-    assert_raise(ArgumentError) { Topic.send(:after_rollback, :on => :save) }
+    assert_raise(ArgumentError) { Topic.after_rollback(on: :save) }
   end
 
   def test_after_commit_callbacks_should_validate_on_condition
-    assert_raise(ArgumentError) { Topic.send(:after_commit, :on => :save) }
+    assert_raise(ArgumentError) { Topic.after_commit(on: :save) }
   end
 
   def test_saving_a_record_with_a_belongs_to_that_specifies_touching_the_parent_should_call_callbacks_on_the_parent_object
@@ -324,6 +287,17 @@ class TransactionCallbacksTest < ActiveRecord::TestCase
 
     assert flag
   end
+
+  private
+
+    def add_transaction_execution_blocks(record)
+      record.after_commit_block(:create) { |r| r.history << :commit_on_create }
+      record.after_commit_block(:update) { |r| r.history << :commit_on_update }
+      record.after_commit_block(:destroy) { |r| r.history << :commit_on_destroy }
+      record.after_rollback_block(:create) { |r| r.history << :rollback_on_create }
+      record.after_rollback_block(:update) { |r| r.history << :rollback_on_update }
+      record.after_rollback_block(:destroy) { |r| r.history << :rollback_on_destroy }
+    end
 end
 
 class CallbacksOnMultipleActionsTest < ActiveRecord::TestCase