Skip to content

Commit cec55aa

Browse files
Dinah Shijhawthorn
andcommitted
Convert strict_loading_mode from class attr to ivar
Strict loading mode `:n_plus_one_only` is only supported on single records, not associations or models. Using an ivar instead of class_attribute ensures that this cannot be set globally. This fixes a bug where setting `strict_loading_mode` caused errors to be silent when `strict_loading_by_default` is true. Fixes rails#42576 Co-Authored-By: John Hawthorn <[email protected]>
1 parent acee501 commit cec55aa

File tree

3 files changed

+4
-48
lines changed

3 files changed

+4
-48
lines changed

activerecord/lib/active_record/associations/association.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ def find_target
240240
if owner.strict_loading_n_plus_one_only? && reflection.macro == :has_many
241241
record.strict_loading!
242242
else
243-
record.strict_loading_mode = owner.strict_loading_mode
243+
record.strict_loading!(false, mode: owner.strict_loading_mode)
244244
end
245245
end
246246
end

activerecord/lib/active_record/core.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ def self.configurations
6868
class_attribute :belongs_to_required_by_default, instance_accessor: false
6969

7070
class_attribute :strict_loading_by_default, instance_accessor: false, default: false
71-
class_attribute :strict_loading_mode, instance_accessor: true, default: :all
7271

7372
class_attribute :has_many_inversing, instance_accessor: false, default: false
7473

@@ -677,6 +676,8 @@ def strict_loading!(value = true, mode: :all)
677676
@strict_loading = value
678677
end
679678

679+
attr_reader :strict_loading_mode
680+
680681
# Returns +true+ if the record uses strict_loading with +:n_plus_one_only+ mode enabled.
681682
def strict_loading_n_plus_one_only?
682683
@strict_loading_mode == :n_plus_one_only
@@ -768,7 +769,7 @@ def init_internals
768769

769770
@primary_key = klass.primary_key
770771
@strict_loading = klass.strict_loading_by_default
771-
@strict_loading_mode = klass.strict_loading_mode
772+
@strict_loading_mode = :all
772773

773774
klass.define_attribute_methods
774775
end

activerecord/test/cases/strict_loading_test.rb

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -77,51 +77,6 @@ def test_strict_loading_n_plus_one_only_mode
7777
end
7878
end
7979

80-
def test_strict_loading_n_plus_one_only_mode_by_default
81-
with_strict_loading_by_default(Developer) do
82-
previous_strict_loading_mode = Developer.strict_loading_mode
83-
Developer.strict_loading_mode = :n_plus_one_only
84-
85-
developer = Developer.first
86-
ship = Ship.first
87-
ShipPart.create!(name: "Stern", ship: ship)
88-
firm = Firm.create!(name: "NASA")
89-
project = Project.create!(name: "Apollo", firm: firm)
90-
91-
ship.update_column(:developer_id, developer.id)
92-
developer.projects << project
93-
developer.reload
94-
95-
assert_predicate developer, :strict_loading?
96-
97-
# Does not raise when loading a has_many association (:projects)
98-
assert_nothing_raised do
99-
developer.projects.to_a
100-
end
101-
102-
# strict_loading is enabled for has_many associations
103-
assert developer.projects.all?(&:strict_loading?)
104-
assert_raises ActiveRecord::StrictLoadingViolationError do
105-
developer.projects.last.firm
106-
end
107-
108-
# Does not raise when a belongs_to association (:ship) loads its
109-
# has_many association (:parts)
110-
assert_nothing_raised do
111-
developer.ship.parts.to_a
112-
end
113-
114-
# strict_loading is enabled for has_many through a belongs_to
115-
assert_not developer.ship.strict_loading?
116-
assert developer.ship.parts.all?(&:strict_loading?)
117-
assert_raises ActiveRecord::StrictLoadingViolationError do
118-
developer.ship.parts.first.trinkets.to_a
119-
end
120-
ensure
121-
Developer.strict_loading_mode = previous_strict_loading_mode
122-
end
123-
end
124-
12580
def test_strict_loading
12681
Developer.all.each { |d| assert_not d.strict_loading? }
12782
Developer.strict_loading.each { |d| assert d.strict_loading? }

0 commit comments

Comments
 (0)