Added support for calling constrained class methods on has_many and has_and_belongs_to_many collections #1764 [Tobias Luetke]

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

12 files changed, 299 insertions, 67 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 3bf9ba5fcf..151333bbb9 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,19 @@
 *SVN*
 
+* Added support for calling constrained class methods on has_many and has_and_belongs_to_many collections #1764 [Tobias Luetke]
+
+    class Comment < AR:B
+      def self.search(q)
+        find(:all, :conditions => ["body = ?", q])
+      end
+    end 
+
+    class Post < AR:B
+      has_many :comments
+    end
+
+    Post.find(1).comments.search('hi') # => SELECT * from comments WHERE post_id = 1 AND body = 'hi'
+
 * Added migration support for SQLite (using temporary tables to simulate ALTER TABLE) #1771 [Sam Stephenson]
 
 * Remove extra definition of supports_migrations? from abstract_adaptor.rb [Nicholas Seckar]
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index 140a12eacf..3b7f2df870 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -107,6 +107,15 @@ module ActiveRecord
       end
 
       private
+      
+        def method_missing(method, *args, &block)
+          if @target.respond_to?(method) or (not @association_class.respond_to?(method) and Class.respond_to?(method))
+            super
+          else
+            @association_class.constrain( :conditions => @finder_sql, :joins => @join_sql) { @association_class.send(method, *args, &block) }
+          end
+        end
+              
         def raise_on_type_mismatch(record)
           raise ActiveRecord::AssociationTypeMismatch, "#{@association_class} expected, got #{record.class}" unless record.is_a?(@association_class)
         end
diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb
index f80a7e996c..ed34b1df6c 100644
--- a/activerecord/lib/active_record/associations/association_proxy.rb
+++ b/activerecord/lib/active_record/associations/association_proxy.rb
@@ -19,11 +19,6 @@ module ActiveRecord
         load_target
       end
 
-      def method_missing(symbol, *args, &block)
-        load_target
-        @target.send(symbol, *args, &block)
-      end
-
       def respond_to?(symbol, include_priv = false)
         proxy_respond_to?(symbol, include_priv) || (load_target && @target.respond_to?(symbol, include_priv))
       end
@@ -44,7 +39,7 @@ module ActiveRecord
         @target = t
         @loaded = true
       end
-
+      
       protected
         def dependent?
           @options[:dependent] || false
@@ -69,8 +64,14 @@ module ActiveRecord
         def extract_options_from_args!(args)
           @owner.send(:extract_options_from_args!, args)
         end
-
+        
       private
+        
+        def method_missing(method, *args, &block)
+          load_target
+          @target.send(method, *args, &block)
+        end
+
         def load_target
           if !@owner.new_record? || foreign_key_present
             begin
diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb
index b142a13153..60c31ce486 100644
--- a/activerecord/lib/active_record/associations/belongs_to_association.rb
+++ b/activerecord/lib/active_record/associations/belongs_to_association.rb
@@ -1,6 +1,12 @@
 module ActiveRecord
   module Associations
     class BelongsToAssociation < AssociationProxy #:nodoc:
+      
+      def initialize(owner, association_name, association_class_name, association_class_primary_key_name, options)
+        super        
+        construct_sql        
+      end
+      
       def reset
         @target = nil
         @loaded = false
@@ -31,6 +37,9 @@ module ActiveRecord
 
         return (@target.nil? ? nil : self)
       end
+      
+      protected
+
 
       private
         def find_target
@@ -48,9 +57,9 @@ module ActiveRecord
         def target_obsolete?
           @owner[@association_class_primary_key_name] != @target.id
         end
-
+        
         def construct_sql
-          # no sql to construct
+          @finder_sql = "#{@association_class.table_name}.#{@association_class.primary_key} = #{@owner.id}"
         end
     end
   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 107f3ef12c..7d85a2268b 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
@@ -42,19 +42,15 @@ module ActiveRecord
       def find_first
         load_target.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"
