Skip to content

Commit

Permalink
Set an implicit order on models
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mec committed Mar 21, 2024
1 parent bf549ba commit eb7a8eb
Show file tree
Hide file tree
Showing 10 changed files with 70 additions and 0 deletions.
1 change: 1 addition & 0 deletions app/models/good_job/batch_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/models/good_job/execution.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/models/good_job/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/models/good_job/process.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions app/models/good_job/setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 || []
Expand Down
13 changes: 13 additions & 0 deletions spec/app/models/good_job/batch_record_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 13 additions & 0 deletions spec/app/models/good_job/execution_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
13 changes: 13 additions & 0 deletions spec/app/models/good_job/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions spec/app/models/good_job/process_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
13 changes: 13 additions & 0 deletions spec/app/models/good_job/setting_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit eb7a8eb

Please sign in to comment.