Make Model.find_by_* and Model.find_all_by_* use relations and remove dynamic method caching

3 files changed, 38 insertions, 78 deletions

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index b6d73265a8..31737043eb 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -671,20 +671,10 @@ module ActiveRecord #:nodoc:
         options = args.extract_options!
 
         if options.empty? && !scoped?(:find)
-          relation = arel_table
+          arel_table
         else
-          relation = construct_finder_arel(options)
-          include_associations = merge_includes(scope(:find, :include), options[:include])
-
-          if include_associations.any?
-            if references_eager_loaded_tables?(options)
-              relation.eager_load(include_associations)
-            else
-              relation.preload(include_associations)
-            end
-          end
+          construct_finder_arel_with_includes(options)
         end
-        relation
       end
 
       # Executes a custom SQL query against your database and returns all the results.  The results will
@@ -1687,6 +1677,8 @@ module ActiveRecord #:nodoc:
 
         def construct_finder_arel(options = {}, scope = scope(:find))
           # TODO add lock to Arel
+          validate_find_options(options)
+
           relation = arel_table(options[:from]).
             joins(construct_join(options[:joins], scope)).
             where(construct_conditions(options[:conditions], scope)).
@@ -1701,6 +1693,21 @@ module ActiveRecord #:nodoc:
           relation
         end
 
+        def construct_finder_arel_with_includes(options = {})
+          relation = construct_finder_arel(options)
+          include_associations = merge_includes(scope(:find, :include), options[:include])
+
+          if include_associations.any?
+            if references_eager_loaded_tables?(options)
+              relation = relation.eager_load(include_associations)
+            else
+              relation = relation.preload(include_associations)
+            end
+          end
+
+          relation
+        end
+
         def construct_finder_sql(options, scope = scope(:find))
           construct_finder_arel(options, scope).to_sql
         end
@@ -1853,48 +1860,9 @@ module ActiveRecord #:nodoc:
             attribute_names = match.attribute_names
             super unless all_attributes_exists?(attribute_names)
             if match.finder?
-              finder = match.finder
-              bang = match.bang?
-              # def self.find_by_login_and_activated(*args)
-              #   options = args.extract_options!
-              #   attributes = construct_attributes_from_arguments(
-              #     [:login,:activated],
-              #     args
-              #   )
-              #   finder_options = { :conditions => attributes }
-              #   validate_find_options(options)
-              #   set_readonly_option!(options)
-              #
-              #   if options[:conditions]
-              #     with_scope(:find => finder_options) do
-              #       find(:first, options)
-              #     end
-              #   else
-              #     find(:first, options.merge(finder_options))
-              #   end
-              # end
-              self.class_eval %{
-                def self.#{method_id}(*args)
-                  options = args.extract_options!
-                  attributes = construct_attributes_from_arguments(
-                    [:#{attribute_names.join(',:')}],
-                    args
-                  )
-                  finder_options = { :conditions => attributes }
-                  validate_find_options(options)
-                  set_readonly_option!(options)
-
-                  #{'result = ' if bang}if options[:conditions]
-                    with_scope(:find => finder_options) do
-                      find(:#{finder}, options)
-                    end
-                  else
-                    find(:#{finder}, options.merge(finder_options))
-                  end
-                  #{'result || raise(RecordNotFound, "Couldn\'t find #{name} with #{attributes.to_a.collect { |pair| pair.join(\' = \') }.join(\', \')}")' if bang}
-                end
-              }, __FILE__, __LINE__
-              send(method_id, *arguments)
+              options = arguments.extract_options!
+              relation = options.any? ? construct_finder_arel_with_includes(options) : scoped
+              relation.send :find_by_attributes, match, attribute_names, *arguments
             elsif match.instantiator?
               instantiator = match.instantiator
               # def self.find_or_create_by_user_id(*args)
diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb
index 96d361093f..a6336e762a 100644
--- a/activerecord/lib/active_record/named_scope.rb
+++ b/activerecord/lib/active_record/named_scope.rb
@@ -26,23 +26,11 @@ module ActiveRecord
         if options.present?
           Scope.new(self, options, &block)
         else
-          if !scoped?(:find)
-            relation = arel_table
-            relation = relation.where(type_condition) if finder_needs_type_condition?
+          unless scoped?(:find)
+            finder_needs_type_condition? ? arel_table.where(type_condition) : arel_table
           else
-            relation = construct_finder_arel
-            include_associations = scope(:find, :include)
-
-            if include_associations.present?
-              if references_eager_loaded_tables?(options)
-                relation.eager_load(include_associations)
-              else
-                relation.preload(include_associations)
-              end
-            end
+            construct_finder_arel_with_includes
           end
-
-          relation
         end
       end
 
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index f9ca6cbbd2..bcae352cc6 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -129,7 +129,7 @@ module ActiveRecord
       self
     end
 
-    private
+    protected
 
     def method_missing(method, *args, &block)
       if @relation.respond_to?(method)
@@ -141,20 +141,24 @@ module ActiveRecord
         super unless @klass.send(:all_attributes_exists?, attributes)
 
         if match.finder?
-          conditions = attributes.inject({}) {|h, a| h[a] = args[attributes.index(a)]; h}
-          result = where(conditions).send(match.finder)
-
-          if match.bang? && result.blank?
-            raise RecordNotFound, "Couldn't find #{@klass.name} with #{conditions.to_a.collect {|p| p.join(' = ')}.join(', ')}"
-          else
-            result
-          end
+          find_by_attributes(match, attributes, *args)
         end
       else
         super
       end
     end
 
+    def find_by_attributes(match, attributes, *args)
+      conditions = attributes.inject({}) {|h, a| h[a] = args[attributes.index(a)]; h}
+      result = where(conditions).send(match.finder)
+
+      if match.bang? && result.blank?
+        raise RecordNotFound, "Couldn't find #{@klass.name} with #{conditions.to_a.collect {|p| p.join(' = ')}.join(', ')}"
+      else
+        result
+      end
+    end
+
     def create_new_relation(relation, readonly = @readonly, preload = @associations_to_preload, eager_load = @eager_load_associations)
       Relation.new(@klass, relation, readonly, preload, eager_load)
     end