Skip to content

Commit

Permalink
Add label as pausable
Browse files Browse the repository at this point in the history
  • Loading branch information
bensheldon committed Jan 19, 2025
1 parent 75d2640 commit 736b14e
Show file tree
Hide file tree
Showing 9 changed files with 182 additions and 37 deletions.
4 changes: 2 additions & 2 deletions app/controllers/good_job/pauses_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ def index
end

def create
return redirect_to({ action: :index }) unless params[:type].in?(%w[queue job_class]) && params[:value].present?
return redirect_to({ action: :index }) unless params[:type].in?(%w[queue job_class label]) && params[:value].present?

GoodJob::Setting.pause(params[:type].to_sym => params[:value])
redirect_to(good_job.pauses_path, notice: "Successfully paused #{params[:type]} '#{params[:value]}'")
end

def destroy
return redirect_to({ action: :index }) unless params[:type].in?(%w[queue job_class]) && params[:value].present?
return redirect_to({ action: :index }) unless params[:type].in?(%w[queue job_class label]) && params[:value].present?

GoodJob::Setting.unpause(params[:type].to_sym => params[:value].to_s)
redirect_to(good_job.pauses_path, notice: "Successfully unpaused #{params[:type]} '#{params[:value]}'")
Expand Down
15 changes: 14 additions & 1 deletion app/models/good_job/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,20 @@ class Job < BaseRecord
paused_query = GoodJob::Setting.where(key: GoodJob::Setting::PAUSES)
paused_queues_query = paused_query.select("jsonb_array_elements_text(value->'queues')")
paused_job_classes_query = paused_query.select("jsonb_array_elements_text(value->'job_classes')")
where.not(queue_name: paused_queues_query).where.not(job_class: paused_job_classes_query)
paused_labels_query = paused_query.select("jsonb_array_elements_text(value->'labels')")

where.not(queue_name: paused_queues_query)
.where.not(job_class: paused_job_classes_query)
.where(
Arel::Nodes::Not.new(
Arel::Nodes::NamedFunction.new(
"COALESCE", [
Arel::Nodes::InfixOperation.new('&&', arel_table['labels'], Arel::Nodes::NamedFunction.new('ARRAY', [paused_labels_query.arel])),
Arel::Nodes::SqlLiteral.new('FALSE'),
]
)
)
)
}

# Order jobs by priority (highest priority first).
Expand Down
37 changes: 25 additions & 12 deletions app/models/good_job/setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,25 +49,28 @@ def self.cron_key_disable(key)
disabled_setting.save!
end

def self.pause(queue: nil, job_class: nil)
raise ArgumentError, "Must provide either queue or job_class, but not both" if queue.nil? == job_class.nil?
def self.pause(queue: nil, job_class: nil, label: nil)
raise ArgumentError, "Must provide exactly one of queue, job_class, or label" unless [queue, job_class, label].count(&:present?) == 1

setting = find_or_initialize_by(key: PAUSES) do |record|
record.value = { "queues" => [], "job_classes" => [] }
record.value = { "queues" => [], "job_classes" => [], "labels" => [] }
end

if queue
setting.value["queues"] ||= []
setting.value["queues"] << queue.to_s unless setting.value["queues"].include?(queue.to_s)
else
elsif job_class
setting.value["job_classes"] ||= []
setting.value["job_classes"] << job_class.to_s unless setting.value["job_classes"].include?(job_class.to_s)
else
setting.value["labels"] ||= []
setting.value["labels"] << label.to_s unless setting.value["labels"].include?(label.to_s)
end
setting.save!
end

def self.unpause(queue: nil, job_class: nil)
raise ArgumentError, "Must provide either queue or job_class, but not both" if queue.nil? == job_class.nil?
def self.unpause(queue: nil, job_class: nil, label: nil)
raise ArgumentError, "Must provide exactly one of queue, job_class, or label" unless [queue, job_class, label].count(&:present?) == 1

setting = find_by(key: PAUSES)
return unless setting
Expand All @@ -76,39 +79,49 @@ def self.unpause(queue: nil, job_class: nil)
return unless setting.value["queues"]&.include?(queue.to_s)

setting.value["queues"].delete(queue.to_s)
else
elsif job_class
return unless setting.value["job_classes"]&.include?(job_class.to_s)

setting.value["job_classes"].delete(job_class.to_s)
else
return unless setting.value["labels"]&.include?(label.to_s)

setting.value["labels"].delete(label.to_s)
end
setting.save!
end

