Merge branch 'master' of github.com:rails/rails

8 files changed, 385 insertions, 23 deletions

diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
index ed2e2e9f8f..fe558f9d3b 100644
--- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
+++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
@@ -29,6 +29,15 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
     assert_equal 2, authors[1].categorizations.size
   end
 
+  def test_eager_association_loading_with_hmt_does_not_table_name_collide_when_joining_associations
+    assert_nothing_raised do
+      Author.joins(:posts).eager_load(:comments).where(:posts => {:taggings_count => 1}).all
+    end
+    authors = Author.joins(:posts).eager_load(:comments).where(:posts => {:taggings_count => 1}).all
+    assert_equal 1, assert_no_queries { authors.size }
+    assert_equal 9, assert_no_queries { authors[0].comments.size }
+  end
+
   def test_eager_association_loading_with_cascaded_two_levels_with_two_has_many_associations
     authors = Author.find(:all, :include=>{:posts=>[:comments, :categorizations]}, :order=>"authors.id")
     assert_equal 2, authors.size
diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb
index 2f4243a6aa..3623680de9 100755
--- a/activerecord/test/cases/base_test.rb
+++ b/activerecord/test/cases/base_test.rb
@@ -2085,6 +2085,7 @@ class BasicsTest < ActiveRecord::TestCase
     assert_equal "topic", xml.root.name
     assert_equal "The First Topic" , xml.elements["//title"].text
     assert_equal "David" , xml.elements["//author-name"].text
+    assert_match "Have a nice day", xml.elements["//content"].text
 
     assert_equal "1", xml.elements["//id"].text
     assert_equal "integer" , xml.elements["//id"].attributes['type']
@@ -2095,10 +2096,6 @@ class BasicsTest < ActiveRecord::TestCase
     assert_equal written_on_in_current_timezone, xml.elements["//written-on"].text
     assert_equal "datetime" , xml.elements["//written-on"].attributes['type']
 
-    assert_match(/^--- Have a nice day\n/ , xml.elements["//content"].text)
-    assert_equal 'Have a nice day' , YAML.load(xml.elements["//content"].text)
-    assert_equal "yaml" , xml.elements["//content"].attributes['type']
-
     assert_equal "david@loudthinking.com", xml.elements["//author-email-address"].text
 
     assert_equal nil, xml.elements["//parent-id"].text
diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb
index dfaecf35cf..aa2d9527f9 100644
--- a/activerecord/test/cases/locking_test.rb
+++ b/activerecord/test/cases/locking_test.rb
@@ -38,6 +38,25 @@ class OptimisticLockingTest < ActiveRecord::TestCase
     assert_raise(ActiveRecord::StaleObjectError) { p2.save! }
   end
 
+  # See Lighthouse ticket #1966
+  def test_lock_destroy
+    p1 = Person.find(1)
+    p2 = Person.find(1)
+    assert_equal 0, p1.lock_version
+    assert_equal 0, p2.lock_version
+
+    p1.first_name = 'stu'
+    p1.save!
+    assert_equal 1, p1.lock_version
+    assert_equal 0, p2.lock_version
+
+    assert_raises(ActiveRecord::StaleObjectError) { p2.destroy }
+
+    assert p1.destroy
+    assert_equal true, p1.frozen?
+    assert_raises(ActiveRecord::RecordNotFound) { Person.find(1) }
+  end
+
   def test_lock_repeating
     p1 = Person.find(1)
     p2 = Person.find(1)
@@ -150,6 +169,32 @@ class OptimisticLockingTest < ActiveRecord::TestCase
       end
     end
   end
+
+  # See Lighthouse ticket #1966
+  def test_destroy_dependents
+    # Establish dependent relationship between People and LegacyThing
+    add_counter_column_to(Person, 'legacy_things_count')
+    LegacyThing.connection.add_column LegacyThing.table_name, 'person_id', :integer
+    LegacyThing.reset_column_information
+    LegacyThing.class_eval do
+      belongs_to :person, :counter_cache => true
+    end
+    Person.class_eval do
+      has_many :legacy_things, :dependent => :destroy
+    end
+
+    # Make sure that counter incrementing doesn't cause problems
+    p1 = Person.new(:first_name => 'fjord')
+    p1.save!
+    t = LegacyThing.new(:person => p1)
+    t.save!
+    p1.reload
+    assert_equal 1, p1.legacy_things_count
+    assert p1.destroy
+    assert_equal true, p1.frozen?
+    assert_raises(ActiveRecord::RecordNotFound) { Person.find(p1.id) }
+    assert_raises(ActiveRecord::RecordNotFound) { LegacyThing.find(t.id) }
+  end
   
   def test_quote_table_name
     ref = references(:michael_magician)
@@ -168,11 +213,11 @@ class OptimisticLockingTest < ActiveRecord::TestCase
 
   private
 
