From 8272630ce8af0546e7d1aa9211a9d91b80700cbd Mon Sep 17 00:00:00 2001
From: Bruce Krysiak <bruce@socialpl.us>
Date: Tue, 10 Mar 2009 11:17:16 +0000
Subject: Ensure ActiveRecord#to_xml respects :skip_types for included
 associations [#1632 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
---
 .../lib/active_record/serializers/xml_serializer.rb    | 18 ++++++++++++------
 activerecord/test/cases/xml_serialization_test.rb      | 13 ++++++++++++-
 2 files changed, 24 insertions(+), 7 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/serializers/xml_serializer.rb b/activerecord/lib/active_record/serializers/xml_serializer.rb
index 4749823b94..fa75874603 100644
--- a/activerecord/lib/active_record/serializers/xml_serializer.rb
+++ b/activerecord/lib/active_record/serializers/xml_serializer.rb
@@ -231,16 +231,22 @@ module ActiveRecord #:nodoc:
     def add_associations(association, records, opts)
       if records.is_a?(Enumerable)
         tag = reformat_name(association.to_s)
+        type = options[:skip_types] ? {} : {:type => "array"}
+
         if records.empty?
-          builder.tag!(tag, :type => :array)
+          builder.tag!(tag, type)
         else
-          builder.tag!(tag, :type => :array) do
+          builder.tag!(tag, type) do
             association_name = association.to_s.singularize
             records.each do |record|
-              record.to_xml opts.merge(
-                :root => association_name,
-                :type => (record.class.to_s.underscore == association_name ? nil : record.class.name)
-              )
+              if options[:skip_types]
+                record_type = {}
+              else
+                record_class = (record.class.to_s.underscore == association_name) ? nil : record.class.name
+                record_type = {:type => record_class}
+              end
+
+              record.to_xml opts.merge(:root => association_name).merge(record_type)
             end
           end
         end
diff --git a/activerecord/test/cases/xml_serialization_test.rb b/activerecord/test/cases/xml_serialization_test.rb
index 39c6ea820d..b49997669e 100644
--- a/activerecord/test/cases/xml_serialization_test.rb
+++ b/activerecord/test/cases/xml_serialization_test.rb
@@ -38,11 +38,15 @@ class XmlSerializationTest < ActiveRecord::TestCase
     assert_match %r{<CreatedAt},    @xml
   end
 
+  def test_should_allow_skipped_types
+    @xml = Contact.new(:age => 25).to_xml :skip_types => true
+    assert %r{<age>25</age>}.match(@xml)
+  end
+
   def test_should_include_yielded_additions
     @xml = Contact.new.to_xml do |xml|
       xml.creator "David"
     end
-
     assert_match %r{<creator>David</creator>}, @xml
   end
 end
@@ -145,6 +149,13 @@ class DatabaseConnectedXmlSerializationTest < ActiveRecord::TestCase
     assert_match %r{<hello-post type="StiPost">}, xml
   end
 
+  def test_included_associations_should_skip_types
+    xml = authors(:david).to_xml :include=>:hello_posts, :indent => 0, :skip_types => true
+    assert_match %r{<hello-posts>}, xml
+    assert_match %r{<hello-post>}, xml
+    assert_match %r{<hello-post>}, xml
+  end
+
   def test_methods_are_called_on_object
     xml = authors(:david).to_xml :methods => :label, :indent => 0
     assert_match %r{<label>.*</label>}, xml
-- 
cgit v1.2.3


From c3aa2bcdcffb42f578b0e89fe08e1c4e234ccf3b Mon Sep 17 00:00:00 2001
From: Manfred Stienstra <manfred@fngtps.com>
Date: Tue, 10 Mar 2009 11:49:58 +0000
Subject: Ensure nested with_scope merges conditions inside out [#2193
 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
---
 activerecord/lib/active_record/base.rb         | 19 ++++++-------
 activerecord/lib/active_record/named_scope.rb  |  4 +--
 activerecord/test/cases/method_scoping_test.rb | 38 ++++++++++++++++++++++----
 activerecord/test/cases/named_scope_test.rb    | 22 +++++++++------
 4 files changed, 57 insertions(+), 26 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index edc145985d..4f39761672 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -416,7 +416,7 @@ module ActiveRecord #:nodoc:
     end
 
     @@subclasses = {}
-    
+
     ##
     # :singleton-method:
     # Contains the database configuration - as is typically stored in config/database.yml -
@@ -661,7 +661,6 @@ module ActiveRecord #:nodoc:
         connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) }
       end
 
-
       # Returns true if a record exists in the table that matches the +id+ or
       # conditions given, or false otherwise. The argument can take five forms:
       #
@@ -1003,7 +1002,6 @@ module ActiveRecord #:nodoc:
         update_counters(id, counter_name => -1)
       end
 
-
       # Attributes named in this macro are protected from mass-assignment,
       # such as <tt>new(attributes)</tt>,
       # <tt>update_attributes(attributes)</tt>, or
@@ -1104,7 +1102,6 @@ module ActiveRecord #:nodoc:
         read_inheritable_attribute(:attr_serialized) or write_inheritable_attribute(:attr_serialized, {})
       end
 
-
       # Guesses the table name (in forced lower-case) based on the name of the class in the inheritance hierarchy descending
       # directly from ActiveRecord::Base. So if the hierarchy looks like: Reply < Message < ActiveRecord::Base, then Message is used
       # to guess the table name even when called on Reply. The rules used to do the guess are handled by the Inflector class
@@ -1417,7 +1414,6 @@ module ActiveRecord #:nodoc:
         end
       end
 
-
       def quote_value(value, column = nil) #:nodoc:
         connection.quote(value,column)
       end
@@ -1486,7 +1482,7 @@ module ActiveRecord #:nodoc:
         elsif match = DynamicScopeMatch.match(method_id)
           return true if all_attributes_exists?(match.attribute_names)
         end
