Skip to content

Commit 529d1f5

Browse files
committed
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.
1 parent 653725e commit 529d1f5

File tree

4 files changed

+73
-0
lines changed

4 files changed

+73
-0
lines changed

activerecord/CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
* Raise on `foreign_key:` being passed as an array in associations
2+
3+
*Nikita Vasilevsky*
4+
15
* Ensure `#signed_id` outputs `url_safe` strings.
26

37
*Jason Meller*

activerecord/lib/active_record/reflection.rb

+12
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ def initialize(name, scope, options, active_record)
382382
@klass = options[:anonymous_class]
383383
@plural_name = active_record.pluralize_table_names ?
384384
name.to_s.pluralize : name.to_s
385+
validate_reflection!
385386
end
386387

387388
def autosave=(autosave)
@@ -433,6 +434,17 @@ def scope_for(relation, owner = nil)
433434
def derive_class_name
434435
name.to_s.camelize
435436
end
437+
438+
def validate_reflection!
439+
return unless options[:foreign_key].is_a?(Array)
440+
441+
message = <<~MSG.squish
442+
Passing #{options[:foreign_key]} array to :foreign_key option
443+
on the #{active_record}##{name} association is not supported.
444+
Use the query_constraints: #{options[:foreign_key]} option instead to represent a composite foreign key.
445+
MSG
446+
raise ArgumentError, message
447+
end
436448
end
437449

438450
# Holds all the metadata about an aggregation as it was specified in the

activerecord/test/cases/associations_test.rb

+24
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,30 @@ def test_querying_by_relation_with_composite_key
194194
assert_equal(expected_posts.map(&:id).sort, blog_posts.map(&:id).sort)
195195
end
196196

197+
def test_has_many_with_foreign_key_as_an_array_raises
198+
expected_message = <<~MSG.squish
199+
Passing [:blog_id, :blog_post_id] array to :foreign_key option
200+
on the Sharded::BlogPost#broken_array_fk_comments association is not supported.
201+
Use the query_constraints: [:blog_id, :blog_post_id] option instead to represent a composite foreign key.
202+
MSG
203+
assert_raises ArgumentError, match: expected_message do
204+
Sharded::BlogPost.has_many :broken_array_fk_comments,
205+
class_name: "Sharded::Comment", foreign_key: [:blog_id, :blog_post_id]
206+
end
207+
end
208+
209+
def test_belongs_to_with_foreign_key_as_an_array_raises
210+
expected_message = <<~MSG.squish
211+
Passing [:blog_id, :blog_post_id] array to :foreign_key option
212+
on the Sharded::Comment#broken_array_fk_blog_post association is not supported.
213+
Use the query_constraints: [:blog_id, :blog_post_id] option instead to represent a composite foreign key.
214+
MSG
215+
assert_raises ArgumentError, match: expected_message do
216+
Sharded::Comment.belongs_to :broken_array_fk_blog_post,
217+
class_name: "Sharded::Blog", foreign_key: [:blog_id, :blog_post_id]
218+
end
219+
end
220+
197221
def test_has_many_association_with_composite_foreign_key_loads_records
198222
blog_post = sharded_blog_posts(:great_post_blog_one)
199223

activerecord/test/cases/reflection_test.rb

+33
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,17 @@ def test_has_many_reflection
226226
assert_equal "companies", Firm.reflect_on_association(:clients_of_firm).table_name
227227
end
228228

229+
def test_has_many_reflection_with_array_fk_raises
230+
expected_message = <<~MSG.squish
231+
Passing [:firm_id, :firm_name] array to :foreign_key option
232+
on the Firm#clients association is not supported.
233+
Use the query_constraints: [:firm_id, :firm_name] option instead to represent a composite foreign key.
234+
MSG
235+
assert_raises ArgumentError, match: expected_message do
236+
ActiveRecord::Reflection.create(:has_many, :clients, nil, { foreign_key: [:firm_id, :firm_name] }, Firm)
237+
end
238+
end
239+
229240
def test_has_one_reflection
230241
reflection_for_account = ActiveRecord::Reflection.create(:has_one, :account, nil, { foreign_key: "firm_id", dependent: :destroy }, Firm)
231242
assert_equal reflection_for_account, Firm.reflect_on_association(:account)
@@ -234,6 +245,17 @@ def test_has_one_reflection
234245
assert_equal "accounts", Firm.reflect_on_association(:account).table_name
235246
end
236247

248+
def has_one_reflection_with_array_fk_raises
249+
expected_message = <<~MSG.squish
250+
Passing [:firm_id, :firm_name] array to :foreign_key option
251+
on the Firm#account association is not supported.
252+
Use the query_constraints: [:firm_id, :firm_name] option instead to represent a composite foreign key.
253+
MSG
254+
assert_raises ArgumentError, match: expected_message do
255+
ActiveRecord::Reflection.create(:has_one, :account, nil, { foreign_key: [:firm_id, :firm_name] }, Firm)
256+
end
257+
end
258+
237259
def test_belongs_to_inferred_foreign_key_from_assoc_name
238260
Company.belongs_to :foo
239261
assert_equal "foo_id", Company.reflect_on_association(:foo).foreign_key
@@ -243,6 +265,17 @@ def test_belongs_to_inferred_foreign_key_from_assoc_name
243265
assert_equal "xyzzy_id", Company.reflect_on_association(:baz).foreign_key
244266
end
245267

268+
def test_belongs_to_reflection_with_array_fk_raises
269+
expected_message = <<~MSG.squish
270+
Passing [:firm_id, :firm_name] array to :foreign_key option
271+
on the Firm#client association is not supported.
272+
Use the query_constraints: [:firm_id, :firm_name] option instead to represent a composite foreign key.
273+
MSG
274+
assert_raises ArgumentError, match: expected_message do
275+
ActiveRecord::Reflection.create(:belongs_to, :client, nil, { foreign_key: [:firm_id, :firm_name] }, Firm)
276+
end
277+
end
278+
246279
def test_association_reflection_in_modules
247280
ActiveRecord::Base.store_full_sti_class = false
248281

0 commit comments

Comments
 (0)