feat: Support GoodJob v4 (#213)

This adds support to GoodJob v4. The main change is that queries now go through `GoodJob::Job` instead of `GoodJob::Execution`, based on the changelog: > Enqueues and executes jobs via the GoodJob::Job model instead of GoodJob::Execution They've also dropped support to Ruby 2.6/2.7, which we still support. I've tweaked the existing Gemfile for AR 6.1 to run against GoodJob v3, so we have test coverage there, and the main Gemfile and AR 7.0 versions will run against v4 going forward. Similarly, we have 2 sample apps for GoodJob, and I've kept one pointing to GoodJob v3 (the multi-db sample), while upgrading the other to v4, so we can test both for now. https://island94.org/2024/07/introducing-goodjob-v4 https://github.com/bensheldon/good_job/tree/v4.0.2?tab=readme-ov-file#upgrading-v3-to-v4 Closes https://linear.app/judo/issue/PRD-579/add-support-for-goodjob-v4
judoscale · Jul 9, 2024 · 1d8f75c · 1d8f75c
1 parent 5541377
commit 1d8f75c
Show file tree

Hide file tree

Showing 10 changed files with 57 additions and 32 deletions.
diff --git a/.github/workflows/judoscale-good_job-test.yml b/.github/workflows/judoscale-good_job-test.yml
@@ -28,6 +28,11 @@ jobs:
             ruby: "2.6"
           - gemfile: Gemfile-activerecord-7-0
             ruby: "2.6"
+          # GoodJob v4+ dropped support for Ruby 2.6/2.7, we test it against these Gemfiles.
+          - gemfile: Gemfile
+            ruby: "2.7"
+          - gemfile: Gemfile-activerecord-7-0
+            ruby: "2.7"
           # AR 6.1 stable doesn't play nice with Ruby 3.2+. Fixes have been backported
           # https://github.com/rails/rails/pull/46895, but no release is planned.
           # To avoid testing against 6-1-stable branch, we'll just skip those for now.

diff --git a/judoscale-good_job/Gemfile-activerecord-6-1 b/judoscale-good_job/Gemfile-activerecord-6-1
@@ -4,6 +4,7 @@ gemspec name: "judoscale-good_job"
 
 gem "judoscale-ruby", path: "../judoscale-ruby"
 gem "activerecord", "~> 6.1"
+gem "good_job", "~> 3.30"
 gem "pg"
 gem "minitest"
 gem "rake"
diff --git a/judoscale-good_job/judoscale-good_job.gemspec b/judoscale-good_job/judoscale-good_job.gemspec
@@ -26,5 +26,5 @@ Gem::Specification.new do |spec|
   spec.required_ruby_version = ">= 2.6.0"
 
   spec.add_dependency "judoscale-ruby", Judoscale::VERSION
-  spec.add_dependency "good_job", ">= 3.0", "< 4.0"
+  spec.add_dependency "good_job", ">= 3.0", "< 5.0"
 end
diff --git a/judoscale-good_job/lib/judoscale/good_job/metrics_collector.rb b/judoscale-good_job/lib/judoscale/good_job/metrics_collector.rb
@@ -9,19 +9,29 @@ module GoodJob
     class MetricsCollector < Judoscale::JobMetricsCollector
       include ActiveRecordHelper
 
+      def self.good_job_v4?
+        Gem::Version.new(::GoodJob::VERSION) >= Gem::Version.new("4")
+      end
+
+      def self.good_job_base_class
+        good_job_v4? ? ::GoodJob::Job : ::GoodJob::Execution
+      end
+
       def self.adapter_config
         Judoscale::Config.instance.good_job
       end
 
       def self.collect?(config)
-        super && ActiveRecordHelper.table_exists_for_model?(::GoodJob::Execution)
+        super && ActiveRecordHelper.table_exists_for_model?(good_job_base_class)
       end
 
       def initialize
         super
 
+        @good_job_base_class = self.class.good_job_base_class
+
         queue_names = run_silently do
-          ::GoodJob::Execution.select("distinct queue_name").map(&:queue_name)
+          @good_job_base_class.select("distinct queue_name").map(&:queue_name)
         end
         self.queues |= queue_names
       end
@@ -32,7 +42,7 @@ def collect
 
         # logically we don't need the finished_at condition, but it lets postgres use the indexes
         oldest_execution_time_by_queue = run_silently do
-          ::GoodJob::Execution
+          @good_job_base_class
             .where(performed_at: nil, finished_at: nil)
             .group(:queue_name)
             .pluck(:queue_name, Arel.sql("min(coalesce(scheduled_at, created_at))"))
@@ -42,7 +52,7 @@ def collect
 
         if track_busy_jobs?
           busy_count_by_queue = run_silently do
-            ::GoodJob::Execution.running.group(:queue_name).count
+            @good_job_base_class.running.group(:queue_name).count
           end
           self.queues |= busy_count_by_queue.keys
         end

diff --git a/judoscale-good_job/test/metrics_collector_test.rb b/judoscale-good_job/test/metrics_collector_test.rb
@@ -143,9 +143,14 @@ def clear_enqueued_jobs
       it "tracks busy jobs when the configuration is enabled" do
         use_adapter_config :good_job, track_busy_jobs: true do
           Delayable.set(queue: "default").perform_later
-          ::GoodJob::Execution.last.update!(performed_at: Time.now.utc)
+          metrics = nil
+          ::GoodJob::JobPerformer.new("default").next do |execution|
+            # Support GoodJob v3 query/scope `GoodJob::Execution.running`, which filters by `performed_at`.
+            # v4 scope `GoodJob::Job.running` joins with the advisory lock created by `JobPerformer#next` instead.
+            execution.update!(performed_at: Time.now.utc)
 
-          metrics = subject.collect
+            metrics = subject.collect
+          end
 
           _(metrics.size).must_equal 2
           _(metrics[1].value).must_equal 1
@@ -158,9 +163,13 @@ def clear_enqueued_jobs
         use_config log_level: :debug do
           use_adapter_config :good_job, track_busy_jobs: true do
             Delayable.set(queue: "default").perform_later
-            ::GoodJob::Execution.last.update!(performed_at: Time.now.utc)
+            ::GoodJob::JobPerformer.new("default").next do |execution|
+              # Support GoodJob v3 query/scope `GoodJob::Execution.running`, which filters by `performed_at`.
+              # v4 scope `GoodJob::Job.running` joins with the advisory lock created by `JobPerformer#next` instead.
+              execution.update!(performed_at: Time.now.utc)
 
-            subject.collect
+              subject.collect
+            end
 
             _(log_string).must_match %r{good_job-qt.default=.+ good_job-busy.default=1}
           end

diff --git a/sample-apps/good_job-multi-db-sample/Gemfile.lock b/sample-apps/good_job-multi-db-sample/Gemfile.lock
@@ -1,21 +1,21 @@
 PATH
   remote: ../../judoscale-good_job
   specs:
-    judoscale-good_job (1.7.0)
-      good_job (>= 3.0, < 4.0)
-      judoscale-ruby (= 1.7.0)
+    judoscale-good_job (1.7.1)
+      good_job (>= 3.0, < 5.0)
+      judoscale-ruby (= 1.7.1)
 
 PATH
   remote: ../../judoscale-rails
   specs:
-    judoscale-rails (1.7.0)
-      judoscale-ruby (= 1.7.0)
+    judoscale-rails (1.7.1)
+      judoscale-ruby (= 1.7.1)
       railties
 
 PATH
   remote: ../../judoscale-ruby
   specs:
-    judoscale-ruby (1.7.0)
+    judoscale-ruby (1.7.1)
 
 GEM
   remote: https://rubygems.org/

diff --git a/sample-apps/good_job-multi-db-sample/app/views/jobs/index.html.erb b/sample-apps/good_job-multi-db-sample/app/views/jobs/index.html.erb
@@ -1,4 +1,4 @@
-<h1>Judoscale: GoodJob Sample</h1>
+<h1>Judoscale: GoodJob MultiDB Sample</h1>
 
 <p>
   Judoscale is reporting metrics to <a href="https://judoscale-adapter-mock.requestcatcher.com" target="_blank" rel="noreferrer">https://judoscale-adapter-mock.requestcatcher.com</a>.

diff --git a/sample-apps/good_job-multi-db-sample/app/views/layouts/application.html.erb b/sample-apps/good_job-multi-db-sample/app/views/layouts/application.html.erb
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <html>
   <head>
-    <title>Judoscale: GoodJob Sample</title>
+    <title>Judoscale: GoodJob MultiDB Sample</title>
     <meta name="viewport" content="width=device-width,initial-scale=1">
     <%= csrf_meta_tags %>
     <%= csp_meta_tag %>

diff --git a/sample-apps/good_job-sample/Gemfile b/sample-apps/good_job-sample/Gemfile
@@ -15,4 +15,4 @@ gem "judoscale-ruby", path: "../../judoscale-ruby"
 gem "judoscale-rails", path: "../../judoscale-rails"
 gem "judoscale-good_job", path: "../../judoscale-good_job"
 
-gem "good_job", "~> 3.7"
+gem "good_job", "~> 4.0"
diff --git a/sample-apps/good_job-sample/Gemfile.lock b/sample-apps/good_job-sample/Gemfile.lock
@@ -1,21 +1,21 @@
 PATH
   remote: ../../judoscale-good_job
   specs:
-    judoscale-good_job (1.7.0)
-      good_job (>= 3.0, < 4.0)
-      judoscale-ruby (= 1.7.0)
+    judoscale-good_job (1.7.1)
+      good_job (>= 3.0, < 5.0)
+      judoscale-ruby (= 1.7.1)
 
 PATH
   remote: ../../judoscale-rails
   specs:
-    judoscale-rails (1.7.0)
-      judoscale-ruby (= 1.7.0)
+    judoscale-rails (1.7.1)
+      judoscale-ruby (= 1.7.1)
       railties
 
 PATH
   remote: ../../judoscale-ruby
   specs:
-    judoscale-ruby (1.7.0)
+    judoscale-ruby (1.7.1)
 
 GEM
   remote: https://rubygems.org/
@@ -57,13 +57,13 @@ GEM
       raabro (~> 1.4)
     globalid (1.2.1)
       activesupport (>= 6.1)
-    good_job (3.99.0)
-      activejob (>= 6.0.0)
-      activerecord (>= 6.0.0)
-      concurrent-ruby (>= 1.0.2)
-      fugit (>= 1.1)
-      railties (>= 6.0.0)
-      thor (>= 0.14.1)
+    good_job (4.0.1)
+      activejob (>= 6.1.0)
+      activerecord (>= 6.1.0)
+      concurrent-ruby (>= 1.3.1)
+      fugit (>= 1.11.0)
+      railties (>= 6.1.0)
+      thor (>= 1.0.0)
     i18n (1.14.5)
       concurrent-ruby (~> 1.0)
     loofah (2.22.0)
@@ -119,7 +119,7 @@ DEPENDENCIES
   activejob (~> 7.0.1)
   activemodel (~> 7.0.1)
   activerecord (~> 7.0.1)
-  good_job (~> 3.7)
+  good_job (~> 4.0)
   judoscale-good_job!
   judoscale-rails!
   judoscale-ruby!