-    def add_counter_column_to(model)
-      model.connection.add_column model.table_name, :test_count, :integer, :null => false, :default => 0
+    def add_counter_column_to(model, col='test_count')
+      model.connection.add_column model.table_name, col, :integer, :null => false, :default => 0
       model.reset_column_information
       # OpenBase does not set a value to existing rows when adding a not null default column
-      model.update_all(:test_count => 0) if current_adapter?(:OpenBaseAdapter)
+      model.update_all(col => 0) if current_adapter?(:OpenBaseAdapter)
     end
 
     def remove_counter_column_from(model)
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index 7a26ee072d..a3d1ceaa1f 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -1136,6 +1136,25 @@ if ActiveRecord::Base.connection.supports_migrations?
       load(MIGRATIONS_ROOT + "/valid/1_people_have_last_names.rb")
     end
 
+    def test_target_version_zero_should_run_only_once
+      # migrate up to 1
+      ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/valid", 1)
+
+      # migrate down to 0
+      ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/valid", 0)
+
+      # now unload the migrations that have been defined
+      PeopleHaveLastNames.unloadable
+      ActiveSupport::Dependencies.remove_unloadable_constants!
+
+      # migrate down to 0 again
+      ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/valid", 0)
+
+      assert !defined? PeopleHaveLastNames
+    ensure
+      load(MIGRATIONS_ROOT + "/valid/1_people_have_last_names.rb")
+    end
+
     def test_migrator_db_has_no_schema_migrations_table
       # Oracle adapter raises error if semicolon is present as last character
       if current_adapter?(:OracleAdapter)
diff --git a/activerecord/test/cases/serialization_test.rb b/activerecord/test/cases/serialization_test.rb
index 8841694271..8c385af97c 100644
--- a/activerecord/test/cases/serialization_test.rb
+++ b/activerecord/test/cases/serialization_test.rb
@@ -44,4 +44,11 @@ class SerializationTest < ActiveRecord::TestCase
       assert_equal @contact_attributes[:awesome], contact.awesome, "For #{format}"
     end
   end
+
+  def test_serialize_should_xml_skip_instruct_for_included_records
+    @contact.alternative = Contact.new(:name => 'Copa Cabana')
+    @serialized = @contact.to_xml(:include => [ :alternative ])
+    assert_equal @serialized.index('<?xml '), 0
+    assert_nil @serialized.index('<?xml ', 1)
+  end
 end
diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb
new file mode 100644
index 0000000000..a07da093f1
--- /dev/null
+++ b/activerecord/test/cases/transaction_callbacks_test.rb
@@ -0,0 +1,244 @@
+require "cases/helper"
+require 'models/topic'
+require 'models/reply'
+
+class TransactionCallbacksTest < ActiveRecord::TestCase
+  self.use_transactional_fixtures = false
+  fixtures :topics
+
+  class TopicWithCallbacks < ActiveRecord::Base
+    set_table_name :topics
+
+    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)}
+
+    def history
+      @history ||= []
+    end
+
+    def after_commit_block(on = nil, &block)
+      @after_commit ||= {}
+      @after_commit[on] ||= []
+      @after_commit[on] << block
+    end
+
+    def after_rollback_block(on = nil, &block)
+      @after_rollback ||= {}
+      @after_rollback[on] ||= []
+      @after_rollback[on] << block
+    end
+
+    def do_after_commit(on)
+      blocks = @after_commit[on] if defined?(@after_commit)
+      blocks.each{|b| b.call(self)} if blocks
+    end
+
+    def do_after_rollback(on)
+      blocks = @after_rollback[on] if defined?(@after_rollback)
+      blocks.each{|b| b.call(self)} if blocks
+    end
+  end
+
+  def setup
+    @first, @second = TopicWithCallbacks.find(1, 3).sort_by { |t| t.id }
+  end
+
+  def test_call_after_commit_after_transaction_commits
+    @first.after_commit_block{|r| r.history << :after_commit}
+    @first.after_rollback_block{|r| r.history << :after_rollback}
+
+    @first.save!
+    assert @first.history, [:after_commit]
+  end
+
+  def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record
+    commit_callback = []
+    @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_commit_block(:create){|r| r.history << :rollback_on_create}
+    @first.after_commit_block(:update){|r| r.history << :rollback_on_update}
+    @first.after_commit_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    @first.save!
+    assert @first.history, [:commit_on_update]
+  end
+
+  def test_only_call_after_commit_on_destroy_after_transaction_commits_for_destroyed_record
+    commit_callback = []
+    @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_commit_block(:create){|r| r.history << :rollback_on_create}
+    @first.after_commit_block(:update){|r| r.history << :rollback_on_update}
+    @first.after_commit_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    @first.destroy
+    assert @first.history, [:commit_on_destroy]
+  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_commit_block(:create){|r| r.history << :rollback_on_create}
+    @new_record.after_commit_block(:update){|r| r.history << :rollback_on_update}
+    @new_record.after_commit_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    @new_record.save!
+    assert @new_record.history, [:commit_on_create]
+  end
+
+  def test_call_after_rollback_after_transaction_rollsback
+    @first.after_commit_block{|r| r.history << :after_commit}
+    @first.after_rollback_block{|r| r.history << :after_rollback}
+
+    Topic.transaction do
+      @first.save!
+      raise ActiveRecord::Rollback
+    end
+
+    assert @first.history, [:after_rollback]
+  end
+
+  def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record
+    commit_callback = []
+    @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_commit_block(:create){|r| r.history << :rollback_on_create}
+    @first.after_commit_block(:update){|r| r.history << :rollback_on_update}
+    @first.after_commit_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    Topic.transaction do
+      @first.save!
+      raise ActiveRecord::Rollback
+    end
+
+    assert @first.history, [:rollback_on_update]
+  end
+
+  def test_only_call_after_rollback_on_destroy_after_transaction_rollsback_for_destroyed_record
+    commit_callback = []
+    @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_commit_block(:create){|r| r.history << :rollback_on_create}
+    @first.after_commit_block(:update){|r| r.history << :rollback_on_update}
+    @first.after_commit_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    Topic.transaction do
+      @first.destroy
+      raise ActiveRecord::Rollback
+    end
+
+    assert @first.history, [:rollback_on_destroy]
+  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_commit_block(:create){|r| r.history << :rollback_on_create}
+    @new_record.after_commit_block(:update){|r| r.history << :rollback_on_update}
+    @new_record.after_commit_block(:destroy){|r| r.history << :rollback_on_destroy}
+
+    Topic.transaction do
+      @new_record.save!
+      raise ActiveRecord::Rollback
+    end
+
+    assert @new_record.history, [:rollback_on_create]
+  end
+
+  def test_call_after_rollback_when_commit_fails
+    @first.connection.class.send(:alias_method, :real_method_commit_db_transaction, :commit_db_transaction)
+    begin
+      @first.connection.class.class_eval do
+        def commit_db_transaction; raise "boom!"; end
+      end
+
+      @first.after_commit_block{|r| r.history << :after_commit}
+      @first.after_rollback_block{|r| r.history << :after_rollback}
+
+      assert !@first.save rescue nil
+      assert @first.history == [:after_rollback]
+    ensure
+      @first.connection.class.send(:remove_method, :commit_db_transaction)
+      @first.connection.class.send(:alias_method, :commit_db_transaction, :real_method_commit_db_transaction)
+    end
+  end
+
+  def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint
+    def @first.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end
+    def @first.commits(i=0); @commits ||= 0; @commits += i if i; end
+    @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)}
+
+    Topic.transaction do
+      @first.save!
+      Topic.transaction(:requires_new => true) do
+        @second.save!
+        raise ActiveRecord::Rollback
+      end
+    end
+
+    assert 1, @first.commits
+    assert 0, @first.rollbacks
+    assert 1, @second.commits
+    assert 1, @second.rollbacks
+  end
+
+  def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint_when_release_savepoint_fails
+    def @first.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end
+    def @first.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
+        @first.save!
+        raise ActiveRecord::Rollback
+      end
+      Topic.transaction(:requires_new => true) do
+        @first.save!
+        raise ActiveRecord::Rollback
+      end
+    end
+
+    assert 1, @first.commits
+    assert 2, @first.rollbacks
+  end
+
+  def test_after_transaction_callbacks_should_not_raise_errors
+    def @first.last_after_transaction_error=(e); @last_transaction_error = e; end
+    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!";}
+
+    @first.save!
+    assert_equal @first.last_after_transaction_error, :commit
+
+    Topic.transaction do
+      @first.save!
+      raise ActiveRecord::Rollback
+    end
+
+    assert_equal @first.last_after_transaction_error, :rollback
+  end
+end
diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb
index c550030329..958a4e4f94 100644
--- a/activerecord/test/cases/transactions_test.rb
+++ b/activerecord/test/cases/transactions_test.rb
@@ -262,22 +262,22 @@ class TransactionTest < ActiveRecord::TestCase
     assert !@first.reload.approved?
     assert !@second.reload.approved?
   end if Topic.connection.supports_savepoints?
-  
+
   def test_many_savepoints
     Topic.transaction do
       @first.content = "One"
       @first.save!
-      
+
       begin
         Topic.transaction :requires_new => true do
           @first.content = "Two"
           @first.save!
-          
+
           begin
             Topic.transaction :requires_new => true do
               @first.content = "Three"
               @first.save!
-              
+
               begin
                 Topic.transaction :requires_new => true do
                   @first.content = "Four"
@@ -286,22 +286,22 @@ class TransactionTest < ActiveRecord::TestCase
                 end
               rescue
               end
