Skip DeleteMessage on duplicated-receive case

cookpad · Jul 21, 2017 · a397414 · a397414
1 parent 504af15
commit a397414
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 4 deletions.
diff --git a/lib/barbeque/message_handler/job_execution.rb b/lib/barbeque/message_handler/job_execution.rb
@@ -16,7 +16,6 @@ def run
         begin
           job_execution = Barbeque::JobExecution.create(message_id: @message.id, job_definition: job_definition, job_queue: @message_queue.job_queue)
         rescue ActiveRecord::RecordNotUnique => e
-          @message_queue.delete_message(@message)
           raise DuplicatedExecution.new(e.message)
         end
         Barbeque::ExecutionLog.save_message(job_execution, @message)

diff --git a/lib/barbeque/message_handler/job_retry.rb b/lib/barbeque/message_handler/job_retry.rb
@@ -18,7 +18,6 @@ def run
         begin
           job_retry = Barbeque::JobRetry.create(message_id: @message.id, job_execution: job_execution)
         rescue ActiveRecord::RecordNotUnique => e
-          @message_queue.delete_message(@message)
           raise DuplicatedExecution.new(e.message)
         end
         @message_queue.delete_message(@message)

diff --git a/spec/barbeque/message_handler/job_execution_spec.rb b/spec/barbeque/message_handler/job_execution_spec.rb
@@ -25,7 +25,6 @@
       allow(Barbeque::Executor::Docker).to receive(:new).with({}).and_return(executor)
       allow(Barbeque::ExecutionLog).to receive(:save_message)
       allow(Barbeque::ExecutionLog).to receive(:save_stdout_and_stderr)
-      expect(message_queue).to receive(:delete_message).with(message)
     end
 
     around do |example|
@@ -36,6 +35,7 @@
 
     it 'creates job_execution associated to job_definition in the message and job_queue' do
       allow(executor).to receive(:start_execution).and_return(open3_result)
+      expect(message_queue).to receive(:delete_message).with(message)
       expect { handler.run }.to change(Barbeque::JobExecution, :count).by(1)
       expect(Barbeque::JobExecution.last.finished_at).to be_nil
       expect(Barbeque::JobExecution.last.job_definition).to eq(job_definition)
@@ -53,6 +53,7 @@
           'BARBEQUE_RETRY_COUNT' => '0',
         )
       }
+      expect(message_queue).to receive(:delete_message).with(message)
       handler.run
     end
 
@@ -74,12 +75,14 @@
       end
 
       it 'updates status to error' do
+        expect(message_queue).to receive(:delete_message).with(message)
         expect(Barbeque::JobExecution.count).to eq(0)
         expect { handler.run }.to raise_error(exception)
         expect(Barbeque::JobExecution.last).to be_error
       end
 
       it 'logs message body' do
+        expect(message_queue).to receive(:delete_message).with(message)
         expect(Barbeque::ExecutionLog).to receive(:save_stdout_and_stderr).with(a_kind_of(Barbeque::JobExecution), '', /something went wrong/)
         expect { handler.run }.to raise_error(exception)
       end

diff --git a/spec/barbeque/message_handler/job_retry_spec.rb b/spec/barbeque/message_handler/job_retry_spec.rb
@@ -23,7 +23,6 @@
       allow(Barbeque::ExecutionLog).to receive(:save_stdout_and_stderr)
       allow(Barbeque::ExecutionLog).to receive(:load).with(execution: job_execution).and_return({ 'message' => message_body })
       allow(Barbeque::Executor::Docker).to receive(:new).with({}).and_return(executor)
-      expect(message_queue).to receive(:delete_message).with(message)
     end
 
     around do |example|
@@ -43,11 +42,13 @@
           'BARBEQUE_RETRY_COUNT' => '1',
         )
       }
+      expect(message_queue).to receive(:delete_message).with(message)
       handler.run
     end
 
     it 'creates job_retry associated to job_execution in the message' do
       expect(executor).to receive(:start_retry)
+      expect(message_queue).to receive(:delete_message).with(message)
       expect { handler.run }.to change(Barbeque::JobRetry, :count).by(1)
       job_retry = Barbeque::JobRetry.last
       expect(job_retry.finished_at).to be_nil
@@ -61,6 +62,7 @@
       end
 
       it 'raises MessageNotFound' do
+        expect(message_queue).to receive(:delete_message).with(message)
         expect { handler.run }.to raise_error(Barbeque::MessageHandler::MessageNotFound)
       end
     end
@@ -83,6 +85,7 @@
       end
 
       it 'updates status to error' do
+        expect(message_queue).to receive(:delete_message).with(message)
         expect(job_execution).to be_failed
         expect(Barbeque::JobRetry.count).to eq(0)
         expect { handler.run }.to raise_error(exception)
@@ -91,6 +94,7 @@
       end
 
       it 'logs empty output' do
+        expect(message_queue).to receive(:delete_message).with(message)
         expect(Barbeque::ExecutionLog).to receive(:save_stdout_and_stderr).with(a_kind_of(Barbeque::JobRetry), '', /something went wrong/)
         expect { handler.run }.to raise_error(exception)
       end