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

18 files changed, 132 insertions, 76 deletions

diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb
index d1e3ff8e38..7887d59aad 100644
--- a/activerecord/lib/active_record/associations/association.rb
+++ b/activerecord/lib/active_record/associations/association.rb
@@ -231,10 +231,7 @@ module ActiveRecord
 
         def build_record(attributes, options)
           reflection.build_association(attributes, options) do |record|
-            record.assign_attributes(
-              create_scope.except(*record.changed),
-              :without_protection => true
-            )
+            record.assign_attributes(create_scope.except(*record.changed), :without_protection => true)
           end
         end
     end
diff --git a/activerecord/lib/active_record/associations/association_scope.rb b/activerecord/lib/active_record/associations/association_scope.rb
index 6cc401e6cc..090f690f0d 100644
--- a/activerecord/lib/active_record/associations/association_scope.rb
+++ b/activerecord/lib/active_record/associations/association_scope.rb
@@ -20,31 +20,19 @@ module ActiveRecord
         # It's okay to just apply all these like this. The options will only be present if the
         # association supports that option; this is enforced by the association builder.
         scope = scope.apply_finder_options(options.slice(
-          :readonly, :include, :order, :limit, :joins, :group, :having, :offset))
+          :readonly, :include, :eager_load, :order, :limit, :joins, :group, :having, :offset, :select))
 
         if options[:through] && !options[:include]
           scope = scope.includes(source_options[:include])
         end
 
-        if select = select_value
-          scope = scope.select(select)
-        end
+        scope = scope.uniq if options[:uniq]
 
         add_constraints(scope)
       end
 
       private
 
-      def select_value
-        select_value = options[:select]
-
-        if reflection.collection?
-          select_value ||= options[:uniq] && "DISTINCT #{reflection.quoted_table_name}.*"
-        end
-
-        select_value
-      end
-
       def add_constraints(scope)
         tables = construct_tables
 
diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb
index 96fca97440..776f0d0469 100644
--- a/activerecord/lib/active_record/associations/builder/association.rb
+++ b/activerecord/lib/active_record/associations/builder/association.rb
@@ -1,7 +1,7 @@
 module ActiveRecord::Associations::Builder
   class Association #:nodoc:
     class_attribute :valid_options
-    self.valid_options = [:class_name, :foreign_key, :select, :conditions, :include, :extend, :readonly, :validate]
+    self.valid_options = [:class_name, :foreign_key, :select, :conditions, :include, :eager_load, :extend, :readonly, :validate]
 
     # Set by subclasses
     class_attribute :macro
@@ -16,6 +16,10 @@ module ActiveRecord::Associations::Builder
       @model, @name, @options = model, name, options
     end
 
+    def mixin
+      @model.generated_feature_methods
+    end
+
     def build
       validate_options
       reflection = model.create_reflection(self.class.macro, name, options, model)
@@ -36,16 +40,14 @@ module ActiveRecord::Associations::Builder
 
       def define_readers
         name = self.name
-
-        model.redefine_method(name) do |*params|
+        mixin.redefine_method(name) do |*params|
           association(name).reader(*params)
         end
       end
 
       def define_writers
         name = self.name
-
-        model.redefine_method("#{name}=") do |value|
+        mixin.redefine_method("#{name}=") do |value|
           association(name).writer(value)
         end
       end
diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb
index f6d26840c2..1759a41d93 100644
--- a/activerecord/lib/active_record/associations/builder/belongs_to.rb
+++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -25,14 +25,14 @@ module ActiveRecord::Associations::Builder
         name         = self.name
 
         method_name = "belongs_to_counter_cache_after_create_for_#{name}"
-        model.redefine_method(method_name) do
+        mixin.redefine_method(method_name) do
           record = send(name)
           record.class.increment_counter(cache_column, record.id) unless record.nil?
         end
         model.after_create(method_name)
 
         method_name = "belongs_to_counter_cache_before_destroy_for_#{name}"
-        model.redefine_method(method_name) do
+        mixin.redefine_method(method_name) do
           record = send(name)
           record.class.decrement_counter(cache_column, record.id) unless record.nil?
         end
@@ -48,7 +48,7 @@ module ActiveRecord::Associations::Builder
         method_name = "belongs_to_touch_after_save_or_destroy_for_#{name}"
         touch       = options[:touch]
 
-        model.redefine_method(method_name) do
+        mixin.redefine_method(method_name) do
           record = send(name)
 
           unless record.nil?
diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb
index f62209a226..35f9a3ae8e 100644
--- a/activerecord/lib/active_record/associations/builder/collection_association.rb
+++ b/activerecord/lib/active_record/associations/builder/collection_association.rb
@@ -58,7 +58,7 @@ module ActiveRecord::Associations::Builder
         super
 
         name = self.name
