Merge pull request #39491 from bogdanvlviv/strict_loading_by_default

Allow to enable/disable strict_loading mode by default for a model.

activerecord/CHANGELOG.md

@@ -1,3 +1,23 @@
+*   Add `ActiveRecord::Base.strict_loading_by_default` and `ActiveRecord::Base.strict_loading_by_default=`
+    to enable/disable strict_loading mode by default for a model. The configuration's value is
+    inheritable by subclasses, but they can override that value and it will not impact parent class.
+
+    Usage:
+
+    ```ruby
+    class Developer < ApplicationRecord
+      self.strict_loading_by_default = true
+
+      has_many :projects
+    end
+
+    dev = Developer.first
+    dev.projects.first
+    # => ActiveRecord::StrictLoadingViolationError Exception: Developer is marked as strict_loading and Project cannot be lazily loaded.
+    ```
+
+    *bogdanvlviv*
+
 *   Deprecate passing an Active Record object to `quote`/`type_cast` directly.
 
     *Ryuta Kamizono*

activerecord/lib/active_record/core.rb

@@ -123,6 +123,8 @@ module ActiveRecord
 
       class_attribute :belongs_to_required_by_default, instance_accessor: false
 
+      class_attribute :strict_loading_by_default, instance_accessor: false, default: false
+
       mattr_accessor :connection_handlers, instance_accessor: false, default: {}
 
       mattr_accessor :writing_role, instance_accessor: false, default: :writing
@@ -600,7 +602,7 @@ module ActiveRecord
         @marked_for_destruction   = false
         @destroyed_by_association = nil
         @_start_transaction_state = nil
-        @strict_loading           = false
+        @strict_loading           = self.class.strict_loading_by_default
 
         self.class.define_attribute_methods
       end

activerecord/test/cases/strict_loading_test.rb

@@ -29,6 +29,43 @@ class StrictLoadingTest < ActiveRecord::TestCase
     Developer.strict_loading.each { |d| assert d.strict_loading? }
   end
 
+  def test_strict_loading_by_default
+    with_strict_loading_by_default(Developer) do
+      Developer.all.each { |d| assert d.strict_loading? }
+    end
+  end
+
+  def test_strict_loading_by_default_can_be_set_per_model
+    model1 = Class.new(ActiveRecord::Base) do
+      self.table_name = "developers"
+      self.strict_loading_by_default = true
+    end.new
+
+    model2 = Class.new(ActiveRecord::Base) do
+      self.table_name = "developers"
+      self.strict_loading_by_default = false
+    end.new
+
+    assert_predicate model1, :strict_loading?
+    assert_not_predicate model2, :strict_loading?
+  end
+
+  def test_strict_loading_by_default_is_inheritable
+    with_strict_loading_by_default(ActiveRecord::Base) do
+      model1 = Class.new(ActiveRecord::Base) do
+        self.table_name = "developers"
+      end.new
+
+      model2 = Class.new(ActiveRecord::Base) do
+        self.table_name = "developers"
+        self.strict_loading_by_default = false
+      end.new
+
+      assert_predicate model1, :strict_loading?
+      assert_not_predicate model2, :strict_loading?
+    end
+  end
+
   def test_raises_if_strict_loading_and_lazy_loading
     dev = Developer.strict_loading.first
     assert_predicate dev, :strict_loading?
@@ -38,6 +75,17 @@ class StrictLoadingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_raises_if_strict_loading_by_default_and_lazy_loading
+    with_strict_loading_by_default(Developer) do
+      dev = Developer.first
+      assert_predicate dev, :strict_loading?
+
+      assert_raises ActiveRecord::StrictLoadingViolationError do
+        dev.audit_logs.to_a
+      end
+    end
+  end
+
   def test_preload_audit_logs_are_strict_loading_because_parent_is_strict_loading
     developer = Developer.first
 
@@ -51,6 +99,21 @@ class StrictLoadingTest < ActiveRecord::TestCase
     assert dev.audit_logs.all?(&:strict_loading?), "Expected all audit logs to be strict_loading"
   end
 
