Skip to content

Commit

Permalink
Merge pull request #45016 from adrianna-chang-shopify/ac-fix-strict-l…
Browse files Browse the repository at this point in the history
…oading-polymorphic-associations

Raise StrictLoadingViolationError with polymorphic relation violations
  • Loading branch information
eileencodes committed May 5, 2022
1 parent f99f422 commit cd7700b
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 @@ -238,7 +238,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 @@ -550,13 +550,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 cd7700b

Please sign in to comment.