From 379c02267b3cbbf6d1ac48bc79ec6ea01af7b53a Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 16 Dec 2010 22:06:19 +0000
Subject: Specify insert_record with NotImplementedError in
 AssociationCollection, to indicate that subclasses should implement it. Also
 add save_record to reduce duplication.

---
 .../lib/active_record/associations/association_collection.rb  | 11 +++++++++++
 1 file changed, 11 insertions(+)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 11a7a725e5..51d8952eca 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -476,6 +476,17 @@ module ActiveRecord
         end
 
       private
+        # Do the relevant stuff to insert the given record into the association collection. The
+        # force param specifies whether or not an exception should be raised on failure. The
+        # validate param specifies whether validation should be performed (if force is false).
+        def insert_record(record, force = true, validate = true)
+          raise NotImplementedError
+        end
+
+        def save_record(record, force, validate)
+          force ? record.save! : record.save(:validate => validate)
+        end
+
         def create_record(attrs)
           attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
           ensure_owner_is_persisted!
-- 
cgit v1.2.3


From ffa57671bb664a715eb2eebaa7476c747abf0fb1 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 16 Dec 2010 22:39:46 +0000
Subject: Delete create, create! and create_record from
 HasManyThroughAssociation in exchange for more generic versions in
 AssociationCollection

---
 .../associations/association_collection.rb         | 26 ++++++++++++----------
 1 file changed, 14 insertions(+), 12 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 51d8952eca..3f80133299 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -263,7 +263,7 @@ module ActiveRecord
         else
           create_record(attrs) do |record|
             yield(record) if block_given?
-            record.save
+            insert_record(record, false)
           end
         end
       end
@@ -271,7 +271,7 @@ module ActiveRecord
       def create!(attrs = {})
         create_record(attrs) do |record|
           yield(record) if block_given?
-          record.save!
+          insert_record(record, true)
         end
       end
 
@@ -488,18 +488,20 @@ module ActiveRecord
         end
 
         def create_record(attrs)
-          attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
           ensure_owner_is_persisted!
 
-          scoped_where = scoped.where_values_hash
-          create_scope = scoped_where ? @scope[:create].merge(scoped_where) : @scope[:create]
-          record = @reflection.klass.send(:with_scope, :create => create_scope) do
-            @reflection.build_association(attrs)
-          end
-          if block_given?
-            add_record_to_target_with_callbacks(record) { |*block_args| yield(*block_args) }
-          else
-            add_record_to_target_with_callbacks(record)
+          attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
+          create_scope = @scope[:create].merge(scoped.where_values_hash || {})
+
+          transaction do
+            record = with_scope(:create => create_scope) do
+              @reflection.build_association(attrs)
+            end
+            if block_given?
+              add_record_to_target_with_callbacks(record) { |*block_args| yield(*block_args) }
+            else
+              add_record_to_target_with_callbacks(record)
+            end
           end
         end
 
-- 
cgit v1.2.3


From 7f5fcc0785e28175b1a54971e5adc34ecd50787d Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Fri, 17 Dec 2010 09:56:47 +0000
Subject: Refactor create_record and build_record in AssociationCollection

---
 .../associations/association_collection.rb         | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 3f80133299..18d05aa133 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -487,32 +487,20 @@ module ActiveRecord
           force ? record.save! : record.save(:validate => validate)
         end
 
-        def create_record(attrs)
+        def create_record(attrs, &block)
           ensure_owner_is_persisted!
 
-          attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
-          create_scope = @scope[:create].merge(scoped.where_values_hash || {})
-
           transaction do
-            record = with_scope(:create => create_scope) do
-              @reflection.build_association(attrs)
-            end
-            if block_given?
-              add_record_to_target_with_callbacks(record) { |*block_args| yield(*block_args) }
-            else
-              add_record_to_target_with_callbacks(record)
+            with_scope(:create => @scope[:create].merge(scoped.where_values_hash || {})) do
+              build_record(attrs, &block)
             end
           end
         end
 
-        def build_record(attrs)
+        def build_record(attrs, &block)
           attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
           record = @reflection.build_association(attrs)
-          if block_given?
-            add_record_to_target_with_callbacks(record) { |*block_args| yield(*block_args) }
-          else
-            add_record_to_target_with_callbacks(record)
-          end
+          add_record_to_target_with_callbacks(record, &block)
         end
 
         def remove_records(*records)
-- 
cgit v1.2.3


From 6e14feb978434802e7a46b26d99d64e31f545fe2 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Mon, 20 Dec 2010 14:24:04 -0800
Subject: use array arithmetic rather than create sets

---
 .../lib/active_record/associations/association_collection.rb        | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 18d05aa133..1c8541be67 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -344,12 +344,10 @@ module ActiveRecord
         other_array.each { |val| raise_on_type_mismatch(val) }
 
         load_target
-        other   = other_array.size < 100 ? other_array : other_array.to_set
-        current = @target.size < 100 ? @target : @target.to_set
 
         transaction do
-          delete(@target.select { |v| !other.include?(v) })
-          concat(other_array.select { |v| !current.include?(v) })
+          delete(@target - other_array)
+          concat(other_array - @target)
         end
       end
 
-- 
cgit v1.2.3


From 3ce3c21997c92cee4123e76d75e39fcfe3d1e209 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Mon, 20 Dec 2010 14:40:07 -0800
Subject: no use for set, no need to to_ary, reduce extra objects

---
 .../lib/active_record/associations/association_collection.rb         | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 1c8541be67..c513e8ab08 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -1,4 +1,3 @@
-require 'set'
 require 'active_support/core_ext/array/wrap'
 
 module ActiveRecord
@@ -75,7 +74,7 @@ module ActiveRecord
           find(:first, *args)
         else
           load_target unless loaded?
-          args = args[1..-1] if args.first.kind_of?(Hash) && args.first.empty?
+          args.shift if args.first.kind_of?(Hash) && args.first.empty?
           @target.first(*args)
         end
       end
@@ -93,7 +92,7 @@ module ActiveRecord
       def to_ary
         load_target
         if @target.is_a?(Array)
-          @target.to_ary
+          @target
         else
           Array.wrap(@target)
         end
-- 
cgit v1.2.3


From ff7bde62c857ec94f45a5be3bc76468deb8b0b3a Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Wed, 22 Dec 2010 00:19:59 +0000
Subject: When a has_many association is not :uniq, appending the same record
 multiple times should append it to the @target multiple times [#5964
 state:resolved]

---
 activerecord/lib/active_record/associations/association_collection.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index c513e8ab08..7964f4fa2b 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -462,10 +462,10 @@ module ActiveRecord
           callback(:before_add, record)
           yield(record) if block_given?
           @target ||= [] unless loaded?
-          if index = @target.index(record)
+          if @reflection.options[:uniq] && index = @target.index(record)
             @target[index] = record
           else
-             @target << record
+            @target << record
           end
           callback(:after_add, record)
           set_inverse_instance(record, @owner)
-- 
cgit v1.2.3


From c6e0433ca3520e9bc999f70840d82a76b9470873 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 23 Dec 2010 17:33:42 +0000
Subject: scoped.where_values_hash is never nil

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 7964f4fa2b..8bb32400cf 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -488,7 +488,7 @@ module ActiveRecord
           ensure_owner_is_persisted!
 
           transaction do
-            with_scope(:create => @scope[:create].merge(scoped.where_values_hash || {})) do
+            with_scope(:create => @scope[:create].merge(scoped.where_values_hash)) do
               build_record(attrs, &block)
             end
           end
-- 
cgit v1.2.3


From e8ada11aac28f0850f0e485acacf34e7eb81aa19 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Fri, 24 Dec 2010 00:29:04 +0000
Subject: Associations: DRY up the code which is generating conditions, and
 make it all use arel rather than SQL strings

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 8bb32400cf..c9f9c925b0 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -111,7 +111,7 @@ module ActiveRecord
         else
           build_record(attributes) do |record|
             block.call(record) if block_given?
-            set_belongs_to_association_for(record)
+            set_owner_attributes(record)
           end
         end
       end
-- 
cgit v1.2.3


From b0498372a10bda006350af42708a5588ab28ffcb Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sun, 26 Dec 2010 19:37:35 +0000
Subject: Add a HasAssociation module for common code for has_* associations

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 ++
 1 file changed, 2 insertions(+)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index c9f9c925b0..86cc17394c 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -18,6 +18,8 @@ module ActiveRecord
     # If you need to work on all current children, new and existing records,
     # +load_target+ and the +loaded+ flag are your friends.
     class AssociationCollection < AssociationProxy #:nodoc:
+      include HasAssociation
+
       delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :to => :scoped
 
       def select(select = nil)
-- 
cgit v1.2.3