+        options = Base.send(:extract_options_from_args!, args)
 
         # If using a custom finder_sql, scan the entire collection.
-        elsif @options[:finder_sql]
+        if @options[:finder_sql]
+          expects_array = args.first.kind_of?(Array)
+          ids = args.flatten.compact.uniq
+
           if ids.size == 1
             id = ids.first
             record = load_target.detect { |record| id == record.id }
@@ -62,22 +58,25 @@ module ActiveRecord
           else
             load_target.select { |record| ids.include?(record.id) }
           end
-
-        # Otherwise, construct a query.
         else
-          ids_list = ids.map { |id| @owner.send(:quote, id) }.join(',')
-          records = find_target(@finder_sql.sub(/(ORDER BY|$)/, " AND j.#{@association_foreign_key} IN (#{ids_list}) \\1"))
-          if records.size == ids.size
-            if ids.size == 1 and !expects_array
-              records.first
-            else
-              records
-            end
-          else
-            raise RecordNotFound, "Couldn't find #{@association_class.name} with ID in (#{ids_list})"
+          conditions = "#{@finder_sql}"
+          if sanitized_conditions = sanitize_sql(options[:conditions])
+            conditions << " AND #{sanitized_conditions}"
+          end
+          options[:conditions] = conditions
+          options[:joins] = @join_sql
+
+          if options[:order] && @options[:order]
+            options[:order] = "#{options[:order]}, #{@options[:order]}"
+          elsif @options[:order]
+            options[:order] = @options[:order]
           end
+
+          # Pass through args exactly as we received them.
+          args << options
+          @association_class.find(*args)
         end
-      end
+      end      
 
       def push_with_attributes(record, join_attributes = {})
         raise_on_type_mismatch(record)
@@ -96,11 +95,17 @@ module ActiveRecord
       end
       
       protected
-        def find_target(sql = @finder_sql)
-          records = @association_class.find_by_sql(sql)
+        def find_target(sql = nil)
+          
+          if sql
+            records = @association_class.find_by_sql(sql) if sql
+          else
+            records = find(:all)
+          end
+          
           @options[:uniq] ? uniq(records) : records
         end
-
+        
         def count_records
           load_target.size
         end
@@ -152,28 +157,17 @@ module ActiveRecord
 
         def construct_sql
           interpolate_sql_options!(@options, :finder_sql)
-          
+
           if @options[:finder_sql]
             @finder_sql = @options[:finder_sql]
           else
-            @finder_sql = 
-              "SELECT t.*, j.* FROM #{@join_table} j, #{@association_table_name} t " +
-              "WHERE t.#{@association_class.primary_key} = j.#{@association_foreign_key} AND " +
-              "j.#{@association_class_primary_key_name} = #{@owner.quoted_id} "
-            
+            @finder_sql = "#{@association_class_primary_key_name} = #{@owner.quoted_id} "
             @finder_sql << " AND #{interpolate_sql(@options[:conditions])}" if @options[:conditions]
-
-            unless @association_class.descends_from_active_record?
-              type_condition = @association_class.send(:subclasses).inject("t.#{@association_class.inheritance_column} = '#{@association_class.name.demodulize}' ") do |condition, subclass| 
-                condition << "OR t.#{@association_class.inheritance_column} = '#{subclass.name.demodulize}' "
-              end
-
-              @finder_sql << " AND (#{type_condition})"
-            end
-	
-            @finder_sql << " ORDER BY #{@order}" if @order
           end
+          
+          @join_sql = "LEFT JOIN #{@join_table} ON #{@association_class.table_name}.#{@association_class.primary_key} = #{@join_table}.#{@association_foreign_key}"
         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 21879adb77..fdeadfc181 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -91,7 +91,7 @@ module ActiveRecord
         @target = []
         self
       end
-
+      
       protected
         def find_target
           find_all
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index 4a6e85c8ee..1550db69b5 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -54,7 +54,7 @@ module ActiveRecord
           return (obj.nil? ? nil : self)
         end
       end
