From 273b21faa911681ed4b6c748676146e0f6eed0a0 Mon Sep 17 00:00:00 2001 From: Rick Olson Date: Fri, 21 Mar 2008 18:09:03 +0000 Subject: [PATCH] Add has_one :through support, finally. Closes #4756 [thechrisoshow] git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@9067 5ecf4fe2-1ee6-0310-87b1-e25e094e27de --- activerecord/CHANGELOG | 2 + .../lib/active_record/association_preload.rb | 34 ++++++++-- .../lib/active_record/associations.rb | 58 ++++++++++++----- .../has_one_through_association.rb | 28 +++++++++ .../cases/associations/join_model_test.rb | 2 +- activerecord/test/cases/associations_test.rb | 63 ++++++++++++++++++- activerecord/test/fixtures/clubs.yml | 6 ++ activerecord/test/fixtures/members.yml | 4 ++ activerecord/test/fixtures/memberships.yml | 20 ++++++ activerecord/test/fixtures/sponsors.yml | 3 + activerecord/test/models/club.rb | 6 ++ activerecord/test/models/member.rb | 9 +++ activerecord/test/models/membership.rb | 9 +++ activerecord/test/models/sponsor.rb | 4 ++ activerecord/test/schema/schema.rb | 21 +++++++ 15 files changed, 246 insertions(+), 23 deletions(-) create mode 100644 activerecord/lib/active_record/associations/has_one_through_association.rb create mode 100644 activerecord/test/fixtures/clubs.yml create mode 100644 activerecord/test/fixtures/members.yml create mode 100644 activerecord/test/fixtures/memberships.yml create mode 100644 activerecord/test/fixtures/sponsors.yml create mode 100644 activerecord/test/models/club.rb create mode 100644 activerecord/test/models/member.rb create mode 100644 activerecord/test/models/membership.rb create mode 100644 activerecord/test/models/sponsor.rb diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index be8765366c..b85cccc78e 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Add has_one :through support. #4756 [thechrisoshow] + * Migrations: create_table supports primary_key_prefix_type. #10314 [student, thechrisoshow] * Added logging for dependency load errors with fixtures #11056 [stuthulhu] diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index cc657dc433..0c31c85bb0 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -48,6 +48,14 @@ module ActiveRecord association_proxy.target.push(*[associated_record].flatten) end end + + def add_preloaded_record_to_collection(parent_records, reflection_name, associated_record) + parent_records.each do |parent_record| + association_proxy = parent_record.send(reflection_name) + association_proxy.loaded + association_proxy.target = associated_record + end + end def set_association_collection_records(id_to_record_map, reflection_name, associated_records, key) associated_records.each do |associated_record| @@ -97,11 +105,27 @@ module ActiveRecord end def preload_has_one_association(records, reflection, preload_options={}) - id_to_record_map, ids = construct_id_map(records) - records.each {|record| record.send("set_#{reflection.name}_target", nil)} + id_to_record_map, ids = construct_id_map(records) + options = reflection.options + if options[:through] + records.each {|record| record.send(reflection.name) && record.send(reflection.name).loaded} + through_records = preload_through_records(records, reflection, options[:through]) + through_reflection = reflections[options[:through]] + through_primary_key = through_reflection.primary_key_name + unless through_records.empty? + source = reflection.source_reflection.name + through_records.first.class.preload_associations(through_records, source) + through_records.compact.each do |through_record| + add_preloaded_record_to_collection(id_to_record_map[through_record[through_primary_key].to_i], + reflection.name, through_record.send(source)) + end + end + else + records.each {|record| record.send("set_#{reflection.name}_target", nil)} - set_association_single_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), - reflection.primary_key_name) + + set_association_single_records(id_to_record_map, reflection.name, find_associated_records(ids, reflection, preload_options), reflection.primary_key_name) + end end def preload_has_many_association(records, reflection, preload_options={}) @@ -126,7 +150,7 @@ module ActiveRecord reflection.primary_key_name) end end - + def preload_through_records(records, reflection, through_association) through_reflection = reflections[through_association] through_primary_key = through_reflection.primary_key_name diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index d7d5d9b312..0e07ee4913 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -6,6 +6,7 @@ require 'active_record/associations/has_one_association' require 'active_record/associations/has_many_association' require 'active_record/associations/has_many_through_association' require 'active_record/associations/has_and_belongs_to_many_association' +require 'active_record/associations/has_one_through_association' module ActiveRecord class HasManyThroughAssociationNotFoundError < ActiveRecordError #:nodoc: @@ -737,6 +738,12 @@ module ActiveRecord # as the default +foreign_key+. # * :include - specify second-order associations that should be eager loaded when this object is loaded. # * :as: Specifies a polymorphic interface (See #belongs_to). + # * :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 + # 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. # * :readonly - if set to +true+, the associated object is readonly through the association. # # Option examples: @@ -746,27 +753,34 @@ module ActiveRecord # has_one :project_manager, :class_name => "Person", :conditions => "role = 'project_manager'" # has_one :attachment, :as => :attachable # has_one :boss, :readonly => :true + # has_one :club, :through => :membership + # has_one :primary_address, :through => :addressables, :conditions => ["addressable.primary = ?", true], :source => :addressable def has_one(association_id, options = {}) - reflection = create_has_one_reflection(association_id, options) + if options[:through] + reflection = create_has_one_through_reflection(association_id, options) + association_accessor_methods(reflection, ActiveRecord::Associations::HasOneThroughAssociation) + else + reflection = create_has_one_reflection(association_id, options) - ivar = "@#{reflection.name}" + ivar = "@#{reflection.name}" - method_name = "has_one_after_save_for_#{reflection.name}".to_sym - define_method(method_name) do - association = instance_variable_get("#{ivar}") if instance_variable_defined?("#{ivar}") + method_name = "has_one_after_save_for_#{reflection.name}".to_sym + define_method(method_name) do + association = instance_variable_get("#{ivar}") if instance_variable_defined?("#{ivar}") - if !association.nil? && (new_record? || association.new_record? || association["#{reflection.primary_key_name}"] != id) - association["#{reflection.primary_key_name}"] = id - association.save(true) + if !association.nil? && (new_record? || association.new_record? || association["#{reflection.primary_key_name}"] != id) + association["#{reflection.primary_key_name}"] = id + association.save(true) + end end + after_save method_name + + association_accessor_methods(reflection, HasOneAssociation) + association_constructor_method(:build, reflection, HasOneAssociation) + association_constructor_method(:create, reflection, HasOneAssociation) + + configure_dependency_for_has_one(reflection) end - after_save method_name - - association_accessor_methods(reflection, HasOneAssociation) - association_constructor_method(:build, reflection, HasOneAssociation) - association_constructor_method(:create, reflection, HasOneAssociation) - - configure_dependency_for_has_one(reflection) end # Adds the following methods for retrieval and query for a single associated object for which this object holds an id: @@ -1058,7 +1072,12 @@ module ActiveRecord association = association_proxy_class.new(self, reflection) end - association.replace(new_value) + if association_proxy_class == HasOneThroughAssociation + association.create_through_record(new_value) + self.send(reflection.name, new_value) + else + association.replace(new_value) + end instance_variable_set(ivar, new_value.nil? ? nil : association) end @@ -1300,6 +1319,13 @@ 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, :conditions, :order, :include, :dependent, :counter_cache, :extend, :as, :through, :source + ) + create_reflection(:has_one, association_id, options, self) + end def create_belongs_to_reflection(association_id, options) options.assert_valid_keys( diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb new file mode 100644 index 0000000000..ecf9d8208d --- /dev/null +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -0,0 +1,28 @@ +module ActiveRecord + module Associations + class HasOneThroughAssociation < ActiveRecord::Associations::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 +end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 1034cb67e9..3d59b97f70 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -631,7 +631,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert_equal comments.first.post, comments[1].post end end - + private # create dynamic Post models to allow different dependency options def find_post_with_dependency(post_id, association, association_name, dependency) diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 50643066b9..768d2b2600 100755 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -20,6 +20,10 @@ require 'models/parrot' require 'models/pirate' require 'models/treasure' require 'models/price_estimate' +require 'models/club' +require 'models/member' +require 'models/membership' +require 'models/sponsor' class AssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :developers, :projects, :developers_projects, @@ -186,7 +190,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_equal companies(:first_firm).account, Account.find(1) assert_equal Account.find(1).credit_limit, companies(:first_firm).account.credit_limit end - + def test_has_one_cache_nils firm = companies(:another_firm) assert_queries(1) { assert_nil firm.account } @@ -476,6 +480,63 @@ class HasOneAssociationsTest < ActiveRecord::TestCase end +class HasOneThroughAssociationsTest < ActiveRecord::TestCase + fixtures :members, :clubs, :memberships, :sponsors + + def setup + @member = members(:groucho) + end + + def test_has_one_through_with_has_one + assert_equal clubs(:boring_club), @member.club + end + + 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 + 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) + assert_equal 2, 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) + assert_equal 2, members.size + assert_not_nil assert_no_queries {members[0].sponsor_club} + end +end class HasManyAssociationsTest < ActiveRecord::TestCase fixtures :accounts, :companies, :developers, :projects, diff --git a/activerecord/test/fixtures/clubs.yml b/activerecord/test/fixtures/clubs.yml new file mode 100644 index 0000000000..1986d28229 --- /dev/null +++ b/activerecord/test/fixtures/clubs.yml @@ -0,0 +1,6 @@ +boring_club: + name: Banana appreciation society +moustache_club: + name: Moustache and Eyebrow Fancier Club +crazy_club: + name: Skull and bones \ No newline at end of file diff --git a/activerecord/test/fixtures/members.yml b/activerecord/test/fixtures/members.yml new file mode 100644 index 0000000000..67a6cc459a --- /dev/null +++ b/activerecord/test/fixtures/members.yml @@ -0,0 +1,4 @@ +groucho: + name: Groucho Marx +some_other_guy: + name: Englebert Humperdink \ No newline at end of file diff --git a/activerecord/test/fixtures/memberships.yml b/activerecord/test/fixtures/memberships.yml new file mode 100644 index 0000000000..99fbe46d9b --- /dev/null +++ b/activerecord/test/fixtures/memberships.yml @@ -0,0 +1,20 @@ +membership_of_boring_club: + joined_on: <%= 3.weeks.ago.to_s(:db) %> + club: boring_club + member: groucho + favourite: false + type: CurrentMembership + +membership_of_favourite_club: + joined_on: <%= 3.weeks.ago.to_s(:db) %> + club: moustache_club + member: groucho + favourite: true + type: Membership + +other_guys_membership: + joined_on: <%= 4.weeks.ago.to_s(:db) %> + club: boring_club + member: some_other_guy + favourite: false + type: CurrentMembership diff --git a/activerecord/test/fixtures/sponsors.yml b/activerecord/test/fixtures/sponsors.yml new file mode 100644 index 0000000000..c116df0534 --- /dev/null +++ b/activerecord/test/fixtures/sponsors.yml @@ -0,0 +1,3 @@ +moustache_club_sponsor_for_groucho: + sponsor_club: moustache_club + sponsorable: groucho (Member) \ No newline at end of file diff --git a/activerecord/test/models/club.rb b/activerecord/test/models/club.rb new file mode 100644 index 0000000000..171445df3e --- /dev/null +++ b/activerecord/test/models/club.rb @@ -0,0 +1,6 @@ +class Club < ActiveRecord::Base + has_many :memberships + has_many :members, :through => :memberships + has_many :current_memberships + has_many :sponsors +end \ No newline at end of file diff --git a/activerecord/test/models/member.rb b/activerecord/test/models/member.rb new file mode 100644 index 0000000000..688725f200 --- /dev/null +++ b/activerecord/test/models/member.rb @@ -0,0 +1,9 @@ +class Member < ActiveRecord::Base + has_one :current_membership + has_many :memberships + has_many :fellow_members, :through => :club, :source => :members + has_one :club, :through => :current_membership + has_one :favourite_club, :through => :memberships, :conditions => ["memberships.favourite = ?", true], :source => :club + has_one :sponsor, :as => :sponsorable + has_one :sponsor_club, :through => :sponsor +end \ No newline at end of file diff --git a/activerecord/test/models/membership.rb b/activerecord/test/models/membership.rb new file mode 100644 index 0000000000..905f948c37 --- /dev/null +++ b/activerecord/test/models/membership.rb @@ -0,0 +1,9 @@ +class Membership < ActiveRecord::Base + belongs_to :member + belongs_to :club +end + +class CurrentMembership < Membership + belongs_to :member + belongs_to :club +end diff --git a/activerecord/test/models/sponsor.rb b/activerecord/test/models/sponsor.rb new file mode 100644 index 0000000000..50c2c2d76c --- /dev/null +++ b/activerecord/test/models/sponsor.rb @@ -0,0 +1,4 @@ +class Sponsor < ActiveRecord::Base + belongs_to :sponsor_club, :class_name => "Club", :foreign_key => "club_id" + belongs_to :sponsorable, :polymorphic => true +end \ No newline at end of file diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 33aa6e2ea4..724226c359 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -49,6 +49,10 @@ ActiveRecord::Schema.define do t.integer :category_id, :null => false t.integer :post_id, :null => false end + + create_table :clubs, :force => true do |t| + t.string :name + end create_table :colnametests, :force => true do |t| t.integer :references, :null => false @@ -117,6 +121,17 @@ ActiveRecord::Schema.define do t.integer :version, :null => false, :default => 0 end + create_table :members, :force => true do |t| + t.string :name + end + + create_table :memberships, :force => true do |t| + t.datetime :joined_on + t.integer :club_id, :member_id + t.boolean :favourite, :default => false + t.string :type + end + create_table :minimalistics, :force => true do |t| end @@ -177,6 +192,12 @@ ActiveRecord::Schema.define do t.integer :post_id, :null => false t.integer :person_id, :null => false end + + create_table :sponsors, :force => true do |t| + t.integer :club_id + t.integer :sponsorable_id + t.integer :sponsorable_type + end create_table :subscribers, :force => true, :id => false do |t| t.string :nick, :null => false