From ff508640e28914da2b546f6a8c9f215bab201b61 Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Mon, 28 Dec 2009 14:13:33 +0100 Subject: Make polymorphic_inverse_of in Reflection throw an InverseOfAssociationNotFoundError if the supplied class doesn't have the appropriate association. [#3520 state:resolved] Signed-off-by: Eloy Duran --- activerecord/lib/active_record/associations.rb | 4 ++-- activerecord/lib/active_record/reflection.rb | 10 +++++----- .../cases/associations/inverse_associations_test.rb | 17 +++++++++++++++-- activerecord/test/models/face.rb | 3 ++- 4 files changed, 24 insertions(+), 10 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index c23c9f63f1..ff8c63bc91 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -3,8 +3,8 @@ require 'active_support/core_ext/enumerable' module ActiveRecord class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc: - def initialize(reflection) - super("Could not find the inverse association for #{reflection.name} (#{reflection.options[:inverse_of].inspect} in #{reflection.class_name})") + def initialize(reflection, associated_class = nil) + super("Could not find the inverse association for #{reflection.name} (#{reflection.options[:inverse_of].inspect} in #{associated_class.nil? ? reflection.class_name : associated_class.name})") end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 72f7df32c7..b751c9ad68 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -239,16 +239,16 @@ module ActiveRecord def inverse_of if has_inverse? @inverse_of ||= klass.reflect_on_association(options[:inverse_of]) - else - nil end end def polymorphic_inverse_of(associated_class) if has_inverse? - associated_class.reflect_on_association(options[:inverse_of]) - else - nil + if inverse_relationship = associated_class.reflect_on_association(options[:inverse_of]) + inverse_relationship + else + raise InverseOfAssociationNotFoundError.new(self, associated_class) + end end end diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 0696f06e5b..457c4da9bf 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -524,8 +524,21 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase 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 } + def test_trying_to_access_inverses_that_dont_exist_shouldnt_raise_an_error + # Ideally this would, if only for symmetry's sake with other association types + assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_polymorphic_man } + end + + def test_trying_to_set_polymorphic_inverses_that_dont_exist_at_all_should_raise_an_error + # fails because no class has the correct inverse_of for horrible_polymorphic_man + assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_polymorphic_man = Man.first } + end + + def test_trying_to_set_polymorphic_inverses_that_dont_exist_on_the_instance_being_set_should_raise_an_error + # passes because Man does have the correct inverse_of + assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).polymorphic_man = Man.first } + # fails because Interest does have the correct inverse_of + assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).polymorphic_man = Interest.first } end end diff --git a/activerecord/test/models/face.rb b/activerecord/test/models/face.rb index 3e2bdc0307..edb75d333f 100644 --- a/activerecord/test/models/face.rb +++ b/activerecord/test/models/face.rb @@ -1,6 +1,7 @@ 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 + # These is a "broken" inverse_of for the purposes of testing belongs_to :horrible_man, :class_name => 'Man', :inverse_of => :horrible_face + belongs_to :horrible_polymorphic_man, :polymorphic => true, :inverse_of => :horrible_polymorphic_face end -- cgit v1.2.3