From 9f5c18ce075179cfc73a00cba9a19d69aaf5274c Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sun, 26 Dec 2010 21:37:25 +0000
Subject: Refactor we_can_set_the_inverse_on_this? to use a less bizarre name
 amongst other things

---
 .../lib/active_record/associations/association_collection.rb        | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 86cc17394c..97d9288874 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -454,9 +454,7 @@ module ActiveRecord
             end
 
           records = @reflection.options[:uniq] ? uniq(records) : records
-          records.each do |record|
-            set_inverse_instance(record, @owner)
-          end
+          records.each { |record| set_inverse_instance(record) }
           records
         end
 
@@ -470,7 +468,7 @@ module ActiveRecord
             @target << record
           end
           callback(:after_add, record)
-          set_inverse_instance(record, @owner)
+          set_inverse_instance(record)
           record
         end
 
-- 
cgit v1.2.3


From 897b56bb2f8c2a904f546db1a32bad074463ec9b Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Mon, 27 Dec 2010 12:47:30 -0700
Subject: I N C E P T I O N: flatten_deeper works around a bug in Ruby 1.8.2.

---
 activerecord/lib/active_record/associations/association_collection.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 97d9288874..108e316672 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -125,7 +125,7 @@ module ActiveRecord
         load_target if @owner.new_record?
 
         transaction do
-          flatten_deeper(records).each do |record|
+          records.flatten.each do |record|
             raise_on_type_mismatch(record)
             add_record_to_target_with_callbacks(record) do |r|
               result &&= insert_record(record) unless @owner.new_record?
@@ -501,7 +501,7 @@ module ActiveRecord
         end
 
         def remove_records(*records)
-          records = flatten_deeper(records)
+          records = records.flatten
           records.each { |record| raise_on_type_mismatch(record) }
 
           transaction do
-- 
cgit v1.2.3


From 62b084f80759300f10a4e5c4235bf1d13693a7d3 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Fri, 31 Dec 2010 10:43:42 +0000
Subject: Specify the STI type condition using SQL IN rather than a whole load
 of ORs. Required a fix to ActiveRecord::Relation#merge for properly merging
 create_with_value. This also fixes a situation where the type condition was
 appearing twice in the resultant SQL query.

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 108e316672..64e10f56e2 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -488,7 +488,7 @@ module ActiveRecord
           ensure_owner_is_persisted!
 
           transaction do
-            with_scope(:create => @scope[:create].merge(scoped.where_values_hash)) do
+            with_scope(:create => @scope[:create].merge(scoped.scope_for_create)) do
               build_record(attrs, &block)
             end
           end
-- 
cgit v1.2.3


From bea4065d3c8c8f845ddda45b3ec98e3fb308d913 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Fri, 31 Dec 2010 18:08:44 +0000
Subject: Refactor BelongsToAssociation to allow
 BelongsToPolymorphicAssociation to inherit from it

---
 activerecord/lib/active_record/associations/association_collection.rb | 4 ----
 1 file changed, 4 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 64e10f56e2..0959ea2c5a 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -31,10 +31,6 @@ module ActiveRecord
         end
       end
 
-      def scoped
-        with_scope(@scope) { @reflection.klass.scoped }
-      end
-
       def find(*args)
         options = args.extract_options!
 
-- 
cgit v1.2.3


From 4e194ed1e68c13901f486334a5a1e9f509b10722 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sun, 2 Jan 2011 14:42:17 +0000
Subject: Rename AssociationProxy#foreign_key_present to foreign_key_present?

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 0959ea2c5a..be5d264a8f 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -364,7 +364,7 @@ module ActiveRecord
         end
 
         def load_target
-          if !@owner.new_record? || foreign_key_present
+          if !@owner.new_record? || foreign_key_present?
             begin
               unless loaded?
                 if @target.is_a?(Array) && @target.any?
-- 
cgit v1.2.3


From 3103296a61709e808aa89c3d37cf22bcdbc5a675 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sun, 2 Jan 2011 20:33:18 +0000
Subject: Let AssociationCollection#find use #scoped to do its finding. Note
 that I am removing
 test_polymorphic_has_many_going_through_join_model_with_disabled_include,
 since this specifies different behaviour for an association than for a
 regular scope. It seems reasonable to expect scopes and association proxies
 to behave in roughly the same way rather than having subtle differences.

---
 .../associations/association_collection.rb         | 81 +++++++++++-----------
 1 file changed, 42 insertions(+), 39 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index be5d264a8f..8b0017e7bf 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -32,37 +32,10 @@ module ActiveRecord
       end
 
       def find(*args)
-        options = args.extract_options!
-
-        # If using a custom finder_sql, scan the entire collection.
         if @reflection.options[:finder_sql]
-          expects_array = args.first.kind_of?(Array)
-          ids           = args.flatten.compact.uniq.map { |arg| arg.to_i }
-
-          if ids.size == 1
-            id = ids.first
-            record = load_target.detect { |r| id == r.id }
-            expects_array ? [ record ] : record
-          else
-            load_target.select { |r| ids.include?(r.id) }
-          end
+          find_by_scan(*args)
         else
-          merge_options_from_reflection!(options)
-          construct_find_options!(options)
-
-          with_scope(:find => @scope[:find].slice(:conditions, :order)) do
-            relation = @reflection.klass.send(:construct_finder_arel, options, @reflection.klass.send(:current_scoped_methods))
-
-            case args.first
-            when :first, :last
-              relation.send(args.first)
-            when :all
-              records = relation.all
-              @reflection.options[:uniq] ? uniq(records) : records
-            else
-              relation.find(*args)
-            end
-          end
+          find_by_sql(*args)
         end
       end
 
@@ -360,7 +333,25 @@ module ActiveRecord
       end
 
       protected
-        def construct_find_options!(options)
+
+        def construct_find_scope
+          {
+            :conditions => construct_conditions,
+            :select     => construct_select,
+            :readonly   => @reflection.options[:readonly],
+            :order      => @reflection.options[:order],
+            :limit      => @reflection.options[:limit],
+            :include    => @reflection.options[:include],
+            :joins      => @reflection.options[:joins],
+            :group      => @reflection.options[:group],
+            :having     => @reflection.options[:having],
+            :offset     => @reflection.options[:offset]
+          }
+        end
+
+        def construct_select
+          @reflection.options[:select] ||
+          @reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*"
         end
 
         def load_target
@@ -394,7 +385,7 @@ module ActiveRecord
           target
         end
 
-        def method_missing(method, *args)
+        def method_missing(method, *args, &block)
           match = DynamicFinderMatch.match(method)
           if match && match.creator?
             attributes = match.attribute_names
@@ -406,15 +397,9 @@ module ActiveRecord
           elsif @reflection.klass.scopes[method]
             @_scopes_cache ||= {}
             @_scopes_cache[method] ||= {}
-            @_scopes_cache[method][args] ||= with_scope(@scope) { @reflection.klass.send(method, *args) }
+            @_scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args)
           else
-            with_scope(@scope) do
-              if block_given?
-                @reflection.klass.send(method, *args) { |*block_args| yield(*block_args) }
-              else
-                @reflection.klass.send(method, *args)
-              end
-            end
+            scoped.readonly(nil).send(method, *args, &block)
           end
         end
 
@@ -547,6 +532,24 @@ module ActiveRecord
             @target.include?(record)
           end
         end
+
+        # If using a custom finder_sql, #find scans the entire collection.
+        def find_by_scan(*args)
+          expects_array = args.first.kind_of?(Array)
+          ids           = args.flatten.compact.uniq.map { |arg| arg.to_i }
+
+          if ids.size == 1
+            id = ids.first
+            record = load_target.detect { |r| id == r.id }
+            expects_array ? [ record ] : record
+          else
+            load_target.select { |r| ids.include?(r.id) }
+          end
+        end
+
+        def find_by_sql(*args)
+          scoped.find(*args)
+        end
     end
   end
 end
-- 
cgit v1.2.3


From 31d101879f1acae604d24d831a4b82a4482acf31 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sun, 2 Jan 2011 21:15:03 +0000
Subject: Use the association directly in other places too

---
 .../lib/active_record/associations/association_collection.rb       | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 8b0017e7bf..6defc465d8 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -155,14 +155,13 @@ module ActiveRecord
 
           @reflection.klass.count_by_sql(custom_counter_sql)
         else
-
           if @reflection.options[:uniq]
             # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
             column_name = "#{@reflection.quoted_table_name}.#{@reflection.klass.primary_key}" unless column_name
             options.merge!(:distinct => true)
           end
 
-          value = @reflection.klass.send(:with_scope, @scope) { @reflection.klass.count(column_name, options) }
+          value = scoped.count(column_name, options)
 
           limit  = @reflection.options[:limit]
           offset = @reflection.options[:offset]
