diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b56eedf591..41dd28987d 100644 --- a/activerecord/CHANGELOG.md +++ b/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. diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 3ec69ac75e..8a80c30374 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/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 diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index dcd6d100ea..543721da1e 100644 --- a/activerecord/lib/active_record/core.rb +++ b/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 diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index 5fc1749640..af6ad919f2 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/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? diff --git a/activerecord/test/cases/strict_loading_test.rb b/activerecord/test/cases/strict_loading_test.rb index 8e7d0795cf..c730fbe588 100644 --- a/activerecord/test/cases/strict_loading_test.rb +++ b/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 diff --git a/guides/source/configuring.md b/guides/source/configuring.md index 570ae24443..b7122e7ee1 100644 --- a/guides/source/configuring.md +++ b/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`.