Added that Base#find takes an optional options hash, including :conditions. Base#find_on_conditions deprecated in favor of #find with :conditions #407 [bitsweat]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@305 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 93 insertions, 56 deletions

diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 5c7770e739..6303ada87b 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -100,13 +100,25 @@ module ActiveRecord
         def interpolate_sql(sql, record = nil)
           @owner.send(:interpolate_sql, sql, record)
         end
-      
+
+        def sanitize_sql(sql)
+          @association_class.send(:sanitize_sql, sql)
+        end
+
+        def extract_options_from_args!(args)
+          @owner.send(:extract_options_from_args!, args)
+        end
+
       private
         def load_collection
-          begin
-            @collection = find_all_records unless loaded?
-          rescue ActiveRecord::RecordNotFound
-            @collection = []
+          if loaded?
+            @collection
+          else
+            begin
+              @collection = find_all_records
+            rescue ActiveRecord::RecordNotFound
+              @collection = []
+            end
           end
         end
 
@@ -114,25 +126,10 @@ module ActiveRecord
           raise ActiveRecord::AssociationTypeMismatch, "#{@association_class} expected, got #{record.class}" unless record.is_a?(@association_class)
         end
 
-
-        def load_collection_to_array
-          return unless @collection_array.nil? 
-          begin
-            @collection_array = find_all_records
-          rescue ActiveRecord::StatementInvalid, ActiveRecord::RecordNotFound
-            @collection_array = []
-          end       
-        end
-        
-        def duplicated_records_array(records)
-          records = [records] unless records.is_a?(Array) || records.is_a?(ActiveRecord::Associations::AssociationCollection)
-          records.dup
-        end
-
         # Array#flatten has problems with rescursive arrays. Going one level deeper solves the majority of the problems.
         def flatten_deeper(array)
           array.collect { |element| element.respond_to?(:flatten) ? element.flatten : element }.flatten
         end
     end
   end
-end
\ No newline at end of file
+end
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 3f90d61e2d..378fc79949 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
@@ -38,15 +38,42 @@ module ActiveRecord
         self
       end
 
-      def find(association_id = nil, &block)
-        if block_given? || @options[:finder_sql]
-          load_collection
-          @collection.find(&block)
+      def find_first
+        load_collection.first
+      end
+
+      def find(*args)
+        # Return an Array if multiple ids are given.
+        expects_array = args.first.kind_of?(Array)
+
+        ids = args.flatten.compact.uniq
+
+        # If no block is given, raise RecordNotFound.
+        if ids.empty?
+          raise RecordNotFound, "Couldn't find #{@association_class.name} without an ID#{conditions}"
+
+        # If using a custom finder_sql, scan the entire collection.
+        elsif @options[:finder_sql]
+          if ids.size == 1
+            id = ids.first
+            record = load_collection.detect { |record| id == record.id }
+            expects_array? ? [record] : record
+          else
+            load_collection.select { |record| ids.include?(record.id) }
+          end
+
+        # Otherwise, construct a query.
         else
-          if loaded?
-            find_all { |record| record.id == association_id.to_i }.first
+          ids_list = ids.map { |id| @owner.send(:quote, id) }.join(',')
+          records = find_all_records(@finder_sql.sub(/ORDER BY/, "AND j.#{@association_foreign_key} IN (#{ids_list}) ORDER BY"))
+          if records.size == ids.size
+            if ids.size == 1 and !expects_array
+              records.first
+            else
+              records
+            end
           else
-            find_all_records(@finder_sql.sub(/ORDER BY/, "AND j.#{@association_foreign_key} = #{@owner.send(:quote, association_id)} ORDER BY")).first
+            raise RecordNotFound, "Couldn't find #{@association_class.name} with ID in (#{ids_list})"
           end
         end
       end
@@ -70,10 +97,9 @@ module ActiveRecord
           records = @association_class.find_by_sql(sql)
           @options[:uniq] ? uniq(records) : records
         end
