Merge pull request #32617 from tgturner/size-should-use-available-association

Loaded associations should not run a new query when size is called

3 files changed, 82 insertions, 1 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 0d8fa48235..36a3d59784 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Ensure `Associations::CollectionAssociation#size` and `Associations::CollectionAssociation#empty?`
+    use loaded association ids if present.
+
+    *Graham Turner*
+
 *   Add support to preload associations of polymorphic associations when not all the records have the requested associations.
 
     *Dana Sherson*
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 671c4c56df..f233947fbd 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -212,6 +212,8 @@ module ActiveRecord
       def size
         if !find_target? || loaded?
           target.size
+        elsif @association_ids && target.empty?
+          @association_ids.size
         elsif !association_scope.group_values.empty?
           load_target.size
         elsif !association_scope.distinct_value && !target.empty?
@@ -231,7 +233,7 @@ module ActiveRecord
       # loaded and you are going to fetch the records anyway it is better to
       # check <tt>collection.length.zero?</tt>.
       def empty?
-        if loaded?
+        if loaded? || @association_ids
           size.zero?
         else
           target.empty? && !scope.exists?
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index 33fe5ccabc..feb7390e2c 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -993,6 +993,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal 1, post.readers.size
   end
 
+  def test_collection_empty_with_dirty_target
+    post = posts(:thinking)
+    assert_equal [], post.reader_ids
+    assert_empty post.readers
+    post.readers.reset
+    post.readers.build
+    assert_equal [], post.reader_ids
+    assert_not_empty post.readers
+  end
+
   def test_collection_size_twice_for_regressions
     post = posts(:thinking)
     assert_equal 0, post.readers.size
@@ -2573,6 +2583,70 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     end
   end
 
+  test "calling size on an association that has not been loaded performs a query" do
+    car = Car.create!
+    Bulb.create(car_id: car.id)
+
+    car_two = Car.create!
+
+    assert_queries(1) do
+      assert_equal 1, car.bulbs.size
+    end
+
+    assert_queries(1) do
+      assert_equal 0, car_two.bulbs.size
+    end
+  end
+
+  test "calling size on an association that has been loaded does not perform query" do
+    car = Car.create!
+    Bulb.create(car_id: car.id)
+    car.bulb_ids
+
+    car_two = Car.create!
+    car_two.bulb_ids
+
+    assert_no_queries do
+      assert_equal 1, car.bulbs.size
+    end
+
+    assert_no_queries do
+      assert_equal 0, car_two.bulbs.size
+    end
+  end
+
+  test "calling empty on an association that has not been loaded performs a query" do
+    car = Car.create!
+    Bulb.create(car_id: car.id)
+
+    car_two = Car.create!
+
+    assert_queries(1) do
+      assert_not_empty car.bulbs
+    end
+
+    assert_queries(1) do
+      assert_empty car_two.bulbs
+    end
+  end
+
+  test "calling empty on an association that has been loaded does not performs query" do
+    car = Car.create!
+    Bulb.create(car_id: car.id)
+    car.bulb_ids
+
+    car_two = Car.create!
+    car_two.bulb_ids
+
+    assert_no_queries do
+      assert_not_empty car.bulbs
+    end
+
+    assert_no_queries do
+      assert_empty car_two.bulbs
+    end
+  end
+
   class AuthorWithErrorDestroyingAssociation < ActiveRecord::Base
     self.table_name = "authors"
     has_many :posts_with_error_destroying,