Share foreign_key_present? implementation in _has_ associations

6 files changed, 71 insertions, 43 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index cd5fdd5964..14af55f327 100644
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -116,6 +116,7 @@ module ActiveRecord
     autoload :Association,           'active_record/associations/association'
     autoload :SingularAssociation,   'active_record/associations/singular_association'
     autoload :CollectionAssociation, 'active_record/associations/collection_association'
+    autoload :ForeignAssociation,    'active_record/associations/foreign_association'
     autoload :CollectionProxy,       'active_record/associations/collection_proxy'
 
     autoload :BelongsToAssociation,            'active_record/associations/belongs_to_association'
diff --git a/activerecord/lib/active_record/associations/foreign_association.rb b/activerecord/lib/active_record/associations/foreign_association.rb
new file mode 100644
index 0000000000..fe48ecec29
--- /dev/null
+++ b/activerecord/lib/active_record/associations/foreign_association.rb
@@ -0,0 +1,11 @@
+module ActiveRecord::Associations
+  module ForeignAssociation
+    def foreign_key_present?
+      if reflection.klass.primary_key
+        owner.attribute_present?(reflection.active_record_primary_key)
+      else
+        false
+      end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index 93084e0dcf..d7f655d00c 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -6,6 +6,7 @@ module ActiveRecord
     # If the association has a <tt>:through</tt> option further specialization
     # is provided by its child HasManyThroughAssociation.
     class HasManyAssociation < CollectionAssociation #:nodoc:
+      include ForeignAssociation
 
       def handle_dependency
         case options[:dependent]
@@ -153,14 +154,6 @@ module ActiveRecord
           end
         end
 
-        def foreign_key_present?
-          if reflection.klass.primary_key
-            owner.attribute_present?(reflection.association_primary_key)
-          else
-            false
-          end
-        end
-
         def concat_records(records, *)
           update_counter_if_success(super, records.length)
         end
diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb
index e6095d84dc..74b8c53758 100644
--- a/activerecord/lib/active_record/associations/has_one_association.rb
+++ b/activerecord/lib/active_record/associations/has_one_association.rb
@@ -2,6 +2,7 @@ module ActiveRecord
   # = Active Record Belongs To Has One Association
   module Associations
     class HasOneAssociation < SingularAssociation #:nodoc:
+      include ForeignAssociation
 
       def handle_dependency
         case options[:dependent]
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index d3b74aa616..21a45042fa 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -31,6 +31,8 @@ require 'models/student'
 require 'models/pirate'
 require 'models/ship'
 require 'models/tyre'
+require 'models/subscriber'
+require 'models/subscription'
 
 class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase
   fixtures :authors, :posts, :comments
@@ -43,12 +45,59 @@ class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCa
   end
 end
 
+class HasManyAssociationsTestPrimaryKeys < ActiveRecord::TestCase
+  fixtures :authors, :essays, :subscribers, :subscriptions, :people
+
+  def test_custom_primary_key_on_new_record_should_fetch_with_query
+    subscriber = Subscriber.new(nick: 'webster132')
+    assert !subscriber.subscriptions.loaded?
+
+    assert_queries 1 do
+      assert_equal 2, subscriber.subscriptions.size
+    end
+
+    assert_equal subscriber.subscriptions, Subscription.where(subscriber_id: 'webster132')
+  end
+
+  def test_association_primary_key_on_new_record_should_fetch_with_query
+    author = Author.new(:name => "David")
+    assert !author.essays.loaded?
+
+    assert_queries 1 do
+      assert_equal 1, author.essays.size
+    end
+
+    assert_equal author.essays, Essay.where(writer_id: "David")
+  end
+
+  def test_has_many_custom_primary_key
+    david = authors(:david)
+    assert_equal david.essays, Essay.where(writer_id: "David")
+  end
+
+  def test_has_many_assignment_with_custom_primary_key
+    david = people(:david)
+
+    assert_equal ["A Modest Proposal"], david.essays.map(&:name)
+    david.essays = [Essay.create!(name: "Remote Work" )]
+    assert_equal ["Remote Work"], david.essays.map(&:name)
+  end
+
+  def test_blank_custom_primary_key_on_new_record_should_not_run_queries
+    author = Author.new
+    assert !author.essays.loaded?
+
+    assert_queries 0 do
+      assert_equal 0, author.essays.size
+    end
+  end
+end
 
 class HasManyAssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :categories, :companies, :developers, :projects,
            :developers_projects, :topics, :authors, :comments,
-           :people, :posts, :readers, :taggings, :cars, :essays,
-           :categorizations, :jobs, :tags
+           :posts, :readers, :taggings, :cars, :jobs, :tags,
+           :categorizations
 
   def setup
     Client.destroyed_client_ids.clear
@@ -1578,39 +1627,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     end
   end
 
-  def test_custom_primary_key_on_new_record_should_fetch_with_query
-    author = Author.new(:name => "David")
-    assert !author.essays.loaded?
-
-    assert_queries 1 do
-      assert_equal 1, author.essays.size
-    end
-
-    assert_equal author.essays, Essay.where(writer_id: "David")
-  end
-
-  def test_has_many_custom_primary_key
-    david = authors(:david)
-    assert_equal david.essays, Essay.where(writer_id: "David")
-  end
-
-  def test_has_many_assignment_with_custom_primary_key
-    david = people(:david)
-
-    assert_equal ["A Modest Proposal"], david.essays.map(&:name)
-    david.essays = [Essay.create!(name: "Remote Work" )]
-    assert_equal ["Remote Work"], david.essays.map(&:name)
-  end
-
-  def test_blank_custom_primary_key_on_new_record_should_not_run_queries
-    author = Author.new
-    assert !author.essays.loaded?
-
-    assert_queries 0 do
-      assert_equal 0, author.essays.size
-    end
-  end
-
   def test_calling_first_or_last_with_integer_on_association_should_not_load_association
     firm = companies(:first_firm)
     firm.clients.create(:name => 'Foo')
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index a69f7a5262..ddf967b166 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -566,6 +566,12 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
     assert_equal author.post, post
   end
 
+  def test_has_one_loading_for_new_record
+    post = Post.create!(author_id: 42, title: 'foo', body: 'bar')
+    author = Author.new(id: 42)
+    assert author.post
+  end
+
   def test_has_one_relationship_cannot_have_a_counter_cache
     assert_raise(ArgumentError) do
       Class.new(ActiveRecord::Base) do