From 71828a4bfbf43a250ce660a47b555b5d69558c02 Mon Sep 17 00:00:00 2001 From: Adrianna Chang Date: Wed, 4 May 2022 09:33:52 -0400 Subject: [PATCH] Raise StrictLoadingViolationError with polymorphic relation violations 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. --- activerecord/lib/active_record/core.rb | 2 +- .../lib/active_record/log_subscriber.rb | 6 +-- activerecord/lib/active_record/reflection.rb | 6 +++ .../test/cases/strict_loading_test.rb | 49 ++++++++++++++++++- 4 files changed, 57 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 99dffc18123a4..28ea49b914753 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -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" diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index b5c2942fdfce2..2e9d1448159a7 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -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 diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index aec46db00492c..99f02e1db6367 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -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 diff --git a/activerecord/test/cases/strict_loading_test.rb b/activerecord/test/cases/strict_loading_test.rb index 161633766321f..5eccf398bea59 100644 --- a/activerecord/test/cases/strict_loading_test.rb +++ b/activerecord/test/cases/strict_loading_test.rb @@ -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