@@ -469,9 +468,7 @@ module ActiveRecord
           ensure_owner_is_persisted!
 
           transaction do
-            with_scope(:create => @scope[:create].merge(scoped.scope_for_create)) do
-              build_record(attrs, &block)
-            end
+            scoped.scoping { build_record(attrs, &block) }
           end
         end
 
-- 
cgit v1.2.3


From 99a8d8430f9b819cd3e8cb3aab44cb04ea402532 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 3 Jan 2011 12:04:33 +0000
Subject: Create the association scope directly rather than going through
 with_scope

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 6defc465d8..4bd8a8e2d2 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -333,7 +333,7 @@ module ActiveRecord
 
       protected
 
-        def construct_find_scope
+        def finder_options
           {
             :conditions => construct_conditions,
             :select     => construct_select,
-- 
cgit v1.2.3


From 102255330bb6ffc5d0ca2888f206813445a29e44 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 5 Jan 2011 10:50:46 -0800
Subject: no need to send a symbol to send()

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 4bd8a8e2d2..bd27365b08 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -521,7 +521,7 @@ module ActiveRecord
 
         def include_in_memory?(record)
           if @reflection.is_a?(ActiveRecord::Reflection::ThroughReflection)
-            @owner.send(proxy_reflection.through_reflection.name.to_sym).any? do |source|
+            @owner.send(proxy_reflection.through_reflection.name).any? do |source|
               target = source.send(proxy_reflection.source_reflection.name)
               target.respond_to?(:include?) ? target.include?(record) : target == record
             end
-- 
cgit v1.2.3


From 770e6893b9f2aaaebe3de10576931dc7194451bc Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 6 Jan 2011 18:04:32 +0000
Subject: Construct an actual ActiveRecord::Relation object for the association
 scope, rather than a hash which is passed to apply_finder_options. This
 allows more flexibility in how the scope is created, for example because
 scope.where(a, b) and scope.where(a).where(b) mean different things.

---
 .../associations/association_collection.rb         | 27 ++++++++--------------
 1 file changed, 10 insertions(+), 17 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index bd27365b08..f33a9ce732 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -333,23 +333,16 @@ module ActiveRecord
 
       protected
 
-        def finder_options
-          {
-            :conditions => construct_conditions,
-            :select     => construct_select,
-            :readonly   => @reflection.options[:readonly],
-            :order      => @reflection.options[:order],
-            :limit      => @reflection.options[:limit],
-            :include    => @reflection.options[:include],
-            :joins      => @reflection.options[:joins],
-            :group      => @reflection.options[:group],
-            :having     => @reflection.options[:having],
-            :offset     => @reflection.options[:offset]
-          }
-        end
-
-        def construct_select
-          @reflection.options[:select] ||
+        def association_scope
+          options = @reflection.options.slice(:order, :limit, :joins, :group, :having, :offset)
+          super.apply_finder_options(options)
+        end
+
+        def select_value
+          super || uniq_select_value
+        end
+
+        def uniq_select_value
           @reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*"
         end
 
-- 
cgit v1.2.3


From 45d0d18baef2de739dae89bb7bc79826392bbde5 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Thu, 6 Jan 2011 19:32:05 +0000
Subject: Not really worth having the HasAssociation module for just a single
 method

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 --
 1 file changed, 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index f33a9ce732..daec8493ac 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -18,8 +18,6 @@ module ActiveRecord
     # If you need to work on all current children, new and existing records,
     # +load_target+ and the +loaded+ flag are your friends.
     class AssociationCollection < AssociationProxy #:nodoc:
-      include HasAssociation
-
       delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :to => :scoped
 
       def select(select = nil)
-- 
cgit v1.2.3


From 6055bbedaa4b7b4bb2377ac87147196eebb2edc1 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sun, 9 Jan 2011 16:34:23 +0000
Subject: Raise ActiveRecord::RecordNotSaved if an AssociationCollection fails
 to be replaced

---
 .../lib/active_record/associations/association_collection.rb        | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index daec8493ac..3d8a23fdca 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -314,7 +314,11 @@ module ActiveRecord
 
         transaction do
           delete(@target - other_array)
-          concat(other_array - @target)
+
+          unless concat(other_array - @target)
+            raise RecordNotSaved, "Failed to replace #{@reflection.name} because one or more of the "
+                                  "new records could not be saved."
+          end
         end
       end
 
-- 
cgit v1.2.3


From 8bee98fe3a3917f86d53f68b7cc11c4aafe5f011 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 12 Jan 2011 12:09:40 -0800
Subject: just use respond_to? and super rather than aliasing around methods

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 3d8a23fdca..6433cc3034 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -329,7 +329,7 @@ module ActiveRecord
         loaded? ? @target.include?(record) : exists?(record)
       end
 
-      def proxy_respond_to?(method, include_private = false)
+      def respond_to?(method, include_private = false)
         super || @reflection.klass.respond_to?(method, include_private)
       end
 
-- 
cgit v1.2.3


From e1beb7d2878cb55a045731b4a4c0c7a6046b3c09 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 12 Jan 2011 15:15:45 -0800
Subject: use array maths rather than *args

---
 .../lib/active_record/associations/association_collection.rb        | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 6433cc3034..da3800bf4a 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -358,8 +358,10 @@ module ActiveRecord
                     if i
                       @target.delete_at(i).tap do |t|
                         keys = ["id"] + t.changes.keys + (f.attribute_names - t.attribute_names)
-                        f.attributes.except(*keys).each do |k,v|
-                          t.send("#{k}=", v)
+                        # FIXME: this call to attributes causes many NoMethodErrors
+                        attributes = f.attributes
+                        (attributes.keys - keys).each do |k|
+                          t.send("#{k}=", attributes[k])
                         end
                       end
                     else
-- 
cgit v1.2.3


From 3165dca28c1db741994c3176e7b158a9f684e816 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Wed, 12 Jan 2011 18:01:02 -0800
Subject: include_in_memory? should check against @target list in case of new
 records. [#6257 state:resolved]

---
 activerecord/lib/active_record/associations/association_collection.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index da3800bf4a..e65ef2b768 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -518,10 +518,10 @@ module ActiveRecord
 
         def include_in_memory?(record)
           if @reflection.is_a?(ActiveRecord::Reflection::ThroughReflection)
-            @owner.send(proxy_reflection.through_reflection.name).any? do |source|
+            @owner.send(proxy_reflection.through_reflection.name).any? { |source|
               target = source.send(proxy_reflection.source_reflection.name)
               target.respond_to?(:include?) ? target.include?(record) : target == record
-            end
+            } || @target.include?(record)
           else
             @target.include?(record)
           end
-- 
cgit v1.2.3


From a0a69b045adeced5754251706a0401b75e7e03ec Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 14 Jan 2011 15:58:59 -0800
Subject: loaded? will not raise an AR::RecordNotFound exception, so move the
 rescue inside the conditional

---
 .../lib/active_record/associations/association_collection.rb      | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index e65ef2b768..bf40446871 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -350,8 +350,8 @@ module ActiveRecord
 
         def load_target
           if !@owner.new_record? || foreign_key_present?
-            begin
-              unless loaded?
+            unless loaded?
+              begin
                 if @target.is_a?(Array) && @target.any?
                   @target = find_target.map do |f|
                     i = @target.index(f)
@@ -371,9 +371,9 @@ module ActiveRecord
                 else
                   @target = find_target
                 end
+              rescue ActiveRecord::RecordNotFound
+                reset
               end
-            rescue ActiveRecord::RecordNotFound
-              reset
             end
           end
 
-- 
cgit v1.2.3


From 2afd6c75f18aee67fab85efef2b970572d459db3 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 14 Jan 2011 16:46:02 -0800
Subject: move complex logic to it's own method

---
 .../associations/association_collection.rb         | 34 ++++++++++++----------
 1 file changed, 19 insertions(+), 15 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index bf40446871..60a8d71d26 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -353,21 +353,7 @@ module ActiveRecord
             unless loaded?
               begin
                 if @target.is_a?(Array) && @target.any?
-                  @target = find_target.map do |f|
-                    i = @target.index(f)
-                    if i
-                      @target.delete_at(i).tap do |t|
-                        keys = ["id"] + t.changes.keys + (f.attribute_names - t.attribute_names)
-                        # FIXME: this call to attributes causes many NoMethodErrors
-                        attributes = f.attributes
-                        (attributes.keys - keys).each do |k|
-                          t.send("#{k}=", attributes[k])
-                        end
-                      end
-                    else
-                      f
-                    end
-                  end + @target
+                  @target = merge_target_lists(find_target, @target)
                 else
                   @target = find_target
                 end
@@ -450,6 +436,24 @@ module ActiveRecord
         end
 
       private
+        def merge_target_lists(loaded, existing)
+          loaded.map do |f|
+            i = existing.index(f)
+            if i
+              existing.delete_at(i).tap do |t|
+                keys = ["id"] + t.changes.keys + (f.attribute_names - t.attribute_names)
+                # FIXME: this call to attributes causes many NoMethodErrors
+                attributes = f.attributes
+                (attributes.keys - keys).each do |k|
+                  t.send("#{k}=", attributes[k])
+                end
+              end
+            else
+              f
+            end
+          end + existing
+        end
+
         # Do the relevant stuff to insert the given record into the association collection. The
         # force param specifies whether or not an exception should be raised on failure. The
         # validate param specifies whether validation should be performed (if force is false).
-- 
cgit v1.2.3


From 17687e4f969af363edf2854dd0c49914098dfda4 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 14 Jan 2011 16:47:31 -0800
Subject: @target is always a list, so stop doing is_a? checks

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 60a8d71d26..9490f5401c 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -352,7 +352,7 @@ module ActiveRecord
           if !@owner.new_record? || foreign_key_present?
             unless loaded?
               begin
-                if @target.is_a?(Array) && @target.any?
+                if @target.any?
                   @target = merge_target_lists(find_target, @target)
                 else
                   @target = find_target
-- 
cgit v1.2.3


From f5d2cb9cac41c1655ad915418bb968011300688a Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 14 Jan 2011 16:50:26 -0800
Subject: we should use [] instead of Array.new

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 9490f5401c..c5c505342e 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -401,7 +401,7 @@ module ActiveRecord
         end
 
         def reset_target!
-          @target = Array.new
+          @target = []
         end
 
         def reset_scopes_cache!
-- 
cgit v1.2.3


From f548054700e59ac77b2a20df96b9ebd98abed195 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 14 Jan 2011 16:54:02 -0800
Subject: we always have a target, so stop checking

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index c5c505342e..864d84d71a 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -363,7 +363,7 @@ module ActiveRecord
             end
           end
 
-          loaded if target
+          loaded
           target
         end
 
-- 
cgit v1.2.3


From 75e29e871e925cb7485cdbc283594d3d97871703 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 14 Jan 2011 16:57:14 -0800
Subject: only find_target can raise the exception, so isolate the rescue
 around that call

---
 .../active_record/associations/association_collection.rb   | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 864d84d71a..1254d68e9a 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -351,15 +351,19 @@ module ActiveRecord
         def load_target
           if !@owner.new_record? || foreign_key_present?
             unless loaded?
+              targets = []
+
               begin
-                if @target.any?
-                  @target = merge_target_lists(find_target, @target)
-                else
-                  @target = find_target
-                end
+                targets = find_target
               rescue ActiveRecord::RecordNotFound
                 reset
               end
+
+              if @target.any?
+                @target = merge_target_lists(targets, @target)
+              else
+                @target = targets
+              end
             end
           end
 
-- 
cgit v1.2.3


From b8ed2d5ddff3db22430de983ba072f57b8aa3c00 Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 14 Jan 2011 17:03:15 -0800
Subject: return early in case the left or right side lists are empty

---
 .../lib/active_record/associations/association_collection.rb     | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 1254d68e9a..bd77aa30c1 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -359,11 +359,7 @@ module ActiveRecord
                 reset
               end
 
-              if @target.any?
-                @target = merge_target_lists(targets, @target)
-              else
-                @target = targets
-              end
+              @target = merge_target_lists(targets, @target)
             end
           end
 
@@ -441,6 +437,9 @@ module ActiveRecord
 
       private
         def merge_target_lists(loaded, existing)
+          return loaded if existing.empty?
+          return existing if loaded.empty?
+
           loaded.map do |f|
             i = existing.index(f)
             if i
-- 
cgit v1.2.3


From 0aef847927ed2f9c3e3dca92207d9a62baa01b1a Mon Sep 17 00:00:00 2001
From: Aaron Patterson <aaron.patterson@gmail.com>
Date: Fri, 14 Jan 2011 17:08:27 -0800
Subject: push !loaded? conditional up

---
 .../associations/association_collection.rb             | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index bd77aa30c1..b75e02c66b 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -349,18 +349,16 @@ module ActiveRecord
         end
 
         def load_target
-          if !@owner.new_record? || foreign_key_present?
-            unless loaded?
-              targets = []
-
-              begin
-                targets = find_target
-              rescue ActiveRecord::RecordNotFound
-                reset
-              end
+          if (!@owner.new_record? || foreign_key_present?) && !loaded?
+            targets = []
 
-              @target = merge_target_lists(targets, @target)
+            begin
+              targets = find_target
+            rescue ActiveRecord::RecordNotFound
+              reset
             end
+
+            @target = merge_target_lists(targets, @target)
           end
 
           loaded
-- 
cgit v1.2.3


From e92c2ffd8eebf4b0b1f260cfe77c0d9690ba8fef Mon Sep 17 00:00:00 2001
From: Alexey Nayden <alexey.nayden@gmail.com>
Date: Thu, 13 Jan 2011 00:24:12 +0300
Subject: Nested attributes and in-memory changed values #first and #[]
 behaviour consistency fix

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index b75e02c66b..24fb49a65d 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -518,7 +518,7 @@ module ActiveRecord
 
         def fetch_first_or_last_using_find?(args)
           (args.first.kind_of?(Hash) && !args.first.empty?) || !(loaded? || @owner.new_record? || @reflection.options[:finder_sql] ||
-            @target.any? { |record| record.new_record? } || args.first.kind_of?(Integer))
+            @target.any? { |record| record.new_record? || record.changed? } || args.first.kind_of?(Integer))
         end
 
         def include_in_memory?(record)
-- 
cgit v1.2.3


From 63c73dd0214188dc91442db538e141e30ec3b1b9 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sun, 23 Jan 2011 21:29:36 +0000
Subject: We shouldn't be using scoped.scoping { ... } to build associated
 records, as this can affect validations/callbacks/etc inside the record
 itself [#6252 state:resolved]

---
 .../active_record/associations/association_collection.rb    | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 24fb49a65d..a37c3dd432 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -466,17 +466,14 @@ module ActiveRecord
           force ? record.save! : record.save(:validate => validate)
         end
 
-        def create_record(attrs, &block)
+        def create_record(attributes, &block)
           ensure_owner_is_persisted!
-
-          transaction do
-            scoped.scoping { build_record(attrs, &block) }
-          end
+          transaction { build_record(attributes, &block) }
         end
 
-        def build_record(attrs, &block)
-          attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash)
-          record = @reflection.build_association(attrs)
+        def build_record(attributes, &block)
+          attributes = scoped.scope_for_create.merge(attributes)
+          record = @reflection.build_association(attributes)
           add_record_to_target_with_callbacks(record, &block)
         end
 
-- 
cgit v1.2.3


From 15601c52e7c7094a6b7b54ef8acfc8299a4d6724 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 19:28:53 +0000
Subject: =?UTF-8?q?Let's=20be=20less=20blas=C3=A9=20about=20method=20visib?=
 =?UTF-8?q?ility=20on=20association=20proxies?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 .../associations/association_collection.rb         | 75 +++++++++++-----------
 1 file changed, 38 insertions(+), 37 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index a37c3dd432..3c939b0ef0 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -333,6 +333,24 @@ module ActiveRecord
         super || @reflection.klass.respond_to?(method, include_private)
       end
 
+      def method_missing(method, *args, &block)
+        match = DynamicFinderMatch.match(method)
+        if match && match.creator?
+          attributes = match.attribute_names
+          return send(:"find_by_#{attributes.join('_and_')}", *args) || create(Hash[attributes.zip(args)])
+        end
+
+        if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method))
+          super
+        elsif @reflection.klass.scopes[method]
+          @_scopes_cache ||= {}
+          @_scopes_cache[method] ||= {}
+          @_scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args)
+        else
+          scoped.readonly(nil).send(method, *args, &block)
+        end
+      end
+
       protected
 
         def association_scope
