Closed xTRiM closed 3 years ago
Using this gem inside models is weird, because models have their own after_commit
and this gem tries to reproduce AR's buitin method to allow its usage outside of models.
I thought that it is obvious: use built-in after_commit
callback in models, bring after_commit_everywhere
if you need to use it outside models.
However, as this gem uses builtin ActiveRecord's mechanisms (building fake records and registering in AR's after_commit callbacks queue), there shouldn't be any difference between them. Different behavior is a bug, I believe.
Please tell me more about your setup:
If you're able to provide executable Gist (like one for reproduction of bugs in ActiveRecord itself) it would be very helpful and appreciated.
You are absolutely right about after_commit in models.
It happened because we were so used to placing after_commit's in services (thank you for the gem!). And it blew up after copying some service code to add "simple bulk updater" to the model (to cut the corner, which shouldn't happen but it did ;).
Actually, the gem itself doesn't even participate in this "bug." It's a psychological bug, when the dev forgets about "outside of models" in the gem description and gets used to placing after_commit's everywhere ;)
Here's a simple gist which shows how it happened (and how it doesn't have to do anything with after_commit_everywhere itself). https://gist.github.com/xTRiM/c10a492fc64da29109bad61716df83a4 Run with --seed 9083 for intended order.
I played with your gist replacing puts
with raise
and swapping between AR's class-level callback and after_commit_everywhere and damn, yes, AR's class-level callback makes another test case to fail. This is so weird!
~I guess that this gem's after_commit
method behaves differently is that internally it uses instance-level callbacks, not class-level (basically, it uses ActiveRecord::Base#after_commit
, not ActiveRecord::Base.after_commit
).~
~This is apparently a bug in ActiveRecord itself, please create a new issue in the Ruby on Rails repository here: https://github.com/rails/rails/issues~
Thank you for reporting this, it is interesting.
Oh, no, I was wrong, forget the nonsense that I wrote.
Actually there is a problem in following code:
class Post < ActiveRecord::Base
def self.bulk_ops
find_each do
after_commit { raise "Some doesn't expect that to happen in future tests, but they should" }
end
end
end
This is the same as:
class Post < ActiveRecord::Base
after_commit do
raise "Some doesn't expect that to happen in SecondTest, but they should"
end
end
So by calling class-level after_commit
method you're effectively adding callback to future records.
That's why following test cases are failing. As they actually should.
Added a note to the README
@Envek Incredible, thank you!
We were looking at some strange bug where one test was weirdly affecting another - jobs enqueued in one test were seemingly trying to run in another.
As it turned out - there was a class method on the model, used for some bulk updates, which were enqueuing async jobs with after_commit {...} block. Dev intended to run after_commit only inside of that bulk update method, but it actually was defined for the whole model, because the AR model has its own after_commit method.
Extracting that bulk method out from the AR model into a separate service solved the issue.
Probably makes sense to put some warning in the Readme about usage within AR models?