From 5cec7ce3cb2eaed0b03c5469b95fef182b604648 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 8 Apr 2020 22:18:45 +0900 Subject: [PATCH] Support bulk insert/upsert on relation to preserve scope values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows to work `author.books.insert_all` as expected. Co-authored-by: Josef Šimánek --- activerecord/CHANGELOG.md | 4 ++ .../lib/active_record/association_relation.rb | 12 ++++ .../associations/collection_proxy.rb | 4 +- activerecord/lib/active_record/insert_all.rb | 12 +++- activerecord/test/cases/insert_all_test.rb | 59 +++++++++++++++++++ 5 files changed, 89 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index bcdb3c97f3ca2..fe5a428f4c29b 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Support bulk insert/upsert on relation to preserve scope values. + + *Josef Šimánek*, *Ryuta Kamizono* + * Preserve column comment value on changing column name on MySQL. *Islam Taha* diff --git a/activerecord/lib/active_record/association_relation.rb b/activerecord/lib/active_record/association_relation.rb index d2abd3a42146b..71f5da59c5a6c 100644 --- a/activerecord/lib/active_record/association_relation.rb +++ b/activerecord/lib/active_record/association_relation.rb @@ -31,6 +31,18 @@ def create!(attributes = nil, &block) scoping { @association.create!(attributes, &block) } end + %w(insert insert_all insert! insert_all! upsert upsert_all).each do |method| + class_eval <<~RUBY + def #{method}(attributes, **kwargs) + if @association.reflection.through_reflection? + raise ArgumentError, "Bulk insert or upsert is currently not supported for has_many through association" + end + + scoping { klass.#{method}(attributes, **kwargs) } + end + RUBY + end + private def exec_queries super do |record| diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 9db74c0697f52..f4d56a00679a0 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -1096,7 +1096,9 @@ def reset_scope # :nodoc: SpawnMethods, ].flat_map { |klass| klass.public_instance_methods(false) - } - self.public_instance_methods(false) - [:select] + [:scoping, :values] + } - self.public_instance_methods(false) - [:select] + [ + :scoping, :values, :insert, :insert_all, :insert!, :insert_all!, :upsert, :upsert_all + ] delegate(*delegate_methods, to: :scope) diff --git a/activerecord/lib/active_record/insert_all.rb b/activerecord/lib/active_record/insert_all.rb index 6f0fd0e5f7f96..a031e85a4aba9 100644 --- a/activerecord/lib/active_record/insert_all.rb +++ b/activerecord/lib/active_record/insert_all.rb @@ -10,9 +10,15 @@ class InsertAll # :nodoc: def initialize(model, inserts, on_duplicate:, returning: nil, unique_by: nil) raise ArgumentError, "Empty list of attributes passed" if inserts.blank? - @model, @connection, @inserts, @keys = model, model.connection, inserts, inserts.first.keys.map(&:to_s).to_set + @model, @connection, @inserts, @keys = model, model.connection, inserts, inserts.first.keys.map(&:to_s) @on_duplicate, @returning, @unique_by = on_duplicate, returning, unique_by + if model.scope_attributes? + @scope_attributes = model.scope_attributes + @keys |= @scope_attributes.keys + end + @keys = @keys.to_set + @returning = (connection.supports_insert_returning? ? primary_keys : false) if @returning.nil? @returning = false if @returning == [] @@ -49,6 +55,8 @@ def update_duplicates? def map_key_with_value inserts.map do |attributes| attributes = attributes.stringify_keys + attributes.merge!(scope_attributes) if scope_attributes + verify_attributes(attributes) keys.map do |key| @@ -58,6 +66,8 @@ def map_key_with_value end private + attr_reader :scope_attributes + def find_unique_index_for(unique_by) name_or_columns = unique_by || model.primary_key match = Array(name_or_columns).map(&:to_s) diff --git a/activerecord/test/cases/insert_all_test.rb b/activerecord/test/cases/insert_all_test.rb index 3098a13f115ca..7cff2f822afc0 100644 --- a/activerecord/test/cases/insert_all_test.rb +++ b/activerecord/test/cases/insert_all_test.rb @@ -1,8 +1,11 @@ # frozen_string_literal: true require "cases/helper" +require "models/author" require "models/book" require "models/speedometer" +require "models/subscription" +require "models/subscriber" class ReadonlyNameBook < Book attr_readonly :name @@ -341,6 +344,62 @@ def test_insert_all_with_enum_values assert_equal ["published", "proposed"], Book.where(isbn: ["1234566", "1234567"]).order(:id).pluck(:status) end + def test_insert_all_on_relation + author = Author.create!(name: "Jimmy") + + assert_difference "author.books.count", +1 do + author.books.insert_all!([{ name: "My little book", isbn: "1974522598" }]) + end + end + + def test_insert_all_on_relation_precedence + author = Author.create!(name: "Jimmy") + second_author = Author.create!(name: "Bob") + + assert_difference "author.books.count", +1 do + author.books.insert_all!([{ name: "My little book", isbn: "1974522598", author_id: second_author.id }]) + end + end + + def test_insert_all_create_with + assert_difference "Book.where(format: 'X').count", +2 do + Book.create_with(format: "X").insert_all!([ { name: "A" }, { name: "B" } ]) + end + end + + def test_insert_all_has_many_through + book = Book.first + assert_raise(ArgumentError) { book.subscribers.insert_all!([ { nick: "Jimmy" } ]) } + end + + def test_upsert_all_on_relation + author = Author.create!(name: "Jimmy") + + assert_difference "author.books.count", +1 do + author.books.upsert_all([{ name: "My little book", isbn: "1974522598" }]) + end + end + + def test_upsert_all_on_relation_precedence + author = Author.create!(name: "Jimmy") + second_author = Author.create!(name: "Bob") + + assert_difference "author.books.count", +1 do + author.books.upsert_all([{ name: "My little book", isbn: "1974522598", author_id: second_author.id }]) + end + end + + def test_upsert_all_create_with + assert_difference "Book.where(format: 'X').count", +2 do + Book.create_with(format: "X").upsert_all([ { name: "A" }, { name: "B" } ]) + end + end + + def test_upsert_all_has_many_through + book = Book.first + assert_raise(ArgumentError) { book.subscribers.upsert_all([ { nick: "Jimmy" } ]) } + end + private def capture_log_output output = StringIO.new