Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

execute_after_commit triggers unexpected after_commit callbacks #6

Open
sevaorlov opened this issue May 6, 2016 · 4 comments
Open

Comments

@sevaorlov
Copy link
Contributor

Hey, @magnusvk, I just realised that this code https://github.com/magnusvk/after_commit_action/blob/master/lib/after_commit_action.rb#L23
leads to triggering all after_commit callbacks on the model that don't have on option, which might not be an expected behaviour.

Checking all open transaction if they include current record can be slow, not sure what is the best solution here.

@sevaorlov
Copy link
Contributor Author

maybe not so slow, so one possible solution I see is

def execute_after_commit(&block)
  transactions = ActiveRecord::Base.connection.transaction_manager.instance_variable_get(:@stack)
  records = transactions.map(&:records).flatten.uniq

  if ActiveRecord::Base.connection.open_transactions == 0 || !records.include?(self)
    block.call
  end
  ...
end

@magnusvk
Copy link
Owner

magnusvk commented May 9, 2016

Hm, well that kind of defeats the purpose, though, right? In that case that code doesn't actually run after commit, it runs right away. The point of adding the record is so that this code can run after the commit. That side effect seems worth it?

@sevaorlov
Copy link
Contributor Author

I think that is ok. When we call execute_after_commit we want to execute the code after current record transaction, but if there is no transaction for that record, that means record will not change and then there is no purpose of waiting some other transactions, we can execute the block straight away.

@magnusvk
Copy link
Owner

I don't think that's always true. What if I specifically warp a block in a transaction, for example? I'd expect after_commit hooks to run after the completion of that block, wouldn't I?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants