Clear out target when no new records

[luanzeba]

This fix was originally part of #45019 but that PR was solving different issues at once, so I decided to break that up into separate PRs for each problem.

Summary

Calling #size on a CollectionAssociation can yield the wrong results when the collection association's target value gets out of sync. This issue can happen for associations with a scope, like so:

class Member < ActiveRecord::Base
  has_many :memberships, -> { where(favorite: true) }
end

Steps to reproduce

Using the example above:

member = Member.create!
membership = member.memberships.create!(favorite: true)
membership.update!(favorite: false)

assert_equal 0, member.memberships.size
assert_equal 0, member.memberships.size

# The first assertion passes and the second fails with:
# Expected: 0
#   Actual: 1

Full reproduction script

# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem 'rails', github: 'rails/rails', branch: 'main'
  gem 'sqlite3'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :members, force: true do |t|
  end

  create_table :memberships, force: true do |t|
    t.integer :member_id
    t.boolean :favorite
  end
end

class Member < ActiveRecord::Base
  has_many :memberships, -> { where(favorite: true) }
end

class Membership < ActiveRecord::Base
  belongs_to :member
end

class BugTest < Minitest::Test
  def test_associations_size
    member = Member.create!
    membership = member.memberships.create!(favorite: true)
    membership.update!(favorite: false)

    assert_equal 0, member.memberships.size
    assert_equal 0, member.memberships.size
  end
end

It's important to have both assertions because CollectionAssociation#size has a conditional with multiple branches and each time we call #size in the test above it takes a different code path.
In the first call the association isn't loaded, so we end up on this branch of the conditional

rails/activerecord/lib/active_record/associations/collection_association.rb

Lines 208 to 210 in 5fdbd21

	elsif !association_scope.distinct_value && !target.empty?
	unsaved_records = target.select(&:new_record?)
	unsaved_records.size + count_records

Inside the count_records method called above we mark the association as loaded and assume the target is empty, but we never ensure that's the case. This assumption can be incorrect when you have a scope on the association as in the example above

rails/activerecord/lib/active_record/associations/has_many_association.rb

Lines 82 to 85 in 5fdbd21

	# If there's nothing in the database and @target has no new records
	# we are certain the current target is an empty array. This is a
	# documented side-effect of the method that may avoid an extra SELECT.
	loaded! if count == 0

In the second call to #size, the association is loaded so we end up on this branch.

rails/activerecord/lib/active_record/associations/collection_association.rb

Lines 202 to 203 in 5fdbd21

	if !find_target? || loaded?
	target.size

Now, the target should be empty, but it actually has 1 record, so we get the test failure.

The fix is to ensure target is empty inside count_records if count is 0

Other Information

I ran this fix against the GitHub monolith tests and got a green build

[jonathanhefner]

Does everything still work as expected if we apply this optimization even with new_record? records? i.e.:

Suggested change

	if count == 0 && target.select(&:new_record?).empty?
	self.target = []
	loaded!
	end
	if count == 0
	target.select!(&:new_record?)
	loaded!
	end

[luanzeba]

That worked!! Thanks for the suggestion 😄 Got all tests passing

[jonathanhefner]

If my previous suggestion works, we'll need to update this comment ("target is an empty array") too.

[luanzeba]

Good call, updated!

rails/activerecord/lib/active_record/associations/has_many_association.rb

Lines 84 to 86 in db0e0c0

	# If there's nothing in the database, @target should only contain new
	# records or be an empty array. This is a documented side-effect of
	# the method that may avoid an extra SELECT.

[jonathanhefner]

Thank you, @luanzeba! And especially thank you for the thorough report and testing! 😍

(Backported to 7-0-stable.)

[bf4]

I'm having a hard time reasoning about why the size of member.favorite_memberships.size is 0. The count of member.favorite_memberships.count is 1, right? Shouldn't the addition of a persisted record to the relation therefore change the size to 1 as well? we're coming across this case where we are surprised that the following is now true

    membership = member.favorite_memberships.create!
    refute_included membership, member.favorite_memberships