Merge pull request #35178 from bogdan/has-many-size

Bugfix has_many association #size when ids reader is cached and assoc…

activerecord/lib/active_record/associations/collection_association.rb

@@ -347,7 +347,6 @@ def _create_record(attributes, raise = false, &block)
               add_to_target(record) do
                 result = insert_record(record, true, raise) {
                   @_was_loaded = loaded?
-                  @association_ids = nil
                 }
               end
               raise ActiveRecord::Rollback unless result
@@ -384,6 +383,7 @@ def remove_records(existing_records, records, method)
 
           delete_records(existing_records, method) if existing_records.any?
           @target -= records
+          @association_ids = nil
 
           records.each { |record| callback(:after_remove, record) }
         end
@@ -424,7 +424,6 @@ def concat_records(records, raise = false)
               unless owner.new_record?
                 result &&= insert_record(record, true, raise) {
                   @_was_loaded = loaded?
-                  @association_ids = nil
                 }
               end
             end
@@ -447,6 +446,7 @@ def replace_on_target(record, index, skip_callbacks)
           if index
             target[index] = record
           elsif @_was_loaded || !loaded?
+            @association_ids = nil
             target << record
           end
 

activerecord/test/cases/associations/has_many_associations_test.rb

@@ -2004,6 +2004,21 @@ def test_get_ids_for_unloaded_associations_does_not_load_them
     assert_not_predicate company.clients, :loaded?
   end
 
+  def test_ids_reader_cache_not_used_for_size_when_association_is_dirty
+    firm = Firm.create!(name: "Startup")
+    assert_equal 0, firm.client_ids.size
+    firm.clients.build
+    assert_equal 1, firm.clients.size
+  end
+
+  def test_ids_reader_cache_should_be_cleared_when_collection_is_deleted
+    firm = companies(:first_firm)
+    assert_equal [2, 3, 11], firm.client_ids
+    client = firm.clients.first
+    firm.clients.delete(client)
+    assert_equal [3, 11], firm.client_ids
+  end
+
   def test_zero_counter_cache_usage_on_unloaded_association
     car = Car.create!(name: "My AppliCar")
     assert_no_queries do