def self.paused?(queue: nil, job_class: nil)
raise ArgumentError, "Must provide either queue or job_class, or neither" if queue && job_class
def self.paused?(queue: nil, job_class: nil, label: nil)
raise ArgumentError, "Must provide at most one of queue, job_class, or label" if [queue, job_class, label].count(&:present?) > 1

if queue
queue.in? paused(:queues)
elsif job_class
job_class.in? paused(:job_classes)
elsif label
label.in? paused(:labels)
else
paused.values.any?(&:any?)
end
end

def self.paused(type = nil)
setting = find_by(key: PAUSES)
pauses = setting&.value&.deep_dup || { "queues" => [], "job_classes" => [] }
pauses = setting&.value&.deep_dup || { "queues" => [], "job_classes" => [], "labels" => [] }
pauses = pauses.with_indifferent_access

if type == :queues
case type
when :queues
pauses["queues"]
elsif type == :job_classes
when :job_classes
pauses["job_classes"]
when :labels
pauses["labels"]
else
{
queues: pauses["queues"] || [],
job_classes: pauses["job_classes"] || [],
labels: pauses["labels"] || [],
}
end
end
Expand Down
2 changes: 2 additions & 0 deletions app/views/good_job/pauses/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
options_for_select([
[t('good_job.pauses.index.queue'), 'queue'],
[t('good_job.pauses.index.job_class'), 'job_class'],
[t('good_job.pauses.index.label'), 'label'],
]),
class: 'form-select',
required: true %>
Expand All @@ -31,3 +32,4 @@

<%= render 'group', paused: @paused, type: :queue %>
<%= render 'group', paused: @paused, type: :job_class %>
<%= render 'group', paused: @paused, type: :label %>
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,7 @@ en:
confirm_pause: Are you sure you want to pause %{value}?
confirm_unpause: Are you sure you want to resume %{value}?
job_class: Job Class
label: Label
pause: Pause
queue: Queue
title: Pauses
Expand Down
12 changes: 7 additions & 5 deletions lib/good_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -304,21 +304,23 @@ def self.pause(queue: nil, job_class: nil)
# Unpause job execution for a given queue or job class.
# @param queue [String, nil] Queue name to unpause
# @param job_class [String, nil] Job class name to unpause
# @param label [String, nil] Label to unpause
# @return [void]
def self.unpause(queue: nil, job_class: nil)
GoodJob::Setting.unpause(queue: queue, job_class: job_class)
def self.unpause(queue: nil, job_class: nil, label: nil)
GoodJob::Setting.unpause(queue: queue, job_class: job_class, label: label)
end

# Check if job execution is paused for a given queue or job class.
# @param queue [String, nil] Queue name to check
# @param job_class [String, nil] Job class name to check
# @param label [String, nil] Label to check
# @return [Boolean]
def self.paused?(queue: nil, job_class: nil)
GoodJob::Setting.paused?(queue: queue, job_class: job_class)
def self.paused?(queue: nil, job_class: nil, label: nil)
GoodJob::Setting.paused?(queue: queue, job_class: job_class, label: label)
end

# Get a list of all paused queues and job classes
# @return [Hash] Hash with :queues and :job_classes arrays of paused items
# @return [Hash] Hash with :queues, :job_classes, :labels arrays of paused items
def self.paused(type = nil)
GoodJob::Setting.paused(type)
end
Expand Down
38 changes: 33 additions & 5 deletions spec/app/models/good_job/job_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,8 @@ def job_params
let!(:default_job) { described_class.create!(queue_name: "default", job_class: "DefaultJob") }
let!(:mailers_job) { described_class.create!(queue_name: "mailers", job_class: "ActionMailer::MailDeliveryJob") }
let!(:reports_job) { described_class.create!(queue_name: "reports", job_class: "ReportsJob") }
let!(:labeled_job) { described_class.create!(queue_name: "default", job_class: "DefaultJob", labels: %w[important urgent]) }
let!(:other_labeled_job) { described_class.create!(queue_name: "default", job_class: "DefaultJob", labels: ["low_priority"]) }

before do
allow(GoodJob.configuration).to receive(:enable_pause).and_return(enable_pause)
Expand All @@ -1084,15 +1086,16 @@ def job_params
let(:enable_pause) { false }

it 'returns all jobs' do
expect(described_class.exclude_paused).to contain_exactly(default_job, mailers_job, reports_job)
expect(described_class.exclude_paused).to contain_exactly(default_job, mailers_job, reports_job, labeled_job, other_labeled_job)
end
end

context 'when enable_pause is true' do
let(:enable_pause) { true }

