Raise on `foreign_key:` being passed as an array in associations

Associations have never allowed nor supported `foreign_key` option
being passed as an Array. This still holds true for Rails 7.1
However with Rails 7.1 supporting composite primary keys it may become
more common for applications to mistakenly pass an array to `foreign_key:`.
This commit adds an exception to raise when `foreign_key:` is passed as
an array.
This commit is contained in:
Nikita Vasilevsky 2023-10-13 17:03:16 +00:00
parent 653725ee7c
commit 529d1f55a8
No known key found for this signature in database
GPG Key ID: 0FF5725CD31059E4
4 changed files with 73 additions and 0 deletions

View File

@ -1,3 +1,7 @@
* Raise on `foreign_key:` being passed as an array in associations
*Nikita Vasilevsky*
* Ensure `#signed_id` outputs `url_safe` strings.
*Jason Meller*

View File

@ -382,6 +382,7 @@ module ActiveRecord
@klass = options[:anonymous_class]
@plural_name = active_record.pluralize_table_names ?
name.to_s.pluralize : name.to_s
validate_reflection!
end
def autosave=(autosave)
@ -433,6 +434,17 @@ module ActiveRecord
def derive_class_name
name.to_s.camelize
end
def validate_reflection!
return unless options[:foreign_key].is_a?(Array)
message = <<~MSG.squish
Passing #{options[:foreign_key]} array to :foreign_key option
on the #{active_record}##{name} association is not supported.
Use the query_constraints: #{options[:foreign_key]} option instead to represent a composite foreign key.
MSG
raise ArgumentError, message
end
end
# Holds all the metadata about an aggregation as it was specified in the

View File

@ -194,6 +194,30 @@ class AssociationsTest < ActiveRecord::TestCase
assert_equal(expected_posts.map(&:id).sort, blog_posts.map(&:id).sort)
end
def test_has_many_with_foreign_key_as_an_array_raises
expected_message = <<~MSG.squish
Passing [:blog_id, :blog_post_id] array to :foreign_key option
on the Sharded::BlogPost#broken_array_fk_comments association is not supported.
Use the query_constraints: [:blog_id, :blog_post_id] option instead to represent a composite foreign key.
MSG
assert_raises ArgumentError, match: expected_message do
Sharded::BlogPost.has_many :broken_array_fk_comments,
class_name: "Sharded::Comment", foreign_key: [:blog_id, :blog_post_id]
end
end
def test_belongs_to_with_foreign_key_as_an_array_raises
expected_message = <<~MSG.squish
Passing [:blog_id, :blog_post_id] array to :foreign_key option
on the Sharded::Comment#broken_array_fk_blog_post association is not supported.
Use the query_constraints: [:blog_id, :blog_post_id] option instead to represent a composite foreign key.
MSG
assert_raises ArgumentError, match: expected_message do
Sharded::Comment.belongs_to :broken_array_fk_blog_post,
class_name: "Sharded::Blog", foreign_key: [:blog_id, :blog_post_id]
end
end
def test_has_many_association_with_composite_foreign_key_loads_records
blog_post = sharded_blog_posts(:great_post_blog_one)

View File

@ -226,6 +226,17 @@ class ReflectionTest < ActiveRecord::TestCase
assert_equal "companies", Firm.reflect_on_association(:clients_of_firm).table_name
end
def test_has_many_reflection_with_array_fk_raises
expected_message = <<~MSG.squish
Passing [:firm_id, :firm_name] array to :foreign_key option
on the Firm#clients association is not supported.
Use the query_constraints: [:firm_id, :firm_name] option instead to represent a composite foreign key.
MSG
assert_raises ArgumentError, match: expected_message do
ActiveRecord::Reflection.create(:has_many, :clients, nil, { foreign_key: [:firm_id, :firm_name] }, Firm)
end
end
def test_has_one_reflection
reflection_for_account = ActiveRecord::Reflection.create(:has_one, :account, nil, { foreign_key: "firm_id", dependent: :destroy }, Firm)
assert_equal reflection_for_account, Firm.reflect_on_association(:account)
@ -234,6 +245,17 @@ class ReflectionTest < ActiveRecord::TestCase
assert_equal "accounts", Firm.reflect_on_association(:account).table_name
end
def has_one_reflection_with_array_fk_raises
expected_message = <<~MSG.squish
Passing [:firm_id, :firm_name] array to :foreign_key option
on the Firm#account association is not supported.
Use the query_constraints: [:firm_id, :firm_name] option instead to represent a composite foreign key.
MSG
assert_raises ArgumentError, match: expected_message do
ActiveRecord::Reflection.create(:has_one, :account, nil, { foreign_key: [:firm_id, :firm_name] }, Firm)
end
end
def test_belongs_to_inferred_foreign_key_from_assoc_name
Company.belongs_to :foo
assert_equal "foo_id", Company.reflect_on_association(:foo).foreign_key
@ -243,6 +265,17 @@ class ReflectionTest < ActiveRecord::TestCase
assert_equal "xyzzy_id", Company.reflect_on_association(:baz).foreign_key
end
def test_belongs_to_reflection_with_array_fk_raises
expected_message = <<~MSG.squish
Passing [:firm_id, :firm_name] array to :foreign_key option
on the Firm#client association is not supported.
Use the query_constraints: [:firm_id, :firm_name] option instead to represent a composite foreign key.
MSG
assert_raises ArgumentError, match: expected_message do
ActiveRecord::Reflection.create(:belongs_to, :client, nil, { foreign_key: [:firm_id, :firm_name] }, Firm)
end
end
def test_association_reflection_in_modules
ActiveRecord::Base.store_full_sti_class = false