+  def test_preload_audit_logs_are_strict_loading_because_it_is_strict_loading_by_default
+    with_strict_loading_by_default(AuditLog) do
+      developer = Developer.first
+
+      3.times do
+        AuditLog.create(developer: developer, message: "I am message")
+      end
+
+      dev = Developer.includes(:audit_logs).first
+
+      assert_not_predicate dev, :strict_loading?
+      assert dev.audit_logs.all?(&:strict_loading?), "Expected all audit logs to be strict_loading"
+    end
+  end
+
   def test_eager_load_audit_logs_are_strict_loading_because_parent_is_strict_loading_in_hm_relation
     developer = Developer.first
 
@@ -76,6 +139,22 @@ class StrictLoadingTest < ActiveRecord::TestCase
     assert dev.audit_logs.all?(&:strict_loading?), "Expected all audit logs to be strict_loading"
   end
 
+  def test_eager_load_audit_logs_are_strict_loading_because_it_is_strict_loading_by_default
+    with_strict_loading_by_default(AuditLog) do
+      developer = Developer.first
+
+      3.times do
+        AuditLog.create(developer: developer, message: "I am message")
+      end
+
+      dev = Developer.eager_load(:audit_logs).first
+
+      assert_not_predicate dev, :strict_loading?
+      assert_predicate AuditLog.last, :strict_loading?
+      assert dev.audit_logs.all?(&:strict_loading?), "Expected all audit logs to be strict_loading"
+    end
+  end
+
   def test_raises_on_unloaded_relation_methods_if_strict_loading
     dev = Developer.strict_loading.first
     assert_predicate dev, :strict_loading?
@@ -85,6 +164,17 @@ class StrictLoadingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_raises_on_unloaded_relation_methods_if_strict_loading_by_default
+    with_strict_loading_by_default(Developer) do
+      dev = Developer.first
+      assert_predicate dev, :strict_loading?
+
+      assert_raises ActiveRecord::StrictLoadingViolationError do
+        dev.audit_logs.first
+      end
+    end
+  end
+
   def test_raises_on_lazy_loading_a_strict_loading_belongs_to_relation
     mentor = Mentor.create!(name: "Mentor")
 
@@ -96,6 +186,19 @@ class StrictLoadingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_raises_on_lazy_loading_a_belongs_to_relation_if_strict_loading_by_default
+    with_strict_loading_by_default(Developer) do
+      mentor = Mentor.create!(name: "Mentor")
+
+      developer = Developer.first
+      developer.update_column(:mentor_id, mentor.id)
+
+      assert_raises ActiveRecord::StrictLoadingViolationError do
+        developer.mentor
+      end
+    end
+  end
+
   def test_does_not_raise_on_eager_loading_a_strict_loading_belongs_to_relation
     mentor = Mentor.create!(name: "Mentor")
 
@@ -105,6 +208,17 @@ class StrictLoadingTest < ActiveRecord::TestCase
     assert_nothing_raised { developer.strict_loading_mentor }
   end
 
+  def test_does_not_raise_on_eager_loading_a_belongs_to_relation_if_strict_loading_by_default
+    with_strict_loading_by_default(Developer) do
+      mentor = Mentor.create!(name: "Mentor")
+
+      Developer.first.update_column(:mentor_id, mentor.id)
+      developer = Developer.includes(:mentor).first
+
+      assert_nothing_raised { developer.mentor }
+    end
+  end
+
   def test_raises_on_lazy_loading_a_strict_loading_has_one_relation
     developer = Developer.first
     ship = Ship.first
@@ -116,6 +230,19 @@ class StrictLoadingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_raises_on_lazy_loading_a_has_one_relation_if_strict_loading_by_default
+    with_strict_loading_by_default(Developer) do
+      developer = Developer.first
+      ship = Ship.first
+
+      ship.update_column(:developer_id, developer.id)
+
+      assert_raises ActiveRecord::StrictLoadingViolationError do
+        developer.ship
+      end
+    end
+  end
+
   def test_does_not_raise_on_eager_loading_a_strict_loading_has_one_relation
     Ship.first.update_column(:developer_id, Developer.first.id)
     developer = Developer.includes(:strict_loading_ship).first