it 'returns all jobs when nothing is paused' do
expect(described_class.exclude_paused).to contain_exactly(default_job, mailers_job, reports_job)
expect(described_class.exclude_paused.count).to eq 5
expect(described_class.exclude_paused).to contain_exactly(default_job, mailers_job, reports_job, labeled_job, other_labeled_job)
end

it 'excludes jobs with paused queue_names' do
Expand All @@ -1104,18 +1107,43 @@ def job_params
it 'excludes jobs with paused job_classes' do
GoodJob::Setting.pause(job_class: "DefaultJob")
GoodJob::Setting.pause(job_class: "ActionMailer::MailDeliveryJob")
expect(described_class.exclude_paused).to contain_exactly(reports_job)
end

puts described_class.exclude_paused.to_sql
it 'excludes jobs with paused labels' do
GoodJob::Setting.pause(label: "important")
expect(described_class.exclude_paused).to contain_exactly(default_job, mailers_job, reports_job, other_labeled_job)
end

expect(described_class.exclude_paused).to contain_exactly(reports_job)
it 'excludes jobs with any paused label when job has multiple labels' do
GoodJob::Setting.pause(label: "urgent")
expect(described_class.exclude_paused).to contain_exactly(default_job, mailers_job, reports_job, other_labeled_job)
end

it 'excludes jobs with both paused queue_names and job_classes' do
GoodJob::Setting.pause(queue: "default")
GoodJob::Setting.pause(job_class: "ActionMailer::MailDeliveryJob")

expect(described_class.exclude_paused).to contain_exactly(reports_job)
end

it 'excludes jobs with paused queue_names, job_classes, or labels' do
GoodJob::Setting.pause(queue: "reports")
GoodJob::Setting.pause(job_class: "ActionMailer::MailDeliveryJob")
GoodJob::Setting.pause(label: "important")
expect(described_class.exclude_paused).to contain_exactly(default_job, other_labeled_job)
end

it 'handles jobs with nil labels' do
GoodJob::Setting.pause(label: "important")
unlabeled_job = described_class.create!(queue_name: "default", job_class: "DefaultJob", labels: nil)
expect(described_class.exclude_paused).to include(unlabeled_job)
end

it 'handles jobs with empty labels array' do
GoodJob::Setting.pause(label: "important")
empty_labeled_job = described_class.create!(queue_name: "default", job_class: "DefaultJob", labels: [])
expect(described_class.exclude_paused).to include(empty_labeled_job)
end
end
end
end
Expand Down
88 changes: 76 additions & 12 deletions spec/app/models/good_job/setting_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,14 @@

describe 'pause settings' do
describe '.pause' do
it 'raises ArgumentError if neither queue nor job_class is provided' do
expect { described_class.pause }.to raise_error(ArgumentError, "Must provide either queue or job_class, but not both")
it 'raises ArgumentError if no valid argument is provided' do
expect { described_class.pause }.to raise_error(ArgumentError, "Must provide exactly one of queue, job_class, or label")
end

it 'raises ArgumentError if both queue and job_class are provided' do
expect { described_class.pause(queue: "default", job_class: "MyJob") }.to raise_error(ArgumentError, "Must provide either queue or job_class, but not both")
it 'raises ArgumentError if multiple arguments are provided' do
expect { described_class.pause(queue: "default", job_class: "MyJob") }.to raise_error(ArgumentError, "Must provide exactly one of queue, job_class, or label")
expect { described_class.pause(queue: "default", label: "my_label") }.to raise_error(ArgumentError, "Must provide exactly one of queue, job_class, or label")
expect { described_class.pause(job_class: "MyJob", label: "my_label") }.to raise_error(ArgumentError, "Must provide exactly one of queue, job_class, or label")
end

context 'with queue' do
Expand Down Expand Up @@ -151,15 +153,42 @@
expect(described_class.find_by(key: described_class::PAUSES).value["job_classes"]).to contain_exactly "MyJob"
end
end

context 'with label' do
it 'inserts values into a json array' do
expect(described_class.where(key: described_class::PAUSES).count).to eq 0

described_class.pause(label: "important")
expect(described_class.where(key: described_class::PAUSES).count).to eq 1
expect(described_class.find_by(key: described_class::PAUSES).value["labels"]).to contain_exactly "important"

described_class.pause(label: "urgent")
expect(described_class.where(key: described_class::PAUSES).count).to eq 1
expect(described_class.find_by(key: described_class::PAUSES).value["labels"]).to contain_exactly "important", "urgent"

