Allow association foreign_key to be an Array

Currently the only way to setup a composite foreign key is to use `query_constraints`.
With our intention to decouple `query_constraints` from `foreign_key` we need to allow
`foreign_key` to be an Array to preserve the current behavior achieved with `query_constraints`.

Author: nvasilevski

activerecord/lib/active_record/reflection.rb

@@ -382,7 +382,6 @@ def initialize(name, scope, options, active_record)
         @klass         = options[:anonymous_class]
         @plural_name   = active_record.pluralize_table_names ?
                             name.to_s.pluralize : name.to_s
-        validate_reflection!
       end
 
       def autosave=(autosave)
@@ -434,17 +433,6 @@ def scope_for(relation, owner = nil)
         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
@@ -511,10 +499,14 @@ def join_table
       end
 
       def foreign_key(infer_from_inverse_of: true)
-        @foreign_key ||= if options[:query_constraints]
+        @foreign_key ||= if options[:foreign_key]
+          if options[:foreign_key].is_a?(Array)
+            options[:foreign_key].map { |fk| fk.to_s.freeze }.freeze
+          else
+            options[:foreign_key].to_s.freeze
+          end
+        elsif options[:query_constraints]
           options[:query_constraints].map { |fk| fk.to_s.freeze }.freeze
-        elsif options[:foreign_key]
-          options[:foreign_key].to_s
         else
           derived_fk = derive_foreign_key(infer_from_inverse_of: infer_from_inverse_of)
 

activerecord/test/cases/associations_test.rb

@@ -196,30 +196,6 @@ def test_querying_by_relation_with_composite_key
     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)
 
@@ -228,6 +204,20 @@ def test_has_many_association_with_composite_foreign_key_loads_records
     assert_includes(comments, sharded_comments(:great_comment_blog_post_one))
   end
 
+  def test_belongs_to_with_explicit_composite_foreign_key
+    car = Cpk::Car.create(make: "Tesla", model: "Model S")
+    review = Cpk::CarReview.create(car: car, comment: "Great car!", rating: 5)
+
+    review.reload
+
+    sql = capture_sql do
+      assert_equal(car, review.car)
+    end
+
+    assert_match(/#{Regexp.escape(Cpk::Car.lease_connection.quote_table_name("cpk_cars.make"))} =/, sql.first)
+    assert_match(/#{Regexp.escape(Cpk::Car.lease_connection.quote_table_name("cpk_cars.model"))} =/, sql.first)
+  end
+
   def test_cpk_model_has_many_records_by_id_attribute
     order = cpk_orders(:cpk_groceries_order_1)
     _order_shop_id, order_id = order.id

activerecord/test/cases/reflection_test.rb

@@ -226,17 +226,6 @@ def test_has_many_reflection
     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)
@@ -245,17 +234,6 @@ def test_has_one_reflection
     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
@@ -265,17 +243,6 @@ def test_belongs_to_inferred_foreign_key_from_assoc_name
     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
 

activerecord/test/models/cpk.rb

@@ -10,3 +10,5 @@
 require_relative "cpk/chapter"
 require_relative "cpk/post"
 require_relative "cpk/comment"
+require_relative "cpk/car"
+require_relative "cpk/car_review"

activerecord/test/models/cpk/car.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+module Cpk
+  class Car < ActiveRecord::Base
+    self.table_name = :cpk_cars
+  end
+end

activerecord/test/models/cpk/car_review.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+module Cpk
+  class CarReview < ActiveRecord::Base
+    self.table_name = :cpk_car_reviews
+
+    belongs_to :car, foreign_key: [:car_make, :car_model]
+  end
+end

activerecord/test/schema/schema.rb

@@ -307,6 +307,18 @@
     t.index :order_id
   end
 
+  create_table :cpk_cars, force: true, primary_key: [:make, :model] do |t|
+    t.string :make, null: false
+    t.string :model, null: false
+  end
+
+  create_table :cpk_car_reviews, force: true do |t|
+    t.string :car_make, null: false
+    t.string :car_model, null: false
+    t.text :comment
+    t.integer :rating
+  end
+
   create_table :paragraphs, force: true do |t|
     t.references :book
   end