-      
+            
       private
         def find_target
           @association_class.find(:first, :conditions => @finder_sql, :order => @options[:order])
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 8420fe2ffe..20ccd9a623 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -346,6 +346,7 @@ module ActiveRecord #:nodoc:
           else
             return args.first if args.first.kind_of?(Array) && args.first.empty?
             expects_array = args.first.kind_of?(Array)
+            
             conditions = " AND #{sanitize_sql(options[:conditions])}" if options[:conditions]
 
             ids = args.flatten.compact.uniq
@@ -703,12 +704,27 @@ module ActiveRecord #:nodoc:
       ensure
         logger.level = old_logger_level if logger
       end
+      
+      # Add constrains to all queries to the same model in the given block.
+      # Currently supported constrains are <tt>:conditions</tt> and <tt>:joins</tt> 
+      #
+      #   Article.constrain(:conditions => "blog_id = 1") do
+      #     Article.find(1) # => SELECT * from articles WHERE blog_id = 1 AND id = 1
+      #   end
+      def constrain(options = {}, &block)
+        begin
+          self.scope_constrains = options
+          block.call if block_given?
+        ensure 
+          self.scope_constrains = nil
+        end
+      end
 
       # Overwrite the default class equality method to provide support for association proxies.
       def ===(object)
         object.is_a?(self)
-      end
-
+      end      
+      
       private
         # Finder methods must instantiate through this method to work with the single-table inheritance model
         # that makes it possible to create objects of different types from the same table.
@@ -742,9 +758,9 @@ module ActiveRecord #:nodoc:
 
         def construct_finder_sql(options)
           sql  = "SELECT #{options[:select] || '*'} FROM #{table_name} "
-          sql << " #{options[:joins]} " if options[:joins]
+          add_joins!(sql, options)
           add_conditions!(sql, options[:conditions])
-          sql << "ORDER BY #{options[:order]} " if options[:order]
+          sql << " ORDER BY #{options[:order]} " if options[:order]
           add_limit!(sql, options)
           sql
         end
@@ -752,11 +768,19 @@ module ActiveRecord #:nodoc:
         def add_limit!(sql, options)
           connection.add_limit_offset!(sql, options)
         end
-
+        
+        def add_joins!(sql, options)
+          join = scope_constrains[:joins] || options[:joins]
+          sql << " #{join} " if join
+        end
+        
         # Adds a sanitized version of +conditions+ to the +sql+ string. Note that it's the passed +sql+ string is changed.
-        def add_conditions!(sql, conditions)
-          sql << "WHERE #{sanitize_sql(conditions)} " unless conditions.nil?
-          sql << (conditions.nil? ? "WHERE " : " AND ") + type_condition unless descends_from_active_record?
+        def add_conditions!(sql, conditions)          
+          condition_segments = [scope_constrains[:conditions]]
+          condition_segments << sanitize_sql(conditions) unless conditions.nil?
+          condition_segments << type_condition unless descends_from_active_record?        
+          condition_segments.compact!  
+          sql << "WHERE #{condition_segments.join(" AND ")} " unless condition_segments.empty?
         end
 
         def type_condition
@@ -789,7 +813,7 @@ module ActiveRecord #:nodoc:
             attributes.each { |attr_name| super unless column_methods_hash.include?(attr_name.to_sym) }
 
             attr_index = -1
-            conditions = attributes.collect { |attr_name| attr_index += 1; "#{attr_name} #{attribute_condition(arguments[attr_index])} " }.join(" AND ")
+            conditions = attributes.collect { |attr_name| attr_index += 1; "#{table_name}.#{attr_name} #{attribute_condition(arguments[attr_index])} " }.join(" AND ")
 
             if arguments[attributes.length].is_a?(Hash)
               find(finder, { :conditions => [conditions, *arguments[0...attributes.length]] }.update(arguments[attributes.length]))
