Skip to content

Commit

Permalink
Deprecate reflection class name to accept a class
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kirs committed Jan 9, 2017
1 parent 80bf338 commit 8312a0d
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 3 deletions.
5 changes: 5 additions & 0 deletions 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 * Raise error when has_many through is defined before through association


Fixes #26834 Fixes #26834
Expand Down
11 changes: 11 additions & 0 deletions activerecord/lib/active_record/reflection.rb
Expand Up @@ -364,6 +364,17 @@ def initialize(name, scope, options, active_record)
@constructable = calculate_constructable(macro, options) @constructable = calculate_constructable(macro, options)
@association_scope_cache = {} @association_scope_cache = {}
@scope_lock = Mutex.new @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 end


def association_scope_cache(conn, owner) def association_scope_cache(conn, owner)
Expand Down
Expand Up @@ -86,8 +86,10 @@ class DeveloperWithSymbolClassName < Developer
has_and_belongs_to_many :projects, class_name: :ProjectWithSymbolsForKeys has_and_belongs_to_many :projects, class_name: :ProjectWithSymbolsForKeys
end end


class DeveloperWithConstantClassName < Developer ActiveSupport::Deprecation.silence do
has_and_belongs_to_many :projects, class_name: ProjectWithSymbolsForKeys class DeveloperWithConstantClassName < Developer
has_and_belongs_to_many :projects, class_name: ProjectWithSymbolsForKeys
end
end end


class DeveloperWithExtendOption < Developer class DeveloperWithExtendOption < Developer
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/cases/reflection_test.rb
Expand Up @@ -404,6 +404,12 @@ def test_symbol_for_class_name
assert_equal Client, Firm.reflect_on_association(:unsorted_clients_with_symbol).klass assert_equal Client, Firm.reflect_on_association(:unsorted_clients_with_symbol).klass
end 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 def test_join_table
category = Struct.new(:table_name, :pluralize_table_names).new("categories", true) category = Struct.new(:table_name, :pluralize_table_names).new("categories", true)
product = Struct.new(:table_name, :pluralize_table_names).new("products", true) product = Struct.new(:table_name, :pluralize_table_names).new("products", true)
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/models/user.rb
Expand Up @@ -5,7 +5,7 @@ class User < ActiveRecord::Base
has_secure_token :auth_token has_secure_token :auth_token


has_and_belongs_to_many :jobs_pool, has_and_belongs_to_many :jobs_pool,
class_name: Job, class_name: "Job",
join_table: "jobs_pool" join_table: "jobs_pool"
end end


Expand Down

0 comments on commit 8312a0d

Please sign in to comment.