1 files changed, 71 insertions, 24 deletions

diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 09692fc3a0..1bfb1ea0c8 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -305,7 +305,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   test "building the association with an array" do
     speedometer = Speedometer.new(speedometer_id: "a")
-    data = [{name: "first"}, {name: "second"}]
+    data = [{ name: "first" }, { name: "second" }]
     speedometer.minivans.build(data)
 
     assert_equal 2, speedometer.minivans.size
@@ -380,47 +380,78 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
     assert_no_queries do
       bulbs.first()
-      bulbs.first({})
     end
 
     assert_no_queries do
       bulbs.second()
-      bulbs.second({})
     end
 
     assert_no_queries do
       bulbs.third()
-      bulbs.third({})
     end
 
     assert_no_queries do
       bulbs.fourth()
-      bulbs.fourth({})
     end
 
     assert_no_queries do
       bulbs.fifth()
-      bulbs.fifth({})
     end
 
     assert_no_queries do
       bulbs.forty_two()
-      bulbs.forty_two({})
     end
 
     assert_no_queries do
       bulbs.third_to_last()
-      bulbs.third_to_last({})
     end
 
     assert_no_queries do
       bulbs.second_to_last()
-      bulbs.second_to_last({})
     end
 
     assert_no_queries do
       bulbs.last()
-      bulbs.last({})
+    end
+  end
+
+  def test_finder_method_with_dirty_target
+    company = companies(:first_firm)
+    new_clients = []
+    assert_no_queries(ignore_none: false) do
+      new_clients << company.clients_of_firm.build(name: "Another Client")
+      new_clients << company.clients_of_firm.build(name: "Another Client II")
+      new_clients << company.clients_of_firm.build(name: "Another Client III")
+    end
+
+    assert_not company.clients_of_firm.loaded?
+    assert_queries(1) do
+      assert_same new_clients[0], company.clients_of_firm.third
+      assert_same new_clients[1], company.clients_of_firm.fourth
+      assert_same new_clients[2], company.clients_of_firm.fifth
+      assert_same new_clients[0], company.clients_of_firm.third_to_last
+      assert_same new_clients[1], company.clients_of_firm.second_to_last
+      assert_same new_clients[2], company.clients_of_firm.last
+    end
+  end
+
+  def test_finder_bang_method_with_dirty_target
+    company = companies(:first_firm)
+    new_clients = []
+    assert_no_queries(ignore_none: false) do
+      new_clients << company.clients_of_firm.build(name: "Another Client")
+      new_clients << company.clients_of_firm.build(name: "Another Client II")
+      new_clients << company.clients_of_firm.build(name: "Another Client III")
+    end
+
+    assert_not company.clients_of_firm.loaded?
+    assert_queries(1) do
+      assert_same new_clients[0], company.clients_of_firm.third!
+      assert_same new_clients[1], company.clients_of_firm.fourth!
+      assert_same new_clients[2], company.clients_of_firm.fifth!
+      assert_same new_clients[0], company.clients_of_firm.third_to_last!
+      assert_same new_clients[1], company.clients_of_firm.second_to_last!
+      assert_same new_clients[2], company.clients_of_firm.last!
     end
   end
 
@@ -487,7 +518,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_finding_array_compatibility
-    assert_equal 3, Firm.order(:id).find{|f| f.id > 0}.clients.length
+    assert_equal 3, Firm.order(:id).find { |f| f.id > 0 }.clients.length
   end
 
   def test_find_many_with_merged_options
@@ -525,14 +556,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_taking_with_a_number
     # taking from unloaded Relation
     bob = Author.find(authors(:bob).id)
+    new_post = bob.posts.build
+    assert_not bob.posts.loaded?
     assert_equal [posts(:misc_by_bob)], bob.posts.take(1)
-    bob = Author.find(authors(:bob).id)
     assert_equal [posts(:misc_by_bob), posts(:other_by_bob)], bob.posts.take(2)
+    assert_equal [posts(:misc_by_bob), posts(:other_by_bob), new_post], bob.posts.take(3)
 
     # taking from loaded Relation
-    bob.posts.to_a
-    assert_equal [posts(:misc_by_bob)], authors(:bob).posts.take(1)
-    assert_equal [posts(:misc_by_bob), posts(:other_by_bob)], authors(:bob).posts.take(2)
+    bob.posts.load
+    assert bob.posts.loaded?
+    assert_equal [posts(:misc_by_bob)], bob.posts.take(1)
+    assert_equal [posts(:misc_by_bob), posts(:other_by_bob)], bob.posts.take(2)
+    assert_equal [posts(:misc_by_bob), posts(:other_by_bob), new_post], bob.posts.take(3)
   end
 
   def test_taking_with_inverse_of
