Merge pull request rails#48357 from gmcgibbon/belongs_to_cpk

Add composite primary key validity check on belongs_to associations.

Author: eileencodes

activerecord/lib/active_record/associations.rb

@@ -184,6 +184,22 @@ def initialize(owner = nil, reflection = nil)
     end
   end
 
+  class CompositePrimaryKeyMismatchError < ActiveRecordError # :nodoc:
+    attr_reader :reflection
+
+    def initialize(reflection = nil)
+      if reflection
+        if reflection.has_one? || reflection.collection?
+          super("Association #{reflection.active_record}##{reflection.name} primary key #{reflection.active_record_primary_key} doesn't match with foreign key #{reflection.foreign_key}. Please specify query_constraints.")
+        else
+          super("Association #{reflection.active_record}##{reflection.name} primary key #{reflection.association_primary_key} doesn't match with foreign key #{reflection.foreign_key}. Please specify query_constraints.")
+        end
+      else
+        super("Association primary key doesn't match with foreign key.")
+      end
+    end
+  end
+
   class AmbiguousSourceReflectionForThroughAssociation < ActiveRecordError # :nodoc:
     def initialize(klass, macro, association_name, options, possible_sources)
       example_options = options.dup

activerecord/lib/active_record/reflection.rb

@@ -253,11 +253,11 @@ def inverse_of
       end
 
       def check_validity_of_inverse!
-        unless polymorphic?
-          if has_inverse? && inverse_of.nil?
+        if !polymorphic? && has_inverse?
+          if inverse_of.nil?
             raise InverseOfAssociationNotFoundError.new(self)
           end
-          if has_inverse? && inverse_of == self
+          if inverse_of == self
             raise InverseOfAssociationRecursiveError.new(self)
           end
         end
@@ -541,6 +541,14 @@ def join_foreign_key
 
       def check_validity!
         check_validity_of_inverse!
+
+        if !polymorphic? && klass.composite_primary_key?
+          if (has_one? || collection?) && Array(active_record_primary_key).length != Array(foreign_key).length
+            raise CompositePrimaryKeyMismatchError.new(self)
+          elsif belongs_to? && Array(association_primary_key).length != Array(foreign_key).length
+            raise CompositePrimaryKeyMismatchError.new(self)
+          end
+        end
       end
 
       def check_eager_loadable!

activerecord/test/cases/associations/belongs_to_associations_test.rb

@@ -30,6 +30,7 @@
 require "models/tree"
 require "models/node"
 require "models/club"
+require "models/cpk"
 
 class BelongsToAssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :companies, :developers, :projects, :topics,
@@ -1745,6 +1746,18 @@ def self.name; "Temp"; end
   ensure
     ActiveRecord.belongs_to_required_validates_foreign_key = original_value
   end
+
+  test "composite primary key malformed association" do
+    error = assert_raises(ActiveRecord::CompositePrimaryKeyMismatchError) do
+      book = Cpk::BrokenBook.new(title: "Some book", order: Cpk::Order.new(id: [1, 2]))
+      book.save!
+    end
+
+    assert_equal(<<~MESSAGE.squish, error.message)
+      Association Cpk::BrokenBook#order primary key ["shop_id", "id"]
+      doesn't match with foreign key order_id. Please specify query_constraints.
+    MESSAGE
+  end
 end
 
 class BelongsToWithForeignKeyTest < ActiveRecord::TestCase

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -3174,6 +3174,18 @@ def test_key_ensuring_owner_was_is_valid_when_dependent_option_is_destroy_async
     end
   end
 
+  test "composite primary key malformed association" do
+    error = assert_raises(ActiveRecord::CompositePrimaryKeyMismatchError) do
+      order = Cpk::BrokenOrder.new(id: [1, 2], books: [Cpk::Book.new(title: "Some book")])
+      order.save!
+    end
+
+    assert_equal(<<~MESSAGE.squish, error.message)
+      Association Cpk::BrokenOrder#books primary key ["shop_id", "id"]
+      doesn't match with foreign key broken_order_id. Please specify query_constraints.
+    MESSAGE
+  end
+
   private
     def force_signal37_to_load_all_clients_of_firm
       companies(:first_firm).clients_of_firm.load_target

activerecord/test/cases/associations/has_one_associations_test.rb

@@ -18,6 +18,7 @@
 require "models/club"
 require "models/membership"
 require "models/parrot"
+require "models/cpk"
 
 class HasOneAssociationsTest < ActiveRecord::TestCase
   self.use_transactional_tests = false unless supports_savepoints?
@@ -902,4 +903,16 @@ def test_has_one_with_touch_option_on_nonpersisted_built_associations_doesnt_upd
       car.build_special_bulb
     end
   end
+
+  test "composite primary key malformed association" do
+    error = assert_raises(ActiveRecord::CompositePrimaryKeyMismatchError) do
+      order = Cpk::BrokenOrder.new(id: [1, 2], book: Cpk::Book.new(title: "Some book"))
+      order.save!
+    end
+
+    assert_equal(<<~MESSAGE.squish, error.message)
+      Association Cpk::BrokenOrder#book primary key ["shop_id", "id"]
+      doesn't match with foreign key broken_order_id. Please specify query_constraints.
+    MESSAGE
+  end
 end

activerecord/test/models/cpk/book.rb

@@ -5,10 +5,13 @@ class Book < ActiveRecord::Base
     self.table_name = :cpk_books
     self.primary_key = [:author_id, :number]
 
-    belongs_to :order
     belongs_to :author, class_name: "Cpk::Author"
   end
 
   class BestSeller < Book
   end
+
+  class BrokenBook < Book
+    belongs_to :order
+  end
 end

activerecord/test/models/cpk/order.rb

@@ -6,6 +6,11 @@ class Order < ActiveRecord::Base
     self.primary_key = [:shop_id, :id]
 
     has_many :order_agreements, primary_key: :id
+    has_many :books, query_constraints: [:shop_id, :order_id]
+  end
+
+  class BrokenOrder < Order
     has_many :books
+    has_one :book
   end
 end

activerecord/test/schema/schema.rb

@@ -246,6 +246,7 @@
     t.string :title
     t.integer :revision
     t.integer :order_id
+    t.integer :shop_id
   end
 
   create_table :cpk_authors, force: true do |t|