From 2d7db7e7b2bae5b5a0aa7ff19c0e0a2be0d55459 Mon Sep 17 00:00:00 2001 From: Brendan Weibrecht Date: Tue, 10 Oct 2023 00:36:11 +1100 Subject: [PATCH 1/5] Run tests on Rails 7.1 --- .github/workflows/tests.yml | 14 +++++++------- spec/gemfiles/Gemfile-rails-7.1 | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) create mode 100644 spec/gemfiles/Gemfile-rails-7.1 diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f92b69b2..41c814ef 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -8,16 +8,16 @@ jobs: strategy: matrix: ruby_version: ['2.7', '3.0', '3.1', '3.2'] - rails_gemfile: ['6.0', '6.1', '7.0'] + rails_gemfile: ['6.0', '6.1', '7.0', '7.1'] postgres_version: ['14'] include: # Postgres versions - - { ruby_version: '3.2', rails_gemfile: '7.0', postgres_version: '9' } - - { ruby_version: '3.2', rails_gemfile: '7.0', postgres_version: '10' } - - { ruby_version: '3.2', rails_gemfile: '7.0', postgres_version: '11' } - - { ruby_version: '3.2', rails_gemfile: '7.0', postgres_version: '12' } - - { ruby_version: '3.2', rails_gemfile: '7.0', postgres_version: '13' } - - { ruby_version: '3.2', rails_gemfile: '7.0', postgres_version: '14' } + - { ruby_version: '3.2', rails_gemfile: '7.1', postgres_version: '9' } + - { ruby_version: '3.2', rails_gemfile: '7.1', postgres_version: '10' } + - { ruby_version: '3.2', rails_gemfile: '7.1', postgres_version: '11' } + - { ruby_version: '3.2', rails_gemfile: '7.1', postgres_version: '12' } + - { ruby_version: '3.2', rails_gemfile: '7.1', postgres_version: '13' } + - { ruby_version: '3.2', rails_gemfile: '7.1', postgres_version: '14' } exclude: [] name: "Test: Ruby ${{ matrix.ruby_version }}, Rails ${{ matrix.rails_gemfile }}, PostgreSQL ${{ matrix.postgres_version }}" services: diff --git a/spec/gemfiles/Gemfile-rails-7.1 b/spec/gemfiles/Gemfile-rails-7.1 new file mode 100644 index 00000000..2da7475c --- /dev/null +++ b/spec/gemfiles/Gemfile-rails-7.1 @@ -0,0 +1,23 @@ +source 'https://rubygems.org' + +gem 'que', path: '../..' + +group :development, :test do + gem 'rake' + + gem 'activerecord', '~> 7.1.0', require: nil + gem 'activejob', '~> 7.1.0', require: nil + gem 'sequel', require: nil + gem 'connection_pool', require: nil + gem 'pond', require: nil + gem 'pg', require: nil, platform: :ruby + gem 'pg_jruby', require: nil, platform: :jruby +end + +group :test do + gem 'minitest', '~> 5.10.1' + gem 'minitest-profile', '0.0.2' + gem 'minitest-hooks', '1.4.0' + gem 'pry' + gem 'pg_examiner', '~> 0.5.2' +end From 3792e549cf6387224c29745032473269e40fd024 Mon Sep 17 00:00:00 2001 From: Brendan Weibrecht Date: Tue, 10 Oct 2023 02:18:04 +1100 Subject: [PATCH 2/5] Use Rails 7.1 in Gemfile (Tests not yet passing) --- Gemfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index 084460f2..7b048d5f 100644 --- a/Gemfile +++ b/Gemfile @@ -6,8 +6,8 @@ source 'https://rubygems.org' group :development, :test do gem 'rake' - gem 'activerecord', '~> 6.0', require: nil - gem 'activejob', '~> 6.0', require: nil + gem 'activerecord', '~> 7.1.0', require: nil + gem 'activejob', '~> 7.1.0', require: nil gem 'sequel', require: nil gem 'connection_pool', require: nil gem 'pond', require: nil From 9de3b96946a5aecd063c64b20bd2c7a5a69cf3bc Mon Sep 17 00:00:00 2001 From: Brendan Weibrecht Date: Tue, 10 Oct 2023 02:21:01 +1100 Subject: [PATCH 3/5] Copy in ActiveJob::QueueAdapters::QueAdapter from Rails removal This no longer exists in Rails, but is still needed. I simply copied the class from the [removal PR](https://github.com/rails/rails/pull/46005), and removed the unnecessary comments, and adjusted the formatting slightly. (Tests not yet passing) --- lib/que/active_job/extensions.rb | 47 ++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/lib/que/active_job/extensions.rb b/lib/que/active_job/extensions.rb index 699d5f15..6921d54c 100644 --- a/lib/que/active_job/extensions.rb +++ b/lib/que/active_job/extensions.rb @@ -108,6 +108,53 @@ def run(args) end end +module ActiveJob + module QueueAdapters + class QueAdapter + def enqueue(job) + job_options = { priority: job.priority, queue: job.queue_name } + que_job = nil + + if require_job_options_kwarg? + que_job = JobWrapper.enqueue job.serialize, job_options: job_options + else + que_job = JobWrapper.enqueue job.serialize, **job_options + end + + job.provider_job_id = que_job.attrs["job_id"] + que_job + end + + def enqueue_at(job, timestamp) + job_options = { priority: job.priority, queue: job.queue_name, run_at: Time.at(timestamp) } + que_job = nil + + if require_job_options_kwarg? + que_job = JobWrapper.enqueue job.serialize, job_options: job_options + else + que_job = JobWrapper.enqueue job.serialize, **job_options + end + + job.provider_job_id = que_job.attrs["job_id"] + que_job + end + + private + + def require_job_options_kwarg? + @require_job_options_kwarg ||= + JobWrapper.method(:enqueue).parameters.any? { |ptype, pname| ptype == :key && pname == :job_options } + end + + class JobWrapper < Que::Job + def run(job_data) + Base.execute job_data + end + end + end + end +end + class ActiveJob::QueueAdapters::QueAdapter class JobWrapper < Que::Job extend Que::ActiveJob::WrapperExtensions::ClassMethods From ba5fe8cb80efd3fd1097bd694210808a84f0ef14 Mon Sep 17 00:00:00 2001 From: Brendan Weibrecht Date: Tue, 10 Oct 2023 02:26:38 +1100 Subject: [PATCH 4/5] Get copied-in ActiveJob::QueueAdapters::QueAdapter working I'd thought we'd need to now use `ActiveJob::Base.queue_adapter = ActiveJob::QueueAdapters::QueAdapter.new` rather than `ActiveJob::Base.queue_adapter = :que`, but it seems that by keeping the adapter class namespace unchanged, we can still use the latter. --- lib/que/active_job/extensions.rb | 73 +++++++++++++++++--------------- spec/spec_helper.rb | 8 ++-- 2 files changed, 44 insertions(+), 37 deletions(-) diff --git a/lib/que/active_job/extensions.rb b/lib/que/active_job/extensions.rb index 6921d54c..53922cf7 100644 --- a/lib/que/active_job/extensions.rb +++ b/lib/que/active_job/extensions.rb @@ -108,47 +108,54 @@ def run(args) end end -module ActiveJob - module QueueAdapters - class QueAdapter - def enqueue(job) - job_options = { priority: job.priority, queue: job.queue_name } - que_job = nil - - if require_job_options_kwarg? - que_job = JobWrapper.enqueue job.serialize, job_options: job_options - else - que_job = JobWrapper.enqueue job.serialize, **job_options +# This is the ActiveJob Que adapter for Rails 7.1+, given ActiveJob::QueueAdapters::QueAdapter has been removed from ActiveJob. +# For backwards compatibility, the class name of ActiveJob::QueueAdapters::QueAdapter::JobWrapper must remain the same, given this string would be in old job records in the database +if ActiveJob.gem_version >= Gem::Version.new('7.1') + module ActiveJob + module QueueAdapters + # Work around `autoload QueAdapter` being left over in ActiveJob after the adapter was removed + remove_const(:QueAdapter) if const_defined?(:QueAdapter) + + class QueAdapter + def enqueue(job) + job_options = { priority: job.priority, queue: job.queue_name } + que_job = nil + + if require_job_options_kwarg? + que_job = JobWrapper.enqueue job.serialize, job_options: job_options + else + que_job = JobWrapper.enqueue job.serialize, **job_options + end + + job.provider_job_id = que_job.attrs["job_id"] + que_job end - job.provider_job_id = que_job.attrs["job_id"] - que_job - end + def enqueue_at(job, timestamp) + job_options = { priority: job.priority, queue: job.queue_name, run_at: Time.at(timestamp) } + que_job = nil - def enqueue_at(job, timestamp) - job_options = { priority: job.priority, queue: job.queue_name, run_at: Time.at(timestamp) } - que_job = nil + if require_job_options_kwarg? + que_job = JobWrapper.enqueue job.serialize, job_options: job_options + else + que_job = JobWrapper.enqueue job.serialize, **job_options + end - if require_job_options_kwarg? - que_job = JobWrapper.enqueue job.serialize, job_options: job_options - else - que_job = JobWrapper.enqueue job.serialize, **job_options + job.provider_job_id = que_job.attrs["job_id"] + que_job end - job.provider_job_id = que_job.attrs["job_id"] - que_job - end - - private + private - def require_job_options_kwarg? - @require_job_options_kwarg ||= - JobWrapper.method(:enqueue).parameters.any? { |ptype, pname| ptype == :key && pname == :job_options } - end + def require_job_options_kwarg? + @require_job_options_kwarg ||= + JobWrapper.method(:enqueue).parameters.any? { |ptype, pname| ptype == :key && pname == :job_options } + end - class JobWrapper < Que::Job - def run(job_data) - Base.execute job_data + class JobWrapper < Que::Job + def run(job_data) + Base.execute job_data + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2f19f794..ac34bb5c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,6 +3,8 @@ # Silence Ruby warnings. $VERBOSE = nil +require 'que' + # ActiveRecord and ActiveJob require ActiveSupport, which affects a bunch of # core classes and may change some behavior that we rely on, so only bring it in # in some spec runs. @@ -10,14 +12,12 @@ require 'active_record' require 'active_job' + require 'que/active_job/extensions' + ActiveJob::Base.queue_adapter = :que ActiveJob::Base.logger = nil - - # require 'que/active_job/extensions' end -require 'que' - # Libraries necessary for tests. require 'uri' require 'pg' From 0c66d030e3e474c94758774b21abcdc0e84cc3b3 Mon Sep 17 00:00:00 2001 From: Brendan Weibrecht Date: Tue, 10 Oct 2023 02:53:08 +1100 Subject: [PATCH 5/5] Simplify ActiveJob::QueueAdapters::QueAdapter slightly by removing #require_job_options_kwarg? We don't need to support old versions of Que in the adapter anymore! Que's codebase always uses the latest Que =P I considered removing & inlining Que::ActiveJob::WrapperExtensions::ClassMethods#enqueue into our Rails 7.1+ QueAdapter, but it still needs to exist in place for support for older versions of Rails =\ Keeping it in place conditionally based on the Rails version would be [a bit complex](https://github.com/que-rb/que/issues/399#issuecomment-1725170442), and so perhaps not an improvement. --- lib/que/active_job/extensions.rb | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/lib/que/active_job/extensions.rb b/lib/que/active_job/extensions.rb index 53922cf7..04607593 100644 --- a/lib/que/active_job/extensions.rb +++ b/lib/que/active_job/extensions.rb @@ -119,39 +119,20 @@ module QueueAdapters class QueAdapter def enqueue(job) job_options = { priority: job.priority, queue: job.queue_name } - que_job = nil - - if require_job_options_kwarg? - que_job = JobWrapper.enqueue job.serialize, job_options: job_options - else - que_job = JobWrapper.enqueue job.serialize, **job_options - end - + que_job = JobWrapper.enqueue job.serialize, **job_options job.provider_job_id = que_job.attrs["job_id"] que_job end def enqueue_at(job, timestamp) job_options = { priority: job.priority, queue: job.queue_name, run_at: Time.at(timestamp) } - que_job = nil - - if require_job_options_kwarg? - que_job = JobWrapper.enqueue job.serialize, job_options: job_options - else - que_job = JobWrapper.enqueue job.serialize, **job_options - end - + que_job = JobWrapper.enqueue job.serialize, **job_options job.provider_job_id = que_job.attrs["job_id"] que_job end private - def require_job_options_kwarg? - @require_job_options_kwarg ||= - JobWrapper.method(:enqueue).parameters.any? { |ptype, pname| ptype == :key && pname == :job_options } - end - class JobWrapper < Que::Job def run(job_data) Base.execute job_data