mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Ensure association target classes exist
Fixes https://github.com/rails/rails/issues/42122 Currently if you call a reader on a singular association, no error is raised if the target class doesn't exist (it just returns `nil`). If you call a reader on a collection association an error is raised, but the error appears to not be intended for this case and doesn't have a very good message. `ActiveRecord::Reflection::AssociationReflection#compute_class` appears to be set up to handle this, but it is not being called correctly. So this PR just makes a call to that (via `reflection.klass`) from `SingularAssociation#reader` and `CollectionAssociation#reader`.
This commit is contained in:
parent
ab13f9549d
commit
db55159de1
5 changed files with 46 additions and 6 deletions
|
@ -215,6 +215,12 @@ module ActiveRecord
|
||||||
end
|
end
|
||||||
|
|
||||||
private
|
private
|
||||||
|
# Reader and writer methods call this so that consistent errors are presented
|
||||||
|
# when the association target class does not exist.
|
||||||
|
def ensure_klass_exists!
|
||||||
|
klass
|
||||||
|
end
|
||||||
|
|
||||||
def find_target
|
def find_target
|
||||||
if violates_strict_loading? && owner.validation_context.nil?
|
if violates_strict_loading? && owner.validation_context.nil?
|
||||||
Base.strict_loading_violation!(owner: owner.class, reflection: reflection)
|
Base.strict_loading_violation!(owner: owner.class, reflection: reflection)
|
||||||
|
|
|
@ -30,6 +30,8 @@ module ActiveRecord
|
||||||
class CollectionAssociation < Association #:nodoc:
|
class CollectionAssociation < Association #:nodoc:
|
||||||
# Implements the reader method, e.g. foo.items for Foo.has_many :items
|
# Implements the reader method, e.g. foo.items for Foo.has_many :items
|
||||||
def reader
|
def reader
|
||||||
|
ensure_klass_exists!
|
||||||
|
|
||||||
if stale_target?
|
if stale_target?
|
||||||
reload
|
reload
|
||||||
end
|
end
|
||||||
|
|
|
@ -5,6 +5,8 @@ module ActiveRecord
|
||||||
class SingularAssociation < Association #:nodoc:
|
class SingularAssociation < Association #:nodoc:
|
||||||
# Implements the reader method, e.g. foo.bar for Foo.has_one :bar
|
# Implements the reader method, e.g. foo.bar for Foo.has_one :bar
|
||||||
def reader
|
def reader
|
||||||
|
ensure_klass_exists!
|
||||||
|
|
||||||
if !loaded? || stale_target?
|
if !loaded? || stale_target?
|
||||||
reload
|
reload
|
||||||
end
|
end
|
||||||
|
|
|
@ -413,14 +413,22 @@ module ActiveRecord
|
||||||
raise ArgumentError, "Polymorphic associations do not support computing the class."
|
raise ArgumentError, "Polymorphic associations do not support computing the class."
|
||||||
end
|
end
|
||||||
|
|
||||||
active_record.send(:compute_type, name).tap do |klass|
|
msg = <<-MSG.squish
|
||||||
unless klass < ActiveRecord::Base
|
|
||||||
raise ArgumentError, <<-MSG.squish
|
|
||||||
Rails couldn't find a valid model for #{name} association.
|
Rails couldn't find a valid model for #{name} association.
|
||||||
Please provide the :class_name option on the association declaration.
|
Please provide the :class_name option on the association declaration.
|
||||||
If :class_name is already provided make sure is an ActiveRecord::Base subclass.
|
If :class_name is already provided make sure is an ActiveRecord::Base subclass.
|
||||||
MSG
|
MSG
|
||||||
|
|
||||||
|
begin
|
||||||
|
klass = active_record.send(:compute_type, name)
|
||||||
|
|
||||||
|
unless klass < ActiveRecord::Base
|
||||||
|
raise ArgumentError, msg
|
||||||
end
|
end
|
||||||
|
|
||||||
|
klass
|
||||||
|
rescue NameError
|
||||||
|
raise NameError, msg
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -359,6 +359,28 @@ class OverridingAssociationsTest < ActiveRecord::TestCase
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
class ModelAssociatedToClassesThatDoNotExist < ActiveRecord::Base
|
||||||
|
self.table_name = "accounts" # this is just to avoid adding a new model just for this test
|
||||||
|
|
||||||
|
has_one :non_existent_has_one_class
|
||||||
|
belongs_to :non_existent_belongs_to_class
|
||||||
|
has_many :non_existent_has_many_classes
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_associations_raise_with_name_error_if_associated_to_classes_that_do_not_exist
|
||||||
|
assert_raises NameError do
|
||||||
|
ModelAssociatedToClassesThatDoNotExist.new.non_existent_has_one_class
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_raises NameError do
|
||||||
|
ModelAssociatedToClassesThatDoNotExist.new.non_existent_belongs_to_class
|
||||||
|
end
|
||||||
|
|
||||||
|
assert_raises NameError do
|
||||||
|
ModelAssociatedToClassesThatDoNotExist.new.non_existent_has_many_classes
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
class PreloaderTest < ActiveRecord::TestCase
|
class PreloaderTest < ActiveRecord::TestCase
|
||||||
|
|
Loading…
Reference in a new issue