Skip to content

Commit

Permalink
fix: while_executing should not invoke conflict strategy when the j…
Browse files Browse the repository at this point in the history
…ob was successfully executed. (#806)
  • Loading branch information
leandrogoe committed Aug 28, 2023
1 parent eec260f commit 3e21885
Show file tree
Hide file tree
Showing 7 changed files with 15 additions and 23 deletions.
1 change: 1 addition & 0 deletions lib/sidekiq_unique_jobs/lock/while_executing.rb
Expand Up @@ -42,6 +42,7 @@ def execute(&block)
with_logging_context do
executed = locksmith.execute do
yield
item[JID]
ensure
unlock_and_callback
end
Expand Down
10 changes: 6 additions & 4 deletions spec/sidekiq_unique_jobs/lock/while_executing_spec.rb
Expand Up @@ -91,10 +91,12 @@

it "reflects execution_failed" do
process_one.execute do
process_two.execute { puts "BOGUS!" }
# NOTE: Below looks weird but tests that
# the result from process_two (which is nil) isn't considered.
jid_one
process_two.execute do
puts "BOGUS!"
nil
end

nil
end

expect(process_one).not_to have_received(:reflect).with(:execution_failed, item_one)
Expand Down
3 changes: 1 addition & 2 deletions spec/sidekiq_unique_jobs/on_conflict/reject_spec.rb
Expand Up @@ -13,8 +13,7 @@
end

before do
allow(strategy).to receive(:deadset).and_return(deadset)
allow(strategy).to receive(:payload).and_return(payload)
allow(strategy).to receive_messages(deadset: deadset, payload: payload)
end

describe "#replace?" do
Expand Down
9 changes: 3 additions & 6 deletions spec/sidekiq_unique_jobs/on_conflict/replace_spec.rb
Expand Up @@ -33,9 +33,8 @@
let(:jid) { "bogus" }

before do
allow(strategy).to receive(:delete_job_by_digest).and_return(jid)
allow(strategy).to receive_messages(delete_job_by_digest: jid, delete_lock: 9)
allow(strategy).to receive(:log_info).and_call_original
allow(strategy).to receive(:delete_lock).and_return(9)
end

it "logs important information" do
Expand All @@ -50,9 +49,8 @@
let(:jid) { "bogus" }

before do
allow(strategy).to receive(:delete_job_by_digest).and_return(jid)
allow(strategy).to receive_messages(delete_job_by_digest: jid, delete_lock: nil)
allow(strategy).to receive(:log_info).and_call_original
allow(strategy).to receive(:delete_lock).and_return(nil)
end

it "logs important information" do
Expand All @@ -68,9 +66,8 @@
let(:block) { nil }

before do
allow(strategy).to receive(:delete_job_by_digest).and_return(jid)
allow(strategy).to receive_messages(delete_job_by_digest: jid, delete_lock: nil)
allow(strategy).to receive(:log_info).and_call_original
allow(strategy).to receive(:delete_lock).and_return(nil)
end

it "does not call block" do
Expand Down
8 changes: 2 additions & 6 deletions spec/sidekiq_unique_jobs/orphans/manager_spec.rb
Expand Up @@ -16,11 +16,9 @@
before do
allow(SidekiqUniqueJobs::Orphans::Observer).to receive(:new).and_return(observer)

allow(described_class).to receive(:task).and_return(task)
allow(described_class).to receive_messages(task: task, log_info: nil)
allow(task).to receive(:add_observer).with(observer)
allow(task).to receive(:execute)

allow(described_class).to receive(:log_info).and_return(nil)
end

context "when registered?" do
Expand Down Expand Up @@ -72,11 +70,9 @@
before do
allow(SidekiqUniqueJobs::Orphans::Observer).to receive(:new).and_return(observer)

allow(described_class).to receive(:task).and_return(task)
allow(described_class).to receive_messages(task: task, log_info: nil)
allow(task).to receive(:add_observer).with(observer)
allow(task).to receive(:execute)

allow(described_class).to receive(:log_info).and_return(nil)
end

context "when unregistered?" do
Expand Down
4 changes: 1 addition & 3 deletions spec/sidekiq_unique_jobs/orphans/reaper_resurrector_spec.rb
Expand Up @@ -16,10 +16,8 @@
end

before do
allow(described_class).to receive(:task).and_return(task)
allow(described_class).to receive_messages(task: task, log_info: nil)
allow(task).to receive(:execute)

allow(described_class).to receive(:log_info).and_return(nil)
end

context "when resurrector is disabled" do
Expand Down
Expand Up @@ -10,8 +10,7 @@

before do
allow(lock).to receive(:locked?).and_return(initially_locked?, locked?)
allow(lock).to receive(:unlock).and_return(true)
allow(lock).to receive(:delete).and_return(true)
allow(lock).to receive_messages(unlock: true, delete: true)
allow(callback).to receive(:call).and_call_original
allow(block).to receive(:call).and_call_original
allow(lock).to receive(:log_warn)
Expand Down

0 comments on commit 3e21885

Please sign in to comment.