From 8312a0d22212798864f142b5a94805e0baa6c562 Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Tue, 3 Jan 2017 10:12:47 -0500 Subject: [PATCH] Deprecate reflection class name to accept a class The idea of `class_name` as an option of reflection is that passing a string would allow us to lazy autoload the class. Using `belongs_to :client, class_name: Customer` is eagerloading models more than necessary and creating possible circular dependencies. --- activerecord/CHANGELOG.md | 5 +++++ activerecord/lib/active_record/reflection.rb | 11 +++++++++++ .../has_and_belongs_to_many_associations_test.rb | 6 ++++-- activerecord/test/cases/reflection_test.rb | 6 ++++++ activerecord/test/models/user.rb | 2 +- 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 97af75546d877..18c9a248b26fb 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* Deprecate passing a class to the `class_name` because it eagerloads more classes than + necessary and potentially creates circular dependencies. + + *Kir Shatrov* + * Raise error when has_many through is defined before through association Fixes #26834 diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 72f1ac4896a96..2da85df9e9d44 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -364,6 +364,17 @@ def initialize(name, scope, options, active_record) @constructable = calculate_constructable(macro, options) @association_scope_cache = {} @scope_lock = Mutex.new + + if options[:class_name] && options[:class_name].class == Class + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Passing a class to the `class_name` is deprecated and will raise + an ArgumentError in Rails 5.2. It eagerloads more classes than + necessary and potentially creates circular dependencies. + + Please pass the class name as a string: + `belongs_to :client, class_name: 'Company'` + MSG + end end def association_scope_cache(conn, owner) diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 54fb61d6a5949..efd2124679d13 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -86,8 +86,10 @@ class DeveloperWithSymbolClassName < Developer has_and_belongs_to_many :projects, class_name: :ProjectWithSymbolsForKeys end -class DeveloperWithConstantClassName < Developer - has_and_belongs_to_many :projects, class_name: ProjectWithSymbolsForKeys +ActiveSupport::Deprecation.silence do + class DeveloperWithConstantClassName < Developer + has_and_belongs_to_many :projects, class_name: ProjectWithSymbolsForKeys + end end class DeveloperWithExtendOption < Developer diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 0ef51272b9db0..2444eccab1f06 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -404,6 +404,12 @@ def test_symbol_for_class_name assert_equal Client, Firm.reflect_on_association(:unsorted_clients_with_symbol).klass end + def test_class_for_class_name + assert_deprecated do + assert_predicate ActiveRecord::Reflection.create(:has_many, :clients, nil, { class_name: Client }, Firm), :validate? + end + end + def test_join_table category = Struct.new(:table_name, :pluralize_table_names).new("categories", true) product = Struct.new(:table_name, :pluralize_table_names).new("products", true) diff --git a/activerecord/test/models/user.rb b/activerecord/test/models/user.rb index 47649e0a77edd..c099c57e370bf 100644 --- a/activerecord/test/models/user.rb +++ b/activerecord/test/models/user.rb @@ -5,7 +5,7 @@ class User < ActiveRecord::Base has_secure_token :auth_token has_and_belongs_to_many :jobs_pool, - class_name: Job, + class_name: "Job", join_table: "jobs_pool" end