diff --git a/app/services/mailings/outbound_queue.rb b/app/services/mailings/outbound_queue.rb index 3713bb1..07407e8 100644 --- a/app/services/mailings/outbound_queue.rb +++ b/app/services/mailings/outbound_queue.rb @@ -1,9 +1,10 @@ class Mailings::OutboundQueue LOCK_PATH = Rails.root.join("tmp/outbound_smtp.lock") STAMP_PATH = Rails.root.join("tmp/outbound_last_sent") + ATTEMPT_PATH = Rails.root.join("tmp/outbound_last_attempt") class << self - attr_accessor :min_interval + attr_accessor :min_interval, :fail_interval def enqueue(recipient) recipient.with_lock do @@ -29,7 +30,9 @@ class Mailings::OutboundQueue begin CampaignMailer.raise_delivery_errors = true outcome = recipient.deliver_queued! - stamp! + stamp_attempt! + stamp! if outcome == :sent + Rails.logger.info("[outbound] #{outcome} mailing=#{recipient.mailing_id} to=#{recipient.email}") outcome == :sent ? :sent : :deferred ensure CampaignMailer.raise_delivery_errors = false if Rails.env.development? @@ -45,7 +48,11 @@ class Mailings::OutboundQueue end def next_queued - queued_scope.order(:updated_at, :id).first + queued_scope.order( + Arel.sql( + "CASE WHEN mailing_recipients.error_message IS NULL OR mailing_recipients.error_message = '' THEN 0 ELSE 1 END, mailing_recipients.updated_at ASC, mailing_recipients.id ASC" + ) + ).first end def queued_scope @@ -53,18 +60,17 @@ class Mailings::OutboundQueue end def seconds_until_next_slot - return 0 if min_interval.to_f <= 0 - - last = last_sent_at - return 0 if last.nil? - - remaining = min_interval - (Time.current - last) - remaining.positive? ? remaining : 0 + [ + remaining(last_sent_at, min_interval), + remaining(last_attempt_at, fail_interval) + ].max end def reset! @last_sent_at = nil + @last_attempt_at = nil File.delete(stamp_path) if File.exist?(stamp_path) + File.delete(attempt_path) if File.exist?(attempt_path) rescue Errno::ENOENT nil end @@ -74,17 +80,36 @@ class Mailings::OutboundQueue File.write(stamp_path, @last_sent_at.to_f.to_s) end - def last_sent_at - return @last_sent_at if @last_sent_at - return unless File.exist?(stamp_path) + def stamp_attempt! + @last_attempt_at = Time.current + File.write(attempt_path, @last_attempt_at.to_f.to_s) + end - @last_sent_at = Time.zone.at(Float(File.read(stamp_path))) - rescue ArgumentError, TypeError, Errno::ENOENT - nil + def last_sent_at + @last_sent_at ||= read_time(stamp_path) + end + + def last_attempt_at + @last_attempt_at ||= read_time(attempt_path) end private + def remaining(timestamp, interval) + return 0 if interval.to_f <= 0 || timestamp.nil? + + leftover = interval - (Time.current - timestamp) + leftover.positive? ? leftover : 0 + end + + def read_time(path) + return unless File.exist?(path) + + Time.zone.at(Float(File.read(path))) + rescue ArgumentError, TypeError, Errno::ENOENT + nil + end + def lock_path LOCK_PATH end @@ -92,7 +117,12 @@ class Mailings::OutboundQueue def stamp_path STAMP_PATH end + + def attempt_path + ATTEMPT_PATH + end end - self.min_interval = Rails.env.test? ? 0.0 : 180.0 + self.min_interval = Rails.env.test? ? 0.0 : 60.0 + self.fail_interval = Rails.env.test? ? 0.0 : 30.0 end diff --git a/test/services/mailings/outbound_queue_test.rb b/test/services/mailings/outbound_queue_test.rb index e83a2a8..7e17392 100644 --- a/test/services/mailings/outbound_queue_test.rb +++ b/test/services/mailings/outbound_queue_test.rb @@ -5,6 +5,7 @@ class Mailings::OutboundQueueTest < ActiveSupport::TestCase setup do Mailings::OutboundQueue.min_interval = 0 + Mailings::OutboundQueue.fail_interval = 0 Mailings::OutboundQueue.reset! Mailings::SmtpGate.min_gap = 0 Mailings::SmtpGate.backoff_base = 0 @@ -13,6 +14,7 @@ class Mailings::OutboundQueueTest < ActiveSupport::TestCase teardown do Mailings::OutboundQueue.min_interval = 0 + Mailings::OutboundQueue.fail_interval = 0 Mailings::OutboundQueue.reset! end