aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord/lib/active_record
diff options
context:
space:
mode:
authorRyuta Kamizono <kamipo@gmail.com>2016-12-23 21:56:20 +0900
committerRyuta Kamizono <kamipo@gmail.com>2016-12-23 21:56:20 +0900
commit9eee7822ac4bce983ad45a98c4d111eb36285199 (patch)
tree20c13d1bcc947a2b6cfc2e8ecd25714f76aca7a5 /activerecord/lib/active_record
parentfd63aa02289d64e9d14fe56723f1de64bca3bb1f (diff)
downloadrails-9eee7822ac4bce983ad45a98c4d111eb36285199.tar.gz
rails-9eee7822ac4bce983ad45a98c4d111eb36285199.tar.bz2
rails-9eee7822ac4bce983ad45a98c4d111eb36285199.zip
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.
Diffstat (limited to 'activerecord/lib/active_record')
-rw-r--r--activerecord/lib/active_record/associations/collection_association.rb25
-rw-r--r--activerecord/lib/active_record/associations/has_many_through_association.rb4
2 files changed, 16 insertions, 13 deletions
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