@@ -840,8 +864,18 @@ module ActiveRecord #:nodoc:
           @@subclasses[self] ||= []
           @@subclasses[self] + extra = @@subclasses[self].inject([]) {|list, subclass| list + subclass.subclasses }
         end
-
-        # Returns the class type of the record using the current module as a prefix. So descendents of
+        
+        def scope_constrains
+          Thread.current[:constrains] ||= {}
+          Thread.current[:constrains][self] ||= {}
+        end
+        
+        def scope_constrains=(value)
+          Thread.current[:constrains] ||= {}
+          Thread.current[:constrains][self] = value
+        end
+        
+         # Returns the class type of the record using the current module as a prefix. So descendents of
         # MyApp::Business::Account would be appear as MyApp::Business::AccountSubclass.
         def compute_type(type_name)
           type_name_with_module(type_name).split("::").inject(Object) do |final_type, part|
diff --git a/activerecord/test/conditions_scoping_test.rb b/activerecord/test/conditions_scoping_test.rb
new file mode 100644
index 0000000000..8ff708658c
--- /dev/null
+++ b/activerecord/test/conditions_scoping_test.rb
@@ -0,0 +1,137 @@
+require 'abstract_unit'
+require 'fixtures/developer'
+require 'fixtures/comment'
+require 'fixtures/post'
+require 'fixtures/category'
+
+class ConditionsScopingTest < Test::Unit::TestCase
+  fixtures :developers
+  
+  def test_set_conditions
+    Developer.constrain(:conditions => 'just a test...') do
+      assert_equal 'just a test...', Thread.current[:constrains][Developer][:conditions]
+    end
+  end
+
+  def test_scoped_find
+    Developer.constrain(:conditions => "name = 'David'") do
+      assert_nothing_raised { Developer.find(1) }
+    end
+  end
+  
+  def test_scoped_find_first
+    Developer.constrain(:conditions => "salary = 100000") do
+      assert_equal Developer.find(10), Developer.find(:first, :order => 'name')
+    end
+  end
+  
+  def test_scoped_find_all
+    Developer.constrain(:conditions => "name = 'David'") do
+      assert_equal [Developer.find(1)], Developer.find(:all)
+      assert_equal [Developer.find(1)], Developer.find(:all, :condtions => '1 = 2')
+    end      
+  end
+  
+  def test_scoped_count
+    Developer.constrain(:conditions => "name = 'David'") do
+      assert_equal 1, Developer.count
+    end        
+
+    Developer.constrain(:conditions => 'salary = 100000') do
+      assert_equal 8, Developer.count
+      assert_equal 1, Developer.count("name LIKE 'fixture_1%'")
+    end        
+  end
+end
+
+class HasManyScopingTest< Test::Unit::TestCase
+  fixtures :comments, :posts
+  
+  def setup
+    @welcome = Post.find(1)
+  end
+  
+  def test_forwarding_of_static_methods
+    assert_equal 'a comment...', Comment.what_are_you
+    assert_equal 'a comment...', @welcome.comments.what_are_you
+  end
+
+  def test_forwarding_to_scoped
+    assert_equal 4, Comment.search_by_type('Comment').size
+    assert_equal 2, @welcome.comments.search_by_type('Comment').size
+  end
+  
+  def test_forwarding_to_dynamic_finders
+    assert_equal 4, Comment.find_all_by_type('Comment').size
+    assert_equal 2, @welcome.comments.find_all_by_type('Comment').size
+  end
+  
+end
+
+
+class HasAndBelongsToManyScopingTest< Test::Unit::TestCase
+  fixtures :posts, :categories
+
+  def setup
+    @welcome = Post.find(1)
+  end
+
+  def test_forwarding_of_static_methods
+    assert_equal 'a category...', Category.what_are_you
+    assert_equal 'a category...', @welcome.categories.what_are_you
+  end
+
+  def test_forwarding_to_dynamic_finders
+    assert_equal 1, Category.find_all_by_type('SpecialCategory').size
+    assert_equal 0, @welcome.categories.find_all_by_type('SpecialCategory').size
+    assert_equal 2, @welcome.categories.find_all_by_type('Category').size
+  end
+
+end
+
+
+=begin
+# We disabled the scoping for has_one and belongs_to as we can't think of a proper use case
+
+
+class BelongsToScopingTest< Test::Unit::TestCase
+  fixtures :comments, :posts
+
+  def setup
+    @greetings = Comment.find(1)
+  end
+
+  def test_forwarding_of_static_method
+    assert_equal 'a post...', Post.what_are_you
+    assert_equal 'a post...', @greetings.post.what_are_you
+  end
+
+  def test_forwarding_to_dynamic_finders
+    assert_equal 4, Post.find_all_by_type('Post').size
+    assert_equal 1, @greetings.post.find_all_by_type('Post').size
+  end
+
+end
+
+
+class HasOneScopingTest< Test::Unit::TestCase
+  fixtures :comments, :posts
+
+  def setup
+    @sti_comments = Post.find(4)
+  end
+
+  def test_forwarding_of_static_methods
+    assert_equal 'a comment...', Comment.what_are_you
+    assert_equal 'a very special comment...', @sti_comments.very_special_comment.what_are_you
+  end
+
+  def test_forwarding_to_dynamic_finders
+    assert_equal 1, Comment.find_all_by_type('VerySpecialComment').size
+    assert_equal 1, @sti_comments.very_special_comment.find_all_by_type('VerySpecialComment').size
+    assert_equal 0, @sti_comments.very_special_comment.find_all_by_type('Comment').size
+  end
+
+end
+
+=end
\ No newline at end of file
diff --git a/activerecord/test/fixtures/category.rb b/activerecord/test/fixtures/category.rb
index 822defa03e..880eb1573d 100644
--- a/activerecord/test/fixtures/category.rb
+++ b/activerecord/test/fixtures/category.rb
@@ -1,5 +1,15 @@
 class Category < ActiveRecord::Base
   has_and_belongs_to_many :posts
