Merge pull request #40511 from eileencodes/add-option-for-handling-strict-loading

Allow applications to change the behavior for a strict loading violation

activerecord/CHANGELOG.md

@@ -1,3 +1,11 @@
+*   Add option to raise or log for `ActiveRecord::StrictLoadingViolationError`.
+
+    Some applications may not want to raise an error in production if using `strict_loading`. This would allow an application to set strict loading to log for the production environment while still raising in development and test environments.
+
+    Set `config.active_record.action_on_strict_loading_violation` to `:log` errors instead of raising.
+
+    *Eileen M. Uchitelle*
+
 *   Allow the inverse of a `has_one` association that was previously autosaved to be loaded.
 
     Fixes #34255.

activerecord/lib/active_record/associations/association.rb

@@ -208,11 +208,11 @@ module ActiveRecord
       private
         def find_target
           if owner.strict_loading?
-            raise StrictLoadingViolationError, "#{owner.class} is marked as strict_loading and #{klass} cannot be lazily loaded."
+            Base.strict_loading_violation!(owner: owner.class, association: klass)
           end
 
           if reflection.strict_loading?
-            raise StrictLoadingViolationError, "The #{reflection.name} association is marked as strict_loading and cannot be lazily loaded."
+            Base.strict_loading_violation!(owner: owner.class, association: reflection.name)
           end
 
           scope = self.scope

activerecord/lib/active_record/core.rb

@@ -133,6 +133,12 @@ module ActiveRecord
 
       class_attribute :belongs_to_required_by_default, instance_accessor: false
 
+      ##
+      # :singleton-method:
+      # Set the application to log or raise when an association violates strict loading.
+      # Defaults to :raise.
+      mattr_accessor :action_on_strict_loading_violation, instance_accessor: false, default: :raise
+
       class_attribute :strict_loading_by_default, instance_accessor: false, default: false
 
       mattr_accessor :writing_role, instance_accessor: false, default: :writing
@@ -263,6 +269,17 @@ module ActiveRecord
       self.default_connection_handler = ConnectionAdapters::ConnectionHandler.new
       self.default_role = writing_role
       self.default_shard = :default
+
+      def self.strict_loading_violation!(owner:, association:) # :nodoc:
+        case action_on_strict_loading_violation
+        when :raise
+          message = "`#{association}` called on `#{owner}` is marked for strict_loading and cannot be lazily loaded."
+          raise ActiveRecord::StrictLoadingViolationError.new(message)
+        when :log
+          name = "strict_loading_violation.active_record"
+          ActiveSupport::Notifications.instrument(name, owner: owner, association: association)
+        end
+      end
     end
 
     module ClassMethods

activerecord/lib/active_record/log_subscriber.rb

@@ -19,6 +19,15 @@ module ActiveRecord
       rt
     end
 
+    def strict_loading_violation(event)
+      debug do
+        owner = event.payload[:owner]
+        association = event.payload[:association]
+
+        color("Strict loading violation: #{association} lazily loaded on #{owner}.", RED)
+      end
+    end
+
     def sql(event)
       self.class.runtime += event.duration
       return unless logger.debug?

activerecord/test/cases/strict_loading_test.rb

@@ -359,6 +359,38 @@ class StrictLoadingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_strict_loading_violation_raises_by_default
+    assert_equal :raise, ActiveRecord::Base.action_on_strict_loading_violation
+
+    developer = Developer.first
+    assert_not_predicate developer, :strict_loading?
+
+    developer.strict_loading!
+    assert_predicate developer, :strict_loading?
+
+    assert_raises ActiveRecord::StrictLoadingViolationError do
+      developer.audit_logs.to_a
+    end
+  end
+
+  def test_strict_loading_violation_can_log_instead_of_raise
+    old_value = ActiveRecord::Base.action_on_strict_loading_violation
+    ActiveRecord::Base.action_on_strict_loading_violation = :log
+    assert_equal :log, ActiveRecord::Base.action_on_strict_loading_violation
+
+    developer = Developer.first
+    assert_not_predicate developer, :strict_loading?
+
+    developer.strict_loading!
+    assert_predicate developer, :strict_loading?
+
+    assert_logged("Strict loading violation: AuditLog lazily loaded on Developer.") do
+      developer.audit_logs.to_a
+    end
+  ensure
+    ActiveRecord::Base.action_on_strict_loading_violation = old_value
+  end
+
   private
     def with_strict_loading_by_default(model)
       previous_strict_loading_by_default = model.strict_loading_by_default
@@ -369,4 +401,19 @@ class StrictLoadingTest < ActiveRecord::TestCase
     ensure
       model.strict_loading_by_default = previous_strict_loading_by_default
     end
+
+    def assert_logged(message)
+      old_logger = ActiveRecord::Base.logger
+      log = StringIO.new
+      ActiveRecord::Base.logger = Logger.new(log)
+
+      begin
+        yield
+
+        log.rewind
+        assert_match message, log.read
+      ensure
+        ActiveRecord::Base.logger = old_logger
+      end
+    end
 end

guides/source/configuring.md

@@ -436,6 +436,11 @@ in controllers and views. This defaults to `false`.
   controls whether a record fails validation if `belongs_to` association is not
   present.
 
+* `config.active_record.action_on_strict_loading_violation` enables raising or
+  logging an exception if strict_loading is set on an association. The default
+  value is `:raise` in all environments. It can be changed to `:log` to send
+  violations to the logger instead of raising.
+
 * `config.active_record.strict_loading_by_default` is a boolean value
   that either enables or disables strict_loading mode by default.
   Defaults to `false`.