Skip to content

Commit

Permalink
Set an implicit order on models (#1293)
Browse files Browse the repository at this point in the history
* 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

* Add `DiscreteExecutions` too

---------

Co-authored-by: Ben Sheldon [he/him] <[email protected]>
  • Loading branch information
mec and bensheldon authored Mar 21, 2024
1 parent 401e11e commit 14950d7
Show file tree
Hide file tree
Showing 11 changed files with 71 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/discrete_execution.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ class DiscreteExecution < BaseRecord
include ErrorEvents

self.table_name = 'good_job_executions'
self.implicit_order_column = 'created_at'

belongs_to :execution, class_name: 'GoodJob::Execution', foreign_key: 'active_job_id', primary_key: 'active_job_id', inverse_of: :discrete_executions, optional: true
belongs_to :job, class_name: 'GoodJob::Job', foreign_key: 'active_job_id', primary_key: 'active_job_id', inverse_of: :discrete_executions, optional: true
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 @@ -6,6 +6,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, default: true)
if default
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 14950d7

Please sign in to comment.