Skip to content

Commit

Permalink
Handle proper values for ActiveJob.enqueue_after_transaction_commit f…
Browse files Browse the repository at this point in the history
…or Rails 7.2 support (#43)

Hi Betterment friends!

As addressed in @smudge's PR
#40, Rails 7.2 introduces the
concept of `enqueue_after_transaction_commit`, which should not be used
in conjunction with Delayed. The implementation, and the associated
tests, make the assumption that the values used by ActiveJob are `true`
and `false`. The implementation in
ActiveJob::EnqueueAfterTransactionCommit
(https://github.com/rails/rails/blob/ac1d7681d05906b2b050eced76c6a2165f420821/activejob/lib/active_job/enqueue_after_transaction_commit.rb#L7-L14)
uses the values of `:always` and `:never`. The issues arises when
hitting the conditional check
(https://github.com/Betterment/delayed/compare/main...warmerzega:delayed:main?expand=1#diff-325ce2458ac2c3e05143813a4899c9f996ae51f54cdb8fa5f79f50efab872f9aR20)
where `!!:never == true` and causes the condition to be true (and
raising the exception) when `job.class.enqueue_after_transaction_commit
== :never`.
  • Loading branch information
warmerzega authored Aug 13, 2024
1 parent fd97a38 commit 37dff23
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,18 @@ jobs:
- gemfiles/rails_6_1.gemfile
- gemfiles/rails_7_0.gemfile
- gemfiles/rails_7_1.gemfile
- gemfiles/rails_7_2.gemfile
exclude:
- ruby: '3.2'
gemfile: gemfiles/rails_5_2.gemfile
- ruby: '3.1'
gemfile: gemfiles/rails_5_2.gemfile
- ruby: '3.0'
gemfile: gemfiles/rails_7_2.gemfile
- ruby: '3.0'
gemfile: gemfiles/rails_5_2.gemfile
- ruby: '2.7'
gemfile: gemfiles/rails_7_2.gemfile
- ruby: '2.7'
gemfile: gemfiles/rails_7_1.gemfile
- ruby: '2.6'
Expand Down
1 change: 1 addition & 0 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ Style/FrozenStringLiteralComment:
- 'gemfiles/rails_6_1.gemfile'
- 'gemfiles/rails_7_0.gemfile'
- 'gemfiles/rails_7_1.gemfile'
- 'gemfiles/rails_7_2.gemfile'
- 'gemfiles/rails_main.gemfile'
- 'lib/delayed.rb'
- 'lib/delayed/active_job_adapter.rb'
Expand Down
6 changes: 6 additions & 0 deletions Appraisals
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ appraise 'rails-7-1' do
gem 'activerecord', '~> 7.1.0'
end

appraise 'rails-7-2' do
gem 'actionmailer', '~> 7.2.0'
gem 'activejob', '~> 7.2.0'
gem 'activerecord', '~> 7.2.0'
end

appraise 'rails-main' do
gem 'actionmailer', github: 'rails/rails', glob: 'actionmailer/*.gemspec'
gem 'activejob', github: 'rails/rails', glob: 'activejob/*.gemspec'
Expand Down
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
delayed (0.5.4)
delayed (0.5.5)
activerecord (>= 5.2)
concurrent-ruby

Expand Down
2 changes: 1 addition & 1 deletion delayed.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Gem::Specification.new do |spec|
spec.require_paths = ['lib']
spec.summary = 'a multi-threaded, SQL-driven ActiveJob backend used at Betterment to process millions of background jobs per day'

spec.version = '0.5.4'
spec.version = '0.5.5'
spec.metadata = {
'changelog_uri' => 'https://github.com/betterment/delayed/blob/main/CHANGELOG.md',
'bug_tracker_uri' => 'https://github.com/betterment/delayed/issues',
Expand Down
18 changes: 18 additions & 0 deletions gemfiles/rails_7_2.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "actionmailer", "~> 7.2.0"
gem "activejob", "~> 7.2.0"
gem "activerecord", "~> 7.2.0"
gem "appraisal"
gem "betterlint"
gem "mysql2"
gem "pg"
gem "rake"
gem "rspec"
gem "sqlite3", "~> 1.7.3"
gem "timecop"
gem "zeitwerk"

gemspec path: "../"
2 changes: 1 addition & 1 deletion lib/delayed/active_job_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def enqueue_at(job, timestamp)
private

def _enqueue(job, opts = {})
if job.class.respond_to?(:enqueue_after_transaction_commit) && job.class.enqueue_after_transaction_commit
if job.class.respond_to?(:enqueue_after_transaction_commit) && job.class.enqueue_after_transaction_commit == :always
raise UnsafeEnqueueError, "The ':delayed' ActiveJob adapter is not compatible with enqueue_after_transaction_commit"
end

Expand Down
15 changes: 13 additions & 2 deletions spec/delayed/active_job_adapter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -288,16 +288,27 @@ def perform(arg, kwarg:)
end

if ActiveJob.gem_version.release >= Gem::Version.new('7.2')
context 'when the given job sets enqueue_after_transaction_commit to true' do
context 'when the given job sets enqueue_after_transaction_commit to :always' do
before do
JobClass.include ActiveJob::EnqueueAfterTransactionCommit # normally run in an ActiveJob railtie
JobClass.enqueue_after_transaction_commit = true
JobClass.enqueue_after_transaction_commit = :always
end

it 'raises an exception on enqueue' do
expect { JobClass.perform_later }.to raise_error(Delayed::ActiveJobAdapter::UnsafeEnqueueError)
end
end

context 'when the given job sets enqueue_after_transaction_commit to :never' do
before do
JobClass.include ActiveJob::EnqueueAfterTransactionCommit # normally run in an ActiveJob railtie
JobClass.enqueue_after_transaction_commit = :never
end

it 'does not raises an exception on enqueue' do
expect { JobClass.perform_later }.not_to raise_error(Delayed::ActiveJobAdapter::UnsafeEnqueueError)
end
end
end

context 'when using the ActiveJob test adapter' do
Expand Down

0 comments on commit 37dff23

Please sign in to comment.