Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Add inverse polymorphic association support. [#3520 state:resolved]
Signed-off-by: Eloy Duran <eloy.de.enige@gmail.com>
  • Loading branch information
oggy authored and alloy committed Dec 28, 2009
1 parent 6c8c85b commit 81ca0cf
Show file tree
Hide file tree
Showing 9 changed files with 139 additions and 32 deletions.
Expand Up @@ -13,6 +13,7 @@ def replace(record)
@updated = true
end

set_inverse_instance(record, @owner)
loaded
record
end
Expand All @@ -22,19 +23,37 @@ def updated?
end

private

# NOTE - for now, we're only supporting inverse setting from belongs_to back onto
# has_one associations.
def we_can_set_the_inverse_on_this?(record)
@reflection.has_inverse? && @reflection.polymorphic_inverse_of(record.class).macro == :has_one
end

def set_inverse_instance(record, instance)
return if record.nil? || !we_can_set_the_inverse_on_this?(record)
inverse_relationship = @reflection.polymorphic_inverse_of(record.class)
unless inverse_relationship.nil?
record.send(:"set_#{inverse_relationship.name}_target", instance)
end
end

def find_target
return nil if association_class.nil?

if @reflection.options[:conditions]
association_class.find(
@owner[@reflection.primary_key_name],
:select => @reflection.options[:select],
:conditions => conditions,
:include => @reflection.options[:include]
)
else
association_class.find(@owner[@reflection.primary_key_name], :select => @reflection.options[:select], :include => @reflection.options[:include])
end
target =
if @reflection.options[:conditions]
association_class.find(
@owner[@reflection.primary_key_name],
:select => @reflection.options[:select],
:conditions => conditions,
:include => @reflection.options[:include]
)
else
association_class.find(@owner[@reflection.primary_key_name], :select => @reflection.options[:select], :include => @reflection.options[:include])
end
set_inverse_instance(target, @owner) if target
target
end

def foreign_key_present
Expand Down
14 changes: 12 additions & 2 deletions activerecord/lib/active_record/reflection.rb
Expand Up @@ -214,8 +214,10 @@ def check_validity!
end

def check_validity_of_inverse!
if has_inverse? && inverse_of.nil?
raise InverseOfAssociationNotFoundError.new(self)
unless options[:polymorphic]
if has_inverse? && inverse_of.nil?
raise InverseOfAssociationNotFoundError.new(self)
end
end
end

Expand All @@ -242,6 +244,14 @@ def inverse_of
end
end

def polymorphic_inverse_of(associated_class)
if has_inverse?
associated_class.reflect_on_association(options[:inverse_of])
else
nil
end
end

private
def derive_class_name
class_name = name.to_s.camelize
Expand Down
100 changes: 81 additions & 19 deletions activerecord/test/cases/associations/inverse_associations_test.rb
Expand Up @@ -85,7 +85,7 @@ class InverseHasOneTests < ActiveRecord::TestCase
fixtures :men, :faces

def test_parent_instance_should_be_shared_with_child_on_find
m = Man.find(:first)
m = men(:gordon)
f = m.face
assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
m.name = 'Bongo'
Expand All @@ -96,15 +96,15 @@ def test_parent_instance_should_be_shared_with_child_on_find


def test_parent_instance_should_be_shared_with_eager_loaded_child_on_find
m = Man.find(:first, :include => :face)
m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :face)
f = m.face
assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
m.name = 'Bongo'
assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance"
f.man.name = 'Mungo'
assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance"

m = Man.find(:first, :include => :face, :order => 'faces.id')
m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :face, :order => 'faces.id')
f = m.face
assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
m.name = 'Bongo'
Expand All @@ -114,7 +114,7 @@ def test_parent_instance_should_be_shared_with_eager_loaded_child_on_find
end

def test_parent_instance_should_be_shared_with_newly_built_child
m = Man.find(:first)
m = men(:gordon)
f = m.build_face(:description => 'haunted')
assert_not_nil f.man
assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
Expand All @@ -125,7 +125,7 @@ def test_parent_instance_should_be_shared_with_newly_built_child
end

def test_parent_instance_should_be_shared_with_newly_created_child
m = Man.find(:first)
m = men(:gordon)
f = m.create_face(:description => 'haunted')
assert_not_nil f.man
assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance"
Expand Down Expand Up @@ -224,7 +224,7 @@ class InverseHasManyTests < ActiveRecord::TestCase
fixtures :men, :interests