-              
+
               @three = @first.reload.content
               raise
             end
           rescue
           end
-          
+
           @two = @first.reload.content
           raise
         end
       rescue
       end
-      
+
       @one = @first.reload.content
     end
-    
+
     assert_equal "One", @one
     assert_equal "Two", @two
     assert_equal "Three", @three
@@ -319,7 +319,34 @@ class TransactionTest < ActiveRecord::TestCase
       end
     end
   end
-  
+
+  def test_restore_active_record_state_for_all_records_in_a_transaction
+    topic_1 = Topic.new(:title => 'test_1')
+    topic_2 = Topic.new(:title => 'test_2')
+    Topic.transaction do
+      assert topic_1.save
+      assert topic_2.save
+      @first.save
+      @second.destroy
+      assert_equal false, topic_1.new_record?
+      assert_not_nil topic_1.id
+      assert_equal false, topic_2.new_record?
+      assert_not_nil topic_2.id
+      assert_equal false, @first.new_record?
+      assert_not_nil @first.id
+      assert_equal true, @second.destroyed?
+      raise ActiveRecord::Rollback
+    end
+
+    assert_equal true, topic_1.new_record?
+    assert_nil topic_1.id
+    assert_equal true, topic_2.new_record?
+    assert_nil topic_2.id
+    assert_equal false, @first.new_record?
+    assert_not_nil @first.id
+    assert_equal false, @second.destroyed?
+  end
+
   if current_adapter?(:PostgreSQLAdapter) && defined?(PGconn::PQTRANS_IDLE)
     def test_outside_transaction_works
       assert Topic.connection.outside_transaction?
@@ -328,7 +355,7 @@ class TransactionTest < ActiveRecord::TestCase
       Topic.connection.rollback_db_transaction
       assert Topic.connection.outside_transaction?
     end
-    
+
     def test_rollback_wont_be_executed_if_no_transaction_active
       assert_raise RuntimeError do
         Topic.transaction do
@@ -338,7 +365,7 @@ class TransactionTest < ActiveRecord::TestCase
         end
       end
     end
-    
+
     def test_open_transactions_count_is_reset_to_zero_if_no_transaction_active
       Topic.transaction do
         Topic.transaction do
@@ -358,12 +385,12 @@ class TransactionTest < ActiveRecord::TestCase
     #
     # We go back to the connection for the column queries because
     # Topic.columns is cached and won't report changes to the DB
-    
+
     assert_nothing_raised do
       Topic.reset_column_information
       Topic.connection.add_column('topics', 'stuff', :string)
       assert Topic.column_names.include?('stuff')
-      
+
       Topic.reset_column_information
       Topic.connection.remove_column('topics', 'stuff')
       assert !Topic.column_names.include?('stuff')
@@ -382,6 +409,12 @@ class TransactionTest < ActiveRecord::TestCase
   end
 
   private
+    def define_callback_method(callback_method)
+      define_method(callback_method) do
+        self.history << [callback_method, :method]
+      end
+    end
+
     def add_exception_raising_after_save_callback_to_topic
       Topic.class_eval <<-eoruby, __FILE__, __LINE__ + 1
         remove_method(:after_save_for_transaction)
@@ -440,7 +473,7 @@ class TransactionsWithTransactionalFixturesTest < ActiveRecord::TestCase
 
   def test_automatic_savepoint_in_outer_transaction
     @first = Topic.find(1)
-    
+
     begin
       Topic.transaction do
         @first.approved = true
diff --git a/activerecord/test/cases/xml_serialization_test.rb b/activerecord/test/cases/xml_serialization_test.rb
index b1c75ec8cd..751946ffc5 100644
--- a/activerecord/test/cases/xml_serialization_test.rb
+++ b/activerecord/test/cases/xml_serialization_test.rb
@@ -79,8 +79,8 @@ class DefaultXmlSerializationTest < ActiveRecord::TestCase
     assert_match %r{<awesome type=\"boolean\">false</awesome>}, @xml
   end
 
-  def test_should_serialize_yaml
-    assert_match %r{<preferences type=\"yaml\">---\s?\n:gem: ruby\n</preferences>}, @xml
+  def test_should_serialize_hash
+    assert_match %r{<preferences>\s*<gem>ruby</gem>\s*</preferences>}m, @xml
   end
 end
 
@@ -234,4 +234,12 @@ class DatabaseConnectedXmlSerializationTest < ActiveRecord::TestCase
     assert types.include?('StiPost')
   end
 
+  def test_should_produce_xml_for_methods_returning_array
+    xml = authors(:david).to_xml(:methods => :social)
+    array = Hash.from_xml(xml)['author']['social']
+    assert_equal 2, array.size
+    assert array.include? 'twitter'
+    assert array.include? 'github'
+  end
+
 end