I don’t know if I consider this an bug in Rails, but at least a gotcha. Wanted to post about it to make sure I’m not just missing something and see if there is any interest in making this less of a gotcha in Rails and what that might look like.
My gotcha is when using the “dirty” ActiveRecord module with an after_commit
hook. It is not reliable. If the model is only saved once it works just fine (so most of the time). But there is another save within the same transaction to that model (which can happen via subtle way with other AR hooks) the previously changed attributes are overwritten in the second save so when the after_commit
runs it doesn’t see any of the changes from the first save.
I put together a quick test file to demonstrate this. The first test passes just fine. The second test does not pass and this might be a surprise is more subtle code.
#!/usr/bin/env ruby
require 'bundler/inline'
gemfile do
source 'https://rubygems.org'
gem 'activerecord'
gem 'sqlite3'
end
require "minitest/autorun"
require 'active_record'
ActiveRecord::Base.establish_connection adapter: 'sqlite3', database: 'test.db'
ActiveRecord::Base.connection.tap do |conn|
conn.create_table :widgets, force: true do |t|
t.string :fld1, :fld2
t.boolean :hook_run, null: false, default: false
end
end
class Widget < ActiveRecord::Base
after_commit(if: :fld1_previously_changed?) { self.hook_run = true }
end
describe 'Changed attributes in after commit hook' do
subject { Widget.new fld1: 'val1' }
it 'provides correct changed attributes after single save in after_commit' do
subject.transaction do
subject.save!
end
expect( subject.hook_run ).must_equal true
end
it 'provides correct changed attributes after a second save in the same txn' do
subject.transaction do
subject.save!
subject.update! fld2: 'val2'
end
expect( subject.hook_run ).must_equal true
end
end
What, if anything, should the Rails framework do to help with this gotcha?
One solution would be to merge the previously_changed
attributes together for each additional saves until the transaction is finally committed. I lean away from this solution as it really changes the semantics of the “dirty” module. While it might eliminate this surprise it could lead to other surprises.
Another solution might just be to issue a warning to the logger if the “dirty” module is used within the after_commit
hooks being run. This is the solution I like the best as it warns the developer that the dirty module is unreliable within an after_commit
hook but it doesn’t stop them from using it.
If you are looking to a work-around to this problem the best solution I have found is to use a combination of hooks and instance variables:
class Widget < ActiveRecord::Base
before_save(if: :fld1_changed?) { @fld1_updated = true }
after_commit if: -> { @fld1_updated } do
self.hook_run = true
remove_instance_variable :@fld1_updated
end
end
Curious what others think. Am I missing something? Should this be addressed by the Rails framework? If so how?