described_class.unpause(label: "important")
described_class.unpause(label: "urgent")
expect(described_class.find_by(key: described_class::PAUSES).value["labels"]).to eq []
end

it 'does not insert duplicate labels' do
described_class.pause(label: "important")
described_class.pause(label: "important")
expect(described_class.where(key: described_class::PAUSES).count).to eq 1
expect(described_class.find_by(key: described_class::PAUSES).value["labels"]).to contain_exactly "important"
end
end
end

describe '.unpause' do
it 'raises ArgumentError if neither queue nor job_class is provided' do
expect { described_class.unpause }.to raise_error(ArgumentError, "Must provide either queue or job_class, but not both")
it 'raises ArgumentError if no valid argument is provided' do
expect { described_class.unpause }.to raise_error(ArgumentError, "Must provide exactly one of queue, job_class, or label")
end

it 'raises ArgumentError if both queue and job_class are provided' do
expect { described_class.unpause(queue: "default", job_class: "MyJob") }.to raise_error(ArgumentError, "Must provide either queue or job_class, but not both")
it 'raises ArgumentError if multiple arguments are provided' do
expect { described_class.unpause(queue: "default", job_class: "MyJob") }.to raise_error(ArgumentError, "Must provide exactly one of queue, job_class, or label")
expect { described_class.unpause(queue: "default", label: "my_label") }.to raise_error(ArgumentError, "Must provide exactly one of queue, job_class, or label")
expect { described_class.unpause(job_class: "MyJob", label: "my_label") }.to raise_error(ArgumentError, "Must provide exactly one of queue, job_class, or label")
end

context 'with queue' do
Expand Down Expand Up @@ -191,11 +220,28 @@
expect(described_class.find_by(key: described_class::PAUSES).value["job_classes"]).to contain_exactly "AnotherJob"
end
end

context 'with label' do
it 'safely handles non-existent settings' do
expect { described_class.unpause(label: "important") }.not_to raise_error
end

it 'removes the label from the paused list' do
described_class.pause(label: "important")
described_class.pause(label: "urgent")
expect(described_class.find_by(key: described_class::PAUSES).value["labels"]).to contain_exactly "important", "urgent"

described_class.unpause(label: "important")
expect(described_class.find_by(key: described_class::PAUSES).value["labels"]).to contain_exactly "urgent"
end
end
end

describe '.paused?' do
it 'raises ArgumentError if both queue and job_class are provided' do
expect { described_class.paused?(queue: "default", job_class: "MyJob") }.to raise_error(ArgumentError, "Must provide either queue or job_class, or neither")
it 'raises ArgumentError if multiple arguments are provided' do
expect { described_class.paused?(queue: "default", job_class: "MyJob") }.to raise_error(ArgumentError, "Must provide at most one of queue, job_class, or label")
expect { described_class.paused?(queue: "default", label: "my_label") }.to raise_error(ArgumentError, "Must provide at most one of queue, job_class, or label")
expect { described_class.paused?(job_class: "MyJob", label: "my_label") }.to raise_error(ArgumentError, "Must provide at most one of queue, job_class, or label")
end

it 'returns true when queue is paused' do
Expand All @@ -215,11 +261,20 @@
it 'returns false when job class is not paused' do
expect(described_class.paused?(job_class: "MyJob")).to be false
end

it 'returns true when label is paused' do
described_class.pause(label: "important")
expect(described_class.paused?(label: "important")).to be true
end

it 'returns false when label is not paused' do
expect(described_class.paused?(label: "important")).to be false
end
end

describe '.paused' do
it 'returns empty arrays when nothing is paused' do
expect(described_class.paused).to eq({ queues: [], job_classes: [] })
expect(described_class.paused).to eq({ queues: [], job_classes: [], labels: [] })
end

it 'returns only queues when type is :queues' do
Expand All @@ -234,12 +289,21 @@
expect(described_class.paused(:job_classes)).to contain_exactly "MyJob"
end

it 'returns both queues and job classes by default' do
it 'returns only labels when type is :labels' do
described_class.pause(queue: "default")
described_class.pause(job_class: "MyJob")
described_class.pause(label: "important")
expect(described_class.paused(:labels)).to contain_exactly "important"
end

it 'returns queues, job classes, and labels by default' do
described_class.pause(queue: "default")
described_class.pause(job_class: "MyJob")
described_class.pause(label: "important")
expect(described_class.paused).to eq({
queues: ["default"],
job_classes: ["MyJob"],
labels: ["important"],
})
end
end
Expand Down
Loading

0 comments on commit 736b14e

Please sign in to comment.