From 34f9d30e399050e776263441ee1d4415d0b2c254 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 22 Jul 2005 20:05:42 +0000 Subject: 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 --- activerecord/CHANGELOG | 14 +++ .../associations/association_collection.rb | 9 ++ .../associations/association_proxy.rb | 15 +-- .../associations/belongs_to_association.rb | 13 +- .../has_and_belongs_to_many_association.rb | 78 ++++++------ .../associations/has_many_association.rb | 2 +- .../associations/has_one_association.rb | 2 +- activerecord/lib/active_record/base.rb | 56 +++++++-- activerecord/test/conditions_scoping_test.rb | 137 +++++++++++++++++++++ activerecord/test/fixtures/category.rb | 12 +- activerecord/test/fixtures/comment.rb | 24 +++- activerecord/test/fixtures/post.rb | 4 + 12 files changed, 299 insertions(+), 67 deletions(-) create mode 100644 activerecord/test/conditions_scoping_test.rb 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 :conditions and :joins + # + # 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; -- cgit v1.2.3