From 6839f040e0720576188d2bc33e93670c12334add Mon Sep 17 00:00:00 2001 From: Rolf Schmidt Date: Wed, 20 Feb 2019 12:18:47 +0100 Subject: [PATCH] Refactoring: Migrated Observer::Ticket::Article::CommunicateEmail::BackgroundJob (Delayed::Job) to TicketArticleCommunicateEmailJob (ActiveJob). --- .../ticket_article_communicate_email_job.rb} | 25 ++++--------- .../ticket/article/communicate_email.rb | 2 +- test/integration/email_deliver_test.rb | 36 +++++++++---------- 3 files changed, 26 insertions(+), 37 deletions(-) rename app/{models/observer/ticket/article/communicate_email/background_job.rb => jobs/ticket_article_communicate_email_job.rb} (93%) diff --git a/app/models/observer/ticket/article/communicate_email/background_job.rb b/app/jobs/ticket_article_communicate_email_job.rb similarity index 93% rename from app/models/observer/ticket/article/communicate_email/background_job.rb rename to app/jobs/ticket_article_communicate_email_job.rb index 9bb5cd94d..38e699e30 100644 --- a/app/models/observer/ticket/article/communicate_email/background_job.rb +++ b/app/jobs/ticket_article_communicate_email_job.rb @@ -1,10 +1,11 @@ -class Observer::Ticket::Article::CommunicateEmail::BackgroundJob - def initialize(id) - @article_id = id - end +class TicketArticleCommunicateEmailJob < ApplicationJob - def perform - record = Ticket::Article.find(@article_id) + retry_on StandardError, attempts: 4, wait: lambda { |executions| + executions * 25.seconds + } + + def perform(article_id) + record = Ticket::Article.find(article_id) # build subject ticket = Ticket.lookup(id: record.ticket_id) @@ -163,16 +164,4 @@ class Observer::Ticket::Article::CommunicateEmail::BackgroundJob raise message end - - def max_attempts - 4 - end - - def reschedule_at(current_time, attempts) - if Rails.env.production? - return current_time + attempts * 25.seconds - end - - current_time + 5.seconds - end end diff --git a/app/models/observer/ticket/article/communicate_email.rb b/app/models/observer/ticket/article/communicate_email.rb index 4f75e3ac5..27c9b91f5 100644 --- a/app/models/observer/ticket/article/communicate_email.rb +++ b/app/models/observer/ticket/article/communicate_email.rb @@ -27,6 +27,6 @@ class Observer::Ticket::Article::CommunicateEmail < ActiveRecord::Observer return true if type.name != 'email' # send background job - Delayed::Job.enqueue(Observer::Ticket::Article::CommunicateEmail::BackgroundJob.new(record.id)) + TicketArticleCommunicateEmailJob.perform_later(record.id) end end diff --git a/test/integration/email_deliver_test.rb b/test/integration/email_deliver_test.rb index 4c40d0924..2b764d261 100644 --- a/test/integration/email_deliver_test.rb +++ b/test/integration/email_deliver_test.rb @@ -2,6 +2,7 @@ require 'test_helper' class EmailDeliverTest < ActiveSupport::TestCase test 'basic check' do + travel_to DateTime.current if ENV['MAIL_SERVER'].blank? raise "Need MAIL_SERVER as ENV variable like export MAIL_SERVER='mx.example.com'" @@ -81,8 +82,7 @@ class EmailDeliverTest < ActiveSupport::TestCase assert_nil(article1.preferences['delivery_status_date']) assert_nil(article1.preferences['delivery_status_message']) - result = Observer::Ticket::Article::CommunicateEmail::BackgroundJob.new(article1.id) - assert(result.perform) + TicketArticleCommunicateEmailJob.new.perform(article1.id) article1_lookup = Ticket::Article.find(article1.id) assert_equal(1, article1_lookup.preferences['delivery_retry']) @@ -115,8 +115,7 @@ class EmailDeliverTest < ActiveSupport::TestCase }, ) assert_raises(RuntimeError) do - result = Observer::Ticket::Article::CommunicateEmail::BackgroundJob.new(article1.id) - assert_not(result.perform) + TicketArticleCommunicateEmailJob.new.perform(article1.id) end article1_lookup = Ticket::Article.find(article1.id) assert_equal(2, article1_lookup.preferences['delivery_retry']) @@ -149,8 +148,7 @@ class EmailDeliverTest < ActiveSupport::TestCase }, ) - result = Observer::Ticket::Article::CommunicateEmail::BackgroundJob.new(article1.id) - assert(result.perform) + TicketArticleCommunicateEmailJob.new.perform(article1.id) article1_lookup = Ticket::Article.find(article1.id) assert_equal(3, article1_lookup.preferences['delivery_retry']) assert_equal('success', article1_lookup.preferences['delivery_status']) @@ -205,9 +203,9 @@ class EmailDeliverTest < ActiveSupport::TestCase ticket1.state = Ticket::State.find_by(name: 'closed') ticket1.save - assert_raises(RuntimeError) do - Scheduler.worker(true) - end + assert(Delayed::Job.where(attempts: 1).none?) + Scheduler.worker(true) + assert(Delayed::Job.where(attempts: 1).exists?) ticket1.reload article2_lookup = Ticket::Article.find(article2.id) @@ -228,10 +226,10 @@ class EmailDeliverTest < ActiveSupport::TestCase assert(article2_lookup.preferences['delivery_status_message']) assert_equal('closed', ticket1.state.name) - sleep 6 - assert_raises(RuntimeError) do - Scheduler.worker(true) - end + travel 26.seconds + assert(Delayed::Job.where(attempts: 2).none?) + Scheduler.worker(true) + assert(Delayed::Job.where(attempts: 2).exists?) ticket1.reload article2_lookup = Ticket::Article.find(article2.id) @@ -253,10 +251,10 @@ class EmailDeliverTest < ActiveSupport::TestCase assert(article2_lookup.preferences['delivery_status_message']) assert_equal('closed', ticket1.state.name) - sleep 11 - assert_raises(RuntimeError) do - Scheduler.worker(true) - end + travel 51.seconds + assert(Delayed::Job.where(attempts: 3).none?) + Scheduler.worker(true) + assert(Delayed::Job.where(attempts: 3).exists?) ticket1.reload article2_lookup = Ticket::Article.find(article2.id) @@ -267,10 +265,12 @@ class EmailDeliverTest < ActiveSupport::TestCase assert(article2_lookup.preferences['delivery_status_message']) assert_equal('closed', ticket1.state.name) - sleep 16 + travel 76.seconds + assert(Delayed::Job.where(attempts: 4).none?) assert_raises(RuntimeError) do Scheduler.worker(true) end + assert(Delayed::Job.where(attempts: 4).exists?) ticket1.reload article2_lookup = Ticket::Article.find(article2.id)