Skip to content

Commit 781ecc2

Browse files
Ensure reload sets correct owner for each association
Fix rails#46363. Currently calling `reload` re-calculates the association cache based on the object freshly loaded from the database. The fresh object gets assigned as `owner` for each association in the association cache. That object however is not the same as `self` (even though they both point to the same record). Under some specific circumstances (using `has_one/has_many through` associations and transactions with `after_commit` callbacks) this might lead to losing the effects of assignments done inside the callback. This commit fixes it by making `reload` point to `self` as `owner` for each association in the association cache.
1 parent 438cad4 commit 781ecc2

File tree

7 files changed

+67
-1
lines changed

7 files changed

+67
-1
lines changed

activerecord/lib/active_record/associations/association.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ module Associations
3333
# <tt>owner</tt>, the collection of its posts as <tt>target</tt>, and
3434
# the <tt>reflection</tt> object represents a <tt>:has_many</tt> macro.
3535
class Association # :nodoc:
36-
attr_reader :owner, :target, :reflection, :disable_joins
36+
attr_accessor :owner
37+
attr_reader :target, :reflection, :disable_joins
3738

3839
delegate :options, to: :reflection
3940

activerecord/lib/active_record/persistence.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,6 +1078,7 @@ def reload(options = nil)
10781078
end
10791079

10801080
@association_cache = fresh_object.instance_variable_get(:@association_cache)
1081+
@association_cache.each_value { |association| association.owner = self }
10811082
@attributes = fresh_object.instance_variable_get(:@attributes)
10821083
@new_record = false
10831084
@previously_new_record = false
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# frozen_string_literal: true
2+
3+
require "cases/helper"
4+
require "models/publication"
5+
require "models/editorship"
6+
require "models/editor"
7+
8+
class ReloadAssociationCacheTest < ActiveRecord::TestCase
9+
def test_reload_sets_correct_owner_for_association_cache
10+
publication = Publication.create!(name: "Rails Way")
11+
assert_equal "Rails Way (touched)", publication.name
12+
publication.reload
13+
assert_equal "Rails Way", publication.name
14+
publication.transaction do
15+
publication.editors = [publication.build_editor_in_chief(name: "Alex Black")]
16+
publication.save!
17+
end
18+
assert_equal "Rails Way (touched)", publication.name
19+
end
20+
end

activerecord/test/models/editor.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
# frozen_string_literal: true
2+
3+
class Editor < ActiveRecord::Base
4+
self.primary_key = "name"
5+
6+
has_one :publication, foreign_key: :editor_in_chief_id, inverse_of: :editor_in_chief
7+
has_many :editorships
8+
end
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
# frozen_string_literal: true
2+
3+
class Editorship < ActiveRecord::Base
4+
belongs_to :publication
5+
belongs_to :editor
6+
end
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# frozen_string_literal: true
2+
3+
class Publication < ActiveRecord::Base
4+
belongs_to :editor_in_chief, class_name: "Editor", inverse_of: :publication, optional: true
5+
has_many :editorships
6+
has_many :editors, through: :editorships
7+
8+
after_initialize do
9+
self.editor_in_chief = build_editor_in_chief(name: "John Doe")
10+
end
11+
12+
after_save_commit :touch_name
13+
def touch_name
14+
self.name = "#{name} (touched)"
15+
end
16+
end

activerecord/test/schema/schema.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,6 +562,15 @@
562562
t.index [:source_id, :sink_id], unique: true, name: "unique_edge_index"
563563
end
564564

565+
create_table :editorships, force: true do |t|
566+
t.string :publication_id
567+
t.string :editor_id
568+
end
569+
570+
create_table :editors, force: true do |t|
571+
t.string :name
572+
end
573+
565574
create_table :engines, force: true do |t|
566575
t.references :car, index: false
567576
end
@@ -1038,6 +1047,11 @@
10381047
t.integer :mentor_id
10391048
end
10401049

1050+
create_table :publications, force: true do |t|
1051+
t.column :name, :string
1052+
t.integer :editor_in_chief_id
1053+
end
1054+
10411055
create_table :randomly_named_table1, force: true do |t|
10421056
t.string :some_attribute
10431057
t.integer :another_attribute

0 commit comments

Comments
 (0)