aboutsummaryrefslogtreecommitdiffstats
path: root/actionmailer
diff options
context:
space:
mode:
authorJeremy Daer <jeremydaer@gmail.com>2016-04-07 13:16:54 -0700
committerJeremy Daer <jeremydaer@gmail.com>2016-04-07 13:36:50 -0700
commit95e06e6682b3bc7f813336142213697eac36e401 (patch)
treee7e058cae32134a316ef9c6b4c8344c95067b5ef /actionmailer
parent9c2945f4cc1017fc9424f2e425e4820da1ef33b0 (diff)
downloadrails-95e06e6682b3bc7f813336142213697eac36e401.tar.gz
rails-95e06e6682b3bc7f813336142213697eac36e401.tar.bz2
rails-95e06e6682b3bc7f813336142213697eac36e401.zip
Disallow calling `#deliver_later` after local message modifications.
They would be lost when the delivery job is enqueued, otherwise. Prevents a common, hard-to-find bug like: ```ruby message = Notifier.welcome(user, foo) message.message_id = my_generated_message_id message.deliver_later ``` The message_id is silently lost here! *Only the mailer arguments are passed to the delivery job.* This raises an exception now. Make modifications to the message within the mailer method or use a custom Active Job to manage delivery instead of using #deliver_later.
Diffstat (limited to 'actionmailer')
-rw-r--r--actionmailer/CHANGELOG.md18
-rw-r--r--actionmailer/lib/action_mailer/message_delivery.rb16
-rw-r--r--actionmailer/test/message_delivery_test.rb8
3 files changed, 40 insertions, 2 deletions
diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md
index 1109912b04..1fef9eae29 100644
--- a/actionmailer/CHANGELOG.md
+++ b/actionmailer/CHANGELOG.md
@@ -1,3 +1,21 @@
+* Disallow calling `#deliver_later` after making local modifications to
+ the message which would be lost when the delivery job is enqueued.
+
+ Prevents a common, hard-to-find bug like
+
+ message = Notifier.welcome(user, foo)
+ message.message_id = my_generated_message_id
+ message.deliver_later
+
+ The message_id is silently lost! *Only the mailer arguments are passed
+ to the delivery job.*
+
+ This raises an exception now. Make modifications to the message within
+ the mailer method instead, or use a custom Active Job to manage delivery
+ instead of using #deliver_later.
+
+ *Jeremy Daer*
+
* Removes `-t` from default Sendmail arguments to match the underlying
`Mail::Sendmail` setting.
diff --git a/actionmailer/lib/action_mailer/message_delivery.rb b/actionmailer/lib/action_mailer/message_delivery.rb
index 5fcb5a0c88..d638057d72 100644
--- a/actionmailer/lib/action_mailer/message_delivery.rb
+++ b/actionmailer/lib/action_mailer/message_delivery.rb
@@ -18,6 +18,7 @@ module ActionMailer
@mailer = mailer
@mail_method = mail_method
@args = args
+ @obj = nil
end
def __getobj__ #:nodoc:
@@ -91,8 +92,19 @@ module ActionMailer
private
def enqueue_delivery(delivery_method, options={})
- args = @mailer.name, @mail_method.to_s, delivery_method.to_s, *@args
- ActionMailer::DeliveryJob.set(options).perform_later(*args)
+ if @obj
+ raise "You've accessed the message before asking to deliver it " \
+ "later, so you may have made local changes that would be " \
+ "silently lost if we enqueued a job to deliver it. Why? Only " \
+ "the mailer method *arguments* are passed with the delivery job! " \
+ "Do not access the message in any way if you mean to deliver it " \
+ "later. Workarounds: 1. don't touch the message before calling " \
+ "#deliver_later, 2. only touch the message *within your mailer " \
+ "method*, or 3. use a custom Active Job instead of #deliver_later."
+ else
+ args = @mailer.name, @mail_method.to_s, delivery_method.to_s, *@args
+ ActionMailer::DeliveryJob.set(options).perform_later(*args)
+ end
end
end
end
diff --git a/actionmailer/test/message_delivery_test.rb b/actionmailer/test/message_delivery_test.rb
index b834cdd08c..f06d69369f 100644
--- a/actionmailer/test/message_delivery_test.rb
+++ b/actionmailer/test/message_delivery_test.rb
@@ -93,4 +93,12 @@ class MessageDeliveryTest < ActiveSupport::TestCase
@mail.deliver_later(queue: :another_queue)
end
end
+
+ test 'deliver_later after accessing the message is disallowed' do
+ @mail.message # Load the message, which calls the mailer method.
+
+ assert_raise RuntimeError do
+ @mail.deliver_later
+ end
+ end
end