diff options
| author | Sean Griffin <sean@thoughtbot.com> | 2014-11-25 10:52:33 -0700 |
|---|---|---|
| committer | Sean Griffin <sean@thoughtbot.com> | 2014-11-25 11:07:24 -0700 |
| commit | dd986814e7f528916a44f841a02c955a4522307c (patch) | |
| tree | e7147e3a5240a2ade86d4e2d9a94c57dbfaaf46a /activerecord/lib/active_record/associations/collection_association.rb | |
| parent | d12f40ff336733eca17ec5216b2f6a6cfa35c57c (diff) | |
| download | rails-dd986814e7f528916a44f841a02c955a4522307c.tar.gz rails-dd986814e7f528916a44f841a02c955a4522307c.tar.bz2 rails-dd986814e7f528916a44f841a02c955a4522307c.zip | |
Setting an association replaces records with the same id in memory
The records weren't being replaced since equality in Active Record is
defined in terms of `id` only. It is reasonable to expect that the
references would be replaced in memory, even if no queries are actually
executed. This change did not appear to affect any other parts of the
code base. I chose not to execute callbacks since we're not actually
modifying the association in a way that will be persisted.
Fixes #17730
Diffstat (limited to 'activerecord/lib/active_record/associations/collection_association.rb')
| -rw-r--r-- | activerecord/lib/active_record/associations/collection_association.rb | 20 |
1 files changed, 18 insertions, 2 deletions
diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index d59cebb08b..2b7d39893d 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -358,6 +358,7 @@ module ActiveRecord if owner.new_record? replace_records(other_array, original_target) else + replace_common_records_in_memory(other_array, original_target) if other_array != original_target transaction { replace_records(other_array, original_target) } end @@ -385,11 +386,18 @@ module ActiveRecord target end - def add_to_target(record, skip_callbacks = false) + def add_to_target(record, skip_callbacks = false, &block) + if association_scope.distinct_value + index = @target.index(record) + end + replace_on_target(record, index, skip_callbacks, &block) + end + + def replace_on_target(record, index, skip_callbacks) callback(:before_add, record) unless skip_callbacks yield(record) if block_given? - if association_scope.distinct_value && index = @target.index(record) + if index @target[index] = record else @target << record @@ -534,6 +542,14 @@ module ActiveRecord target end + def replace_common_records_in_memory(new_target, original_target) + common_records = new_target & original_target + common_records.each do |record| + skip_callbacks = true + replace_on_target(record, @target.index(record), skip_callbacks) + end + end + def concat_records(records, should_raise = false) result = true |