From db55159de1655a258d3e767e6663dc954803b926 Mon Sep 17 00:00:00 2001 From: Alex Ghiculescu Date: Sun, 2 May 2021 11:42:11 -0500 Subject: [PATCH] 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`. --- .../active_record/associations/association.rb | 6 +++++ .../associations/collection_association.rb | 2 ++ .../associations/singular_association.rb | 2 ++ activerecord/lib/active_record/reflection.rb | 20 ++++++++++++----- activerecord/test/cases/associations_test.rb | 22 +++++++++++++++++++ 5 files changed, 46 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 33da92149b..20cdc69fb9 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -215,6 +215,12 @@ module ActiveRecord end 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 if violates_strict_loading? && owner.validation_context.nil? Base.strict_loading_violation!(owner: owner.class, reflection: reflection) diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 29ef11475f..da6c70fe93 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -30,6 +30,8 @@ module ActiveRecord class CollectionAssociation < Association #:nodoc: # Implements the reader method, e.g. foo.items for Foo.has_many :items def reader + ensure_klass_exists! + if stale_target? reload end diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index b7e5987c4b..c754e2d7c5 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -5,6 +5,8 @@ module ActiveRecord class SingularAssociation < Association #:nodoc: # Implements the reader method, e.g. foo.bar for Foo.has_one :bar def reader + ensure_klass_exists! + if !loaded? || stale_target? reload end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index d43aa00da1..d0d9d68aa9 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -413,14 +413,22 @@ module ActiveRecord raise ArgumentError, "Polymorphic associations do not support computing the class." end - active_record.send(:compute_type, name).tap do |klass| + msg = <<-MSG.squish + Rails couldn't find a valid model for #{name} association. + Please provide the :class_name option on the association declaration. + If :class_name is already provided make sure is an ActiveRecord::Base subclass. + MSG + + begin + klass = active_record.send(:compute_type, name) + unless klass < ActiveRecord::Base - raise ArgumentError, <<-MSG.squish - Rails couldn't find a valid model for #{name} association. - Please provide the :class_name option on the association declaration. - If :class_name is already provided make sure is an ActiveRecord::Base subclass. - MSG + raise ArgumentError, msg end + + klass + rescue NameError + raise NameError, msg end end diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 5992f3f181..68571ee7c5 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -359,6 +359,28 @@ class OverridingAssociationsTest < ActiveRecord::TestCase 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 class PreloaderTest < ActiveRecord::TestCase