From cec55aafb9191b0bd357679453170c60196479fa Mon Sep 17 00:00:00 2001 From: Dinah Shi Date: Thu, 24 Jun 2021 11:54:40 -0400 Subject: [PATCH] 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 #42576 Co-Authored-By: John Hawthorn --- .../active_record/associations/association.rb | 2 +- activerecord/lib/active_record/core.rb | 5 ++- .../test/cases/strict_loading_test.rb | 45 ------------------- 3 files changed, 4 insertions(+), 48 deletions(-) diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 20cdc69fb9..2b88f53df0 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -240,7 +240,7 @@ module ActiveRecord if owner.strict_loading_n_plus_one_only? && reflection.macro == :has_many record.strict_loading! else - record.strict_loading_mode = owner.strict_loading_mode + record.strict_loading!(false, mode: owner.strict_loading_mode) end end end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 6c30cc4b79..eea6210e41 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -68,7 +68,6 @@ module ActiveRecord class_attribute :belongs_to_required_by_default, instance_accessor: false class_attribute :strict_loading_by_default, instance_accessor: false, default: false - class_attribute :strict_loading_mode, instance_accessor: true, default: :all class_attribute :has_many_inversing, instance_accessor: false, default: false @@ -677,6 +676,8 @@ module ActiveRecord @strict_loading = value end + attr_reader :strict_loading_mode + # Returns +true+ if the record uses strict_loading with +:n_plus_one_only+ mode enabled. def strict_loading_n_plus_one_only? @strict_loading_mode == :n_plus_one_only @@ -768,7 +769,7 @@ module ActiveRecord @primary_key = klass.primary_key @strict_loading = klass.strict_loading_by_default - @strict_loading_mode = klass.strict_loading_mode + @strict_loading_mode = :all klass.define_attribute_methods end diff --git a/activerecord/test/cases/strict_loading_test.rb b/activerecord/test/cases/strict_loading_test.rb index 200d3f4dd1..052cca9fc7 100644 --- a/activerecord/test/cases/strict_loading_test.rb +++ b/activerecord/test/cases/strict_loading_test.rb @@ -77,51 +77,6 @@ class StrictLoadingTest < ActiveRecord::TestCase end end - def test_strict_loading_n_plus_one_only_mode_by_default - with_strict_loading_by_default(Developer) do - previous_strict_loading_mode = Developer.strict_loading_mode - Developer.strict_loading_mode = :n_plus_one_only - - developer = Developer.first - ship = Ship.first - ShipPart.create!(name: "Stern", ship: ship) - firm = Firm.create!(name: "NASA") - project = Project.create!(name: "Apollo", firm: firm) - - ship.update_column(:developer_id, developer.id) - developer.projects << project - developer.reload - - assert_predicate developer, :strict_loading? - - # Does not raise when loading a has_many association (:projects) - assert_nothing_raised do - developer.projects.to_a - end - - # strict_loading is enabled for has_many associations - assert developer.projects.all?(&:strict_loading?) - assert_raises ActiveRecord::StrictLoadingViolationError do - developer.projects.last.firm - end - - # Does not raise when a belongs_to association (:ship) loads its - # has_many association (:parts) - assert_nothing_raised do - developer.ship.parts.to_a - end - - # strict_loading is enabled for has_many through a belongs_to - assert_not developer.ship.strict_loading? - assert developer.ship.parts.all?(&:strict_loading?) - assert_raises ActiveRecord::StrictLoadingViolationError do - developer.ship.parts.first.trinkets.to_a - end - ensure - Developer.strict_loading_mode = previous_strict_loading_mode - end - end - def test_strict_loading Developer.all.each { |d| assert_not d.strict_loading? } Developer.strict_loading.each { |d| assert d.strict_loading? }