-        
+
         super
       end
 
@@ -2014,7 +2010,6 @@ module ActiveRecord #:nodoc:
           end
         end
 
-
         # Defines an "attribute" method (like +inheritance_column+ or
         # +table_name+). A new (class) method will be created with the
         # given name. If a value is specified, the new method will
@@ -2111,7 +2106,7 @@ module ActiveRecord #:nodoc:
           end
 
           # Merge scopings
-          if action == :merge && current_scoped_methods
+          if [:merge, :reverse_merge].include?(action) && current_scoped_methods
             method_scoping = current_scoped_methods.inject(method_scoping) do |hash, (method, params)|
               case hash[method]
                 when Hash
@@ -2133,7 +2128,11 @@ module ActiveRecord #:nodoc:
                       end
                     end
                   else
-                    hash[method] = hash[method].merge(params)
+                    if action == :reverse_merge
+                      hash[method] = hash[method].merge(params)
+                    else
+                      hash[method] = params.merge(hash[method])
+                    end
                   end
                 else
                   hash[method] = params
@@ -2143,7 +2142,6 @@ module ActiveRecord #:nodoc:
           end
 
           self.scoped_methods << method_scoping
-
           begin
             yield
           ensure
@@ -2749,7 +2747,6 @@ module ActiveRecord #:nodoc:
         assign_multiparameter_attributes(multi_parameter_attributes)
       end
 
-
       # Returns a hash of all the attributes with their names as keys and the values of the attributes as values.
       def attributes
         self.attribute_names.inject({}) do |attrs, name|
diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index 43411dfb55..519941dd94 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -104,7 +104,7 @@ module ActiveRecord
         end
       end
     end
-    
+
     class Scope
       attr_reader :proxy_scope, :proxy_options, :current_scoped_methods_when_defined
       NON_DELEGATE_METHODS = %w(nil? send object_id class extend find size count sum average maximum minimum paginate first last empty? any? respond_to?).to_set
@@ -175,7 +175,7 @@ module ActiveRecord
         if scopes.include?(method)
           scopes[method].call(self, *args)
         else
-          with_scope :find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ?  proxy_options[:conditions] : {} do
+          with_scope({:find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ?  proxy_options[:conditions] : {}}, :reverse_merge) do
             method = :new if method == :build
             if current_scoped_methods_when_defined
               with_scope current_scoped_methods_when_defined do
diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb
index c676c1c72b..68a7017f86 100644
--- a/activerecord/test/cases/method_scoping_test.rb
+++ b/activerecord/test/cases/method_scoping_test.rb
@@ -262,6 +262,15 @@ class NestedScopingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_merge_inner_scope_has_priority
+    Developer.with_scope(:find => { :limit => 5 }) do
+      Developer.with_scope(:find => { :limit => 10 }) do
+        merged_option = Developer.instance_eval('current_scoped_methods')[:find]
+        assert_equal({ :limit => 10 }, merged_option)
+      end
+    end
+  end
+
   def test_replace_options
     Developer.with_scope(:find => { :conditions => "name = 'David'" }) do
       Developer.with_exclusive_scope(:find => { :conditions => "name = 'Jamis'" }) do
@@ -400,6 +409,29 @@ class NestedScopingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_nested_scoped_create
+    comment = nil
+    Comment.with_scope(:create => { :post_id => 1}) do
+      Comment.with_scope(:create => { :post_id => 2}) do
+        assert_equal({ :post_id => 2 }, Comment.send(:current_scoped_methods)[:create])
+        comment = Comment.create :body => "Hey guys, nested scopes are broken. Please fix!"
+      end
+    end
+    assert_equal 2, comment.post_id
+  end
+
+  def test_nested_exclusive_scope_for_create
+    comment = nil
+    Comment.with_scope(:create => { :body => "Hey guys, nested scopes are broken. Please fix!" }) do
+      Comment.with_exclusive_scope(:create => { :post_id => 1 }) do
+        assert_equal({ :post_id => 1 }, Comment.send(:current_scoped_methods)[:create])
+        comment = Comment.create :body => "Hey guys"
+      end
+    end
+    assert_equal 1, comment.post_id
+    assert_equal 'Hey guys', comment.body
+  end
+
   def test_merged_scoped_find_on_blank_conditions
     [nil, " ", [], {}].each do |blank|
       Developer.with_scope(:find => {:conditions => blank}) do
@@ -523,7 +555,6 @@ class HasManyScopingTest< ActiveRecord::TestCase
   end
 end
 
-
 class HasAndBelongsToManyScopingTest< ActiveRecord::TestCase
   fixtures :posts, :categories, :categories_posts
 
@@ -549,7 +580,6 @@ class HasAndBelongsToManyScopingTest< ActiveRecord::TestCase
   end
 end
 
-
 class DefaultScopingTest < ActiveRecord::TestCase
   fixtures :developers
 
@@ -577,7 +607,7 @@ class DefaultScopingTest < ActiveRecord::TestCase
     # Scopes added on children should append to parent scope
     expected_klass_scope = [{ :create => {}, :find => { :order => 'salary DESC' }}, { :create => {}, :find => {} }]
     assert_equal expected_klass_scope, klass.send(:scoped_methods)
-    
+
     # Parent should still have the original scope
     assert_equal scope, DeveloperOrderedBySalary.send(:scoped_methods)
   end
@@ -620,7 +650,6 @@ end
 =begin
 # We disabled the scoping for has_one and belongs_to as we can't think of a proper use case
 
-
 class BelongsToScopingTest< ActiveRecord::TestCase
   fixtures :comments, :posts
 
@@ -640,7 +669,6 @@ class BelongsToScopingTest< ActiveRecord::TestCase
 
 end
 
-
 class HasOneScopingTest< ActiveRecord::TestCase
   fixtures :comments, :posts
 
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index 9f3a3848e2..4b2be0987e 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -247,7 +247,7 @@ class NamedScopeTest < ActiveRecord::TestCase
     topic = Topic.approved.create!({})
     assert topic.approved
   end
