Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise StrictLoadingViolationError with polymorphic relation violations #45016

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/core.rb
Expand Up @@ -213,7 +213,7 @@ def self.connection_class_for_self # :nodoc:
def self.strict_loading_violation!(owner:, reflection:) # :nodoc:
case ActiveRecord.action_on_strict_loading_violation
when :raise
message = "`#{owner}` is marked for strict_loading. The `#{reflection.klass}` association named `:#{reflection.name}` cannot be lazily loaded."
message = reflection.strict_loading_violation_message(owner)
raise ActiveRecord::StrictLoadingViolationError.new(message)
when :log
name = "strict_loading_violation.active_record"
Expand Down
6 changes: 2 additions & 4 deletions activerecord/lib/active_record/log_subscriber.rb
Expand Up @@ -22,10 +22,8 @@ def self.reset_runtime
def strict_loading_violation(event)
debug do
owner = event.payload[:owner]
association = event.payload[:reflection].klass
name = event.payload[:reflection].name

color("Strict loading violation: #{owner} is marked for strict loading. The #{association} association named :#{name} cannot be lazily loaded.", RED)
reflection = event.payload[:reflection]
color(reflection.strict_loading_violation_message(owner), RED)
end
end

Expand Down
6 changes: 6 additions & 0 deletions activerecord/lib/active_record/reflection.rb
Expand Up @@ -297,6 +297,12 @@ def strict_loading?
options[:strict_loading]
end

def strict_loading_violation_message(owner)
message = +"`#{owner}` is marked for strict_loading."
message << " The #{polymorphic? ? "polymorphic association" : "#{klass} association"}"
message << " named `:#{name}` cannot be lazily loaded."
end

protected
def actual_source_reflection # FIXME: this is a horrible name
self
Expand Down
49 changes: 48 additions & 1 deletion activerecord/test/cases/strict_loading_test.rb
Expand Up @@ -549,13 +549,60 @@ def test_strict_loading_violation_can_log_instead_of_raise
developer.strict_loading!
assert_predicate developer, :strict_loading?

assert_logged("Strict loading violation: Developer is marked for strict loading. The AuditLog association named :audit_logs cannot be lazily loaded.") do
expected_log = <<-MSG.squish
`Developer` is marked for strict_loading.
The AuditLog association named `:audit_logs` cannot be lazily loaded.
MSG
assert_logged(expected_log) do
developer.audit_logs.to_a
end
ensure
ActiveRecord.action_on_strict_loading_violation = old_value
end

def test_strict_loading_violation_on_polymorphic_relation
pirate = Pirate.create!(catchphrase: "Arrr!")
Treasure.create!(looter: pirate)

treasure = Treasure.last
treasure.strict_loading!
assert_predicate treasure, :strict_loading?

error = assert_raises ActiveRecord::StrictLoadingViolationError do
treasure.looter
end

expected_error_message = <<-MSG.squish
`Treasure` is marked for strict_loading.
The polymorphic association named `:looter` cannot be lazily loaded.
MSG

assert_equal(expected_error_message, error.message)
end

def test_strict_loading_violation_logs_on_polymorphic_relation
old_value = ActiveRecord.action_on_strict_loading_violation
ActiveRecord.action_on_strict_loading_violation = :log
assert_equal :log, ActiveRecord.action_on_strict_loading_violation

pirate = Pirate.create!(catchphrase: "Arrr!")
Treasure.create!(looter: pirate)

treasure = Treasure.last
treasure.strict_loading!
assert_predicate treasure, :strict_loading?

expected_log = <<-MSG.squish
`Treasure` is marked for strict_loading.
The polymorphic association named `:looter` cannot be lazily loaded.
MSG
assert_logged(expected_log) do
treasure.looter
end
ensure
ActiveRecord.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
Expand Down