@@ -340,14 +358,6 @@ module ActiveRecord
           super.apply_finder_options(options)
         end
 
-        def select_value
-          super || uniq_select_value
-        end
-
-        def uniq_select_value
-          @reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*"
-        end
-
         def load_target
           if (!@owner.new_record? || foreign_key_present?) && !loaded?
             targets = []
@@ -365,22 +375,28 @@ module ActiveRecord
           target
         end
 
-        def method_missing(method, *args, &block)
-          match = DynamicFinderMatch.match(method)
-          if match && match.creator?
-            attributes = match.attribute_names
-            return send(:"find_by_#{attributes.join('_and_')}", *args) || create(Hash[attributes.zip(args)])
-          end
-
-          if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method))
-            super
-          elsif @reflection.klass.scopes[method]
-            @_scopes_cache ||= {}
-            @_scopes_cache[method] ||= {}
-            @_scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args)
+        def add_record_to_target_with_callbacks(record)
+          callback(:before_add, record)
+          yield(record) if block_given?
+          @target ||= [] unless loaded?
+          if @reflection.options[:uniq] && index = @target.index(record)
+            @target[index] = record
           else
-            scoped.readonly(nil).send(method, *args, &block)
+            @target << record
           end
+          callback(:after_add, record)
+          set_inverse_instance(record)
+          record
+        end
+
+      private
+
+        def select_value
+          super || uniq_select_value
+        end
+
+        def uniq_select_value
+          @reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*"
         end
 
         def custom_counter_sql
