Use rails enum for error_event and lock_type (#1420)

* Move `error_event` to rails enum

* Move `lock_type` to rails enum

app/models/concerns/good_job/error_events.rb

@@ -5,41 +5,20 @@ module GoodJob
   module ErrorEvents
     extend ActiveSupport::Concern
 
-    ERROR_EVENTS = [
-      ERROR_EVENT_INTERRUPTED = 'interrupted',
-      ERROR_EVENT_UNHANDLED = 'unhandled',
-      ERROR_EVENT_HANDLED = 'handled',
-      ERROR_EVENT_RETRIED = 'retried',
-      ERROR_EVENT_RETRY_STOPPED = 'retry_stopped',
-      ERROR_EVENT_DISCARDED = 'discarded',
-    ].freeze
-
-    ERROR_EVENT_ENUMS = {
-      ERROR_EVENT_INTERRUPTED => 0,
-      ERROR_EVENT_UNHANDLED => 1,
-      ERROR_EVENT_HANDLED => 2,
-      ERROR_EVENT_RETRIED => 3,
-      ERROR_EVENT_RETRY_STOPPED => 4,
-      ERROR_EVENT_DISCARDED => 5,
-    }.freeze
-
-    # TODO: GoodJob v4 can make this an `enum` once migrations are guaranteed.
-    def error_event
-      return unless self.class.columns_hash['error_event']
-
-      enum = read_attribute(:error_event)
-      return unless enum
-
-      ERROR_EVENT_ENUMS.key(enum)
-    end
-
-    def error_event=(event)
-      return unless self.class.columns_hash['error_event']
-
-      enum = ERROR_EVENT_ENUMS[event]
-      raise(ArgumentError, "Invalid error_event: #{event}") if event && !enum
-
-      write_attribute(:error_event, enum)
+    included do
+      error_event_enum = {
+        interrupted: 0,
+        unhandled: 1,
+        handled: 2,
+        retried: 3,
+        retry_stopped: 4,
+        discarded: 5,
+      }
+      if Gem::Version.new(Rails.version) >= Gem::Version.new('7.1.0.a')
+        enum :error_event, error_event_enum, validate: { allow_nil: true }
+      else
+        enum error_event: error_event_enum
+      end
     end
   end
 end

app/models/good_job/base_execution.rb

@@ -409,15 +409,15 @@ def perform(lock_id:)
 
             interrupt_error_string = self.class.format_error(GoodJob::InterruptError.new("Interrupted after starting perform at '#{existing_performed_at}'"))
             self.error = interrupt_error_string
-            self.error_event = ERROR_EVENT_INTERRUPTED
+            self.error_event = :interrupted
             monotonic_duration = (::Process.clock_gettime(::Process::CLOCK_MONOTONIC) - monotonic_start).seconds
 
             discrete_execution_attrs = {
               error: interrupt_error_string,
               finished_at: job_performed_at,
+              error_event: :interrupted,
+              duration: monotonic_duration,
             }
-            discrete_execution_attrs[:error_event] = GoodJob::ErrorEvents::ERROR_EVENT_ENUMS[GoodJob::ErrorEvents::ERROR_EVENT_INTERRUPTED]
-            discrete_execution_attrs[:duration] = monotonic_duration
             discrete_executions.where(finished_at: nil).where.not(performed_at: nil).update_all(discrete_execution_attrs) # rubocop:disable Rails/SkipsModelValidations
           end
 
@@ -451,13 +451,13 @@ def perform(lock_id:)
             handled_error ||= current_thread.error_on_retry || current_thread.error_on_discard
 
             error_event = if handled_error == current_thread.error_on_discard
-                            ERROR_EVENT_DISCARDED
+                            :discarded
                           elsif handled_error == current_thread.error_on_retry
-                            ERROR_EVENT_RETRIED
+                            :retried
                           elsif handled_error == current_thread.error_on_retry_stopped
-                            ERROR_EVENT_RETRY_STOPPED
+                            :retry_stopped
                           elsif handled_error
-                            ERROR_EVENT_HANDLED
+                            :handled
                           end
 
             instrument_payload.merge!(
@@ -469,11 +469,11 @@ def perform(lock_id:)
             ExecutionResult.new(value: value, handled_error: handled_error, error_event: error_event, retried: current_thread.execution_retried)
           rescue StandardError => e
             error_event = if e.is_a?(GoodJob::InterruptError)
-                            ERROR_EVENT_INTERRUPTED
+                            :interrupted
                           elsif e == current_thread.error_on_retry_stopped
-                            ERROR_EVENT_RETRY_STOPPED
+                            :retry_stopped
                           else
-                            ERROR_EVENT_UNHANDLED
+                            :unhandled
                           end
 
             instrument_payload[:unhandled_error] = e

app/models/good_job/job.rb

@@ -146,7 +146,7 @@ def retry_job
 
           self.class.transaction(joinable: false, requires_new: true) do
             new_active_job = active_job.retry_job(wait: 0, error: error)
-            self.error_event = ERROR_EVENT_RETRIED if error
+            self.error_event = :retried if error
             save!
           end
         end
@@ -212,7 +212,7 @@ def _discard_job(message)
         update(
           finished_at: Time.current,
           error: self.class.format_error(job_error),
-          error_event: ERROR_EVENT_DISCARDED
+          error_event: :discarded
         )
       end
 

app/models/good_job/process.rb

@@ -15,15 +15,15 @@ class Process < BaseRecord
 
     self.table_name = 'good_job_processes'
     self.implicit_order_column = 'created_at'
-    LOCK_TYPES = [
-      LOCK_TYPE_ADVISORY = 'advisory',
-    ].freeze
 
-    LOCK_TYPE_ENUMS = {
-      LOCK_TYPE_ADVISORY => 1,
-    }.freeze
-
-    self.table_name = 'good_job_processes'
+    lock_type_enum = {
+      advisory: 0,
+    }
+    if Gem::Version.new(Rails.version) >= Gem::Version.new('7.1.0.a')
+      enum :lock_type, lock_type_enum, validate: { allow_nil: true }
+    else
+      enum lock_type: lock_type_enum
+    end
 
     has_many :locked_jobs, class_name: "GoodJob::Job", foreign_key: :locked_by_id, inverse_of: :locked_by_process, dependent: nil
     after_destroy { locked_jobs.update_all(locked_by_id: nil) } # rubocop:disable Rails/SkipsModelValidations
@@ -34,7 +34,7 @@ class Process < BaseRecord
     # @return [ActiveRecord::Relation]
     scope :active, (lambda do
       query = joins_advisory_locks
-      query.where(lock_type: LOCK_TYPE_ENUMS[LOCK_TYPE_ADVISORY]).advisory_locked
+      query.where(lock_type: :advisory).advisory_locked
         .or(query.where(lock_type: nil).where(arel_table[:updated_at].gt(EXPIRED_INTERVAL.ago)))
     end)
 
@@ -44,7 +44,7 @@ class Process < BaseRecord
     # @return [ActiveRecord::Relation]
     scope :inactive, (lambda do
       query = joins_advisory_locks
-      query.where(lock_type: LOCK_TYPE_ENUMS[LOCK_TYPE_ADVISORY]).advisory_unlocked
+      query.where(lock_type: :advisory).advisory_unlocked
         .or(query.where(lock_type: nil).where(arel_table[:updated_at].lt(EXPIRED_INTERVAL.ago)))
     end)
 
@@ -63,7 +63,7 @@ def self.create_record(id:, with_advisory_lock: false)
       }
       if with_advisory_lock
         attributes[:create_with_advisory_lock] = true
-        attributes[:lock_type] = LOCK_TYPE_ADVISORY
+        attributes[:lock_type] = :advisory
       end
       create!(attributes)
     end
@@ -124,21 +124,5 @@ def basename
     def schedulers
       state.fetch("schedulers", [])
     end
-
-    def lock_type
-      return unless self.class.columns_hash['lock_type']
-
-      enum = super
-      LOCK_TYPE_ENUMS.key(enum) if enum
-    end
-
-    def lock_type=(value)
-      return unless self.class.columns_hash['lock_type']
-
-      enum = LOCK_TYPE_ENUMS[value]
-      raise(ArgumentError, "Invalid error_event: #{value}") if value && !enum
-
-      super(enum)
-    end
   end
 end

lib/good_job/capsule_tracker.rb

@@ -84,7 +84,7 @@ def register(with_advisory_lock: false)
             if !advisory_locked? || !advisory_locked_connection?
               @record.class.transaction do
                 @record.advisory_lock!
-                @record.update(lock_type: GoodJob::Process::LOCK_TYPE_ADVISORY)
+                @record.update(lock_type: :advisory)
               end
               @advisory_locked_connection = WeakRef.new(@record.class.connection)
             end

spec/app/models/concerns/good_job/filterable_spec.rb

@@ -13,7 +13,7 @@
       serialized_params: { example_key: 'example_value' },
       labels: %w[buffalo gopher],
       error: "ExampleJob::ExampleError: a message",
-      error_event: GoodJob::ErrorEvents::ERROR_EVENT_RETRIED
+      error_event: "retried"
     )
   end
 

