mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Allow applications to change the behavior for a strict loading violation
Originally strict loading violations would always raise an error if turned on for an association. This change allows for an application to optionally log instead of raise. The behavior here is similar to how strong parameters work. By default all environments will raise. An application may want to use log in production while working on finding all lazily loaded associations. Set `config.active_record.action_on_strict_loading_violation` to `:log` in your application to log instead of raise.
This commit is contained in:
parent
3c99817394
commit
656c6faf78
6 changed files with 88 additions and 2 deletions
|
@ -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.
|
* Allow the inverse of a `has_one` association that was previously autosaved to be loaded.
|
||||||
|
|
||||||
Fixes #34255.
|
Fixes #34255.
|
||||||
|
|
|
@ -208,11 +208,11 @@ module ActiveRecord
|
||||||
private
|
private
|
||||||
def find_target
|
def find_target
|
||||||
if owner.strict_loading?
|
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
|
end
|
||||||
|
|
||||||
if reflection.strict_loading?
|
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
|
end
|
||||||
|
|
||||||
scope = self.scope
|
scope = self.scope
|
||||||
|
|
|
@ -133,6 +133,12 @@ module ActiveRecord
|
||||||
|
|
||||||
class_attribute :belongs_to_required_by_default, instance_accessor: false
|
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
|
class_attribute :strict_loading_by_default, instance_accessor: false, default: false
|
||||||
|
|
||||||
mattr_accessor :writing_role, instance_accessor: false, default: :writing
|
mattr_accessor :writing_role, instance_accessor: false, default: :writing
|
||||||
|
@ -263,6 +269,17 @@ module ActiveRecord
|
||||||
self.default_connection_handler = ConnectionAdapters::ConnectionHandler.new
|
self.default_connection_handler = ConnectionAdapters::ConnectionHandler.new
|
||||||
self.default_role = writing_role
|
self.default_role = writing_role
|
||||||
self.default_shard = :default
|
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
|
end
|
||||||
|
|
||||||
module ClassMethods
|
module ClassMethods
|
||||||
|
|
|
@ -19,6 +19,15 @@ module ActiveRecord
|
||||||
rt
|
rt
|
||||||
end
|
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)
|
def sql(event)
|
||||||
self.class.runtime += event.duration
|
self.class.runtime += event.duration
|
||||||
return unless logger.debug?
|
return unless logger.debug?
|
||||||
|
|
|
@ -359,6 +359,38 @@ class StrictLoadingTest < ActiveRecord::TestCase
|
||||||
end
|
end
|
||||||
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
|
private
|
||||||
def with_strict_loading_by_default(model)
|
def with_strict_loading_by_default(model)
|
||||||
previous_strict_loading_by_default = model.strict_loading_by_default
|
previous_strict_loading_by_default = model.strict_loading_by_default
|
||||||
|
@ -369,4 +401,19 @@ class StrictLoadingTest < ActiveRecord::TestCase
|
||||||
ensure
|
ensure
|
||||||
model.strict_loading_by_default = previous_strict_loading_by_default
|
model.strict_loading_by_default = previous_strict_loading_by_default
|
||||||
end
|
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
|
end
|
||||||
|
|
|
@ -436,6 +436,11 @@ in controllers and views. This defaults to `false`.
|
||||||
controls whether a record fails validation if `belongs_to` association is not
|
controls whether a record fails validation if `belongs_to` association is not
|
||||||
present.
|
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
|
* `config.active_record.strict_loading_by_default` is a boolean value
|
||||||
that either enables or disables strict_loading mode by default.
|
that either enables or disables strict_loading mode by default.
|
||||||
Defaults to `false`.
|
Defaults to `false`.
|
||||||
|
|
Loading…
Reference in a new issue