-            
+
         def count_records
-          load_collection
-          @collection.size
+          load_collection.size
         end
 
         def insert_record(record)
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index bca52d61d0..665d28869f 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -3,12 +3,13 @@ module ActiveRecord
     class HasManyAssociation < AssociationCollection #:nodoc:
       def initialize(owner, association_name, association_class_name, association_class_primary_key_name, options)
         super(owner, association_name, association_class_name, association_class_primary_key_name, options)
-        @conditions = @association_class.send(:sanitize_conditions, options[:conditions])
+        @conditions = sanitize_sql(options[:conditions])
 
         if options[:finder_sql]
           @finder_sql = interpolate_sql(options[:finder_sql])
         else
-          @finder_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id} #{@conditions ? " AND " + interpolate_sql(@conditions) : ""}"
+          @finder_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id}"
+          @finder_sql << " AND #{@conditions}" if @conditions
         end
 
         if options[:counter_sql]
@@ -35,29 +36,46 @@ module ActiveRecord
         record
       end
 
-      def find_all(runtime_conditions = nil, orderings = nil, limit = nil, joins = nil, &block)
-        if block_given? || @options[:finder_sql]
-          load_collection
-          @collection.find_all(&block)
+      def find_all(runtime_conditions = nil, orderings = nil, limit = nil, joins = nil)
+        if @options[:finder_sql]
+          records = @association_class.find_by_sql(@finder_sql)
         else
-          @association_class.find_all(
-            "#{@association_class_primary_key_name} = #{@owner.quoted_id}" +
-            "#{@conditions ? " AND " + @conditions : ""}#{runtime_conditions ? " AND " + @association_class.send(:sanitize_conditions, runtime_conditions) : ""}",
-            orderings, 
-            limit, 
-            joins
-          )
+          sql = @finder_sql
+          sql << " AND #{sanitize_sql(runtime_conditions)}" if runtime_conditions
+          orderings ||= @options[:order]
+          records = @association_class.find_all(sql, orderings, limit, joins)
         end
       end
 
-      def find(association_id = nil, &block)
-        if block_given? || @options[:finder_sql]
-          load_collection
-          @collection.find(&block)
+      # Find the first associated record.  All arguments are optional.
+      def find_first(conditions = nil, orderings = nil)
+        find_all(conditions, orderings, 1).first
+      end
+
+      def find(*args)
+        # Return an Array if multiple ids are given.
+        expects_array = args.first.kind_of?(Array)
+
+        ids = args.flatten.compact.uniq
+
+        # If no ids given, raise RecordNotFound.
+        if ids.empty?
+          raise RecordNotFound, "Couldn't find #{@association_class.name} without an ID"
+
+        # If using a custom finder_sql, scan the entire collection.
+        elsif @options[:finder_sql]
+          if ids.size == 1
+            id = ids.first
+            record = load_collection.detect { |record| id == record.id }
+            expects_array? ? [record] : record
+          else
+            load_collection.select { |record| ids.include?(record.id) }
+          end
+
+        # Otherwise, delegate to association class with conditions.
         else
-          @association_class.find_on_conditions(association_id,
-            "#{@association_class_primary_key_name} = #{@owner.quoted_id}#{@conditions ? " AND " + @conditions : ""}"
-          )
+          args << { :conditions => "#{@association_class_primary_key_name} = '#{@owner.id}' #{@conditions ? " AND " + @conditions : ""}" }
+          @association_class.find(*args)
         end
       end
 
@@ -71,11 +89,7 @@ module ActiveRecord
 
       protected
         def find_all_records
-          if @options[:finder_sql]
-            @association_class.find_by_sql(@finder_sql)
-          else
-            @association_class.find_all(@finder_sql, @options[:order] ? @options[:order] : nil)
-          end
+          find_all
         end
 
         def count_records