From eb7a8eb698e9887b8aa7c8ce0378b3e6d9cb12e5 Mon Sep 17 00:00:00 2001 From: mec rawlings Date: Thu, 21 Mar 2024 08:19:41 +0000 Subject: [PATCH] Set an implicit order on models As we use uuids as primary keys the order of objects can be unexpected without an explicit sort order. By setting the `implicit_order_column` to `created_at` we get back the expected behaviour of sequential primary keys. https://api.rubyonrails.org/classes/ActiveRecord/ModelSchema.html#method-c-implicit_order_column --- app/models/good_job/batch_record.rb | 1 + app/models/good_job/execution.rb | 1 + app/models/good_job/job.rb | 1 + app/models/good_job/process.rb | 1 + app/models/good_job/setting.rb | 1 + spec/app/models/good_job/batch_record_spec.rb | 13 +++++++++++++ spec/app/models/good_job/execution_spec.rb | 13 +++++++++++++ spec/app/models/good_job/job_spec.rb | 13 +++++++++++++ spec/app/models/good_job/process_spec.rb | 13 +++++++++++++ spec/app/models/good_job/setting_spec.rb | 13 +++++++++++++ 10 files changed, 70 insertions(+) diff --git a/app/models/good_job/batch_record.rb b/app/models/good_job/batch_record.rb index f59ba3135..05fea7372 100644 --- a/app/models/good_job/batch_record.rb +++ b/app/models/good_job/batch_record.rb @@ -7,6 +7,7 @@ class BatchRecord < BaseRecord include AdvisoryLockable self.table_name = 'good_job_batches' + self.implicit_order_column = 'created_at' has_many :jobs, class_name: 'GoodJob::Job', inverse_of: :batch, foreign_key: :batch_id, dependent: nil has_many :executions, class_name: 'GoodJob::Execution', foreign_key: :batch_id, inverse_of: :batch, dependent: nil diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index 56520fdb0..8e99cef34 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -16,6 +16,7 @@ class Execution < BaseExecution self.table_name = 'good_jobs' self.advisory_lockable_column = 'active_job_id' + self.implicit_order_column = 'created_at' define_model_callbacks :perform define_model_callbacks :perform_unlocked, only: :after diff --git a/app/models/good_job/job.rb b/app/models/good_job/job.rb index 30c26d243..26384964e 100644 --- a/app/models/good_job/job.rb +++ b/app/models/good_job/job.rb @@ -26,6 +26,7 @@ def table_name=(_value) self.primary_key = 'active_job_id' self.advisory_lockable_column = 'active_job_id' + self.implicit_order_column = 'created_at' belongs_to :batch, class_name: 'GoodJob::BatchRecord', inverse_of: :jobs, optional: true has_many :executions, -> { order(created_at: :asc) }, class_name: 'GoodJob::Execution', foreign_key: 'active_job_id', inverse_of: :job # rubocop:disable Rails/HasManyOrHasOneDependent diff --git a/app/models/good_job/process.rb b/app/models/good_job/process.rb index ca1bfd7bd..a6fd76ec8 100644 --- a/app/models/good_job/process.rb +++ b/app/models/good_job/process.rb @@ -14,6 +14,7 @@ class Process < BaseRecord EXPIRED_INTERVAL = 5.minutes self.table_name = 'good_job_processes' + self.implicit_order_column = 'created_at' cattr_reader :mutex, default: Mutex.new cattr_accessor :_current_id, default: nil diff --git a/app/models/good_job/setting.rb b/app/models/good_job/setting.rb index 8416f87d4..3fd2f70b3 100644 --- a/app/models/good_job/setting.rb +++ b/app/models/good_job/setting.rb @@ -5,6 +5,7 @@ class Setting < BaseRecord CRON_KEYS_DISABLED = "cron_keys_disabled" self.table_name = 'good_job_settings' + self.implicit_order_column = 'created_at' def self.cron_key_enabled?(key) cron_disabled = find_by(key: CRON_KEYS_DISABLED)&.value || [] diff --git a/spec/app/models/good_job/batch_record_spec.rb b/spec/app/models/good_job/batch_record_spec.rb index b7192226e..dce2b9287 100644 --- a/spec/app/models/good_job/batch_record_spec.rb +++ b/spec/app/models/good_job/batch_record_spec.rb @@ -17,4 +17,17 @@ expect(batch.id).to eq record.id end end + + describe 'implicit sort order' do + it 'is by created at' do + first_job = described_class.create(id: '67160140-1bec-4c3b-bc34-1a8b36f87b21') + described_class.create(id: '3732d706-fd5a-4c39-b1a5-a9bc6d265811') + last_job = described_class.create(id: '4fbae77c-6f22-488f-ad42-5bd20f39c28c') + + result = described_class.all + + expect(result.first).to eq first_job + expect(result.last).to eq last_job + end + end end diff --git a/spec/app/models/good_job/execution_spec.rb b/spec/app/models/good_job/execution_spec.rb index 0243030ae..369683120 100644 --- a/spec/app/models/good_job/execution_spec.rb +++ b/spec/app/models/good_job/execution_spec.rb @@ -25,6 +25,19 @@ def perform(result_value = nil, raise_error: false) stub_const 'TestJob::ExpectedError', Class.new(StandardError) end + describe 'implicit sort order' do + it 'is by created at' do + first_job = described_class.create(active_job_id: '67160140-1bec-4c3b-bc34-1a8b36f87b21') + described_class.create(active_job_id: '3732d706-fd5a-4c39-b1a5-a9bc6d265811') + last_job = described_class.create(active_job_id: '4fbae77c-6f22-488f-ad42-5bd20f39c28c') + + result = described_class.all + + expect(result.first).to eq first_job + expect(result.last).to eq last_job + end + end + describe '.enqueue' do let(:active_job) { TestJob.new } diff --git a/spec/app/models/good_job/job_spec.rb b/spec/app/models/good_job/job_spec.rb index 347db8dfd..b41657300 100644 --- a/spec/app/models/good_job/job_spec.rb +++ b/spec/app/models/good_job/job_spec.rb @@ -99,6 +99,19 @@ def perform(*) end end + describe 'implicit sort order' do + it 'is by created_at' do + first_job = described_class.create(active_job_id: '67160140-1bec-4c3b-bc34-1a8b36f87b21') + described_class.create(active_job_id: '3732d706-fd5a-4c39-b1a5-a9bc6d265811') + last_job = described_class.create(active_job_id: '4fbae77c-6f22-488f-ad42-5bd20f39c28c') + + result = described_class.all + + expect(result.first).to eq first_job + expect(result.last).to eq last_job + end + end + describe '#job_class' do it 'is the job class' do expect(job.id).to eq head_execution.active_job_id diff --git a/spec/app/models/good_job/process_spec.rb b/spec/app/models/good_job/process_spec.rb index b0a740c4f..b6732638e 100644 --- a/spec/app/models/good_job/process_spec.rb +++ b/spec/app/models/good_job/process_spec.rb @@ -23,6 +23,19 @@ end end + describe 'implicit sort order' do + it 'is by created_at' do + first_job = described_class.create(id: '67160140-1bec-4c3b-bc34-1a8b36f87b21') + described_class.create(id: '3732d706-fd5a-4c39-b1a5-a9bc6d265811') + last_job = described_class.create(id: '4fbae77c-6f22-488f-ad42-5bd20f39c28c') + + result = described_class.all + + expect(result.first).to eq first_job + expect(result.last).to eq last_job + end + end + describe '.ns_current_state' do it 'contains information about the process' do expect(described_class.ns_current_state).to include( diff --git a/spec/app/models/good_job/setting_spec.rb b/spec/app/models/good_job/setting_spec.rb index ac92f6349..9239d0250 100644 --- a/spec/app/models/good_job/setting_spec.rb +++ b/spec/app/models/good_job/setting_spec.rb @@ -3,6 +3,19 @@ require 'rails_helper' RSpec.describe GoodJob::Setting do + describe 'implicit sort order' do + it 'is by created_at' do + first_job = described_class.create(id: '67160140-1bec-4c3b-bc34-1a8b36f87b21') + described_class.create(id: '3732d706-fd5a-4c39-b1a5-a9bc6d265811') + last_job = described_class.create(id: '4fbae77c-6f22-488f-ad42-5bd20f39c28c') + + result = described_class.all + + expect(result.first).to eq first_job + expect(result.last).to eq last_job + end + end + describe 'cron_key_disabled setting' do describe '.cron_key_enabled?' do it 'returns true when the key is not disabled' do