diff options
| author | Pratik Naik <pratiknaik@gmail.com> | 2008-10-16 22:20:32 +0200 |
|---|---|---|
| committer | Pratik Naik <pratiknaik@gmail.com> | 2008-10-16 22:20:32 +0200 |
| commit | 0242d817035e60357e559af0e9441a1f24a764e4 (patch) | |
| tree | 1efa579fc261e55f76bacfa9818eb4a649d68294 /activerecord | |
| parent | 7e2777348b88bf096ebc51e364554af19d7bcc55 (diff) | |
| parent | 9cb5400871b660e2c6d1654346650f07bb52a0c0 (diff) | |
| download | rails-0242d817035e60357e559af0e9441a1f24a764e4.tar.gz rails-0242d817035e60357e559af0e9441a1f24a764e4.tar.bz2 rails-0242d817035e60357e559af0e9441a1f24a764e4.zip | |
Merge with mainstream rails
Diffstat (limited to 'activerecord')
16 files changed, 136 insertions, 52 deletions
diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 99e3973830..6e194ab9b4 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -193,6 +193,7 @@ module ActiveRecord end def preload_has_one_association(records, reflection, preload_options={}) + return if records.first.send("loaded_#{reflection.name}?") id_to_record_map, ids = construct_id_map(records) options = reflection.options records.each {|record| record.send("set_#{reflection.name}_target", nil)} @@ -214,6 +215,7 @@ module ActiveRecord end def preload_has_many_association(records, reflection, preload_options={}) + return if records.first.send(reflection.name).loaded? options = reflection.options primary_key_name = reflection.through_reflection_primary_key_name @@ -271,6 +273,7 @@ module ActiveRecord end def preload_belongs_to_association(records, reflection, preload_options={}) + return if records.first.send("loaded_#{reflection.name}?") options = reflection.options primary_key_name = reflection.primary_key_name diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index d1a0b2f96a..ad093d83d4 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1235,7 +1235,7 @@ module ActiveRecord association = instance_variable_get(ivar) if instance_variable_defined?(ivar) - if association.nil? || !association.loaded? || force_reload + if association.nil? || force_reload association = association_proxy_class.new(self, reflection) retval = association.reload if retval.nil? and association_proxy_class == BelongsToAssociation @@ -1248,6 +1248,11 @@ module ActiveRecord association.target.nil? ? nil : association end + define_method("loaded_#{reflection.name}?") do + association = instance_variable_get(ivar) if instance_variable_defined?(ivar) + association && association.loaded? + end + define_method("#{reflection.name}=") do |new_value| association = instance_variable_get(ivar) if instance_variable_defined?(ivar) @@ -1264,17 +1269,6 @@ module ActiveRecord end end - if association_proxy_class == BelongsToAssociation - define_method("#{reflection.primary_key_name}=") do |target_id| - if instance_variable_defined?(ivar) - if association = instance_variable_get(ivar) - association.reset - end - end - write_attribute(reflection.primary_key_name, target_id) - end - end - define_method("set_#{reflection.name}_target") do |target| return if target.nil? and association_proxy_class == BelongsToAssociation association = association_proxy_class.new(self, reflection) diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 463de9d819..09a80be266 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -320,6 +320,10 @@ module ActiveRecord exists?(record) end + def proxy_respond_to?(method) + super || @reflection.klass.respond_to?(method) + end + protected def construct_find_options!(options) end diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index b617147f67..d1a79df6e6 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -140,6 +140,15 @@ module ActiveRecord @target.inspect end + def send(method, *args) + if proxy_respond_to?(method) + super + else + load_target + @target.send(method, *args) + end + end + protected # Does the association have a <tt>:dependent</tt> option? def dependent? @@ -197,6 +206,8 @@ module ActiveRecord # Forwards any missing method call to the \target. def method_missing(method, *args) if load_target + raise NoMethodError unless @target.respond_to?(method) + if block_given? @target.send(method, *args) { |*block_args| yield(*block_args) } else diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index c92ef5c2c9..960323004d 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -57,7 +57,7 @@ module ActiveRecord protected def owner_quoted_id if @reflection.options[:primary_key] - quote_value(@owner.send(@reflection.options[:primary_key])) + @owner.class.quote_value(@owner.send(@reflection.options[:primary_key])) else @owner.quoted_id end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 10dc1a81f3..97c6cd4331 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -120,10 +120,6 @@ module ActiveRecord sql end - def sanitize_limit(limit) - limit.to_s[/,/] ? limit.split(',').map{ |i| i.to_i }.join(',') : limit.to_i - end - # Appends a locking clause to an SQL statement. # This method *modifies* the +sql+ parameter. # # SELECT * FROM suppliers FOR UPDATE @@ -185,6 +181,21 @@ module ActiveRecord def delete_sql(sql, name = nil) update_sql(sql, name) end + + # Sanitizes the given LIMIT parameter in order to prevent SQL injection. + # + # +limit+ may be anything that can evaluate to a string via #to_s. It + # should look like an integer, or a comma-delimited list of integers. + # + # Returns the sanitized limit parameter, either as an integer, or as a + # string which contains a comma-delimited list of integers. + def sanitize_limit(limit) + if limit.to_s =~ /,/ + limit.to_s.split(',').map{ |i| i.to_i }.join(',') + else + limit.to_i + end + end end end end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 37b6836a89..40a8503980 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -47,19 +47,6 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal apple.id, citibank.firm_id end - def test_foreign_key_assignment - # Test using an existing record - signals37 = accounts(:signals37) - assert_equal companies(:first_firm), signals37.firm - signals37.firm_id = companies(:another_firm).id - assert_equal companies(:another_firm), signals37.firm - - # Test using a new record - account = Account.new - account.firm_id = companies(:another_firm).id - assert_equal companies(:another_firm), account.firm - end - def test_no_unexpected_aliasing first_firm = companies(:first_firm) another_firm = companies(:another_firm) @@ -441,4 +428,14 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert log.valid? assert log.save end + + def test_belongs_to_proxy_should_not_respond_to_private_methods + assert_raises(NoMethodError) { companies(:first_firm).private_method } + assert_raises(NoMethodError) { companies(:second_client).firm.private_method } + end + + def test_belongs_to_proxy_should_respond_to_private_methods_via_send + companies(:first_firm).send(:private_method) + companies(:second_client).firm.send(:private_method) + end end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 7f42577ab0..5f43975d5a 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -18,7 +18,7 @@ require 'models/developer' require 'models/project' class EagerAssociationTest < ActiveRecord::TestCase - fixtures :posts, :comments, :authors, :categories, :categories_posts, + fixtures :posts, :comments, :authors, :author_addresses, :categories, :categories_posts, :companies, :accounts, :tags, :taggings, :people, :readers, :owners, :pets, :author_favorites, :jobs, :references, :subscribers, :subscriptions, :books, :developers, :projects, :developers_projects @@ -111,6 +111,46 @@ class EagerAssociationTest < ActiveRecord::TestCase end end + def test_finding_with_includes_on_has_many_association_with_same_include_includes_only_once + author_id = authors(:david).id + author = assert_queries(3) { Author.find(author_id, :include => {:posts_with_comments => :comments}) } # find the author, then find the posts, then find the comments + author.posts_with_comments.each do |post_with_comments| + assert_equal post_with_comments.comments.length, post_with_comments.comments.count + assert_equal nil, post_with_comments.comments.uniq! + end + end + + def test_finding_with_includes_on_has_one_assocation_with_same_include_includes_only_once + author = authors(:david) + post = author.post_about_thinking_with_last_comment + last_comment = post.last_comment + author = assert_queries(3) { Author.find(author.id, :include => {:post_about_thinking_with_last_comment => :last_comment})} # find the author, then find the posts, then find the comments + assert_no_queries do + assert_equal post, author.post_about_thinking_with_last_comment + assert_equal last_comment, author.post_about_thinking_with_last_comment.last_comment + end + end + + def test_finding_with_includes_on_belongs_to_association_with_same_include_includes_only_once + post = posts(:welcome) + author = post.author + author_address = author.author_address + post = assert_queries(3) { Post.find(post.id, :include => {:author_with_address => :author_address}) } # find the post, then find the author, then find the address + assert_no_queries do + assert_equal author, post.author_with_address + assert_equal author_address, post.author_with_address.author_address + end + end + + def test_finding_with_includes_on_null_belongs_to_association_with_same_include_includes_only_once + post = posts(:welcome) + post.update_attributes!(:author => nil) + post = assert_queries(2) { Post.find(post.id, :include => {:author_with_address => :author_address}) } # find the post, then find the author which is null so no query for the address + assert_no_queries do + assert_equal nil, post.author_with_address + end + end + def test_loading_from_an_association posts = authors(:david).posts.find(:all, :include => :comments, :order => "posts.id") assert_equal 2, posts.first.comments.size diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 1bc9c39c19..dada2d1603 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1080,5 +1080,9 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end end + def test_sending_new_to_association_proxy_should_have_same_effect_as_calling_new + clients_assoc = companies(:first_firm).clients + assert_equal clients_assoc.new.attributes, clients_assoc.send(:new).attributes + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index ec06be5eba..14032a67c0 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -349,4 +349,14 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert companies(:first_firm).readonly_account.readonly? end + def test_has_one_proxy_should_not_respond_to_private_methods + assert_raises(NoMethodError) { accounts(:signals37).private_method } + assert_raises(NoMethodError) { companies(:first_firm).account.private_method } + end + + def test_has_one_proxy_should_respond_to_private_methods_via_send + accounts(:signals37).send(:private_method) + companies(:first_firm).account.send(:private_method) + end + end diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index 77e3cb1776..ff4021fe02 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -110,4 +110,14 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase new_member.club = new_club = Club.create(:name => "LRUG") assert_equal new_club, new_member.club.target end + + def test_has_one_through_proxy_should_not_respond_to_private_methods + assert_raises(NoMethodError) { clubs(:moustache_club).private_method } + assert_raises(NoMethodError) { @member.club.private_method } + end + + def test_has_one_through_proxy_should_respond_to_private_methods_via_send + clubs(:moustache_club).send(:private_method) + @member.club.send(:private_method) + end end diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index cde451de0e..056a29438a 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -182,26 +182,6 @@ class AssociationProxyTest < ActiveRecord::TestCase assert_nil p.author.reset end - def test_reset_loads_association_next_time - welcome = posts(:welcome) - david = authors(:david) - author_assoc = welcome.author - - assert_equal david, welcome.author # So we can be sure the test works correctly - author_assoc.reset - assert !author_assoc.loaded? - assert_nil author_assoc.target - assert_equal david, welcome.author - end - - def test_assigning_association_id_after_reload - welcome = posts(:welcome) - welcome.reload - assert_nothing_raised do - welcome.author_id = authors(:david).id - end - end - def test_reload_returns_assocition david = developers(:david) assert_nothing_raised do diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 37551c8157..e5b19ff9e4 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -17,6 +17,8 @@ class Author < ActiveRecord::Base proxy_target end end + has_one :post_about_thinking, :class_name => 'Post', :conditions => "posts.title like '%thinking%'" + has_one :post_about_thinking_with_last_comment, :class_name => 'Post', :conditions => "posts.title like '%thinking%'", :include => :last_comment has_many :comments, :through => :posts has_many :comments_containing_the_letter_e, :through => :posts, :source => :comments has_many :comments_with_order_and_conditions, :through => :posts, :source => :comments, :order => 'comments.body', :conditions => "comments.body like 'Thank%'" diff --git a/activerecord/test/models/club.rb b/activerecord/test/models/club.rb index 3ddb691dfb..6e7cdd643a 100644 --- a/activerecord/test/models/club.rb +++ b/activerecord/test/models/club.rb @@ -4,4 +4,10 @@ class Club < ActiveRecord::Base has_many :current_memberships has_one :sponsor has_one :sponsored_member, :through => :sponsor, :source => :sponsorable, :source_type => "Member" + + private + + def private_method + "I'm sorry sir, this is a *private* club, not a *pirate* club" + end end
\ No newline at end of file diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 0eb8ae0a15..62d20861f3 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -13,6 +13,12 @@ class Company < AbstractCompany def arbitrary_method "I am Jack's profound disappointment" end + + private + + def private_method + "I am Jack's innermost fears and aspirations" + end end module Namespaced @@ -129,9 +135,14 @@ class Account < ActiveRecord::Base true end - protected def validate errors.add_on_empty "credit_limit" end + + private + + def private_method + "Sir, yes sir!" + end end diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 6da37c31ff..e0d8be676a 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -13,6 +13,7 @@ class Post < ActiveRecord::Base end belongs_to :author_with_posts, :class_name => "Author", :foreign_key => :author_id, :include => :posts + belongs_to :author_with_address, :class_name => "Author", :foreign_key => :author_id, :include => :author_address has_one :last_comment, :class_name => 'Comment', :order => 'id desc' |