From c2fc8626d2b2f8162e757e79065d750ab0417377 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Mon, 14 Dec 2020 14:26:42 +0100 Subject: [PATCH] compute_class raise when klass is not ActiveRecord::Base subclass Changes `ActiveRecord::Reflection#compute_class` in order to raise an `ArgumentError` when the computed class is not a subclass of `ActiveRecord::Base`. Raising a meaningful error ease the user interaction in the following scenarios: - Rails couldn't compute correctly the `class_name` - User sets as `:class_name` an invalid class Closes #15811. --- activerecord/lib/active_record/reflection.rb | 11 ++++++++- activerecord/test/cases/reflection_test.rb | 14 +++++++++++ .../test/models/user_with_invalid_relation.rb | 24 +++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 activerecord/test/models/user_with_invalid_relation.rb diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 9ecda5859e..67ee8d6206 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -406,7 +406,16 @@ module ActiveRecord if polymorphic? raise ArgumentError, "Polymorphic associations do not support computing the class." end - active_record.send(:compute_type, name) + + active_record.send(:compute_type, name).tap do |klass| + 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 + end + end end attr_reader :type, :foreign_type diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index f847c76d90..5f744fd36f 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -26,6 +26,7 @@ require "models/department" require "models/cake_designer" require "models/drink_designer" require "models/recipe" +require "models/user_with_invalid_relation" class ReflectionTest < ActiveRecord::TestCase include ActiveRecord::Reflection @@ -137,6 +138,19 @@ class ReflectionTest < ActiveRecord::TestCase assert_equal "PluralIrregular", reflection.class_name end + def test_reflection_klass_is_not_ar_subclass + [:account_invalid, + :account_class_name, + :info_invalids, + :infos_class_name, + :infos_through_class_name, + ].each do |rel| + assert_raise(ArgumentError) do + UserWithInvalidRelation.reflect_on_association(rel).klass + end + end + end + def test_aggregation_reflection reflection_for_address = AggregateReflection.new( :address, nil, { mapping: [ %w(address_street street), %w(address_city city), %w(address_country country) ] }, Customer diff --git a/activerecord/test/models/user_with_invalid_relation.rb b/activerecord/test/models/user_with_invalid_relation.rb new file mode 100644 index 0000000000..5299540793 --- /dev/null +++ b/activerecord/test/models/user_with_invalid_relation.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +class UserWithInvalidRelation < ActiveRecord::Base + has_one :account_invalid + + has_one :account_class_name, class_name: "AccountInvalid" + + has_many :user_info_invalid + has_many :info_invalids, through: :user_info_invalid + + has_many :infos_class_name, through: :user_info, class_name: "InfoInvalid" + + has_many :user_infos_class_name, class_name: "UserInfoInvalid" + has_many :infos_through_class_name, through: :user_infos_class_name, class_name: "InfoInvalid" +end + +class AccountInvalid; end + +class InfoInvalid; end + +class UserInfoInvalid < ActiveRecord::Base + belongs_to :info_invalid + belongs_to :user_invalid +end