1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Polymorphic has_one touch: Reset association cache result after create transaction

In case of a polymorphic association there's no automatic inverse_of to assign the
inverse record. So to get the record there needs to be a query executed,
however, if the query fires within the transaction that's trying to create
the associated record, no record can be found. And worse, the nil result is cached
on the association so after the transaction commits the record can't be found.

That's what happens if touch is enabled on a polymorphic has_one association.

Consider a Comment with a commentable association that needs to be touched.

For `Comment.create(commentable: Post.new)`, the existing code essentially
does `commentable.send(:comment)` within the create transaction for the comment
and thus not finding the comment.

Now we're purposefully clearing the cache in case we've tried accessing
the association within the transaction and found no object.

Before:

```
kaspth-imac 2.6.3 ~/code/rails/activerecord master *= ARCONN=postgresql bin/test test/cases/associations/has_one_associations_test.rb -n /commit/
Using postgresql
Run options: -n /commit/ --seed 46022

D, [2019-07-19T03:30:37.864537 #96022] DEBUG -- :   Chef Load (0.2ms)  SELECT "chefs".* FROM "chefs" WHERE "chefs"."employable_id" = $1 AND "chefs"."employable_type" = $2 LIMIT $3  [["employable_id", 1], ["employable_type", "DrinkDesignerWithPolymorphicTouchChef"], ["LIMIT", 1]]
D, [2019-07-19T03:30:37.865013 #96022] DEBUG -- :   Chef Create (0.2ms)  INSERT INTO "chefs" ("employable_id", "employable_type") VALUES ($1, $2) RETURNING "id"  [["employable_id", 1], ["employable_type", "DrinkDesignerWithPolymorphicTouchChef"]]
D, [2019-07-19T03:30:37.865201 #96022] DEBUG -- :   TRANSACTION (0.1ms)  RELEASE SAVEPOINT active_record_1
D, [2019-07-19T03:30:37.874136 #96022] DEBUG -- :   TRANSACTION (0.1ms)  ROLLBACK
D, [2019-07-19T03:30:37.874323 #96022] DEBUG -- :   TRANSACTION (0.1ms)  ROLLBACK
F

Failure:
HasOneAssociationsTest#test_polymorphic_has_one_with_touch_option_on_create_wont_cache_assocation_so_fetching_after_transaction_commit_works [/Users/kaspth/code/rails/activerecord/test/cases/associations/has_one_associations_test.rb:716]:
--- expected
+++ actual
@@ -1 +1 @@
-#<Chef id: 1, employable_id: 1, employable_type: "DrinkDesignerWithPolymorphicTouchChef", department_id: nil, employable_list_type: nil, employable_list_id: nil>
+nil
```

After:

```
kaspth-imac 2.6.3 ~/code/rails/activerecord master *= ARCONN=postgresql bin/test test/cases/associations/has_one_associations_test.rb -n /commit/
Using postgresql
Run options: -n /commit/ --seed 46022

D, [2019-07-19T03:30:22.479387 #95973] DEBUG -- :   Chef Create (0.3ms)  INSERT INTO "chefs" ("employable_id", "employable_type") VALUES ($1, $2) RETURNING "id"  [["employable_id", 1], ["employable_type", "DrinkDesignerWithPolymorphicTouchChef"]]
D, [2019-07-19T03:30:22.479574 #95973] DEBUG -- :   TRANSACTION (0.1ms)  RELEASE SAVEPOINT active_record_1
D, [2019-07-19T03:30:22.482051 #95973] DEBUG -- :   Chef Load (0.1ms)  SELECT "chefs".* FROM "chefs" WHERE "chefs"."employable_id" = $1 AND "chefs"."employable_type" = $2 LIMIT $3  [["employable_id", 1], ["employable_type", "DrinkDesignerWithPolymorphicTouchChef"], ["LIMIT", 1]]
D, [2019-07-19T03:30:22.482317 #95973] DEBUG -- :   TRANSACTION (0.1ms)  ROLLBACK
D, [2019-07-19T03:30:22.482437 #95973] DEBUG -- :   TRANSACTION (0.1ms)  ROLLBACK
.

Finished in 0.088498s, 11.2997 runs/s, 22.5994 assertions/s.
1 runs, 2 assertions, 0 failures, 0 errors, 0 skips
```