@@ -419,21 +435,6 @@ module ActiveRecord
           records
         end
 
-        def add_record_to_target_with_callbacks(record)
-          callback(:before_add, record)
-          yield(record) if block_given?
-          @target ||= [] unless loaded?
-          if @reflection.options[:uniq] && index = @target.index(record)
-            @target[index] = record
-          else
-            @target << record
-          end
-          callback(:after_add, record)
-          set_inverse_instance(record)
-          record
-        end
-
-      private
         def merge_target_lists(loaded, existing)
           return loaded if existing.empty?
           return existing if loaded.empty?
-- 
cgit v1.2.3


From de05e2fb15ee4fd521aae202eb4517ae05114c28 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 20:47:06 +0000
Subject: Abstract load_target conditional logic

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 3c939b0ef0..95526be82f 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -359,7 +359,7 @@ module ActiveRecord
         end
 
         def load_target
-          if (!@owner.new_record? || foreign_key_present?) && !loaded?
+          if find_target?
             targets = []
 
             begin
-- 
cgit v1.2.3


From aa86420be24d7df9c07379bcf6f33904d0d41adc Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 20:57:11 +0000
Subject: Rename AssociationProxy#loaded to loaded! as it mutates the
 association

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 95526be82f..50c69abefb 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -371,7 +371,7 @@ module ActiveRecord
             @target = merge_target_lists(targets, @target)
           end
 
-          loaded
+          loaded!
           target
         end
 
-- 
cgit v1.2.3


From db503c4142d85cc1d2d511873f8eb7e7250bbedb Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 20:58:43 +0000
Subject: load_target returns the target

---
 activerecord/lib/active_record/associations/association_collection.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 50c69abefb..c112f2b5d0 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -22,8 +22,7 @@ module ActiveRecord
 
       def select(select = nil)
         if block_given?
-          load_target
-          @target.select.each { |e| yield e }
+          load_target.select.each { |e| yield e }
         else
           scoped.select(select)
         end
-- 
cgit v1.2.3


From 1e7cf6c7e9aede02e417885847346c225d309302 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 22:11:20 +0000
Subject: Try to make fetch_first_or_last_using_find? more readable

---
 .../associations/association_collection.rb         | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index c112f2b5d0..0ab2b4fb9d 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -513,9 +513,27 @@ module ActiveRecord
           end
         end
 
+        # Should we deal with assoc.first or assoc.last by issuing an independent query to
+        # the database, or by getting the target, and then taking the first/last item from that?
+        #
+        # If the args is just a non-empty options hash, go to the database.
+        #
+        # Otherwise, go to the database only if none of the following are true:
+        #   * target already loaded
+        #   * owner is new record
+        #   * custom :finder_sql exists
+        #   * target contains new or changed record(s)
+        #   * the first arg is an integer (which indicates the number of records to be returned)
         def fetch_first_or_last_using_find?(args)
-          (args.first.kind_of?(Hash) && !args.first.empty?) || !(loaded? || @owner.new_record? || @reflection.options[:finder_sql] ||
-            @target.any? { |record| record.new_record? || record.changed? } || args.first.kind_of?(Integer))
+          if args.first.kind_of?(Hash) && !args.first.empty?
+            true
+          else
+            !(loaded? ||
+              @owner.new_record? ||
+              @reflection.options[:finder_sql] ||
+              @target.any? { |record| record.new_record? || record.changed? } ||
+              args.first.kind_of?(Integer))
+          end
         end
 
         def include_in_memory?(record)
-- 
cgit v1.2.3


From 1a4bbaf106f09c8a67c7e4da109a46449c06374f Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 22:14:32 +0000
Subject: Use scoped.first and scoped.last instead of find(:first, ...) and
 find(:last, ...)

---
 activerecord/lib/active_record/associations/association_collection.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 0ab2b4fb9d..2b0ab32f34 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -39,7 +39,7 @@ module ActiveRecord
       # Fetches the first one using SQL if possible.
       def first(*args)
         if fetch_first_or_last_using_find?(args)
-          find(:first, *args)
+          scoped.first(*args)
         else
           load_target unless loaded?
           args.shift if args.first.kind_of?(Hash) && args.first.empty?
@@ -50,7 +50,7 @@ module ActiveRecord
       # Fetches the last one using SQL if possible.
       def last(*args)
         if fetch_first_or_last_using_find?(args)
-          find(:last, *args)
+          scoped.last(*args)
         else
           load_target unless loaded?
           @target.last(*args)
-- 
cgit v1.2.3


From e8d7152a89f16a487a3d67d16a63bf065183b405 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 22:15:38 +0000
Subject: Use scoped.find directly rather than having a find_by_sql method

---
 .../lib/active_record/associations/association_collection.rb        | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 2b0ab32f34..5b9f2f99db 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -32,7 +32,7 @@ module ActiveRecord
         if @reflection.options[:finder_sql]
           find_by_scan(*args)
         else
-          find_by_sql(*args)
+          scoped.find(*args)
         end
       end
 
@@ -560,10 +560,6 @@ module ActiveRecord
             load_target.select { |r| ids.include?(r.id) }
           end
         end
-
-        def find_by_sql(*args)
-          scoped.find(*args)
-        end
     end
   end
 end
-- 
cgit v1.2.3


From b7bcc7e1905062f330e0a84b93a1ecacfea2a4c0 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 22:25:32 +0000
Subject: DRY up first/last and hence make last benefit from the bugfix in
 first

---
 .../associations/association_collection.rb         | 28 ++++++++++------------
 1 file changed, 13 insertions(+), 15 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 5b9f2f99db..a59de18313 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -36,25 +36,12 @@ module ActiveRecord
         end
       end
 
-      # Fetches the first one using SQL if possible.
       def first(*args)
-        if fetch_first_or_last_using_find?(args)
-          scoped.first(*args)
-        else
-          load_target unless loaded?
-          args.shift if args.first.kind_of?(Hash) && args.first.empty?
-          @target.first(*args)
-        end
+        first_or_last(:first, *args)
       end
 
-      # Fetches the last one using SQL if possible.
       def last(*args)
-        if fetch_first_or_last_using_find?(args)
-          scoped.last(*args)
-        else
-          load_target unless loaded?
-          @target.last(*args)
-        end
+        first_or_last(:last, *args)
       end
 
       def to_ary
@@ -560,6 +547,17 @@ module ActiveRecord
             load_target.select { |r| ids.include?(r.id) }
           end
         end
+
+        # Fetches the first/last using SQL if possible, otherwise from the target array.
+        def first_or_last(type, *args)
+          if fetch_first_or_last_using_find?(args)
+            scoped.send(type, *args)
+          else
+            load_target unless loaded?
+            args.shift if args.first.kind_of?(Hash) && args.first.empty?
+            @target.send(type, *args)
+          end
+        end
     end
   end
 end
-- 
cgit v1.2.3


From 47309826e4d7d402c248ff507c4c4ef7a867449a Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 22:27:48 +0000
Subject: load_target will return the target. it also will not load if loaded?
 is true.

---
 activerecord/lib/active_record/associations/association_collection.rb | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index a59de18313..07b5b07195 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -553,9 +553,8 @@ module ActiveRecord
           if fetch_first_or_last_using_find?(args)
             scoped.send(type, *args)
           else
-            load_target unless loaded?
             args.shift if args.first.kind_of?(Hash) && args.first.empty?
-            @target.send(type, *args)
+            load_target.send(type, *args)
           end
         end
     end
-- 
cgit v1.2.3


From 5dd3dadcdd17b1bb367a127c10ae7aee4916e15c Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 22:34:16 +0000
Subject: target is always an array

---
 .../lib/active_record/associations/association_collection.rb         | 5 -----
 1 file changed, 5 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 07b5b07195..76cdc6edd5 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -46,11 +46,6 @@ module ActiveRecord
 
       def to_ary
         load_target
-        if @target.is_a?(Array)
-          @target
-        else
-          Array.wrap(@target)
-        end
       end
       alias_method :to_a, :to_ary
 