-  
+
   def test_should_build_with_proxy_options_chained
     topic = Topic.approved.by_lifo.build({})
     assert topic.approved
@@ -287,15 +287,21 @@ class NamedScopeTest < ActiveRecord::TestCase
     assert_equal post.comments.size, Post.scoped(:joins => join).scoped(:joins => join, :conditions => "posts.id = #{post.id}").size
   end
 
-  def test_chanining_should_use_latest_conditions_when_creating
-    post1 = Topic.rejected.approved.new
-    assert post1.approved?
+  def test_chaining_should_use_latest_conditions_when_creating
+    post = Topic.rejected.new
+    assert !post.approved?
+
+    post = Topic.rejected.approved.new
+    assert post.approved?
 
-    post2 = Topic.approved.rejected.new
-    assert ! post2.approved?
+    post = Topic.approved.rejected.new
+    assert !post.approved?
+
+    post = Topic.approved.rejected.approved.new
+    assert post.approved?
   end
 
-  def test_chanining_should_use_latest_conditions_when_searching
+  def test_chaining_should_use_latest_conditions_when_searching
     # Normal hash conditions
     assert_equal Topic.all(:conditions => {:approved => true}), Topic.rejected.approved.all
     assert_equal Topic.all(:conditions => {:approved => false}), Topic.approved.rejected.all
@@ -306,7 +312,7 @@ class NamedScopeTest < ActiveRecord::TestCase
     # Nested hash conditions with different keys
     assert_equal [posts(:sti_comments)], Post.with_special_comments.with_post(4).all.uniq
   end
-  
+
   def test_methods_invoked_within_scopes_should_respect_scope
     assert_equal [], Topic.approved.by_rejected_ids.proxy_options[:conditions][:id]
   end
-- 
cgit v1.2.3


From 19ad375e7afa99dc8bc5d859c478bae19c5ddc18 Mon Sep 17 00:00:00 2001
From: Jeremy Kemper <jeremy@bitsweat.net>
Date: Tue, 10 Mar 2009 23:11:05 -0700
Subject: Don't duplicate :order from scope and options, it makes mysql do
 extra work

---
 activerecord/lib/active_record/base.rb         | 4 +++-
 activerecord/test/cases/method_scoping_test.rb | 6 ++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 4f39761672..62bdf0483a 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1741,7 +1741,9 @@ module ActiveRecord #:nodoc:
           scoped_order = scope[:order] if scope
           if order
             sql << " ORDER BY #{order}"
-            sql << ", #{scoped_order}" if scoped_order
+            if scoped_order && scoped_order != order
+              sql << ", #{scoped_order}"
+            end
           else
             sql << " ORDER BY #{scoped_order}" if scoped_order
           end
diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb
index 68a7017f86..3c34cdeade 100644
--- a/activerecord/test/cases/method_scoping_test.rb
+++ b/activerecord/test/cases/method_scoping_test.rb
@@ -378,8 +378,10 @@ class NestedScopingTest < ActiveRecord::TestCase
   def test_merged_scoped_find
     poor_jamis = developers(:poor_jamis)
     Developer.with_scope(:find => { :conditions => "salary < 100000" }) do
-      Developer.with_scope(:find => { :offset => 1 }) do
-        assert_equal(poor_jamis, Developer.find(:first, :order => 'id asc'))
+      Developer.with_scope(:find => { :offset => 1, :order => 'id asc' }) do
+        assert_sql /ORDER BY id asc / do
+          assert_equal(poor_jamis, Developer.find(:first, :order => 'id asc'))
+        end
       end
     end
   end
-- 
cgit v1.2.3


From 04333482bd665e4d4ced167ff4f566ecfc0cc919 Mon Sep 17 00:00:00 2001
From: Pratik Naik <pratiknaik@gmail.com>
Date: Wed, 11 Mar 2009 14:08:40 +0000
Subject: Rename ActiveRecord::Base.each to ActiveRecord::Base.find_each

---
 activerecord/CHANGELOG                    | 3 +--
 activerecord/lib/active_record/batches.rb | 4 ++--
 activerecord/test/cases/batches_test.rb   | 6 +++---
 3 files changed, 6 insertions(+), 7 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 404481ea38..6d2f597004 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,10 +1,9 @@
 *2.3.1 [RC2] (March 5, 2009)*
 
-* Added ActiveRecord::Base.each and ActiveRecord::Base.find_in_batches for batch processing [DHH/Jamis Buck]
+* Added ActiveRecord::Base.find_each and ActiveRecord::Base.find_in_batches for batch processing [DHH/Jamis Buck]
 
 * Added that ActiveRecord::Base.exists? can be called with no arguments #1817 [Scott Taylor]
 
-
 *2.3.0 [RC1] (February 1st, 2009)*
 
 * Add Support for updating deeply nested models from a single form. #1202 [Eloy Duran]
diff --git a/activerecord/lib/active_record/batches.rb b/activerecord/lib/active_record/batches.rb
index 9e9c8fbbf4..4ea932f34d 100644
--- a/activerecord/lib/active_record/batches.rb
+++ b/activerecord/lib/active_record/batches.rb
@@ -11,14 +11,14 @@ module ActiveRecord
       #
       # Example:
       #
-      #   Person.each(:conditions => "age > 21") do |person|
+      #   Person.find_each(:conditions => "age > 21") do |person|
       #     person.party_all_night!
       #   end
       #
       # Note: This method is only intended to use for batch processing of large amounts of records that wouldn't fit in
       # memory all at once. If you just need to loop over less than 1000 records, it's probably better just to use the
       # regular find methods.
-      def each(options = {})
+      def find_each(options = {})
         find_in_batches(options) do |records|
           records.each { |record| yield record }
         end
diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb
index 108d679108..705da9f88e 100644
--- a/activerecord/test/cases/batches_test.rb
+++ b/activerecord/test/cases/batches_test.rb
@@ -11,7 +11,7 @@ class EachTest < ActiveRecord::TestCase
   
   def test_each_should_excecute_one_query_per_batch
     assert_queries(Post.count + 1) do
-      Post.each(:batch_size => 1) do |post| 
+      Post.find_each(:batch_size => 1) do |post|
         assert_kind_of Post, post
       end
     end
@@ -19,13 +19,13 @@ class EachTest < ActiveRecord::TestCase
 
   def test_each_should_raise_if_the_order_is_set
     assert_raise(RuntimeError) do
-      Post.each(:order => "title") { |post| post }
+      Post.find_each(:order => "title") { |post| post }
     end
   end
 
   def test_each_should_raise_if_the_limit_is_set
     assert_raise(RuntimeError) do
-      Post.each(:limit => 1) { |post| post }
+      Post.find_each(:limit => 1) { |post| post }
     end
   end
   
-- 
cgit v1.2.3


From 0b6f514947c38bf7af2b7e98905a49a94832e5f6 Mon Sep 17 00:00:00 2001
From: Pratik Naik <pratiknaik@gmail.com>
Date: Wed, 11 Mar 2009 14:26:26 +0000
Subject: Add NamedScope#find_each tests [#2201 state:resolved]

---
 activerecord/test/cases/named_scope_test.rb | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index 4b2be0987e..a441a8f6e3 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -15,7 +15,7 @@ class NamedScopeTest < ActiveRecord::TestCase
     assert_equal Topic.find(:all),   Topic.base
     assert_equal Topic.find(:all),   Topic.base.to_a
     assert_equal Topic.find(:first), Topic.base.first
-    assert_equal Topic.find(:all),   Topic.base.each { |i| i }
+    assert_equal Topic.find(:all),   Topic.base.map { |i| i }
   end
 
   def test_found_items_are_cached
@@ -316,6 +316,20 @@ class NamedScopeTest < ActiveRecord::TestCase
   def test_methods_invoked_within_scopes_should_respect_scope
     assert_equal [], Topic.approved.by_rejected_ids.proxy_options[:conditions][:id]
   end
+
+  def test_named_scopes_batch_finders
+    assert_equal 3, Topic.approved.count
+
+    assert_queries(4) do
+      Topic.approved.find_each(:batch_size => 1) {|t| assert t.approved? }
+    end
+
+    assert_queries(3) do
+      Topic.approved.find_in_batches(:batch_size => 2) do |group|
+        group.each {|t| assert t.approved? }
+      end
+    end
+  end
 end
 
 class DynamicScopeMatchTest < ActiveRecord::TestCase  
-- 
cgit v1.2.3


From f23adf0ed4bd596c9e7688455489a79f8b35eb3f Mon Sep 17 00:00:00 2001
From: Pratik Naik <pratiknaik@gmail.com>
Date: Wed, 11 Mar 2009 14:52:51 +0000
Subject: Add tests for AssociationCollection#find_each and
 AssociationCollection#find_in_batches

---
 .../associations/association_collection.rb         |  2 +-
 .../associations/has_many_associations_test.rb     | 39 ++++++++++++++++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 0fefec1216..f024f99a34 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -60,7 +60,7 @@ module ActiveRecord
           @reflection.klass.find(*args)
         end
       end
-      
+
       # Fetches the first one using SQL if possible.
       def first(*args)
         if fetch_first_or_last_using_find?(args)
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index b7fa9d9d7c..99e4dc7e6e 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -219,6 +219,45 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal 1, firm.clients.find(:all, :conditions => "name = 'Summit'").length
   end
 
+  def test_find_each
+    firm = companies(:first_firm)
+
+    assert ! firm.clients.loaded?
+
+    assert_queries(3) do
+      firm.clients.find_each(:batch_size => 1) {|c| assert_equal firm.id, c.firm_id }
+    end
+
+    assert ! firm.clients.loaded?
+  end
+
+  def test_find_each_with_conditions
+    firm = companies(:first_firm)
+
+    assert_queries(2) do
+      firm.clients.find_each(:batch_size => 1, :conditions => {:name => "Microsoft"}) do |c|
+        assert_equal firm.id, c.firm_id
+        assert_equal "Microsoft", c.name
+      end
+    end
+
+    assert ! firm.clients.loaded?
+  end
+
+  def test_find_in_batches
+    firm = companies(:first_firm)
+
+    assert ! firm.clients.loaded?
+
+    assert_queries(2) do
+      firm.clients.find_in_batches(:batch_size => 2) do |clients|
+        clients.each {|c| assert_equal firm.id, c.firm_id }
+      end
+    end
+
+    assert ! firm.clients.loaded?
+  end
+
   def test_find_all_sanitized
     firm = Firm.find(:first)
     summit = firm.clients.find(:all, :conditions => "name = 'Summit'")
-- 
cgit v1.2.3


From 106976df0911e423042ec4abc165fd561766a047 Mon Sep 17 00:00:00 2001
From: Pratik Naik <pratiknaik@gmail.com>
Date: Wed, 11 Mar 2009 15:24:30 +0000
Subject: Ensure ActiveRecord::Base.find_in_batches fires doesnt fire an extra
 query unless needed

---
 activerecord/lib/active_record/batches.rb   |  5 ++++-
 activerecord/test/cases/batches_test.rb     | 12 ++++++++++++
 activerecord/test/cases/named_scope_test.rb |  2 +-
 3 files changed, 17 insertions(+), 2 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/batches.rb b/activerecord/lib/active_record/batches.rb
index 4ea932f34d..03bd4f9f93 100644
--- a/activerecord/lib/active_record/batches.rb
+++ b/activerecord/lib/active_record/batches.rb
@@ -49,12 +49,15 @@ module ActiveRecord
         raise "You can't specify a limit, it's forced to be the batch_size"  if options[:limit]
 
         start = options.delete(:start).to_i
