Raise `ActiveRecord:: InverseOfAssociationNotFoundError` if invalid inverse_of is specified #51767

alpaca-tc · 2024-05-08T15:43:03Z

Motivation / Background

This error occurs when the association specified in the inverse_of option does not exist on the associated class.
Previously this would be implicitly ignored, so the developer wouldn't know they tried to make an invalid association.

Detail

Post.belongs_to(:user, inverse_of: :comment) # Correct inverse_of is :post
user = User.create!

 # Before:
Post.new(user: user) #=> No error

 # After:
Post.new(user: user)
#=> ActiveRecord::InverseOfAssociationNotFoundError: Could not find the inverse association for user (:comment in User).

Additional information

When checking the bi-directionality of inverse_of reflection, it is intuitive to
compare classes. However, the implementation compares table_name instead, because
class name comparisons are challenging with anonymous classes (such as those used in Rails testing).

Checklist

Before submitting the PR make sure the following are checked:

This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
Tests are added or updated if you fix a bug or add a feature.
CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

yahonda · 2024-05-16T06:38:15Z

Although English isn't my native language, I find the current error message like Inverse association human (:interests in Human) is mismatch table_name." unclear for me, I assume this statement is based on #41552 , where the "recursive" is an adjective, while "mismatch" is a noun.

alpaca-tc · 2024-05-28T14:58:36Z

@yahonda I modified the class name of the exception has been changed from InverseOfAssociationTableNameMismatchError to InverseOfAssociationNotBidirectionalError.
This is because the old name is not essential, since it expresses implementation details.
In addition, the error message has been changed to point out that it is not bi-directional.

Also, the PR description has been modified to explain why table_name is used for bi-directional comparisons. Originally I said STI considered, but it was correct that it considered the anonymous class.

Please re-review this PR 😄 🙏

rafaelfranca · 2024-05-28T22:09:43Z

activerecord/lib/active_record/associations.rb

+      if reflection
+        super("Inverse association #{reflection.name} (#{reflection.inverse_of.name.inspect} in #{reflection.class_name}) is not bi-directional.")
+      else
+        super("Inverse association is not bi-directional.")


Even I not sure what this error message means. It seems like it is a requirement for inverse_of to be by directional, but I don't think this is the problem here, is it?

The problem is that the association on the other model doesn't exist, so we should be explicit about that.

Something like:

Inverse association doesn't not exist in the associated class

@rafaelfranca I have corrected the message you commented. 🙏
Should I L80 have also been changed?

Should I L80 have also been changed?

Yes, I made some other comments below too, thanks for your PR! 🙇

activerecord/test/cases/associations/inverse_associations_test.rb

activerecord/CHANGELOG.md

zzak · 2024-05-30T12:46:19Z

activerecord/lib/active_record/associations.rb

@@ -71,6 +71,20 @@ def initialize(reflection = nil)
    end
  end

+  class InverseOfAssociationNotBidirectionalError < ActiveRecordError # :nodoc:


I think there was already some discussion about the name, but I prefer InverseOfAssociationNotFoundError, as it's explicit that the "inverse of" doesn't exist.

This matches the style for HasManyThroughAssociationNotFoundError.

activerecord/lib/active_record/associations.rb

…verse_of is specified. This error occurs when the association specified in the `inverse_of` option does not exist on the associated class. Previously this would be implicitly ignored, so the developer wouldn't know they tried to make an invalid association. ```ruby Post.belongs_to(:user, inverse_of: :comment) # Correct inverse_of is :post user = User.create! # Before: Post.new(user: user) #=> No error # After: Post.new(user: user) #=> ActiveRecord::InverseOfAssociationNotFoundError: Could not find the inverse association for user (:comment in User). ``` When checking the bi-directionality of inverse_of reflection, it is intuitive to compare classes. However, the implementation compares table_name instead, because class name comparisons are challenging with anonymous classes (such as those used in Rails testing). Co-authored-by: zzak <zzakscott@gmail.com>

alpaca-tc · 2024-05-30T15:09:15Z

Thank you for the review 🙏

I have incorporated suggestions and modified the code to use InverseOfAssociationNotFoundError. The changes have been force-pushed. Please review the updates.

rails-bot bot added the activerecord label May 8, 2024

alpaca-tc force-pushed the assert-inverse-of branch 2 times, most recently from 6bf94e7 to 30c0c53 Compare May 10, 2024 08:41

alpaca-tc force-pushed the assert-inverse-of branch 3 times, most recently from fd5c8e6 to 3c2711c Compare May 28, 2024 14:46

rafaelfranca requested changes May 28, 2024

View reviewed changes

alpaca-tc force-pushed the assert-inverse-of branch from 3c2711c to 318d569 Compare May 29, 2024 02:30