@@ -123,6 +250,15 @@ class StrictLoadingTest < ActiveRecord::TestCase
     assert_nothing_raised { developer.strict_loading_ship }
   end
 
+  def test_does_not_raise_on_eager_loading_a_has_one_relation_if_strict_loading_by_default
+    with_strict_loading_by_default(Developer) do
+      Ship.first.update_column(:developer_id, Developer.first.id)
+      developer = Developer.includes(:ship).first
+
+      assert_nothing_raised { developer.ship }
+    end
+  end
+
   def test_raises_on_lazy_loading_a_strict_loading_has_many_relation
     developer = Developer.first
 
@@ -137,6 +273,22 @@ class StrictLoadingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_raises_on_lazy_loading_a_has_many_relation_if_strict_loading_by_default
+    with_strict_loading_by_default(Developer) do
+      developer = Developer.first
+
+      AuditLog.create(
+        3.times.map do
+          { developer_id: developer.id, message: "I am message" }
+        end
+      )
+
+      assert_raises ActiveRecord::StrictLoadingViolationError do
+        developer.audit_logs.first
+      end
+    end
+  end
+
   def test_does_not_raise_on_eager_loading_a_strict_loading_has_many_relation
     developer = Developer.first
 
@@ -151,6 +303,22 @@ class StrictLoadingTest < ActiveRecord::TestCase
     assert_nothing_raised { developer.strict_loading_opt_audit_logs.first }
   end
 
+  def test_does_not_raise_on_eager_loading_a_has_many_relation_if_strict_loading_by_default
+    with_strict_loading_by_default(Developer) do
+      developer = Developer.first
+
+      AuditLog.create(
+        3.times.map do
+          { developer_id: developer.id, message: "I am message" }
+        end
+      )
+
+      developer = Developer.includes(:audit_logs).first
+
+      assert_nothing_raised { developer.audit_logs.first }
+    end
+  end
+
   def test_raises_on_lazy_loading_a_strict_loading_habtm_relation
     developer = Developer.first
     developer.projects << Project.first
@@ -162,10 +330,43 @@ class StrictLoadingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_raises_on_lazy_loading_a_habtm_relation_if_strict_loading_by_default
+    with_strict_loading_by_default(Developer) do
+      developer = Developer.first
+      developer.projects << Project.first
+
+      assert_not developer.projects.loaded?
+
+      assert_raises ActiveRecord::StrictLoadingViolationError do
+        developer.projects.first
+      end
+    end
+  end
+
   def test_does_not_raise_on_eager_loading_a_strict_loading_habtm_relation
     Developer.first.projects << Project.first
     developer = Developer.includes(:strict_loading_projects).first
 
     assert_nothing_raised { developer.strict_loading_projects.first }
   end
+
+  def test_does_not_raise_on_eager_loading_a_habtm_relation_if_strict_loading_by_default
+    with_strict_loading_by_default(Developer) do
+      Developer.first.projects << Project.first
+      developer = Developer.includes(:projects).first
+
+      assert_nothing_raised { developer.projects.first }
+    end
+  end
+
+  private
+    def with_strict_loading_by_default(model)
+      previous_strict_loading_by_default = model.strict_loading_by_default
+
+      model.strict_loading_by_default = true
+
+      yield
+    ensure
+      model.strict_loading_by_default = previous_strict_loading_by_default
+    end
 end

guides/source/configuring.md

@@ -432,6 +432,10 @@ in controllers and views. This defaults to `false`.
   controls whether a record fails validation if `belongs_to` association is not
   present.
 
+* `config.active_record.strict_loading_by_default` is a boolean value
+  that either enables or disables strict_loading mode by default.
+  Defaults to `false`.
+
 * `config.active_record.warn_on_records_fetched_greater_than` allows setting a
   warning threshold for query result size. If the number of records returned
   by a query exceeds the threshold, a warning is logged. This can be used to