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.

8 files changed, 68 insertions, 99 deletions

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
diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
index e86f4c0283..ab42d0f215 100644
--- a/activerecord/lib/active_record/associations/association_proxy.rb
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -182,20 +182,6 @@ module ActiveRecord
           @reflection.klass.send(:sanitize_sql, sql, table_name)
         end
 
-        # Merges into +options+ the ones coming from the reflection.
-        def merge_options_from_reflection!(options)
-          options.reverse_merge!(
-            :group   => @reflection.options[:group],
-            :having  => @reflection.options[:having],
-            :limit   => @reflection.options[:limit],
-            :offset  => @reflection.options[:offset],
-            :joins   => @reflection.options[:joins],
-            :include => @reflection.options[:include],
-            :select  => @reflection.options[:select],
-            :readonly  => @reflection.options[:readonly]
-          )
-        end
-
         # Forwards +with_scope+ to the reflection.
         def with_scope(*args, &block)
           target_klass.send :with_scope, *args, &block
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 5336b6cc28..3abe3c2dae 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
@@ -2,6 +2,7 @@ module ActiveRecord
   # = Active Record Has And Belongs To Many Association
   module Associations
     class HasAndBelongsToManyAssociation < AssociationCollection #:nodoc:
+
       def columns
         @reflection.columns(@reflection.options[:join_table], "#{@reflection.options[:join_table]} Columns")
       end
@@ -15,11 +16,6 @@ module ActiveRecord
       end
 
       protected
-        def construct_find_options!(options)
-          options[:joins]      = Arel::SqlLiteral.new(@scope[:find][:joins])
-          options[:readonly]   = finding_with_ambiguous_select?(options[:select] || @reflection.options[:select])
-          options[:select]   ||= (@reflection.options[:select] || Arel::SqlLiteral.new('*'))
-        end
 
         def count_records
           load_target.size
@@ -84,22 +80,23 @@ module ActiveRecord
         end
 
         def construct_find_scope
-          {
-            :conditions => construct_conditions,
-            :joins      => construct_joins,
-            :readonly   => false,
-            :order      => @reflection.options[:order],
-            :include    => @reflection.options[:include],
-            :limit      => @reflection.options[:limit]
-          }
+          super.merge(
+            :joins    => construct_joins,
+            :readonly => ambiguous_select?(@reflection.options[:select]),
+            :select   => @reflection.options[:select] || Arel.star
+          )
         end
 
         # Join tables with additional columns on top of the two foreign keys must be considered
         # ambiguous unless a select clause has been explicitly defined. Otherwise you can get
         # broken records back, if, for example, the join column also has an id column. This will
         # then overwrite the id column of the records coming back.
-        def finding_with_ambiguous_select?(select_clause)
-          !select_clause && columns.size != 2
+        def ambiguous_select?(select)
+          extra_join_columns? && select.nil?
+        end
+
+        def extra_join_columns?
+          columns.size > 2
         end
 
       private
@@ -114,6 +111,13 @@ module ActiveRecord
         def invertible_for?(record)
           false
         end
+
+        def find_by_sql(*args)
+          options   = args.extract_options!
+          ambiguous = ambiguous_select?(@reflection.options[:select] || options[:select])
+
+          scoped.readonly(ambiguous).find(*(args << options))
+        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 0d044b28e4..ca02aa8612 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -71,16 +71,6 @@ module ActiveRecord
           end
         end
 
-        def construct_find_scope
-          {
-            :conditions => construct_conditions,
-            :readonly   => false,
-            :order      => @reflection.options[:order],
-            :limit      => @reflection.options[:limit],
-            :include    => @reflection.options[:include]
-          }
-        end
-
         def construct_create_scope
           construct_owner_attributes
         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 d432e8486d..400db6baf1 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -38,11 +38,6 @@ module ActiveRecord
           end
         end
 
-        def construct_find_options!(options)
-          options[:joins]   = [construct_joins] + Array.wrap(options[:joins])
-          options[:include] = @reflection.source_reflection.options[:include] if options[:include].nil? && @reflection.source_reflection.options[:include]
-        end
-
         def insert_record(record, force = true, validate = true)
           if record.new_record?
             return false unless save_record(record, force, validate)
diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb
index 2b28dda363..b2f1f941c0 100644
--- a/activerecord/lib/active_record/associations/through_association.rb
+++ b/activerecord/lib/active_record/associations/through_association.rb
@@ -13,15 +13,11 @@ module ActiveRecord
       protected
 
       def construct_find_scope
-        {
-          :conditions => construct_conditions,
-          :joins      => construct_joins,
-          :include    => @reflection.options[:include] || @reflection.source_reflection.options[:include],
-          :select     => construct_select,
-          :order      => @reflection.options[:order],
-          :limit      => @reflection.options[:limit],
-          :readonly   => @reflection.options[:readonly]
-        }
+        super.merge(
+          :joins   => construct_joins,
+          :include => @reflection.options[:include] ||
+                      @reflection.source_reflection.options[:include]
+        )
       end
 
       # This scope affects the creation of the associated records (not the join records). At the
@@ -44,11 +40,6 @@ module ActiveRecord
         super(aliased_through_table, @reflection.through_reflection)
       end
 
-      def construct_select
-        @reflection.options[:select] ||
-        @reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*"
-      end
-
       def construct_joins
         right = aliased_through_table
         left  = @reflection.klass.arel_table
diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb
index 16023defe3..f3a32e85e6 100644
--- a/activerecord/lib/active_record/nested_attributes.rb
+++ b/activerecord/lib/active_record/nested_attributes.rb
@@ -393,7 +393,7 @@ module ActiveRecord
         association.to_a
       else
         attribute_ids = attributes_collection.map {|a| a['id'] || a[:id] }.compact
-        attribute_ids.empty? ? [] : association.all(:conditions => {association.primary_key => attribute_ids})
+        attribute_ids.empty? ? [] : association.all(:conditions => {association.klass.primary_key => attribute_ids})
       end
 
       attributes_collection.each do |attributes|
diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb
index e1f7fa2949..a8fb1bccf4 100644
--- a/activerecord/lib/active_record/relation/spawn_methods.rb
+++ b/activerecord/lib/active_record/relation/spawn_methods.rb
@@ -102,7 +102,7 @@ module ActiveRecord
 
       options.assert_valid_keys(VALID_FIND_OPTIONS)
       finders = options.dup
-      finders.delete_if { |key, value| value.nil? }
+      finders.delete_if { |key, value| value.nil? && key != :limit }
 
       ([:joins, :select, :group, :order, :having, :limit, :offset, :from, :lock, :readonly] & finders.keys).each do |finder|
         relation = relation.send(finder, finders[finder])