From 5924911cd3007809767b37b47d3b922c6b24fb22 Mon Sep 17 00:00:00 2001 From: pivotal Date: Fri, 22 Aug 2008 19:17:44 -0700 Subject: [PATCH] HasOneThroughAssociation now correctly sets the association target on assignment; HasOneThroughAssociation now correctly resets target to nil on reset, rather than empty array. --- activerecord/lib/active_record/associations.rb | 41 ++++++++++---------- .../associations/has_one_through_association.rb | 18 +++++--- .../has_one_through_associations_test.rb | 30 ++++++++++----- 3 files changed, 51 insertions(+), 38 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index b9039ce..b6cc668 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -487,14 +487,14 @@ module ActiveRecord # # Since only one table is loaded at a time, conditions or orders cannot reference tables other than the main one. If this is the case # Active Record falls back to the previously used LEFT OUTER JOIN based strategy. For example - # + # # Post.find(:all, :include => [ :author, :comments ], :conditions => ['comments.approved = ?', true]) # # will result in a single SQL query with joins along the lines of: LEFT OUTER JOIN comments ON comments.post_id = posts.id and # LEFT OUTER JOIN authors ON authors.id = posts.author_id. Note that using conditions like this can have unintended consequences. # In the above example posts with no approved comments are not returned at all, because the conditions apply to the SQL statement as a whole # and not just to the association. You must disambiguate column references for this fallback to happen, for example - # :order => "author.name DESC" will work but :order => "name DESC" will not. + # :order => "author.name DESC" will work but :order => "name DESC" will not. # # If you do want eagerload only some members of an association it is usually more natural to :include an association # which has conditions defined on it: @@ -520,10 +520,10 @@ module ActiveRecord # # Address.find(:all, :include => :addressable) # - # will execute one query to load the addresses and load the addressables with one query per addressable type. + # will execute one query to load the addresses and load the addressables with one query per addressable type. # For example if all the addressables are either of class Person or Company then a total of 3 queries will be executed. The list of # addressable types to load is determined on the back of the addresses loaded. This is not supported if Active Record has to fallback - # to the previous implementation of eager loading and will raise ActiveRecord::EagerLoadPolymorphicError. The reason is that the parent + # to the previous implementation of eager loading and will raise ActiveRecord::EagerLoadPolymorphicError. The reason is that the parent # model's type is a column value so its corresponding table name cannot be put in the +FROM+/+JOIN+ clauses of that query. # # == Table Aliasing @@ -842,15 +842,15 @@ module ActiveRecord # but not include the joined columns. Do not forget to include the primary and foreign keys, otherwise it will raise an error. # [:through] # Specifies a Join Model through which to perform the query. Options for :class_name and :foreign_key - # are ignored, as the association uses the source reflection. You can only use a :through query through a + # are ignored, as the association uses the source reflection. You can only use a :through query through a # has_one or belongs_to association on the join model. # [:source] # Specifies the source association name used by has_one :through queries. Only use it if the name cannot be # inferred from the association. has_one :favorite, :through => :favorites will look for a - # :favorite on Favorite, unless a :source is given. + # :favorite on Favorite, unless a :source is given. # [:source_type] # Specifies type of the source association used by has_one :through queries where the source - # association is a polymorphic +belongs_to+. + # association is a polymorphic +belongs_to+. # [:readonly] # If true, the associated object is readonly through the association. # [:validate] @@ -960,7 +960,7 @@ module ActiveRecord # destroyed. This requires that a column named #{table_name}_count (such as +comments_count+ for a belonging Comment class) # is used on the associate class (such as a Post class). You can also specify a custom counter cache column by providing # a column name instead of a +true+/+false+ value to this option (e.g., :counter_cache => :my_custom_counter.) - # When creating a counter cache column, the database statement or migration must specify a default value of 0, failing to do + # When creating a counter cache column, the database statement or migration must specify a default value of 0, failing to do # this results in a counter with +NULL+ value, which will never increment. # Note: Specifying a counter cache will add it to that model's list of readonly attributes using +attr_readonly+. # [:include] @@ -1154,8 +1154,8 @@ module ActiveRecord # the +has_and_belongs_to_many+ association will use "project_id" as the default :association_foreign_key. # [:conditions] # Specify the conditions that the associated object must meet in order to be included as a +WHERE+ - # SQL fragment, such as authorized = 1. Record creations from the association are scoped if a hash is used. - # has_many :posts, :conditions => {:published => true} will create published posts with @blog.posts.create + # SQL fragment, such as authorized = 1. Record creations from the association are scoped if a hash is used. + # has_many :posts, :conditions => {:published => true} will create published posts with @blog.posts.create # or @blog.posts.build. # [:order] # Specify the order in which the associated objects are returned as an ORDER BY SQL fragment, @@ -1270,10 +1270,9 @@ module ActiveRecord association.create_through_record(new_value) self.send(reflection.name, new_value) else - association.replace(new_value) + association.replace(new_value) + instance_variable_set(ivar, new_value.nil? ? nil : association) end - - instance_variable_set(ivar, new_value.nil? ? nil : association) end define_method("set_#{reflection.name}_target") do |target| @@ -1323,7 +1322,7 @@ module ActiveRecord end end end - + def add_single_associated_validation_callbacks(association_name) method_name = "validate_associated_records_for_#{association_name}".to_sym define_method(method_name) do @@ -1332,10 +1331,10 @@ module ActiveRecord errors.add "#{association_name}" unless association.target.nil? || association.valid? end end - + validate method_name end - + def add_multiple_associated_validation_callbacks(association_name) method_name = "validate_associated_records_for_#{association_name}".to_sym ivar = "@#{association_name}" @@ -1529,7 +1528,7 @@ module ActiveRecord create_reflection(:has_one, association_id, options, self) end - + def create_has_one_through_reflection(association_id, options) options.assert_valid_keys( :class_name, :foreign_key, :remote, :select, :conditions, :order, :include, :dependent, :counter_cache, :extend, :as, :through, :source, :source_type, :validate @@ -1969,11 +1968,11 @@ module ActiveRecord @aliased_prefix = "t#{ join_dependency.joins.size }" @parent_table_name = parent.active_record.table_name @aliased_table_name = aliased_table_name_for(table_name) - + if reflection.macro == :has_and_belongs_to_many @aliased_join_table_name = aliased_table_name_for(reflection.options[:join_table], "_join") end - + if reflection.macro == :has_many && reflection.options[:through] @aliased_join_table_name = aliased_table_name_for(reflection.through_reflection.klass.table_name, "_join") end @@ -2110,7 +2109,7 @@ module ActiveRecord end protected - + def aliased_table_name_for(name, suffix = nil) if !parent.table_joins.blank? && parent.table_joins.to_s.downcase =~ %r{join(\s+\w+)?\s+#{name.downcase}\son} @join_dependency.table_aliases[name] += 1 @@ -2128,7 +2127,7 @@ module ActiveRecord name end - + def pluralize(table_name) ActiveRecord::Base.pluralize_table_names ? table_name.to_s.pluralize : table_name end diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb index c846956..915410b 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -1,28 +1,32 @@ module ActiveRecord module Associations class HasOneThroughAssociation < HasManyThroughAssociation - + def create_through_record(new_value) #nodoc: klass = @reflection.through_reflection.klass current_object = @owner.send(@reflection.through_reflection.name) - + if current_object klass.destroy(current_object) @owner.clear_association_cache end - + @owner.send(@reflection.through_reflection.name, klass.send(:create, construct_join_attributes(new_value))) end - + private def find(*args) super(args.merge(:limit => 1)) end - + def find_target super.first - end - end + end + + def reset_target! + @target = nil + end + end 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 3eb66bc..dba5cfb 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -6,7 +6,7 @@ require 'models/sponsor' class HasOneThroughAssociationsTest < ActiveRecord::TestCase fixtures :members, :clubs, :memberships, :sponsors - + def setup @member = members(:groucho) end @@ -18,47 +18,47 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_has_one_through_with_has_many assert_equal clubs(:moustache_club), @member.favourite_club end - + def test_creating_association_creates_through_record new_member = Member.create(:name => "Chris") new_member.club = Club.create(:name => "LRUG") assert_not_nil new_member.current_membership assert_not_nil new_member.club end - + def test_replace_target_record new_club = Club.create(:name => "Marx Bros") @member.club = new_club @member.reload assert_equal new_club, @member.club end - + def test_replacing_target_record_deletes_old_association assert_no_difference "Membership.count" do new_club = Club.create(:name => "Bananarama") @member.club = new_club - @member.reload + @member.reload end end - + def test_has_one_through_polymorphic assert_equal clubs(:moustache_club), @member.sponsor_club end - + def has_one_through_to_has_many assert_equal 2, @member.fellow_members.size end - + def test_has_one_through_eager_loading members = Member.find(:all, :include => :club, :conditions => ["name = ?", "Groucho Marx"]) assert_equal 1, members.size assert_not_nil assert_no_queries {members[0].club} end - + def test_has_one_through_eager_loading_through_polymorphic members = Member.find(:all, :include => :sponsor_club, :conditions => ["name = ?", "Groucho Marx"]) assert_equal 1, members.size - assert_not_nil assert_no_queries {members[0].sponsor_club} + assert_not_nil assert_no_queries {members[0].sponsor_club} end def test_has_one_through_polymorphic_with_source_type @@ -71,4 +71,14 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase assert_not_nil assert_no_queries {clubs[0].sponsored_member} end + def test_uninitialized_has_one_through_should_return_nil_for_unsaved_record + assert_nil Member.new.club + end + + def test_assigning_association_correctly_assigns_target + new_member = Member.create(:name => "Chris") + new_member.club = new_club = Club.create(:name => "LRUG") + assert_equal new_club, new_member.club.target + end + end -- 1.5.6.4