Notice the select now fires after the commit.
This commit is contained in:
Kasper Timm Hansen 2019-07-19 03:41:05 +02:00
parent 65dcc9d159
commit cea392eb96
No known key found for this signature in database
GPG key ID: 191153215EDA53D8
5 changed files with 37 additions and 12 deletions

View file

@ -56,6 +56,10 @@ module ActiveRecord
@inversed = false @inversed = false
end end
def reset_negative_cache # :nodoc:
reset if loaded? && target.nil?
end
# Reloads the \target and returns +self+ on success. # Reloads the \target and returns +self+ on success.
# The QueryCache is cleared if +force+ is true. # The QueryCache is cleared if +force+ is true.
def reload(force = false) def reload(force = false)

View file

@ -32,15 +32,12 @@ module ActiveRecord::Associations::Builder # :nodoc:
end end
end end
def self.touch_record(o, name, touch) def self.touch_record(record, name, touch)
record = o.send name instance = record.send(name)
return unless record && record.persisted? if instance&.persisted?
touch != true ?
if touch != true instance.touch(touch) : instance.touch
record.touch(touch)
else
record.touch
end end
end end
@ -48,11 +45,9 @@ module ActiveRecord::Associations::Builder # :nodoc:
name = reflection.name name = reflection.name
touch = reflection.options[:touch] touch = reflection.options[:touch]
callback = lambda { |record| callback = -> (record) { HasOne.touch_record(record, name, touch) }
HasOne.touch_record(record, name, touch)
}
model.after_create callback, if: :saved_changes? model.after_create callback, if: :saved_changes?
model.after_create_commit { association(name).reset_negative_cache }
model.after_update callback, if: :saved_changes? model.after_update callback, if: :saved_changes?
model.after_destroy callback model.after_destroy callback
model.after_touch callback model.after_touch callback

View file

@ -712,6 +712,24 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
} }
end end
def test_polymorphic_has_one_with_touch_option_on_create_wont_cache_association_so_fetching_after_transaction_commit_works
assert_queries(4) {
chef = Chef.create(employable: DrinkDesignerWithPolymorphicTouchChef.new)
employable = chef.employable
assert_equal chef, employable.chef
}
end
def test_polymorphic_has_one_with_touch_option_on_update_will_touch_record_by_fetching_from_database_if_needed
DrinkDesignerWithPolymorphicTouchChef.create(chef: Chef.new)
designer = DrinkDesignerWithPolymorphicTouchChef.last
assert_queries(3) {
designer.update(name: "foo")
}
end
def test_has_one_with_touch_option_on_update def test_has_one_with_touch_option_on_update
new_club = Club.create(name: "1000 Oaks") new_club = Club.create(name: "1000 Oaks")
new_club.create_membership new_club.create_membership

View file

@ -10,5 +10,11 @@ class DrinkDesignerWithPolymorphicDependentNullifyChef < ActiveRecord::Base
has_one :chef, as: :employable, dependent: :nullify has_one :chef, as: :employable, dependent: :nullify
end end
class DrinkDesignerWithPolymorphicTouchChef < ActiveRecord::Base
self.table_name = "drink_designers"
has_one :chef, as: :employable, touch: true
end
class MocktailDesigner < DrinkDesigner class MocktailDesigner < DrinkDesigner
end end

View file

@ -1070,6 +1070,7 @@ ActiveRecord::Schema.define do
create_table :cake_designers, force: true do |t| create_table :cake_designers, force: true do |t|
end end
create_table :drink_designers, force: true do |t| create_table :drink_designers, force: true do |t|
t.string :name
end end
create_table :chefs, force: true do |t| create_table :chefs, force: true do |t|
t.integer :employable_id t.integer :employable_id
@ -1077,6 +1078,7 @@ ActiveRecord::Schema.define do
t.integer :department_id t.integer :department_id
t.string :employable_list_type t.string :employable_list_type
t.integer :employable_list_id t.integer :employable_list_id
t.timestamps
end end
create_table :recipes, force: true do |t| create_table :recipes, force: true do |t|
t.integer :chef_id t.integer :chef_id