-- 
cgit v1.2.3


From fdee153ff8cde2d44d751f8f961e2df13a80cd5c Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 22:41:34 +0000
Subject: Get rid of separate reset_target! and reset_scopes_cache! methods

---
 .../associations/association_collection.rb         | 43 +++++++++-------------
 1 file changed, 17 insertions(+), 26 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 76cdc6edd5..586965bb3b 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -50,9 +50,9 @@ module ActiveRecord
       alias_method :to_a, :to_ary
 
       def reset
-        reset_target!
-        reset_scopes_cache!
-        @loaded = false
+        @_scopes_cache = {}
+        @loaded        = false
+        @target        = []
       end
 
       def build(attributes = {}, &block)
@@ -106,10 +106,20 @@ module ActiveRecord
       #
       # See delete for more info.
       def delete_all
-        load_target
-        delete(@target)
-        reset_target!
-        reset_scopes_cache!
+        delete(load_target).tap do
+          reset
+          loaded!
+        end
+      end
+
+      # Destroy all the records from this association.
+      #
+      # See destroy for more info.
+      def destroy_all
+        destroy(load_target).tap do
+          reset
+          loaded!
+        end
       end
 
       # Calculate sum using SQL, not Enumerable
@@ -194,17 +204,6 @@ module ActiveRecord
         self
       end
 
-      # Destroy all the records from this association.
-      #
-      # See destroy for more info.
-      def destroy_all
-        load_target
-        destroy(@target).tap do
-          reset_target!
-          reset_scopes_cache!
-        end
-      end
-
       def create(attrs = {})
         if attrs.is_a?(Array)
           attrs.collect { |attr| create(attr) }
@@ -395,14 +394,6 @@ module ActiveRecord
           interpolate_sql(@reflection.options[:finder_sql])
         end
 
-        def reset_target!
-          @target = []
-        end
-
-        def reset_scopes_cache!
-          @_scopes_cache = {}
-        end
-
         def find_target
           records =
             if @reflection.options[:finder_sql]
-- 
cgit v1.2.3


From ca7785847eafaf687b10c7757dc034d08d1e6ba8 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 22:47:18 +0000
Subject: Condense first_or_last a bit more

---
 .../lib/active_record/associations/association_collection.rb | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 586965bb3b..7151f53fdb 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -498,7 +498,7 @@ module ActiveRecord
         #   * target contains new or changed record(s)
         #   * the first arg is an integer (which indicates the number of records to be returned)
         def fetch_first_or_last_using_find?(args)
-          if args.first.kind_of?(Hash) && !args.first.empty?
+          if args.first.is_a?(Hash)
             true
           else
             !(loaded? ||
@@ -536,12 +536,10 @@ module ActiveRecord
 
         # Fetches the first/last using SQL if possible, otherwise from the target array.
         def first_or_last(type, *args)
-          if fetch_first_or_last_using_find?(args)
-            scoped.send(type, *args)
-          else
-            args.shift if args.first.kind_of?(Hash) && args.first.empty?
-            load_target.send(type, *args)
-          end
+          args.shift if args.first.is_a?(Hash) && args.first.empty?
+
+          collection = fetch_first_or_last_using_find?(args) ? scoped : load_target
+          collection.send(type, *args)
         end
     end
   end
-- 
cgit v1.2.3


From 140b269fb7aee85e6da32da5305f771099d35af5 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 23:01:30 +0000
Subject: Call sum on the scope directly, rather than relying on method_missing
 and calculate

---
 activerecord/lib/active_record/associations/association_collection.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 7151f53fdb..7fd15e4bd6 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -125,9 +125,9 @@ module ActiveRecord
       # Calculate sum using SQL, not Enumerable
       def sum(*args)
         if block_given?
-          calculate(:sum, *args) { |*block_args| yield(*block_args) }
+          scoped.sum(*args) { |*block_args| yield(*block_args) }
         else
-          calculate(:sum, *args)
+          scoped.sum(*args)
         end
       end
 
-- 
cgit v1.2.3


From 1da1ac159f9391b9a053a0fb0d426499b9edd5b7 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 23:11:38 +0000
Subject: Just use primary_key here, AR::Relation will resolve the ambiguity
 before it is converted to SQL

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 7fd15e4bd6..7504773639 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -146,7 +146,7 @@ module ActiveRecord
         else
           if @reflection.options[:uniq]
             # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
-            column_name = "#{@reflection.quoted_table_name}.#{@reflection.klass.primary_key}" unless column_name
+            column_name ||= @reflection.klass.primary_key
             options.merge!(:distinct => true)
           end
 
-- 
cgit v1.2.3


From 88df88095c82cde53501abe2a44f6c1f66c272b4 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 23:30:11 +0000
Subject: AssociationCollection#to_ary should definitely dup the target! Also
 changed #replace which was previously incorrect, but the test passed due to
 the fact that to_a was not duping.

---
 .../lib/active_record/associations/association_collection.rb        | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 7504773639..f2997b9db3 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -45,7 +45,7 @@ module ActiveRecord
       end
 
       def to_ary
-        load_target
+        load_target.dup
       end
       alias_method :to_a, :to_ary
 
@@ -289,13 +289,13 @@ module ActiveRecord
       # This will perform a diff and delete/add only records that have changed.
       def replace(other_array)
         other_array.each { |val| raise_on_type_mismatch(val) }
-
-        load_target
+        original_target = load_target.dup
 
         transaction do
           delete(@target - other_array)
 
           unless concat(other_array - @target)
+            @target = original_target
             raise RecordNotSaved, "Failed to replace #{@reflection.name} because one or more of the "
                                   "new records could not be saved."
           end
-- 
cgit v1.2.3


From 2e24cf7cc2392c8fa252ef3b4831a4516ae852d6 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 23:43:44 +0000
Subject: AssociationCollection#clear can basically just use #delete_all,
 except it should return self.

---
 .../associations/association_collection.rb           | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index f2997b9db3..8e63159190 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -112,6 +112,13 @@ module ActiveRecord
         end
       end
 
+      # Identical to delete_all, except that the return value is the association (for chaining)
+      # rather than the records which have been removed.
+      def clear
+        delete_all
+        self
+      end
+
       # Destroy all the records from this association.
       #
       # See destroy for more info.
@@ -191,19 +198,6 @@ module ActiveRecord
         load_target
       end
 
-      # Removes all records from this association.  Returns +self+ so method calls may be chained.
-      def clear
-        unless length.zero? # forces load_target if it hasn't happened already
-          if @reflection.options[:dependent] == :destroy
-            destroy_all
-          else
-            delete_all
-          end
-        end
-
-        self
-      end
-
       def create(attrs = {})
         if attrs.is_a?(Array)
           attrs.collect { |attr| create(attr) }
-- 
cgit v1.2.3


From 0645fd2c80c05109f148e28cdf78d636406cb6d7 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 23:50:04 +0000
Subject: Don't use method_missing when we don't have to

---
 activerecord/lib/active_record/associations/association_collection.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 8e63159190..b1a8d772f3 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -257,7 +257,7 @@ module ActiveRecord
 
       def any?
         if block_given?
-          method_missing(:any?) { |*block_args| yield(*block_args) }
+          load_target.any? { |*block_args| yield(*block_args) }
         else
           !empty?
         end
@@ -266,7 +266,7 @@ module ActiveRecord
       # Returns true if the collection has more than 1 record. Equivalent to collection.size > 1.
       def many?
         if block_given?
-          method_missing(:many?) { |*block_args| yield(*block_args) }
+          load_target.many? { |*block_args| yield(*block_args) }
         else
           size > 1
         end
-- 
cgit v1.2.3


From 59d54c3ebaa08c3298a36d1655a2492074ed8a87 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 24 Jan 2011 23:55:14 +0000
Subject: Make AssociationCollection#include? a bit more readable

---
 .../active_record/associations/association_collection.rb   | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index b1a8d772f3..2811f53424 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -297,10 +297,16 @@ module ActiveRecord
       end
 
       def include?(record)
-        return false unless record.is_a?(@reflection.klass)
-        return include_in_memory?(record) if record.new_record?
-        load_target if @reflection.options[:finder_sql] && !loaded?
-        loaded? ? @target.include?(record) : exists?(record)
+        if record.is_a?(@reflection.klass)
+          if record.new_record?
+            include_in_memory?(record)
+          else
+            load_target if @reflection.options[:finder_sql]
+            loaded? ? @target.include?(record) : scoped.exists?(record)
+          end
+        else
+          false
+        end
       end
 
       def respond_to?(method, include_private = false)
-- 
cgit v1.2.3


From d55406d2e991056b08f69eb68bcf9b17da807b6c Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sun, 30 Jan 2011 19:07:08 +0000
Subject: Make record.association.destroy(*records) on habtm and hm:t only
 delete records in the join table. This is to make the destroy method more
 consistent across the different types of associations. For more details see
 the CHANGELOG entry.

---
 .../lib/active_record/associations/association_collection.rb      | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 2811f53424..9bd59132f5 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -192,7 +192,7 @@ module ActiveRecord
       def destroy(*records)
         records = find(records) if records.any? {|record| record.kind_of?(Fixnum) || record.kind_of?(String)}
         remove_records(records) do |_records, old_records|
-          old_records.each { |record| record.destroy }
+          delete_records(old_records, :destroy) if old_records.any?
         end
 
         load_target
@@ -462,6 +462,12 @@ module ActiveRecord
           end
         end
 
+        # Delete the given records from the association, using one of the methods :destroy,
+        # :delete_all or :nullify. The default method used is given by the :dependent option.
+        def delete_records(records, method = @reflection.options[:dependent])
+          raise NotImplementedError
+        end
+
         def callback(method, record)
           callbacks_for(method).each do |callback|
             case callback
-- 
cgit v1.2.3


From d9870d92f733f0ef4452a0b6df338ed9dbcc05b3 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sat, 5 Feb 2011 13:18:27 +0000
Subject: This string should continue

---
 activerecord/lib/active_record/associations/association_collection.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 9bd59132f5..acb9fe7ff8 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -290,7 +290,7 @@ module ActiveRecord
 
           unless concat(other_array - @target)
             @target = original_target
-            raise RecordNotSaved, "Failed to replace #{@reflection.name} because one or more of the "
+            raise RecordNotSaved, "Failed to replace #{@reflection.name} because one or more of the " \
                                   "new records could not be saved."
           end
         end
-- 
cgit v1.2.3


From e62b57647258fad34129975c5a264d19af2dbbe8 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Sun, 6 Feb 2011 22:14:16 +0000
Subject: Refactor the implementations of AssociatioCollection#delete and
 #destroy to be more consistent with each other, and to stop passing blocks
 around, thus making the execution easier to follow.

---
 .../associations/association_collection.rb         | 26 +++++++++-------------
 1 file changed, 11 insertions(+), 15 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index acb9fe7ff8..3e3237c348 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -178,10 +178,7 @@ 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)