-        model.redefine_method("#{name.to_s.singularize}_ids") do
+        mixin.redefine_method("#{name.to_s.singularize}_ids") do
           association(name).ids_reader
         end
       end
@@ -67,7 +67,7 @@ module ActiveRecord::Associations::Builder
         super
 
         name = self.name
-        model.redefine_method("#{name.to_s.singularize}_ids=") do |ids|
+        mixin.redefine_method("#{name.to_s.singularize}_ids=") do |ids|
           association(name).ids_writer(ids)
         end
       end
diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
index 30fc44b4c2..0b634ab944 100644
--- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb
@@ -18,7 +18,7 @@ module ActiveRecord::Associations::Builder
         model.send(:include, Module.new {
           class_eval <<-RUBY, __FILE__, __LINE__ + 1
             def destroy_associations
-              association(#{name.to_sym.inspect}).delete_all
+              association(#{name.to_sym.inspect}).delete_all_on_destroy
               super
             end
           RUBY
diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb
index ecbc70888f..9c24f40690 100644
--- a/activerecord/lib/active_record/associations/builder/has_many.rb
+++ b/activerecord/lib/active_record/associations/builder/has_many.rb
@@ -28,7 +28,7 @@ module ActiveRecord::Associations::Builder
 
       def define_destroy_dependency_method
         name = self.name
-        model.send(:define_method, dependency_method_name) do
+        mixin.redefine_method(dependency_method_name) do
           send(name).each do |o|
             # No point in executing the counter update since we're going to destroy the parent anyway
             counter_method = ('belongs_to_counter_cache_before_destroy_for_' + self.class.name.downcase).to_sym
@@ -45,15 +45,21 @@ module ActiveRecord::Associations::Builder
 
       def define_delete_all_dependency_method
         name = self.name
-        model.send(:define_method, dependency_method_name) do
+        mixin.redefine_method(dependency_method_name) do
+          association(name).delete_all_on_destroy
+        end
+      end
+
+      def define_nullify_dependency_method
+        name = self.name
+        mixin.redefine_method(dependency_method_name) do
           send(name).delete_all
         end
       end
-      alias :define_nullify_dependency_method :define_delete_all_dependency_method
 
       def define_restrict_dependency_method
         name = self.name
-        model.send(:define_method, dependency_method_name) do
+        mixin.redefine_method(dependency_method_name) do
           raise ActiveRecord::DeleteRestrictionError.new(name) unless send(name).empty?
         end
       end
diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb
index 88c0d3e90f..7a6cd3890f 100644
--- a/activerecord/lib/active_record/associations/builder/has_one.rb
+++ b/activerecord/lib/active_record/associations/builder/has_one.rb
@@ -44,18 +44,17 @@ module ActiveRecord::Associations::Builder
       end
 
       def define_destroy_dependency_method
-        model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1)
-          def #{dependency_method_name}
-            association(#{name.to_sym.inspect}).delete
-          end
-        eoruby
+        name = self.name
+        mixin.redefine_method(dependency_method_name) do
+          association(name).delete
+        end
       end
       alias :define_delete_dependency_method :define_destroy_dependency_method
       alias :define_nullify_dependency_method :define_destroy_dependency_method
 
       def define_restrict_dependency_method
         name = self.name
-        model.redefine_method(dependency_method_name) do
+        mixin.redefine_method(dependency_method_name) do
           raise ActiveRecord::DeleteRestrictionError.new(name) unless send(name).nil?
         end
       end
diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb
index 0cbbba041a..436b6c1524 100644
--- a/activerecord/lib/active_record/associations/builder/singular_association.rb
+++ b/activerecord/lib/active_record/associations/builder/singular_association.rb
@@ -16,15 +16,15 @@ module ActiveRecord::Associations::Builder
       def define_constructors
         name = self.name
 
-        model.redefine_method("build_#{name}") do |*params, &block|
+        mixin.redefine_method("build_#{name}") do |*params, &block|
           association(name).build(*params, &block)
         end
 
-        model.redefine_method("create_#{name}") do |*params, &block|
+        mixin.redefine_method("create_#{name}") do |*params, &block|
           association(name).create(*params, &block)
         end
 
-        model.redefine_method("create_#{name}!") do |*params, &block|
+        mixin.redefine_method("create_#{name}!") do |*params, &block|
           association(name).create!(*params, &block)
         end
       end
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index cec876149c..fe9f30bd2a 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -49,10 +49,18 @@ module ActiveRecord
           end
         else
           column  = "#{reflection.quoted_table_name}.#{reflection.association_primary_key}"
+          relation = scoped
 
-          scoped.select(column).map! do |record|
-            record.send(reflection.association_primary_key)
+          including = (relation.eager_load_values + relation.includes_values).uniq
+
+          if including.any?
+            join_dependency = ActiveRecord::Associations::JoinDependency.new(reflection.klass, including, [])
+            relation = join_dependency.join_associations.inject(relation) do |r, association|
+              association.join_relation(r)
+            end
           end
+
+          relation.uniq.pluck(column)
         end
       end
 
@@ -152,6 +160,13 @@ module ActiveRecord
         end
       end
 
+      # Called when the association is declared as :dependent => :delete_all. This is
+      # an optimised version which avoids loading the records into memory. Not really
+      # for public consumption.
+      def delete_all_on_destroy
+        scoped.delete_all
+      end
+
       # Destroy all the records from this association.
       #
       # See destroy for more info.
@@ -235,7 +250,7 @@ module ActiveRecord
       # This method is abstract in the sense that it relies on
       # +count_records+, which is a method descendants have to provide.
       def size
-        if owner.new_record? || (loaded? && !options[:uniq])
+        if !find_target? || (loaded? && !options[:uniq])
           target.size
         elsif !loaded? && options[:group]
           load_target.size
@@ -344,8 +359,12 @@ module ActiveRecord
           if options[:counter_sql]
             interpolate(options[:counter_sql])
           else
-            # replace the SELECT clause with COUNT(*), preserving any hints within /* ... */
-            interpolate(options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
+            # replace the SELECT clause with COUNT(SELECTS), preserving any hints within /* ... */
+            interpolate(options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) do
+              count_with = $2.to_s
+              count_with = '*' if count_with.blank? || count_with =~ /,/
+              "SELECT #{$1}COUNT(#{count_with}) FROM"
+            end
           end
         end
 
diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb
index 3181ca9a32..eb320bc774 100644
--- a/activerecord/lib/active_record/associations/collection_proxy.rb
+++ b/activerecord/lib/active_record/associations/collection_proxy.rb
@@ -39,10 +39,9 @@ module ActiveRecord
       instance_methods.each { |m| undef_method m unless m.to_s =~ /^(?:nil\?|send|object_id|to_a)$|^__|^respond_to|proxy_/ }
 
       delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from,
-               :lock, :readonly, :having, :to => :scoped
+               :lock, :readonly, :having, :pluck, :to => :scoped
 
-      delegate :target, :load_target, :loaded?, :scoped,
-               :to => :@association
+      delegate :target, :load_target, :loaded?, :to => :@association
 
       delegate :select, :find, :first, :last,
                :build, :create, :create!,
@@ -62,6 +61,13 @@ module ActiveRecord
         @association
       end
 
+      def scoped
+        association = @association
+        association.scoped.extending do
+          define_method(:proxy_association) { association }
+        end
+      end
+
       def respond_to?(name, include_private = false)
         super ||
         (load_target && target.respond_to?(name, include_private)) ||
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index 1f917f58f2..a4cea99372 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -32,6 +32,10 @@ module ActiveRecord
         record
       end
 
+      # ActiveRecord::Relation#delete_all needs to support joins before we can use a
+      # SQL-only implementation.
+      alias delete_all_on_destroy delete_all
+
       private
 
         def count_records
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 50ee60284c..059e6c77bc 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -99,6 +99,10 @@ module ActiveRecord
             end
           end
         end
+
+        def foreign_key_present?
+          owner.attribute_present?(reflection.association_primary_key)
+        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 2e818dca5d..9657cb081d 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -6,6 +6,13 @@ module ActiveRecord
     class HasManyThroughAssociation < HasManyAssociation #:nodoc:
       include ThroughAssociation
 
+      def initialize(owner, reflection)
+        super
+
+        @through_records     = {}
+        @through_association = nil
+      end
+
       # Returns the size of the collection by executing a SELECT COUNT(*) query if the collection hasn't been
       # loaded and calling collection.size if it has. If it's more likely than not that the collection does
       # have a size larger than zero, and you need to fetch that collection afterwards, it'll take one fewer
@@ -42,27 +49,40 @@ module ActiveRecord
           end
         end
 
-        through_record(record).save!
+        save_through_record(record)
         update_counter(1)
         record
       end
 
-      private
+      # ActiveRecord::Relation#delete_all needs to support joins before we can use a
+      # SQL-only implementation.
+      alias delete_all_on_destroy delete_all
 
-        def through_record(record)
-          through_association = owner.association(through_reflection.name)
-          attributes = construct_join_attributes(record)
+      private
 
-          through_record = Array.wrap(through_association.target).find { |candidate|
-            candidate.attributes.slice(*attributes.keys) == attributes
-          }
+        def through_association
+          @through_association ||= owner.association(through_reflection.name)
+        end
 
-          unless through_record
-            through_record = through_association.build(attributes)
+        # We temporarily cache through record that has been build, because if we build a
+        # through record in build_record and then subsequently call insert_record, then we
+        # want to use the exact same object.
+        #
+        # However, after insert_record has been called, we clear the cache entry because
+        # we want it to be possible to have multiple instances of the same record in an
+        # association
+        def build_through_record(record)
+          @through_records[record.object_id] ||= begin
+            through_record = through_association.build(construct_join_attributes(record))
             through_record.send("#{source_reflection.name}=", record)
+            through_record
           end
+        end
 
-          through_record
+        def save_through_record(record)
+          build_through_record(record).save!
+        ensure
+          @through_records.delete(record.object_id)
         end
 
         def build_record(attributes, options = {})
@@ -73,9 +93,9 @@ module ActiveRecord
           inverse = source_reflection.inverse_of
           if inverse
             if inverse.macro == :has_many
-              record.send(inverse.name) << through_record(record)
+              record.send(inverse.name) << build_through_record(record)
             elsif inverse.macro == :has_one
-              record.send("#{inverse.name}=", through_record(record))
+              record.send("#{inverse.name}=", build_through_record(record))
             end
           end
 
@@ -104,8 +124,7 @@ module ActiveRecord
         def delete_records(records, method)
           ensure_not_nested
 
-          through = owner.association(through_reflection.name)
-          scope   = through.scoped.where(construct_join_attributes(*records))
+          scope = through_association.scoped.where(construct_join_attributes(*records))
 
           case method
           when :destroy
@@ -116,7 +135,7 @@ module ActiveRecord
             count = scope.delete_all
           end
 
-          delete_through_records(through, records)
+          delete_through_records(records)
 
           if through_reflection.macro == :has_many && update_through_counter?(method)
             update_counter(-count, through_reflection)
@@ -125,15 +144,25 @@ module ActiveRecord
           update_counter(-count)
         end
 
-        def delete_through_records(through, records)
-          if through_reflection.macro == :has_many
-            records.each do |record|
-              through.target.delete(through_record(record))
-            end
-          else
-            records.each do |record|
-              through.target = nil if through.target == through_record(record)
+        def through_records_for(record)
+          attributes = construct_join_attributes(record)
+          candidates = Array.wrap(through_association.target)
+          candidates.find_all { |c| c.attributes.slice(*attributes.keys) == attributes }
+        end
+
+        def delete_through_records(records)
+          records.each do |record|
+            through_records = through_records_for(record)
+
+            if through_reflection.macro == :has_many
+              through_records.each { |r| through_association.target.delete(r) }
+            else
+              if through_records.include?(through_association.target)
+                through_association.target = nil
+              end
             end
+
+            @through_records.delete(record.object_id)
           end
         end
 
diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb
index 6c878f0f00..827b01c5ac 100644
--- a/activerecord/lib/active_record/associations/join_dependency.rb
+++ b/activerecord/lib/active_record/associations/join_dependency.rb
@@ -184,7 +184,7 @@ module ActiveRecord
 
         macro = join_part.reflection.macro
         if macro == :has_one
-          return if record.association_cache.key?(join_part.reflection.name)
+          return record.association(join_part.reflection.name).target if record.association_cache.key?(join_part.reflection.name)
           association = join_part.instantiate(row) unless row[join_part.aliased_primary_key].nil?
           set_target_and_inverse(join_part, association, record)
         else
diff --git a/activerecord/lib/active_record/associations/preloader/association.rb b/activerecord/lib/active_record/associations/preloader/association.rb
index 779f8164cc..298decb0f1 100644
--- a/activerecord/lib/active_record/associations/preloader/association.rb
+++ b/activerecord/lib/active_record/associations/preloader/association.rb
@@ -100,6 +100,7 @@ module ActiveRecord
 
           scope = scope.select(preload_options[:select] || options[:select] || table[Arel.star])
           scope = scope.includes(preload_options[:include] || options[:include])
+          scope = scope.eager_load(preload_options[:eager_load] || options[:eager_load])
 
           if options[:as]
             scope = scope.where(
diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb
index ad6374d09a..97898c53ae 100644
--- a/activerecord/lib/active_record/associations/preloader/through_association.rb
+++ b/activerecord/lib/active_record/associations/preloader/through_association.rb
@@ -53,6 +53,7 @@ module ActiveRecord
           else
             if options[:conditions]
               through_options[:include]    = options[:include] || options[:source]
+              through_options[:eager_load] = options[:eager_load] || options[:source]
               through_options[:conditions] = options[:conditions]
             end
 
diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb
index b347a94978..f95e5337c2 100644
--- a/activerecord/lib/active_record/associations/through_association.rb
+++ b/activerecord/lib/active_record/associations/through_association.rb
@@ -44,7 +44,7 @@ module ActiveRecord
           join_attributes = {
             source_reflection.foreign_key =>
               records.map { |record|
-                record.send(source_reflection.association_primary_key)
+                record.send(source_reflection.association_primary_key(reflection.klass))
               }
           }