FIX: Stop race condition when topic notification jobs are scheduled during a database transaction
This was not picked up by tests because scheduled jobs are run immediately and in the current thread (and therefore the current database transaction). This particular case sometimes occurs inside multiple nested transactions, so simply moving the offending line outside of the transaction is not enough. Implemented TransactionHelper, which allows us to use `TransactionHelper.after_commit` to define code to be run after the current transaction has been committed.
This commit is contained in:
parent
2dc3a50dac
commit
32db976156
|
@ -0,0 +1,38 @@
|
||||||
|
##
|
||||||
|
# Allows running arbitrary code after the current transaction has been committed.
|
||||||
|
# Works even with nexted transactions. Useful for scheduling sidekiq jobs.
|
||||||
|
# Slightly simplified version of https://dev.to/evilmartians/rails-aftercommit-everywhere--4j9g
|
||||||
|
# Usage:
|
||||||
|
# Topic.transaction do
|
||||||
|
# puts "Some work before scheduling"
|
||||||
|
# TransactionHelper.after_commit do
|
||||||
|
# puts "Running after commit"
|
||||||
|
# end
|
||||||
|
# puts "Some work after scheduling"
|
||||||
|
# end
|
||||||
|
#
|
||||||
|
# Produces:
|
||||||
|
# > Some work before scheduling
|
||||||
|
# > Some work after scheduling
|
||||||
|
# > Running after commit
|
||||||
|
|
||||||
|
module TransactionHelper
|
||||||
|
class AfterCommitWrapper
|
||||||
|
def initialize
|
||||||
|
@callback = Proc.new
|
||||||
|
end
|
||||||
|
|
||||||
|
def committed!(*)
|
||||||
|
@callback.call
|
||||||
|
end
|
||||||
|
|
||||||
|
def before_committed!(*); end
|
||||||
|
def rolledback!(*); end
|
||||||
|
end
|
||||||
|
|
||||||
|
def self.after_commit(&blk)
|
||||||
|
ActiveRecord::Base.connection.add_transaction_record(
|
||||||
|
AfterCommitWrapper.new(&blk)
|
||||||
|
)
|
||||||
|
end
|
||||||
|
end
|
|
@ -686,7 +686,9 @@ class Topic < ActiveRecord::Base
|
||||||
|
|
||||||
if post = self.ordered_posts.first
|
if post = self.ordered_posts.first
|
||||||
notified_user_ids = [post.user_id, post.last_editor_id].uniq
|
notified_user_ids = [post.user_id, post.last_editor_id].uniq
|
||||||
Jobs.enqueue(:notify_category_change, post_id: post.id, notified_user_ids: notified_user_ids)
|
TransactionHelper.after_commit do
|
||||||
|
Jobs.enqueue(:notify_category_change, post_id: post.id, notified_user_ids: notified_user_ids)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
@ -0,0 +1,32 @@
|
||||||
|
require 'rails_helper'
|
||||||
|
|
||||||
|
describe TransactionHelper do
|
||||||
|
|
||||||
|
it "runs callbacks after outermost transaction is committed" do
|
||||||
|
outputString = "1"
|
||||||
|
|
||||||
|
# Main transaction
|
||||||
|
ActiveRecord::Base.transaction do
|
||||||
|
outputString += "2"
|
||||||
|
|
||||||
|
# Nested transaction
|
||||||
|
ActiveRecord::Base.transaction do
|
||||||
|
outputString += "3"
|
||||||
|
|
||||||
|
TransactionHelper.after_commit do
|
||||||
|
outputString += "6"
|
||||||
|
end
|
||||||
|
outputString += "4"
|
||||||
|
end
|
||||||
|
|
||||||
|
TransactionHelper.after_commit do
|
||||||
|
outputString += "7"
|
||||||
|
end
|
||||||
|
|
||||||
|
outputString += "5"
|
||||||
|
end
|
||||||
|
|
||||||
|
expect(outputString).to eq("1234567")
|
||||||
|
end
|
||||||
|
|
||||||
|
end
|
Loading…
Reference in New Issue