spec/lib/good_job/active_job_extensions/interrupt_errors_spec.rb

@@ -26,7 +26,7 @@ def perform
       expect { GoodJob.perform_inline }.to raise_error(GoodJob::InterruptError)
       expect(GoodJob::Job.last).to have_attributes(
         error: start_with('GoodJob::InterruptError: Interrupted after starting perform at'),
-        error_event: GoodJob::Job::ERROR_EVENT_INTERRUPTED
+        error_event: "interrupted"
       )
     end
 
@@ -44,7 +44,7 @@ def perform
           performed_at: be_blank,
           finished_at: be_blank,
           error: start_with('GoodJob::InterruptError: Interrupted after starting perform at'),
-          error_event: GoodJob::Job::ERROR_EVENT_RETRIED
+          error_event: "retried"
         )
 
         initial_discrete_execution = job.discrete_executions.first
@@ -53,7 +53,7 @@ def perform
           finished_at: be_present,
           duration: be_present,
           error: start_with('GoodJob::InterruptError: Interrupted after starting perform at'),
-          error_event: GoodJob::Job::ERROR_EVENT_INTERRUPTED
+          error_event: "interrupted"
         )
 
         retried_discrete_execution = job.discrete_executions.last
@@ -62,7 +62,7 @@ def perform
           finished_at: be_present,
           duration: be_present,
           error: start_with('GoodJob::InterruptError: Interrupted after starting perform at'),
-          error_event: GoodJob::Job::ERROR_EVENT_RETRIED
+          error_event: "retried"
         )
       end
     end