-        remove_records(records) do |_records, old_records|
-          delete_records(old_records) if old_records.any?
-          _records.each { |record| @target.delete(record) }
-        end
+        delete_or_destroy(records, @reflection.options[:dependent])
       end
 
       # Destroy +records+ and remove them from this association calling
@@ -190,12 +187,8 @@ module ActiveRecord
       # Note that this method will _always_ remove records from the database
       # ignoring the +:dependent+ option.
       def destroy(*records)
-        records = find(records) if records.any? {|record| record.kind_of?(Fixnum) || record.kind_of?(String)}
-        remove_records(records) do |_records, old_records|
-          delete_records(old_records, :destroy) if old_records.any?
-        end
-
-        load_target
+        records = find(records) if records.any? { |record| record.kind_of?(Fixnum) || record.kind_of?(String) }
+        delete_or_destroy(records, :destroy)
       end
 
       def create(attrs = {})
@@ -450,21 +443,24 @@ module ActiveRecord
           add_record_to_target_with_callbacks(record, &block)
         end
 
-        def remove_records(*records)
+        def delete_or_destroy(records, method)
           records = records.flatten
           records.each { |record| raise_on_type_mismatch(record) }
+          existing_records = records.reject { |r| r.new_record? }
 
           transaction do
             records.each { |record| callback(:before_remove, record) }
-            old_records = records.reject { |r| r.new_record? }
-            yield(records, old_records)
+
+            delete_records(existing_records, method) if existing_records.any?
+            records.each { |record| @target.delete(record) }
+
             records.each { |record| callback(:after_remove, record) }
           end
         end
 
         # Delete the given records from the association, using one of the methods :destroy,
-        # :delete_all or :nullify. The default method used is given by the :dependent option.
-        def delete_records(records, method = @reflection.options[:dependent])
+        # :delete_all or :nullify (or nil, in which case a default is used).
+        def delete_records(records, method)
           raise NotImplementedError
         end
 
-- 
cgit v1.2.3


From a7e19b30ca71f62af516675023659be061b2b70a Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Fri, 11 Feb 2011 22:22:19 +0000
Subject: Add interpolation of association conditions back in, in the form of
 proc { ... } rather than instance_eval-ing strings

---
 .../lib/active_record/associations/association_collection.rb      | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 3e3237c348..8e006e4d9d 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -374,17 +374,15 @@ module ActiveRecord
 
         def custom_counter_sql
           if @reflection.options[:counter_sql]
-            counter_sql = @reflection.options[:counter_sql]
+            interpolate(@reflection.options[:counter_sql])
           else
             # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */
