From dd986814e7f528916a44f841a02c955a4522307c Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Tue, 25 Nov 2014 10:52:33 -0700 Subject: 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 --- .../associations/collection_association.rb | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') 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 -- cgit v1.2.3