`Record#dup` adds this additional new record to associations this record belongs to when `inverse_of` specified

[aglushkov]

Steps to reproduce

Record#dup adds this additional new record to associations this record belongs to when inverse_of specified.

So when parent association is saved, it saves this duplicated record.

• It does not saves new records with config.load_defaults 6.0
• It does not saves new records when inverse_of option not specified

# 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", "~> 7.0.0"
  gem "sqlite3"
  gem "debug"
end

require "active_record/railtie"
require "debug"

class Application < Rails::Application
  config.load_defaults 6.1 #------------ WORKS with 6.0 ------------
end

ENV["DATABASE_URL"] = "sqlite3::memory:"
Rails.application.initialize!


ActiveRecord::Schema.define do
  create_table :categories, force: true do |t|
    t.string :name, default: 'default'
  end

  create_table :topics, force: true do |t|
    t.belongs_to :category, null: false
  end
end

class Category < ActiveRecord::Base
  has_many :topics, inverse_of: :category # WORKS without inverse_of even with load_defaults 6.1
end

class Topic < ActiveRecord::Base
  belongs_to :category, inverse_of: :topics # WORKS without inverse_of even with load_defaults 6.1
end

category = Category.create!
topic = Topic.create!(category: category)

# Add topic duplicate
topic_dup = topic.dup
category_from_dup = topic.dup.category

# We have extra topic with id = nil
puts category_from_dup.topics.size
puts category_from_dup.topics.inspect

# Save category
category_from_dup.update!(name: 'name')

# It creates additional topic
puts category_from_dup.reload.topics.size
puts category_from_dup.topics.inspect

Expected behavior

No additional records should be created. Same as it works with load_defaults 6.0. Or same as it works without inverse_of option

Actual behavior

We've got additional record.dup assigned to parent association

System configuration

Rails version: Rails 7.0.2.3

Ruby version: ruby 3.0.2p107 (2021-07-07 revision 0db68f0233) [x86_64-linux]

[ghiculescu]

Presumably the bug is related to has_many_inversing, could you try just toggling that setting rather than load_defaults to confirm?

[aglushkov]

Yes with load_defaults 6.1 and has_many_inversing = false it works correctly

[ghiculescu]

Confirmed that this also fails against Rails 6.1.

[koshigoe]

I have a similar problem.

parent.collection.create!(parent: parent) may add double new item to parent.collection, but saved item is one. parent.<< is also.

Steps to reproduce

• ActiveRecord::Base.has_many_inversing = true
• use belongs_to with inverse_of:
• loaded collection members before call collection.create!(parent: parent, ...)
  • Must pass parent instance to reproduce problem (Not parent_id, Don't omit parent attribute)

# frozen_string_literal: true

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'
  gem 'activerecord', '6.1.6.1'
  gem 'sqlite3'
  gem 'aggregate_assertions'
end

require 'active_record'

ActiveRecord::Base.has_many_inversing = true
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: 'test.db')

ActiveRecord::Schema.define do
  create_table :parents, force: true

  create_table :nodes, force: true do |t|
    t.references :parent, foreign_key: false
    t.string :name
  end
end

class Parent < ActiveRecord::Base
  has_many :nodes
end

class Node < ActiveRecord::Base
  belongs_to :parent, inverse_of: :nodes
end

require 'minitest/autorun'

class CollectionAssociationTest < Minitest::Test
  def teardown
    Parent.delete_all
    Node.delete_all
  end

  def test_collection_create_with_parent_instance_if_collection_loaded
    parent = Parent.create!(id: 123)
    parent.nodes.to_a

    parent.nodes.create!(name: 'sample', parent: parent)

    aggregate_assertions do
      assert(parent.nodes.all?(&:persisted?))
      assert_equal([[123, 'sample']], parent.nodes.pluck(:parent_id, :name))
      assert_equal([[123, 'sample']], Node.where(parent_id: parent.id).pluck(:parent_id, :name))
    end
  end

  def test_collection_create_with_parent_instance_if_collection_unloaded
    parent = Parent.create!(id: 123)

    parent.nodes.create!(name: 'sample', parent: parent)

    aggregate_assertions do
      assert(parent.nodes.all?(&:persisted?))
      assert_equal([[123, 'sample']], parent.nodes.pluck(:parent_id, :name))
      assert_equal([[123, 'sample']], Node.where(parent_id: parent.id).pluck(:parent_id, :name))
    end
  end

  def test_collection_push_with_parent_instance_if_collection_loaded
    parent = Parent.create!(id: 123)
    parent.nodes.to_a

    parent.nodes << Node.new(name: 'sample', parent: parent)

    aggregate_assertions do
      assert(parent.nodes.all?(&:persisted?))
      assert_equal([[123, 'sample']], parent.nodes.pluck(:parent_id, :name))
      assert_equal([[123, 'sample']], Node.where(parent_id: parent.id).pluck(:parent_id, :name))
    end
  end

  def test_collection_push_with_parent_instance_if_collection_unloaded
    parent = Parent.create!(id: 123)

    parent.nodes << Node.new(name: 'sample', parent: parent)

    aggregate_assertions do
      assert(parent.nodes.all?(&:persisted?))
      assert_equal([[123, 'sample']], parent.nodes.pluck(:parent_id, :name))
      assert_equal([[123, 'sample']], Node.where(parent_id: parent.id).pluck(:parent_id, :name))
    end
  end