def test_parent_instance_should_be_shared_with_every_child_on_find
m = Man.find(:first)
m = men(:gordon)
is = m.interests
is.each do |i|
assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
Expand All @@ -236,7 +236,7 @@ def test_parent_instance_should_be_shared_with_every_child_on_find
end

def test_parent_instance_should_be_shared_with_eager_loaded_children
m = Man.find(:first, :include => :interests)
m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :interests)
is = m.interests
is.each do |i|
assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
Expand All @@ -246,7 +246,7 @@ def test_parent_instance_should_be_shared_with_eager_loaded_children
assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance"
end

m = Man.find(:first, :include => :interests, :order => 'interests.id')
m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :interests, :order => 'interests.id')
is = m.interests
is.each do |i|
assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
Expand All @@ -255,11 +255,10 @@ def test_parent_instance_should_be_shared_with_eager_loaded_children
i.man.name = 'Mungo'
assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance"
end

end

def test_parent_instance_should_be_shared_with_newly_built_child
m = Man.find(:first)
m = men(:gordon)
i = m.interests.build(:topic => 'Industrial Revolution Re-enactment')
assert_not_nil i.man
assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
Expand All @@ -282,7 +281,7 @@ def test_parent_instance_should_be_shared_with_newly_block_style_built_child
end

def test_parent_instance_should_be_shared_with_newly_created_child
m = Man.find(:first)
m = men(:gordon)
i = m.interests.create(:topic => 'Industrial Revolution Re-enactment')
assert_not_nil i.man
assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance"
Expand Down Expand Up @@ -316,7 +315,7 @@ def test_parent_instance_should_be_shared_with_newly_block_style_created_child
end

def test_parent_instance_should_be_shared_with_poked_in_child
m = Man.find(:first)
m = men(:gordon)
i = Interest.create(:topic => 'Industrial Revolution Re-enactment')
m.interests << i
assert_not_nil i.man
Expand Down Expand Up @@ -360,7 +359,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase
fixtures :men, :faces, :interests

def test_child_instance_should_be_shared_with_parent_on_find
f = Face.find(:first)
f = faces(:trusting)
m = f.man
assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
f.description = 'gormless'
Expand All @@ -370,16 +369,15 @@ def test_child_instance_should_be_shared_with_parent_on_find
end

def test_eager_loaded_child_instance_should_be_shared_with_parent_on_find
f = Face.find(:first, :include => :man)
f = Face.find(:first, :include => :man, :conditions => {:description => 'trusting'})
m = f.man
assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
f.description = 'gormless'
assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance"
m.face.description = 'pleasing'
assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance"


f = Face.find(:first, :include => :man, :order => 'men.id')
f = Face.find(:first, :include => :man, :order => 'men.id', :conditions => {:description => 'trusting'})
m = f.man
assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
f.description = 'gormless'
Expand All @@ -389,7 +387,7 @@ def test_eager_loaded_child_instance_should_be_shared_with_parent_on_find
end

def test_child_instance_should_be_shared_with_newly_built_parent
f = Face.find(:first)
f = faces(:trusting)
m = f.build_man(:name => 'Charles')
assert_not_nil m.face
assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
Expand All @@ -400,7 +398,7 @@ def test_child_instance_should_be_shared_with_newly_built_parent
end

def test_child_instance_should_be_shared_with_newly_created_parent
f = Face.find(:first)
f = faces(:trusting)
m = f.create_man(:name => 'Charles')
assert_not_nil m.face
assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance"
Expand All @@ -411,7 +409,7 @@ def test_child_instance_should_be_shared_with_newly_created_parent
end

def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many
i = Interest.find(:first)
i = interests(:trainspotting)
m = i.man
assert_not_nil m.interests
iz = m.interests.detect {|iz| iz.id == i.id}
Expand Down Expand Up @@ -452,6 +450,70 @@ def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
end
end

class InversePolymorphicBelongsToTests < ActiveRecord::TestCase
fixtures :men, :faces, :interests

def test_child_instance_should_be_shared_with_parent_on_find
f = Face.find(:first, :conditions => {:description => 'confused'})
m = f.polymorphic_man
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same before changes to child instance"
f.description = 'gormless'
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to child instance"
m.polymorphic_face.description = 'pleasing'
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance"
end