+        batch_size = options.delete(:batch_size) || 1000
 
-        with_scope(:find => options.merge(:order => batch_order, :limit => options.delete(:batch_size) || 1000)) do
+        with_scope(:find => options.merge(:order => batch_order, :limit => batch_size)) do
           records = find(:all, :conditions => [ "#{table_name}.#{primary_key} >= ?", start ])
 
           while records.any?
             yield records
+
+            break if records.size < batch_size
             records = find(:all, :conditions => [ "#{table_name}.#{primary_key} > ?", records.last.id ])
           end
         end
diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb
index 705da9f88e..5009a90846 100644
--- a/activerecord/test/cases/batches_test.rb
+++ b/activerecord/test/cases/batches_test.rb
@@ -46,4 +46,16 @@ class EachTest < ActiveRecord::TestCase
       end
     end
   end
+
+  def test_find_in_batches_shouldnt_excute_query_unless_needed
+    post_count = Post.count
+
+    assert_queries(2) do
+      Post.find_in_batches(:batch_size => post_count) {|batch| assert_kind_of Array, batch }
+    end
+
+    assert_queries(1) do
+      Post.find_in_batches(:batch_size => post_count + 1) {|batch| assert_kind_of Array, batch }
+    end
+  end
 end
\ No newline at end of file
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index a441a8f6e3..8045b136c2 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -324,7 +324,7 @@ class NamedScopeTest < ActiveRecord::TestCase
       Topic.approved.find_each(:batch_size => 1) {|t| assert t.approved? }
     end
 
-    assert_queries(3) do
+    assert_queries(2) do
       Topic.approved.find_in_batches(:batch_size => 2) do |group|
         group.each {|t| assert t.approved? }
       end
-- 
cgit v1.2.3