-            counter_sql = @reflection.options[:finder_sql].sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
+            interpolate(@reflection.options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
           end
-
-          interpolate_sql(counter_sql)
         end
 
         def custom_finder_sql
-          interpolate_sql(@reflection.options[:finder_sql])
+          interpolate(@reflection.options[:finder_sql])
         end
 
         def find_target
-- 
cgit v1.2.3


From 7ce7ae0c8bd94e7f96c1448183b83da85ef91edb Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 14 Feb 2011 00:14:16 +0000
Subject: Get rid of AssociationCollection#save_record

---
 .../associations/association_collection.rb          | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 8e006e4d9d..4ead67b282 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -197,16 +197,15 @@ module ActiveRecord
         else
           create_record(attrs) do |record|
             yield(record) if block_given?
-            insert_record(record, false)
+            insert_record(record)
           end
         end
       end
 
-      def create!(attrs = {})
-        create_record(attrs) do |record|
-          yield(record) if block_given?
-          insert_record(record, true)
-        end
+      def create!(attrs = {}, &block)
+        record = create(attrs, &block)
+        Array.wrap(record).each(&:save!)
+        record
       end
 
       # Returns the size of the collection by executing a SELECT COUNT(*)
@@ -419,17 +418,11 @@ module ActiveRecord
           end + existing
         end
 
-        # Do the relevant stuff to insert the given record into the association collection. The
-        # force param specifies whether or not an exception should be raised on failure. The
-        # validate param specifies whether validation should be performed (if force is false).
-        def insert_record(record, force = true, validate = true)
+        # Do the relevant stuff to insert the given record into the association collection.
+        def insert_record(record, validate = true)
           raise NotImplementedError
         end
 
-        def save_record(record, force, validate)
-          force ? record.save! : record.save(:validate => validate)
-        end
-
         def create_record(attributes, &block)
           ensure_owner_is_persisted!
           transaction { build_record(attributes, &block) }
-- 
cgit v1.2.3


From b93d2189c4cf370927b9796e0008034a5268214f Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 14 Feb 2011 00:34:54 +0000
Subject: Get rid of create_record as it is not only used in one place

---
 .../active_record/associations/association_collection.rb  | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 4ead67b282..ff4c2b51ac 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -195,9 +195,13 @@ module ActiveRecord
         if attrs.is_a?(Array)
           attrs.collect { |attr| create(attr) }
         else
-          create_record(attrs) do |record|
-            yield(record) if block_given?
-            insert_record(record)
+          ensure_owner_is_persisted!
+
+          transaction do
+            build_record(attrs) do |record|
+              yield(record) if block_given?
+              insert_record(record)
+            end
           end
         end
       end
@@ -423,11 +427,6 @@ module ActiveRecord
           raise NotImplementedError
         end
 
-        def create_record(attributes, &block)
-          ensure_owner_is_persisted!
-          transaction { build_record(attributes, &block) }
-        end
-
         def build_record(attributes, &block)
           attributes = scoped.scope_for_create.merge(attributes)
           record = @reflection.build_association(attributes)
-- 
cgit v1.2.3


From 686418c65754701b9cdc213de4b6905f465fdc60 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 14 Feb 2011 00:44:10 +0000
Subject: Move create and create! next to build

---
 .../associations/association_collection.rb         | 42 +++++++++++-----------
 1 file changed, 21 insertions(+), 21 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index ff4c2b51ac..e391d9cce6 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -66,6 +66,27 @@ module ActiveRecord
         end
       end
 
+      def create(attrs = {})
+        if attrs.is_a?(Array)
+          attrs.collect { |attr| create(attr) }
+        else
+          ensure_owner_is_persisted!
+
+          transaction do
+            build_record(attrs) do |record|
+              yield(record) if block_given?
+              insert_record(record)
+            end
+          end
+        end
+      end
+
+      def create!(attrs = {}, &block)
+        record = create(attrs, &block)
+        Array.wrap(record).each(&:save!)
+        record
+      end
+
       # Add +records+ to this association.  Returns +self+ so method calls may be chained.
       # Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically.
       def <<(*records)
@@ -191,27 +212,6 @@ module ActiveRecord
         delete_or_destroy(records, :destroy)
       end
 
-      def create(attrs = {})
-        if attrs.is_a?(Array)
-          attrs.collect { |attr| create(attr) }
-        else
-          ensure_owner_is_persisted!
-
-          transaction do
-            build_record(attrs) do |record|
-              yield(record) if block_given?
-              insert_record(record)
-            end
-          end
-        end
-      end
-
-      def create!(attrs = {}, &block)
-        record = create(attrs, &block)
-        Array.wrap(record).each(&:save!)
-        record
-      end
-
       # Returns the size of the collection by executing a SELECT COUNT(*)
       # query if the collection hasn't been loaded, and calling
       # <tt>collection.size</tt> if it has.
-- 
cgit v1.2.3


From 641a3068ea11c20e09358d6911404a265da32a9f Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 14 Feb 2011 00:58:34 +0000
Subject: Don't pass the block through build_record

---
 .../associations/association_collection.rb             | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index e391d9cce6..a850cf39cd 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -59,21 +59,21 @@ module ActiveRecord
         if attributes.is_a?(Array)
           attributes.collect { |attr| build(attr, &block) }
         else
-          build_record(attributes) do |record|
-            block.call(record) if block_given?
+          add_record_to_target_with_callbacks(build_record(attributes)) do |record|
+            yield(record) if block_given?
             set_owner_attributes(record)
           end
         end
       end
 
-      def create(attrs = {})
-        if attrs.is_a?(Array)
-          attrs.collect { |attr| create(attr) }
+      def create(attributes = {})
+        if attributes.is_a?(Array)
+          attributes.collect { |attr| create(attr) }
         else
           ensure_owner_is_persisted!
 
           transaction do
-            build_record(attrs) do |record|
+            add_record_to_target_with_callbacks(build_record(attributes)) do |record|
               yield(record) if block_given?
               insert_record(record)
             end
@@ -427,10 +427,8 @@ module ActiveRecord
           raise NotImplementedError
         end
 
-        def build_record(attributes, &block)
-          attributes = scoped.scope_for_create.merge(attributes)
-          record = @reflection.build_association(attributes)
-          add_record_to_target_with_callbacks(record, &block)
+        def build_record(attributes)
+          @reflection.build_association(scoped.scope_for_create.merge(attributes))
         end
 
         def delete_or_destroy(records, method)
-- 
cgit v1.2.3


From db03308451497dd6c7fa6e531b378f63f0781e7c Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 14 Feb 2011 01:02:42 +0000
Subject: Rename add_record_to_target_with_callbacks to add_to_target

---
 .../lib/active_record/associations/association_collection.rb      | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index a850cf39cd..b8187b1c6a 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -59,7 +59,7 @@ module ActiveRecord
         if attributes.is_a?(Array)
           attributes.collect { |attr| build(attr, &block) }
         else
-          add_record_to_target_with_callbacks(build_record(attributes)) do |record|
+          add_to_target(build_record(attributes)) do |record|
             yield(record) if block_given?
             set_owner_attributes(record)
           end
@@ -73,7 +73,7 @@ module ActiveRecord
           ensure_owner_is_persisted!
 
           transaction do
-            add_record_to_target_with_callbacks(build_record(attributes)) do |record|
+            add_to_target(build_record(attributes)) do |record|
               yield(record) if block_given?
               insert_record(record)
             end
@@ -96,7 +96,7 @@ module ActiveRecord
         transaction do
           records.flatten.each do |record|
             raise_on_type_mismatch(record)
-            add_record_to_target_with_callbacks(record) do |r|
+            add_to_target(record) do |r|
               result &&= insert_record(record) unless @owner.new_record?
             end
           end
@@ -351,7 +351,7 @@ module ActiveRecord
           target
         end
 
-        def add_record_to_target_with_callbacks(record)
+        def add_to_target(record)
           callback(:before_add, record)
           yield(record) if block_given?
           @target ||= [] unless loaded?
-- 
cgit v1.2.3


From c9b685e681ea5851c2baaaec780fcc9c6a9e2775 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 14 Feb 2011 01:07:48 +0000
Subject: @target should always be an array

---
 activerecord/lib/active_record/associations/association_collection.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index b8187b1c6a..8b5e600c96 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -354,12 +354,13 @@ module ActiveRecord
         def add_to_target(record)
           callback(:before_add, record)
           yield(record) if block_given?
-          @target ||= [] unless loaded?
+
           if @reflection.options[:uniq] && index = @target.index(record)
             @target[index] = record
           else
             @target << record
           end
+
           callback(:after_add, record)
           set_inverse_instance(record)
           record
-- 
cgit v1.2.3


From 5d6d669bfe1e480dd4d0cc5042b7faba4b469846 Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 14 Feb 2011 01:19:42 +0000
Subject: Inline ensure_owner_is_persisted! as it is only called from one place

---
 .../lib/active_record/associations/association_collection.rb | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 8b5e600c96..888ebdf7af 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -67,11 +67,13 @@ module ActiveRecord
       end
 
       def create(attributes = {})
+        unless @owner.persisted?
+          raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
+        end
+
         if attributes.is_a?(Array)
           attributes.collect { |attr| create(attr) }
         else
-          ensure_owner_is_persisted!
-
           transaction do
             add_to_target(build_record(attributes)) do |record|
               yield(record) if block_given?
@@ -471,12 +473,6 @@ module ActiveRecord
           @owner.class.send(full_callback_name.to_sym) || []
         end
 
-        def ensure_owner_is_persisted!
-          unless @owner.persisted?
-            raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
-          end
-        end
-
         # Should we deal with assoc.first or assoc.last by issuing an independent query to
         # the database, or by getting the target, and then taking the first/last item from that?
         #
-- 
cgit v1.2.3


From b9ea751d0e56bd00d341766977a607ed3f7ddd0f Mon Sep 17 00:00:00 2001
From: Jon Leighton <j@jonathanleighton.com>
Date: Mon, 14 Feb 2011 01:40:00 +0000
Subject: Add a transaction wrapper in add_to_target. This means that #build
 will now also use a transaction. IMO this is reasonable given that the
 before_add and after_add callbacks might do anything, and this great
 consistency allows us to abstract out the duplicate code from #build and
 #create.

---
 .../associations/association_collection.rb         | 54 +++++++++++-----------
 1 file changed, 27 insertions(+), 27 deletions(-)

(limited to 'activerecord/lib/active_record/associations/association_collection.rb')

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 888ebdf7af..ca350f51c9 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -56,31 +56,15 @@ module ActiveRecord
       end
 
       def build(attributes = {}, &block)
-        if attributes.is_a?(Array)
-          attributes.collect { |attr| build(attr, &block) }
-        else
-          add_to_target(build_record(attributes)) do |record|
-            yield(record) if block_given?
-            set_owner_attributes(record)
-          end
-        end
+        build_or_create(attributes, :build, &block)
       end
 
-      def create(attributes = {})
+      def create(attributes = {}, &block)
         unless @owner.persisted?
           raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
         end
 
-        if attributes.is_a?(Array)
-          attributes.collect { |attr| create(attr) }
-        else
-          transaction do
-            add_to_target(build_record(attributes)) do |record|
-              yield(record) if block_given?
-              insert_record(record)
-            end
-          end
-        end
+        build_or_create(attributes, :create, &block)
       end
 
       def create!(attrs = {}, &block)
@@ -354,17 +338,20 @@ module ActiveRecord
         end
 
         def add_to_target(record)
-          callback(:before_add, record)
-          yield(record) if block_given?
+          transaction do
+            callback(:before_add, record)
+            yield(record) if block_given?
 
-          if @reflection.options[:uniq] && index = @target.index(record)
-            @target[index] = record
-          else
-            @target << record
+            if @reflection.options[:uniq] && index = @target.index(record)
+              @target[index] = record
+            else
+              @target << record
+            end
+
+            callback(:after_add, record)
+            set_inverse_instance(record)
           end
 
-          callback(:after_add, record)
-          set_inverse_instance(record)
           record
         end
 
@@ -425,6 +412,19 @@ module ActiveRecord
           end + existing
         end
 
+        def build_or_create(attributes, method)
+          records = Array.wrap(attributes).map do |attrs|
+            record = build_record(attrs)
+
+            add_to_target(record) do
+              yield(record) if block_given?
+              insert_record(record) if method == :create
+            end
+          end
+
+          attributes.is_a?(Array) ? records : records.first
+        end
+
         # Do the relevant stuff to insert the given record into the association collection.
         def insert_record(record, validate = true)
           raise NotImplementedError
-- 
cgit v1.2.3