def test_eager_loaded_child_instance_should_be_shared_with_parent_on_find
f = Face.find(:first, :conditions => {:description => 'confused'}, :include => :man)
m = f.polymorphic_man
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same before changes to child instance"
f.description = 'gormless'
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to child instance"
m.polymorphic_face.description = 'pleasing'
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance"

f = Face.find(:first, :conditions => {:description => 'confused'}, :include => :man, :order => 'men.id')
m = f.polymorphic_man
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same before changes to child instance"
f.description = 'gormless'
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to child instance"
m.polymorphic_face.description = 'pleasing'
assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance"
end

def test_child_instance_should_be_shared_with_replaced_parent
face = faces(:confused)
old_man = face.polymorphic_man
new_man = Man.new

assert_not_nil face.polymorphic_man
face.polymorphic_man.replace(new_man)

assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same before changes to parent instance"
face.description = 'Bongo'
assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to parent instance"
new_man.polymorphic_face.description = 'Mungo'
assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to replaced-parent-owned instance"
end

def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many
i = interests(:llama_wrangling)
m = i.polymorphic_man
assert_not_nil m.polymorphic_interests
iz = m.polymorphic_interests.detect {|iz| iz.id == i.id}
assert_not_nil iz
assert_equal i.topic, iz.topic, "Interest topics should be the same before changes to child"
i.topic = 'Eating cheese with a spoon'
assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to child"
iz.topic = 'Cow tipping'
assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to parent-owned instance"
end

def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error
assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_man }
end
end

# NOTE - these tests might not be meaningful, ripped as they were from the parental_control plugin
# which would guess the inverse rather than look for an explicit configuration option.
class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase
Expand Down
4 changes: 4 additions & 0 deletions activerecord/test/fixtures/faces.yml
Expand Up @@ -5,3 +5,7 @@ trusting:
weather_beaten:
description: weather beaten
man: steve

confused:
description: confused
polymorphic_man: gordon (Man)
6 changes: 5 additions & 1 deletion activerecord/test/fixtures/interests.yml
Expand Up @@ -23,7 +23,11 @@ woodsmanship:
zine: going_out
man: steve

survial:
survival:
topic: Survival
zine: going_out
man: steve

llama_wrangling:
topic: Llama Wrangling
polymorphic_man: gordon (Man)
1 change: 1 addition & 0 deletions activerecord/test/models/face.rb
@@ -1,5 +1,6 @@
class Face < ActiveRecord::Base
belongs_to :man, :inverse_of => :face
belongs_to :polymorphic_man, :polymorphic => true, :inverse_of => :polymorphic_face
# This is a "broken" inverse_of for the purposes of testing
belongs_to :horrible_man, :class_name => 'Man', :inverse_of => :horrible_face
end
1 change: 1 addition & 0 deletions activerecord/test/models/interest.rb
@@ -1,4 +1,5 @@
class Interest < ActiveRecord::Base
belongs_to :man, :inverse_of => :interests
belongs_to :polymorphic_man, :polymorphic => true, :inverse_of => :polymorphic_interests
belongs_to :zine, :inverse_of => :interests
end
2 changes: 2 additions & 0 deletions activerecord/test/models/man.rb
@@ -1,6 +1,8 @@
class Man < ActiveRecord::Base
has_one :face, :inverse_of => :man
has_one :polymorphic_face, :class_name => 'Face', :as => :polymorphic_man, :inverse_of => :polymorphic_man
has_many :interests, :inverse_of => :man
has_many :polymorphic_interests, :class_name => 'Interest', :as => :polymorphic_man, :inverse_of => :polymorphic_man
# These are "broken" inverse_of associations for the purposes of testing
has_one :dirty_face, :class_name => 'Face', :inverse_of => :dirty_man
has_many :secret_interests, :class_name => 'Interest', :inverse_of => :secret_man
Expand Down
4 changes: 4 additions & 0 deletions activerecord/test/schema/schema.rb
Expand Up @@ -520,11 +520,15 @@ def create_table(*args, &block)
create_table :faces, :force => true do |t|
t.string :description
t.integer :man_id
t.integer :polymorphic_man_id
t.string :polymorphic_man_type
end

create_table :interests, :force => true do |t|
t.string :topic
t.integer :man_id
t.integer :polymorphic_man_id
t.string :polymorphic_man_type
t.integer :zine_id
end

Expand Down

0 comments on commit 81ca0cf

Please sign in to comment.