@@ -618,6 +653,18 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_raise(ActiveRecord::RecordNotFound) { firm.clients.find(2, 99) }
   end
 
+  def test_find_one_message_on_primary_key
+    firm = Firm.all.merge!(order: "id").first
+
+    e = assert_raises(ActiveRecord::RecordNotFound) do
+      firm.clients.find(0)
+    end
+    assert_equal 0, e.id
+    assert_equal "id", e.primary_key
+    assert_equal "Client", e.model
+    assert_match (/\ACouldn't find Client with 'id'=0/), e.message
+  end
+
   def test_find_ids_and_inverse_of
     force_signal37_to_load_all_clients_of_firm
 
@@ -644,7 +691,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert ! firm.clients.loaded?
 
     assert_queries(4) do
-      firm.clients.find_each(batch_size: 1) {|c| assert_equal firm.id, c.firm_id }
+      firm.clients.find_each(batch_size: 1) { |c| assert_equal firm.id, c.firm_id }
     end
 
     assert ! firm.clients.loaded?
@@ -670,7 +717,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
     assert_queries(2) do
       firm.clients.find_in_batches(batch_size: 2) do |clients|
-        clients.each {|c| assert_equal firm.id, c.firm_id }
+        clients.each { |c| assert_equal firm.id, c.firm_id }
       end
     end
 
@@ -701,7 +748,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_find_all_with_include_and_conditions
     assert_nothing_raised do
-      Developer.all.merge!(joins: :audit_logs, where: {"audit_logs.message" => nil, :name => "Smith"}).to_a
+      Developer.all.merge!(joins: :audit_logs, where: { "audit_logs.message" => nil, :name => "Smith" }).to_a
     end
   end
 
@@ -888,7 +935,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build_many
     company = companies(:first_firm)
-    new_clients = assert_no_queries(ignore_none: false) { company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) }
+    new_clients = assert_no_queries(ignore_none: false) { company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) }
     assert_equal 2, new_clients.size
   end
 
@@ -914,7 +961,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_build_via_block
     company = companies(:first_firm)
-    new_client = assert_no_queries(ignore_none: false) { company.clients_of_firm.build {|client| client.name = "Another Client" } }
+    new_client = assert_no_queries(ignore_none: false) { company.clients_of_firm.build { |client| client.name = "Another Client" } }
     assert !company.clients_of_firm.loaded?
 
     assert_equal "Another Client", new_client.name
@@ -925,7 +972,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   def test_build_many_via_block
     company = companies(:first_firm)
     new_clients = assert_no_queries(ignore_none: false) do
-      company.clients_of_firm.build([{"name" => "Another Client"}, {"name" => "Another Client II"}]) do |client|
+      company.clients_of_firm.build([{ "name" => "Another Client" }, { "name" => "Another Client II" }]) do |client|
         client.name = "changed"
       end
     end
@@ -962,7 +1009,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
   end
 
   def test_create_many
-    companies(:first_firm).clients_of_firm.create([{"name" => "Another Client"}, {"name" => "Another Client II"}])
+    companies(:first_firm).clients_of_firm.create([{ "name" => "Another Client" }, { "name" => "Another Client II" }])
     assert_equal 4, companies(:first_firm).clients_of_firm.reload.size
   end
 
@@ -1564,7 +1611,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
 
   def test_restrict_with_error_with_locale
     I18n.backend = I18n::Backend::Simple.new
-    I18n.backend.store_translations "en", activerecord: {attributes: {restricted_with_error_firm: {companies: "client companies"}}}
+    I18n.backend.store_translations "en", activerecord: { attributes: { restricted_with_error_firm: { companies: "client companies" } } }
     firm = RestrictedWithErrorFirm.create!(name: "restrict")
     firm.companies.create(name: "child")
 
@@ -1742,7 +1789,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
       lambda { authors(:mary).comments = [comments(:greetings), comments(:more_greetings)] },
       lambda { authors(:mary).comments << Comment.create!(body: "Yay", post_id: 424242) },
       lambda { authors(:mary).comments.delete(authors(:mary).comments.first) },
-    ].each {|block| assert_raise(ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection, &block) }
+    ].each { |block| assert_raise(ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection, &block) }
   end
 
   def test_dynamic_find_should_respect_association_order_for_through