diff options
author | Jeremy Daer <jeremydaer@gmail.com> | 2016-04-08 18:40:39 -0700 |
---|---|---|
committer | Jeremy Daer <jeremydaer@gmail.com> | 2016-04-08 18:40:39 -0700 |
commit | 2080ff2872e9604155a276a830b38844dc45c3b4 (patch) | |
tree | 9c94f21d1a686324a170e8da398b0be7d008530b /actionmailer | |
parent | 55c1436e67d4c9ecabb0f98529b13ad129ba2864 (diff) | |
parent | 95e06e6682b3bc7f813336142213697eac36e401 (diff) | |
download | rails-2080ff2872e9604155a276a830b38844dc45c3b4.tar.gz rails-2080ff2872e9604155a276a830b38844dc45c3b4.tar.bz2 rails-2080ff2872e9604155a276a830b38844dc45c3b4.zip |
Merge pull request #24457 from jeremy/mailer/dont-deliver-later-after-message-is-loaded
Disallow calling `#deliver_later` after local message modifications.
Diffstat (limited to 'actionmailer')
-rw-r--r-- | actionmailer/CHANGELOG.md | 18 | ||||
-rw-r--r-- | actionmailer/lib/action_mailer/message_delivery.rb | 16 | ||||
-rw-r--r-- | actionmailer/test/message_delivery_test.rb | 8 |
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 |