Skip to content

Commit

Permalink
Raise StrictLoadingViolationError with polymorphic relation violations
Browse files Browse the repository at this point in the history
Performing strict_loading! on a model with a polymorphic association
currently raises an ArgumentError because it attempts to look up the
klass on the polymorphic association, which is not possible. The error
message for StrictLoadingViolationError on models with polymorphic
relationships should omit the klass for the association.
  • Loading branch information
adrianna-chang-shopify committed May 5, 2022
1 parent 1b884cf commit 71828a4
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 6 deletions.
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

0 comments on commit 71828a4

Please sign in to comment.