diff options
| author | Larry Reid <lcreid@jadesystems.ca> | 2018-07-16 10:44:55 -0700 |
|---|---|---|
| committer | Larry Reid <lcreid@jadesystems.ca> | 2018-07-23 14:53:27 -0700 |
| commit | 332e7601a98ebff6a7494a556c7fe97c5691f085 (patch) | |
| tree | dd60a40250979168ca972db33a189774dc5e75e5 /activerecord/lib | |
| parent | bd139a59405044c27bf367cde671e4bdbf7ccb57 (diff) | |
| download | rails-332e7601a98ebff6a7494a556c7fe97c5691f085.tar.gz rails-332e7601a98ebff6a7494a556c7fe97c5691f085.tar.bz2 rails-332e7601a98ebff6a7494a556c7fe97c5691f085.zip | |
Fix circular `autosave: true`
Use a variable local to the `save_collection_association` method in
`activerecord/lib/active_record/autosave_association.rb`, instead of an
instance variable.
Prior to this PR, when there was a circular series of `autosave: true`
associations, the callback for a `has_many` association was run while
another instance of the same callback on the same association hadn't
finished running. When control returned to the first instance of the
callback, the instance variable had changed, and subsequent associated
records weren't saved correctly. Specifically, the ID field for the
`belongs_to` corresponding to the `has_many` was `nil`.
Remove unnecessary test and comments.
Fixes #28080.
Diffstat (limited to 'activerecord/lib')
| -rw-r--r-- | activerecord/lib/active_record/autosave_association.rb | 8 |
1 files changed, 6 insertions, 2 deletions
diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index a405f05e0b..78acc06eae 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -382,10 +382,14 @@ module ActiveRecord if association = association_instance_get(reflection.name) autosave = reflection.options[:autosave] + # By saving the instance variable in a local variable, we make the + # whole callback re-entrant, fixing issue #28080 + new_record_before_save = @new_record_before_save + # reconstruct the scope now that we know the owner's id association.reset_scope - if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) + if records = associated_records_to_validate_or_save(association, new_record_before_save, autosave) if autosave records_to_destroy = records.select(&:marked_for_destruction?) records_to_destroy.each { |record| association.destroy(record) } @@ -397,7 +401,7 @@ module ActiveRecord saved = true - if autosave != false && (@new_record_before_save || record.new_record?) + if autosave != false && (new_record_before_save || record.new_record?) if autosave saved = association.insert_record(record, false) elsif !reflection.nested? |