end

__END__

Fetching gem metadata from https://rubygems.org/........
Resolving dependencies...
Using bundler 2.2.33
Using concurrent-ruby 1.1.10
Using sqlite3 1.4.4
Using zeitwerk 2.6.0
Using minitest 5.16.2
Using i18n 1.12.0
Using tzinfo 2.0.5
Using aggregate_assertions 0.1.1
Using activesupport 6.1.6.1
Using activemodel 6.1.6.1
Using activerecord 6.1.6.1
-- create_table(:parents, {:force=>true})
   -> 0.0064s
-- create_table(:nodes, {:force=>true})
   -> 0.0016s
Run options: --seed 6135

# Running:

.FF.

Finished in 0.045052s, 88.7863 runs/s, 266.3589 assertions/s.

  1) Failure:
CollectionAssociationTest#test_collection_push_with_parent_instance_if_collection_loaded [issue_test.rb:75]:
--- expected
+++ actual
@@ -1 +1 @@
-[[123, "sample"]]
+[[123, "sample"], [123, "sample"]]


  2) Failure:
CollectionAssociationTest#test_collection_create_with_parent_instance_if_collection_loaded [issue_test.rb:50]:
--- expected
+++ actual
@@ -1 +1 @@
-[[123, "sample"]]
+[[123, "sample"], [123, "sample"]]


4 runs, 12 assertions, 2 failures, 0 errors, 0 skips

[clouvet]

@koshigoe, the pull request (https://github.com/rails/rails/pull/45140) I opened for this issue addresses your test cases too.

[clouvet]

I think I spoke too soon about https://github.com/rails/rails/pull/45140 solving the problem. It breaks inversing for unsaved objects.

I think it's possible to rephrase the reproduction steps a bit to point closer to the issue:

# 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"
  gem "sqlite3"
  gem "debug"
end

require "active_record/railtie"
require "debug"
require 'minitest/autorun'

class Application < Rails::Application
  config.load_defaults 6.1 #------------ WORKS with 6.0 ------------
end

ENV["DATABASE_URL"] = "sqlite3::memory:"
Rails.application.initialize!


ActiveRecord::Schema.define do
  create_table :categories, force: true do |t|
    t.string :name, default: 'default'
  end

  create_table :topics, force: true do |t|
    t.belongs_to :category, null: false
  end
end

class Category < ActiveRecord::Base
  has_many :topics, inverse_of: :category # WORKS without inverse_of even with load_defaults 6.1
end

class Topic < ActiveRecord::Base
  belongs_to :category, inverse_of: :topics # WORKS without inverse_of even with load_defaults 6.1
end

class InversingTest < ActiveSupport::TestCase

  def test_that_inversed_association_collections_are_equal_for_new_and_saved_objects
    category = Category.create!
    topic = Topic.create!(category: category)
    topic_dup = topic.dup
    
    assert_equal category, topic_dup.category
    assert_equal category.topics, topic_dup.category.topics
  end

end

That produces the following result:

# Running:

F

Failure:
InversingTest#test_that_inversed_association_collections_are_equal_for_new_and_saved_objects [test2.rb:53]:
--- expected
+++ actual
@@ -1 +1 @@
-#<ActiveRecord::Associations::CollectionProxy [#<Topic id: 1, category_id: 1>]>
+#<ActiveRecord::Associations::CollectionProxy [#<Topic id: 1, category_id: 1>, #<Topic id: nil, category_id: 1>]>

If category and topic_dup.category are equal, then shouldn't category.topics and topic_dup.category.topics be equal also?

[lst4rksugarwork]

Hey @clouvet did you ever figure this out? I'm seeing a similar situation and cannot figure out how to get the parent record to stop saving a duplicate one

[clouvet]

@lst4rksugarwork Sorry, but I wasn't able to make any progress beyond what I demonstrated here and on the attempted pull request: https://github.com/rails/rails/pull/45140.