From 92dadf6d7927fde1482ba1d96d0916093bfb83ca Mon Sep 17 00:00:00 2001
From: Will Bryant <will.bryant@gmail.com>
Date: Thu, 12 Mar 2009 02:13:26 +1300
Subject: Fixed autosave checks on objects with hm:t in :include  [#2213
 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
---
 activerecord/lib/active_record/autosave_association.rb   |  2 +-
 .../cases/associations/belongs_to_associations_test.rb   | 16 ++++++++++++++++
 .../test/cases/associations/has_one_associations_test.rb | 16 ++++++++++++++++
 .../associations/has_one_through_associations_test.rb    | 16 ++++++++++++++++
 4 files changed, 49 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 1c3d0567c1..6dcc5005d1 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -310,7 +310,7 @@ module ActiveRecord
     # This all happens inside a transaction, _if_ the Transactions module is included into
     # ActiveRecord::Base after the AutosaveAssociation module, which it does by default.
     def save_has_one_association(reflection)
-      if association = association_instance_get(reflection.name)
+      if (association = association_instance_get(reflection.name)) && !association.target.nil?
         if reflection.options[:autosave] && association.marked_for_destruction?
           association.destroy
         elsif new_record? || association.new_record? || association[reflection.primary_key_name] != id || reflection.options[:autosave]
diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb
index ff3e54712e..13a78a1890 100644
--- a/activerecord/test/cases/associations/belongs_to_associations_test.rb
+++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb
@@ -326,4 +326,20 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
     companies(:first_firm).send(:private_method)
     companies(:second_client).firm.send(:private_method)
   end
+
+  def test_save_of_record_with_loaded_belongs_to
+    @account = companies(:first_firm).account
+
+    assert_nothing_raised do
+      Account.find(@account.id).save!
+      Account.find(@account.id, :include => :firm).save!
+    end
+
+    @account.firm.delete
+
+    assert_nothing_raised do
+      Account.find(@account.id).save!
+      Account.find(@account.id, :include => :firm).save!
+    end
+  end
 end
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index 4947f1543c..1ddb3f49bf 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -290,4 +290,20 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
     companies(:first_firm).account.send(:private_method)
   end
 
+  def test_save_of_record_with_loaded_has_one
+    @firm = companies(:first_firm)
+    assert_not_nil @firm.account
+
+    assert_nothing_raised do
+      Firm.find(@firm.id).save!
+      Firm.find(@firm.id, :include => :account).save!
+    end
+
+    @firm.account.destroy
+
+    assert_nothing_raised do
+      Firm.find(@firm.id).save!
+      Firm.find(@firm.id, :include => :account).save!
+    end
+  end
 end
diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb
index f96b55513e..12c598751b 100644
--- a/activerecord/test/cases/associations/has_one_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb
@@ -173,4 +173,20 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase
     assert_not_nil assert_no_queries { @new_detail.member_type }
   end
 
+  def test_save_of_record_with_loaded_has_one_through
+    @club = @member.club
+    assert_not_nil @club.sponsored_member
+
+    assert_nothing_raised do
+      Club.find(@club.id).save!
+      Club.find(@club.id, :include => :sponsored_member).save!
+    end
+
+    @club.sponsor.destroy
+
+    assert_nothing_raised do
+      Club.find(@club.id).save!
+      Club.find(@club.id, :include => :sponsored_member).save!
+    end
+  end
 end
-- 
cgit v1.2.3


From db26ace030f6704da6fc80bcc6cd00a2aee664ce Mon Sep 17 00:00:00 2001
From: Murray Steele <muz@h-lame.com>
Date: Thu, 12 Mar 2009 13:49:16 +0000
Subject: Ensure NoMethodError isn't raised when some of the nested eager
 loaded associations are empty [#1696 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
---
 activerecord/lib/active_record/associations.rb     | 13 +++++++---
 .../associations/eager_load_nested_include_test.rb | 29 ++++++++++++++++++++++
 2 files changed, 38 insertions(+), 4 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 301b3a3b58..6d25b36aea 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1856,9 +1856,10 @@ module ActiveRecord
             def construct(parent, associations, joins, row)
               case associations
                 when Symbol, String
-                  while (join = joins.shift).reflection.name.to_s != associations.to_s
-                    raise ConfigurationError, "Not Enough Associations" if joins.empty?
-                  end
+                  join = joins.detect{|j| j.reflection.name.to_s == associations.to_s && j.parent_table_name == parent.class.table_name }
+                  raise(ConfigurationError, "No such association") if join.nil?
+
+                  joins.delete(join)
                   construct_association(parent, join, row)
                 when Array
                   associations.each do |association|
@@ -1866,7 +1867,11 @@ module ActiveRecord
                   end
                 when Hash
                   associations.keys.sort{|a,b|a.to_s<=>b.to_s}.each do |name|
-                    association = construct_association(parent, joins.shift, row)
+                    join = joins.detect{|j| j.reflection.name.to_s == name.to_s && j.parent_table_name == parent.class.table_name }
+                    raise(ConfigurationError, "No such association") if join.nil?
+
+                    association = construct_association(parent, join, row)
+                    joins.delete(join)
                     construct(association, associations[name], joins, row) if association
                   end
                 else
diff --git a/activerecord/test/cases/associations/eager_load_nested_include_test.rb b/activerecord/test/cases/associations/eager_load_nested_include_test.rb
index 12dec5ccd1..1b2e0fc11e 100644
--- a/activerecord/test/cases/associations/eager_load_nested_include_test.rb
+++ b/activerecord/test/cases/associations/eager_load_nested_include_test.rb
@@ -1,4 +1,9 @@
 require 'cases/helper'
+require 'models/author'
+require 'models/post'
+require 'models/comment'
+require 'models/category'
+require 'models/categorization'
 
 module Remembered
   def self.included(base)
@@ -99,3 +104,27 @@ class EagerLoadPolyAssocsTest < ActiveRecord::TestCase
     end
   end
 end
+
+class EagerLoadNestedIncludeWithMissingDataTest < ActiveRecord::TestCase
+  def setup
+    @davey_mcdave = Author.create(:name => 'Davey McDave')
+    @first_post = @davey_mcdave.posts.create(:title => 'Davey Speaks', :body => 'Expressive wordage')
+    @first_comment = @first_post.comments.create(:body => 'Inflamatory doublespeak')
+    @first_categorization = @davey_mcdave.categorizations.create(:category => Category.first, :post => @first_post)
+  end
+
+  def teardown
+    @davey_mcdave.destroy
+    @first_post.destroy
+    @first_comment.destroy
+    @first_categorization.destroy
+  end
+
+  def test_missing_data_in_a_nested_include_should_not_cause_errors_when_constructing_objects
+    assert_nothing_raised do
+      # @davey_mcdave doesn't have any author_favorites
+      includes = {:posts => :comments, :categorizations => :category, :author_favorites => :favorite_author }
+      Author.all :include => includes, :conditions => {:authors => {:name => @davey_mcdave.name}}, :order => 'categories.name'
+    end
+  end
+end
\ No newline at end of file
-- 
cgit v1.2.3


From 91b98cf0a5417ce4042a0b3cd1930d5a221b737f Mon Sep 17 00:00:00 2001
From: Elijah Miller <elijah.miller@gmail.com>
Date: Thu, 12 Mar 2009 15:06:19 +0000
Subject: Returning nil from named scope lambda is equivalent to an empty hash
 [#1773 state:resolved]

Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
---
 activerecord/lib/active_record/named_scope.rb | 1 +
 activerecord/test/cases/named_scope_test.rb   | 6 ++++++
 activerecord/test/models/topic.rb             | 4 +++-
 3 files changed, 10 insertions(+), 1 deletion(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index 519941dd94..1f3ef300f2 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -117,6 +117,7 @@ module ActiveRecord
       delegate :scopes, :with_scope, :to => :proxy_scope
 
       def initialize(proxy_scope, options, &block)
+        options ||= {}
         [options[:extend]].flatten.each { |extension| extend extension } if options[:extend]
         extend Module.new(&block) if block_given?
         unless Scope === proxy_scope
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index 8045b136c2..ae6a54a5bd 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -99,6 +99,12 @@ class NamedScopeTest < ActiveRecord::TestCase
     assert_equal topics_written_before_the_second, Topic.written_before(topics(:second).written_on)
   end
 
+  def test_procedural_scopes_returning_nil
+    all_topics = Topic.find(:all)
+
+    assert_equal all_topics, Topic.written_before(nil)
+  end
+
   def test_scopes_with_joins
     address = author_addresses(:david_address)
     posts_with_authors_at_address = Post.find(
diff --git a/activerecord/test/models/topic.rb b/activerecord/test/models/topic.rb
index f1b7bbae3b..51012d22ed 100644
--- a/activerecord/test/models/topic.rb
+++ b/activerecord/test/models/topic.rb
@@ -1,7 +1,9 @@
 class Topic < ActiveRecord::Base
   named_scope :base
   named_scope :written_before, lambda { |time|
-    { :conditions => ['written_on < ?', time] }
+    if time
+      { :conditions => ['written_on < ?', time] }
+    end
   }
   named_scope :approved, :conditions => {:approved => true}
   named_scope :rejected, :conditions => {:approved => false}
-- 
cgit v1.2.3


From 47bdf3bf40ec17e1f8ca1c0e3d7f697d0c4cd1bf Mon Sep 17 00:00:00 2001
From: Luca Guidi <guidi.luca@gmail.com>
Date: Thu, 12 Mar 2009 15:24:37 +0000
Subject: Ensure AutosaveAssociation runs remove callbacks [#2146
 state:resolved]

Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
Signed-off-by: Pratik Naik <pratiknaik@gmail.com>
---
 .../associations/association_collection.rb         | 51 ++++++++++++++--------
 .../lib/active_record/autosave_association.rb      |  2 +-
 .../has_and_belongs_to_many_associations_test.rb   | 27 ++++++++++++
 .../associations/has_many_associations_test.rb     | 24 ++++++++++
 .../has_many_through_associations_test.rb          | 18 ++++++++
 .../test/cases/autosave_association_test.rb        | 35 +++++++++++++++
 activerecord/test/models/pirate.rb                 | 49 ++++++++++++++++++++-
 7 files changed, 185 insertions(+), 21 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index f024f99a34..ad375be184 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -186,7 +186,6 @@ module ActiveRecord
         end
       end
 
-
       # Removes +records+ from this association calling +before_remove+ and
       # +after_remove+ callbacks.
       #
@@ -195,22 +194,25 @@ module ActiveRecord
       # are actually removed from the database, that depends precisely on
       # +delete_records+. They are in any case removed from the collection.
       def delete(*records)
-        records = flatten_deeper(records)
-        records.each { |record| raise_on_type_mismatch(record) }
-        
-        transaction do
-          records.each { |record| callback(:before_remove, record) }
-          
-          old_records = records.reject {|r| r.new_record? }
+        remove_records(records) do |records, old_records|
           delete_records(old_records) if old_records.any?
-          
-          records.each do |record|
-            @target.delete(record)
-            callback(:after_remove, record)
-          end
+          records.each { |record| @target.delete(record) }
         end
       end
 
+      # Destroy +records+ and remove from this association calling +before_remove+
+      # and +after_remove+ callbacks.
+      #
+      # Note this method will always remove records from database ignoring the
+      # +:dependent+ option.
+      def destroy(*records)
+        remove_records(records) do |records, old_records|
+          old_records.each { |record| record.destroy }
+        end
+
+        load_target
+      end
+
       # Removes all records from this association.  Returns +self+ so method calls may be chained.
       def clear
         return self if length.zero? # forces load_target if it hasn't happened already
@@ -223,15 +225,14 @@ module ActiveRecord
 
         self
       end
-      
-      def destroy_all
-        transaction do
-          each { |record| record.destroy }
-        end
 
+      # Destory all the records from this association
+      def destroy_all
+        load_target
+        destroy(@target)
         reset_target!
       end
-      
+
       def create(attrs = {})
         if attrs.is_a?(Array)
           attrs.collect { |attr| create(attr) }
@@ -431,6 +432,18 @@ module ActiveRecord
           record
         end
 
+        def remove_records(*records)
+          records = flatten_deeper(records)
+          records.each { |record| raise_on_type_mismatch(record) }
+
+          transaction do
+            records.each { |record| callback(:before_remove, record) }
+            old_records = records.reject { |r| r.new_record? }
+            yield(records, old_records)
+            records.each { |record| callback(:after_remove, record) }
+          end
+        end
+
         def callback(method, record)
           callbacks_for(method).each do |callback|
             ActiveSupport::Callbacks::Callback.new(method, callback, record).call(@owner, record)
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb
index 6dcc5005d1..741aa2acbe 100644
--- a/activerecord/lib/active_record/autosave_association.rb
+++ b/activerecord/lib/active_record/autosave_association.rb
@@ -283,7 +283,7 @@ module ActiveRecord
         if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave)
           records.each do |record|
             if autosave && record.marked_for_destruction?
-              record.destroy
+              association.destroy(record)
             elsif @new_record_before_save || record.new_record?
               if autosave
                 association.send(:insert_record, record, false, false)
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index ca1772d1ca..5e8b2cadfc 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -381,6 +381,33 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
     assert_date_from_db Date.new(2004, 10, 10), Developer.find(1).projects.first.joined_on.to_date
   end
 
+  def test_destroying
+    david = Developer.find(1)
+    active_record = Project.find(1)
+    david.projects.reload
+    assert_equal 2, david.projects.size
+    assert_equal 3, active_record.developers.size
+
+    assert_difference "Project.count", -1 do
+      david.projects.destroy(active_record)
+    end
+
+    assert_equal 1, david.reload.projects.size
+    assert_equal 1, david.projects(true).size
+  end
+
+  def test_destroying_array
+    david = Developer.find(1)
+    david.projects.reload
+
+    assert_difference "Project.count", -Project.count do
+      david.projects.destroy(Project.find(:all))
+    end
+
+    assert_equal 0, david.reload.projects.size
+    assert_equal 0, david.projects(true).size
+  end
+
   def test_destroy_all
     david = Developer.find(1)
     david.projects.reload
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 99e4dc7e6e..30edf79a26 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -680,6 +680,30 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_raise(ActiveRecord::AssociationTypeMismatch) { david.projects.delete(Project.find(1).developers) }
   end
 
+  def test_destroying
+    force_signal37_to_load_all_clients_of_firm
+
+    assert_difference "Client.count", -1 do
+      companies(:first_firm).clients_of_firm.destroy(companies(:first_firm).clients_of_firm.first)
+    end
+
+    assert_equal 0, companies(:first_firm).reload.clients_of_firm.size
+    assert_equal 0, companies(:first_firm).clients_of_firm(true).size
+  end
+
+  def test_destroying_a_collection
+    force_signal37_to_load_all_clients_of_firm
+    companies(:first_firm).clients_of_firm.create("name" => "Another Client")
+    assert_equal 2, companies(:first_firm).clients_of_firm.size
+
+    assert_difference "Client.count", -2 do
+      companies(:first_firm).clients_of_firm.destroy([companies(:first_firm).clients_of_firm[0], companies(:first_firm).clients_of_firm[1]])
+    end
+
+    assert_equal 0, companies(:first_firm).reload.clients_of_firm.size
+    assert_equal 0, companies(:first_firm).clients_of_firm(true).size
+  end
+
   def test_destroy_all
     force_signal37_to_load_all_clients_of_firm
     assert !companies(:first_firm).clients_of_firm.empty?, "37signals has clients after load"
diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb
index c3ad0ee6de..97efca7891 100644
--- a/activerecord/test/cases/associations/has_many_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb
@@ -92,6 +92,24 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase
     assert posts(:welcome).reload.people(true).empty?
   end
 
+  def test_destroy_association
+    assert_difference "Person.count", -1 do
+      posts(:welcome).people.destroy(people(:michael))
+    end
+
+    assert posts(:welcome).reload.people.empty?
+    assert posts(:welcome).people(true).empty?
+  end
+
+  def test_destroy_all
+    assert_difference "Person.count", -1 do
+      posts(:welcome).people.destroy_all
+    end
+
+    assert posts(:welcome).reload.people.empty?
+    assert posts(:welcome).people(true).empty?
+  end
+
   def test_replace_association
     assert_queries(4){posts(:welcome);people(:david);people(:michael); posts(:welcome).people(true)}
     
diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb
index b179bd827a..436f50d395 100644
--- a/activerecord/test/cases/autosave_association_test.rb
+++ b/activerecord/test/cases/autosave_association_test.rb
@@ -556,6 +556,41 @@ class TestDestroyAsPartOfAutosaveAssociation < ActiveRecord::TestCase
       assert_raise(RuntimeError) { assert !@pirate.save }
       assert_equal before, @pirate.reload.send(association_name)
     end
+
+    # Add and remove callbacks tests for association collections.
+    %w{ method proc }.each do |callback_type|
+      define_method("test_should_run_add_callback_#{callback_type}s_for_#{association_name}") do
+        association_name_with_callbacks = "#{association_name}_with_#{callback_type}_callbacks"
+
+        pirate = Pirate.new(:catchphrase => "Arr")
+        pirate.send(association_name_with_callbacks).build(:name => "Crowe the One-Eyed")
+
+        expected = [
+          "before_adding_#{callback_type}_#{association_name.singularize}_<new>",
+          "after_adding_#{callback_type}_#{association_name.singularize}_<new>"
+        ]
+
+        assert_equal expected, pirate.ship_log
+      end
+
+      define_method("test_should_run_remove_callback_#{callback_type}s_for_#{association_name}") do
+        association_name_with_callbacks = "#{association_name}_with_#{callback_type}_callbacks"
+
+        @pirate.send(association_name_with_callbacks).create!(:name => "Crowe the One-Eyed")
+        @pirate.send(association_name_with_callbacks).each { |c| c.mark_for_destruction }
+        child_id = @pirate.send(association_name_with_callbacks).first.id
+
+        @pirate.ship_log.clear
+        @pirate.save
+
+        expected = [
+          "before_removing_#{callback_type}_#{association_name.singularize}_#{child_id}",
+          "after_removing_#{callback_type}_#{association_name.singularize}_#{child_id}"
+        ]
+
+        assert_equal expected, @pirate.ship_log
+      end
+    end
   end
 end
 
diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb
index 7bc50e0e7b..238917bf30 100644
--- a/activerecord/test/models/pirate.rb
+++ b/activerecord/test/models/pirate.rb
@@ -1,16 +1,63 @@
 class Pirate < ActiveRecord::Base
   belongs_to :parrot
   has_and_belongs_to_many :parrots
-  has_many :treasures, :as => :looter
+  has_and_belongs_to_many :parrots_with_method_callbacks, :class_name => "Parrot",
+    :before_add    => :log_before_add,
+    :after_add     => :log_after_add,
+    :before_remove => :log_before_remove,
+    :after_remove  => :log_after_remove
+  has_and_belongs_to_many :parrots_with_proc_callbacks, :class_name => "Parrot",
+    :before_add    => proc {|p,pa| p.ship_log << "before_adding_proc_parrot_#{pa.id || '<new>'}"},
+    :after_add     => proc {|p,pa| p.ship_log << "after_adding_proc_parrot_#{pa.id || '<new>'}"},
+    :before_remove => proc {|p,pa| p.ship_log << "before_removing_proc_parrot_#{pa.id}"},
+    :after_remove  => proc {|p,pa| p.ship_log << "after_removing_proc_parrot_#{pa.id}"}
 
+  has_many :treasures, :as => :looter
   has_many :treasure_estimates, :through => :treasures, :source => :price_estimates
 
   # These both have :autosave enabled because accepts_nested_attributes_for is used on them.
   has_one :ship
   has_many :birds
+  has_many :birds_with_method_callbacks, :class_name => "Bird",
+    :before_add    => :log_before_add,
+    :after_add     => :log_after_add,
+    :before_remove => :log_before_remove,
+    :after_remove  => :log_after_remove
+  has_many :birds_with_proc_callbacks, :class_name => "Bird",
+    :before_add     => proc {|p,b| p.ship_log << "before_adding_proc_bird_#{b.id || '<new>'}"},
+    :after_add      => proc {|p,b| p.ship_log << "after_adding_proc_bird_#{b.id || '<new>'}"},
+    :before_remove  => proc {|p,b| p.ship_log << "before_removing_proc_bird_#{b.id}"},
+    :after_remove   => proc {|p,b| p.ship_log << "after_removing_proc_bird_#{b.id}"}
 
   accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
   accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
+  accepts_nested_attributes_for :parrots_with_method_callbacks, :parrots_with_proc_callbacks,
+    :birds_with_method_callbacks, :birds_with_proc_callbacks, :allow_destroy => true
 
   validates_presence_of :catchphrase
+
+  def ship_log
+    @ship_log ||= []
+  end
+
+  private
+    def log_before_add(record)
+      log(record, "before_adding_method")
+    end
+
+    def log_after_add(record)
+      log(record, "after_adding_method")
+    end
+
+    def log_before_remove(record)
+      log(record, "before_removing_method")
+    end
+
+    def log_after_remove(record)
+      log(record, "after_removing_method")
+    end
+
+    def log(record, callback)
+      ship_log << "#{callback}_#{record.class.name.downcase}_#{record.id || '<new>'}"
+    end
 end
-- 
cgit v1.2.3