+  
+  def self.what_are_you
+    'a category...'
+  end
 end
 
-class SpecialCategory < Category; end;
+class SpecialCategory < Category
+  
+  def self.what_are_you
+    'a special category...'
+  end  
+  
+end
diff --git a/activerecord/test/fixtures/comment.rb b/activerecord/test/fixtures/comment.rb
index 982cbc6a7a..0605fd7046 100644
--- a/activerecord/test/fixtures/comment.rb
+++ b/activerecord/test/fixtures/comment.rb
@@ -1,7 +1,27 @@
 class Comment < ActiveRecord::Base
   belongs_to :post
+  
+  def self.what_are_you
+    'a comment...'
+  end
+  
+  def self.search_by_type(q)
+    self.find(:all, :conditions => ['type = ?', q])
+  end
 end
 
-class SpecialComment < Comment; end;
+class SpecialComment < Comment; 
 
-class VerySpecialComment < Comment; end;
+  def self.what_are_you
+    'a special comment...'
+  end
+  
+end;
+
+class VerySpecialComment < Comment; 
+  
+  def self.what_are_you
+    'a very special comment...'
+  end
+  
+end;
diff --git a/activerecord/test/fixtures/post.rb b/activerecord/test/fixtures/post.rb
index e347d94fb8..f5adac41dc 100644
--- a/activerecord/test/fixtures/post.rb
+++ b/activerecord/test/fixtures/post.rb
@@ -5,6 +5,10 @@ class Post < ActiveRecord::Base
   has_many   :special_comments, :class_name => "SpecialComment"
   has_and_belongs_to_many :categories
   has_and_belongs_to_many :special_categories, :join_table => "categories_posts"
+  
+  def self.what_are_you
+    'a post...'
+  end
 end
 
 class SpecialPost < Post; end;