From 9eee7822ac4bce983ad45a98c4d111eb36285199 Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 23 Dec 2016 21:56:20 +0900
Subject: Add a record to target before any callbacks loads the record

`append_record` was added at 15ddd51 for not double adding the record.
But adding `append_record` (checking `@target.include?(record)`) caused
performance regression #27434. Instead of checking not double adding the
record, add a record to target before any callbacks loads the record.

Fixes #27434.
---
 .../associations/collection_association.rb         | 25 ++++++++++++++--------
 .../associations/has_many_through_association.rb   |  4 ----
 2 files changed, 16 insertions(+), 13 deletions(-)

(limited to 'activerecord/lib/active_record')

diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb
index 46923f690a..13f77c7d4d 100644
--- a/activerecord/lib/active_record/associations/collection_association.rb
+++ b/activerecord/lib/active_record/associations/collection_association.rb
@@ -299,12 +299,23 @@ module ActiveRecord
       def replace_on_target(record, index, skip_callbacks)
         callback(:before_add, record) unless skip_callbacks
 
-        yield(record) if block_given?
+        begin
+          if index
+            record_was = target[index]
+            target[index] = record
+          else
+            target << record
+          end
 
-        if index
-          @target[index] = record
-        else
-          append_record(record)
+          yield(record) if block_given?
+        rescue
+          if index
+            target[index] = record_was
+          else
+            target.delete(record)
+          end
+
+          raise
         end
 
         callback(:after_add, record) unless skip_callbacks
@@ -502,10 +513,6 @@ module ActiveRecord
             load_target.select { |r| ids.include?(r.id.to_s) }
           end
         end
-
-        def append_record(record)
-          @target << record unless @target.include?(record)
-        end
     end
   end
 end
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 8c90aea975..0c0aefe3b9 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -206,10 +206,6 @@ module ActiveRecord
         def invertible_for?(record)
           false
         end
-
-        def append_record(record)
-          @target << record
-        end
     end
   end
 end
-- 
cgit v1.2.3