Backport v7 fix for conflicts (#461)

* Backport fix from v7

* Adds coverage

lib/sidekiq_unique_jobs/lock/base_lock.rb

@@ -118,11 +118,11 @@ def unlock_with_callback
         return log_warn("might need to be unlocked manually") unless unlock
 
         callback_safely
-        item[JID_KEY]
       end
 
       def callback_safely
         callback&.call
+        item[JID_KEY]
       rescue StandardError
         log_warn("unlocked successfully but the #after_unlock callback failed!")
         raise

lib/sidekiq_unique_jobs/lock/while_executing.rb

@@ -33,14 +33,13 @@ def lock
       #   These jobs are locked in the server process not from the client
       # @yield to the worker class perform method
       def execute
-        return strategy.call unless locksmith.lock(item[LOCK_TIMEOUT_KEY])
+        return strategy&.call unless locksmith.lock(item[LOCK_TIMEOUT_KEY])
 
         yield
+        unlock_with_callback
       rescue Exception # rubocop:disable Lint/RescueException
         delete!
         raise
-      else
-        unlock_with_callback
       end
 
       private

spec/integration/sidekiq_unique_jobs/lock/while_executing_spec.rb

@@ -4,16 +4,19 @@
 RSpec.describe SidekiqUniqueJobs::Lock::WhileExecuting, redis: :redis do
   include SidekiqHelpers
 
-  let(:process_one) { described_class.new(item_one, callback) }
-  let(:process_two) { described_class.new(item_two, callback) }
+  let(:process_one) { described_class.new(item_one, callback_one) }
+  let(:process_two) { described_class.new(item_two, callback_two) }
+  let(:strategy_one) { nil }
+  let(:strategy_two) { nil }
 
   let(:jid_one)      { "jid one" }
   let(:jid_two)      { "jid two" }
   let(:worker_class) { WhileExecutingJob }
   let(:unique)       { :while_executing }
   let(:queue)        { :while_executing }
   let(:args)         { %w[array of arguments] }
-  let(:callback)     { -> {} }
+  let(:callback_one) { -> {} }
+  let(:callback_two) { -> {} }
   let(:item_one) do
     { "jid" => jid_one,
       "class" => worker_class.to_s,
@@ -30,7 +33,10 @@
   end
 
   before do
-    allow(callback).to receive(:call).and_call_original
+    allow(callback_one).to receive(:call).and_call_original if callback_one
+    allow(callback_two).to receive(:call).and_call_original if callback_two
+    allow(process_one).to receive(:strategy).and_return(strategy_one)
+    allow(process_two).to receive(:strategy).and_return(strategy_two)
   end
 
   describe "#lock" do
@@ -52,7 +58,7 @@
 
     it "calls back" do
       process_one.execute {}
-      expect(callback).to have_received(:call)
+      expect(callback_one).to have_received(:call)
     end
 
     it "prevents other processes from executing" do
@@ -62,7 +68,31 @@
         expect(unset).to eq(true)
       end
 
-      expect(callback).to have_received(:call).once
+      expect(callback_one).to have_received(:call).once
+      expect(callback_two).not_to have_received(:call)
+    end
+
+    context "when no callback is defined" do
+      let(:callback_one) { -> { true } }
+      let(:callback_two) { nil }
+
+      let(:strategy_one) { SidekiqUniqueJobs::OnConflict::Reschedule.new(item_one) }
+      let(:strategy_two) { SidekiqUniqueJobs::OnConflict::Reschedule.new(item_two) }
+
+      before do
+        allow(strategy_one).to receive(:call).and_call_original
+        allow(strategy_two).to receive(:call).and_call_original
+      end
+
+      it "works" do
+        process_one.execute do
+          process_two.execute { puts "BOGUS!" }
+        end
+
+        expect(callback_one).to have_received(:call).once
+        expect(strategy_one).not_to have_received(:call)
+        expect(strategy_two).to have_received(:call).once
+      end
     end
 
     context "when worker raises error" do