Author: Hugo Melo <hugo@riseup.net>
Add unit tests and fixes
app/mailers/mail_queuer.rb | 56 ++++++++----- test/unit/mail_queuer_test.rb | 142 +++++++++++++++++++++++++++++++++++++
diff --git a/app/mailers/mail_queuer.rb b/app/mailers/mail_queuer.rb
index 744705a3a3179e78ed4ad55d062f48dd3149c15f..fd706bce55f785908b2827a79639913e27f36aae 100644
--- a/app/mailers/mail_queuer.rb
+++ b/app/mailers/mail_queuer.rb
@@ -16,14 +16,15 @@ def self.schedule message, options = {}
delivery_method = ApplicationMailer.delivery_methods.find{ |n, k| k == message.delivery_method.class }.first
delivery_method_options = message.delivery_method.settings
- set(options).perform_later message.encoded, delivery_method.to_s, delivery_method_options.to_yaml
+ set(options).perform_later message.encoded, message.bcc, delivery_method.to_s, delivery_method_options.to_yaml
end
##
# Mail delivery
#
- def perform message, delivery_method, delivery_method_options
+ def perform message, bcc, delivery_method, delivery_method_options
m = Mail.read_from_string message
+ m.bcc = bcc
ApplicationMailer.wrap_delivery_behavior m, delivery_method.to_sym, YAML.load(delivery_method_options)
m.deliver_now
end
@@ -31,24 +32,38 @@ end
module InstanceMethods
def deliver
+ message = nil
last_sched = MailSchedule.last
last_sched.with_lock do
if last_sched != MailSchedule.last
- deliver_now
+ message = deliver_now
else
- deliver_schedule last_sched
+ message = deliver_schedule last_sched
end
end
+ message
end
def deliver_schedule last_sched
limit = ENV['MAIL_QUEUER_LIMIT'].to_i - 1
- to = self.to ||= []
orig_to = to.dup
- bcc = self.bcc ||= []
+ orig_cc = cc.dup
+ dests = {
+ to: self.to,
+ cc: self.cc,
+ bcc: self.bcc,
+ }
loop do
- dest_count = to.size + bcc.size
+ # mailers don't like emails without :to,
+ # so fill one if not present
+ dests[:to] = [orig_to.first] if dests[:to].blank?
+
+ dest_count = dests[:to].size + dests[:cc].size + dests[:bcc].size
+ # dests are changed on email splits, so set it to the remaining values
+ self.to = dests[:to]
+ self.cc = dests[:cc]
+ self.bcc = dests[:bcc]
##
# The last schedule is outside the quota period
@@ -62,13 +77,13 @@
if dest_count <= available_limit
last_sched.update dest_count: last_sched.dest_count + dest_count
Job.schedule self, wait_until: last_sched.scheduled_to
- return
+ return self
# avoid breaking mail which respect the mail limit. Schedule it all to the next hour
- elsif dest_count <= limit
+ elsif dest_count <= limit #&& ENV['AVOID_SPLIT']
last_sched = MailSchedule.create! dest_count: dest_count, scheduled_to: last_sched.scheduled_to+1.hour
Job.schedule self, wait_until: last_sched.scheduled_to
- return
+ return self
else # dest_count > limit
if available_limit == 0
@@ -79,23 +94,20 @@ # reuse current schedule
last_sched.update dest_count: limit # limit = last.sched.dest_count + available_limit
end
+ # needs to preserve replies when spliting the email
+ self.reply_to = orig_to + orig_cc if self.reply_to.blank?
+
##
- # Drop `to` until empty and then start dropping from bcc
- # #to and #bcc are changed destructively
- # for the next recursion
+ # start sending :to, followed by :cc, and so :bcc creating new schedules as needed
#
- message = self.dup
+ [:to, :cc, :bcc].each do |field|
+ next self[field] = [] if available_limit == 0
- if to.size > 0
- message.to = to.shift available_limit
- message.bcc = []
- else
- message.to = [orig_to.first]
- message.bcc = bcc.shift available_limit
- message.reply_to = orig_to if reply_to.blank?
+ self[field] = dests[field].shift(available_limit)
+ available_limit -= self.send(field).size
end
- Job.schedule message, wait_until: last_sched.scheduled_to
+ Job.schedule self, wait_until: last_sched.scheduled_to
end
end
end
diff --git a/test/unit/mail_queuer_test.rb b/test/unit/mail_queuer_test.rb
new file mode 100644
index 0000000000000000000000000000000000000000..ce486bf603da6ca5f55460147154e0c33fb15609
--- /dev/null
+++ b/test/unit/mail_queuer_test.rb
@@ -0,0 +1,142 @@
+ENV['MAIL_QUEUER'] = '1'
+require_relative "../test_helper"
+
+class MailQueuerTest < ActiveSupport::TestCase
+
+ def setup
+ MailSchedule.delete_all
+ Delayed::Job.delete_all
+
+ ENV['MAIL_QUEUER_LIMIT'] = '100'
+
+ MailSchedule.create dest_count: 0, scheduled_to: Time.now
+ ApplicationMailer.deliveries = []
+ end
+
+ class Mailer < ApplicationMailer
+ def test to:[], cc:[], bcc:[], from: Environment.default.noreply_email
+ mail to: to, cc: cc, bcc: bcc, from: from,
+ subject: "test", body: 'test'
+ end
+ end
+
+ should 'fit the available limit' do
+ to = 80.times.map{ |i| "b#{i}@example.com" }
+ cc = 10.times.map{ |i| "b#{i}@example.com" }
+ bcc = 9.times.map{ |i| "b#{i}@example.com" }
+ Mailer.test(to: to, cc: cc, bcc: bcc).deliver
+
+ Delayed::Worker.new.work_off
+ message = ApplicationMailer.deliveries.last
+ assert_equal 99, MailSchedule.first.dest_count
+ assert_equal to, message.to
+ assert_equal cc, message.cc
+ assert_equal bcc, message.bcc
+ end
+
+
+ should 'break the mail when :to is bigger than limit' do
+ to = 100.times.map{ |i| "b#{i}@example.com" }
+ cc = ["cc@example.com"]
+ bcc = ["bcc@example.com"]
+ Mailer.test(to: to, cc: cc, bcc: bcc).deliver
+
+ Delayed::Worker.new.work_off
+ assert_equal 1, ApplicationMailer.deliveries.count
+ message = ApplicationMailer.deliveries.first
+ assert_equal 99, MailSchedule.first.dest_count
+ assert_equal to+cc, message.reply_to
+ assert_equal to.first(99), message.to
+ assert message.cc.blank?
+ assert message.bcc.blank?
+
+ message = job_message
+ assert_equal 3, MailSchedule.last.dest_count
+ assert_equal to+cc, message.reply_to
+ assert_equal [to.last], message.to
+ assert_equal cc, message.cc
+ assert_equal bcc, message.bcc
+ end
+
+ should 'break the mail with cc is bigger than limit' do
+ to = 10.times.map{ |i| "b#{i}@example.com" }
+ cc = 100.times.map{ |i| "b#{i}@example.com" }
+ bcc = ["bcc@example.com"]
+ Mailer.test(to: to, cc: cc, bcc: bcc).deliver
+
+ Delayed::Worker.new.work_off
+ assert_equal 1, ApplicationMailer.deliveries.count
+ message = ApplicationMailer.deliveries.first
+ assert_equal 99, MailSchedule.first.dest_count
+ assert_equal to+cc, message.reply_to
+ assert_equal to, message.to
+ assert_equal cc.first(89), message.cc
+ assert message.bcc.blank?
+
+ message = job_message
+ assert_equal 13, MailSchedule.last.dest_count
+ assert_equal to+cc, message.reply_to
+ assert_equal [to.first], message.to
+ assert_equal cc[89..-1], message.cc
+ assert_equal bcc, message.bcc
+ end
+
+ should 'break the mail with bcc is bigger than limit' do
+ to = 10.times.map{ |i| "b#{i}@example.com" }
+ cc = 10.times.map{ |i| "b#{i}@example.com" }
+ bcc = 80.times.map{ |i| "b#{i}@example.com" }
+ Mailer.test(to: to, cc: cc, bcc: bcc).deliver
+
+ Delayed::Worker.new.work_off
+ assert_equal 1, ApplicationMailer.deliveries.count
+ message = ApplicationMailer.deliveries.first
+ assert_equal 99, MailSchedule.first.dest_count
+ assert_equal to+cc, message.reply_to
+ assert_equal to, message.to
+ assert_equal cc, message.cc
+ assert_equal bcc.first(79), message.bcc
+
+ message = job_message
+ assert_equal 2, MailSchedule.last.dest_count
+ assert_equal to+cc, message.reply_to
+ assert_equal [to.first], message.to
+ assert message.cc.blank?
+ assert_equal [bcc.last], message.bcc
+ end
+
+ should 'send in the next hour if available_limit < dest_count < limit' do
+ MailSchedule.last.update dest_count: 90
+
+ to = 10.times.map{ |i| "b#{i}@example.com" }
+ cc = 10.times.map{ |i| "b#{i}@example.com" }
+ bcc = 9.times.map{ |i| "b#{i}@example.com" }
+ Mailer.test(to: to, cc: cc, bcc: bcc).deliver
+
+ assert_equal 2, MailSchedule.count
+ assert_equal 90, MailSchedule.first.dest_count
+ assert_equal 29, MailSchedule.last.dest_count
+ assert_equal MailSchedule.last.scheduled_to, MailSchedule.first.scheduled_to + 1.hour
+
+ Delayed::Worker.new.work_off
+ assert_equal nil, ApplicationMailer.deliveries.last
+
+ message = job_message
+ assert_equal to, message.to
+ assert_equal cc, message.cc
+ assert_equal bcc, message.bcc
+ end
+
+ protected
+
+ def job_message job=Delayed::Job.last
+ y = YAML.load job.handler
+ l = y.job_data['arguments']
+ m = l.first
+ bcc = l.second
+
+ msg = Mail.read_from_string m
+ msg.bcc = bcc
+ msg
+ end
+
+end