Skip to content

Commit d76d8f7

Browse files
eileencodeseileencodes
authored
Merge pull request rails#34591 from gmcgibbon/new_delivery_job
Add MailDeliveryJob for unified mail delivery
2 parents 65c4b1b + f5050d9 commit d76d8f7

12 files changed

+179
-23
lines changed

‎actionmailer/CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
* Deliver parameterized mail with `ActionMailer::DeliveryJob` and remove `ActionMailer::Parameterized::DeliveryJob`.
1+
* Add `MailDeliveryJob` for delivering both regular and parameterized mail. Deprecate using `DeliveryJob` and `Parameterized::DeliveryJob`.
22

33
*Gannon McGibbon*
44

‎actionmailer/lib/action_mailer.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ module ActionMailer
5252
autoload :TestHelper
5353
autoload :MessageDelivery
5454
autoload :DeliveryJob
55+
autoload :MailDeliveryJob
5556

5657
def self.eager_load!
5758
super

‎actionmailer/lib/action_mailer/base.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ def _protected_ivars # :nodoc:
461461

462462
helper ActionMailer::MailHelper
463463

464-
class_attribute :delivery_job, default: ::ActionMailer::DeliveryJob
464+
class_attribute :delivery_job, default: ::ActionMailer::MailDeliveryJob
465465
class_attribute :default_params, default: {
466466
mime_version: "1.0",
467467
charset: "UTF-8",

‎actionmailer/lib/action_mailer/delivery_job.rb

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,16 @@ class DeliveryJob < ActiveJob::Base # :nodoc:
1212

1313
rescue_from StandardError, with: :handle_exception_with_mailer_class
1414

15-
def perform(mailer, mail_method, delivery_method, params, *args) #:nodoc:
16-
mailer_class = params ? mailer.constantize.with(params) : mailer.constantize
17-
mailer_class.public_send(mail_method, *args).send(delivery_method)
15+
before_perform do
16+
ActiveSupport::Deprecation.warn <<~MSG.squish
17+
Sending mail with DeliveryJob and Parameterized::DeliveryJob
18+
is deprecated and will be removed in Rails 6.1.
19+
Please use MailDeliveryJob instead.
20+
MSG
21+
end
22+
23+
def perform(mailer, mail_method, delivery_method, *args) #:nodoc:
24+
mailer.constantize.public_send(mail_method, *args).send(delivery_method)
1825
end
1926

2027
private

‎actionmailer/lib/action_mailer/mail_delivery_job.rb

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# frozen_string_literal: true
2+
3+
require "active_job"
4+
5+
module ActionMailer
6+
# The <tt>ActionMailer::NewDeliveryJob</tt> class is used when you
7+
# want to send emails outside of the request-response cycle. It supports
8+
# sending either parameterized or normal mail.
9+
#
10+
# Exceptions are rescued and handled by the mailer class.
11+
class MailDeliveryJob < ActiveJob::Base # :nodoc:
12+
queue_as { ActionMailer::Base.deliver_later_queue_name }
13+
14+
rescue_from StandardError, with: :handle_exception_with_mailer_class
15+
16+
def perform(mailer, mail_method, delivery_method, args:, params: nil) #:nodoc:
17+
mailer_class = params ? mailer.constantize.with(params) : mailer.constantize
18+
mailer_class.public_send(mail_method, *args).send(delivery_method)
19+
end
20+
21+
private
22+
# "Deserialize" the mailer class name by hand in case another argument
23+
# (like a Global ID reference) raised DeserializationError.
24+
def mailer_class
25+
if mailer = Array(@serialized_arguments).first || Array(arguments).first
26+
mailer.constantize
27+
end
28+
end
29+
30+
def handle_exception_with_mailer_class(exception)
31+
if klass = mailer_class
32+
klass.handle_exception exception
33+
else
34+
raise exception
35+
end
36+
end
37+
end
38+
end

‎actionmailer/lib/action_mailer/message_delivery.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,18 @@ def enqueue_delivery(delivery_method, options = {})
135135
"#deliver_later, 2. only touch the message *within your mailer " \
136136
"method*, or 3. use a custom Active Job instead of #deliver_later."
137137
else
138-
args = @mailer_class.name, @action.to_s, delivery_method.to_s, nil, *@args
139138
job = @mailer_class.delivery_job
139+
args = arguments_for(job, delivery_method)
140140
job.set(options).perform_later(*args)
141141
end
142142
end
143+
144+
def arguments_for(delivery_job, delivery_method)
145+
if delivery_job <= MailDeliveryJob
146+
[@mailer_class.name, @action.to_s, delivery_method.to_s, args: @args]
147+
else
148+
[@mailer_class.name, @action.to_s, delivery_method.to_s, *@args]
149+
end
150+
end
143151
end
144152
end

‎actionmailer/lib/action_mailer/parameterized.rb

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,12 @@ def respond_to_missing?(method, include_all = false)
121121
end
122122
end
123123

124+
class DeliveryJob < ActionMailer::DeliveryJob # :nodoc:
125+
def perform(mailer, mail_method, delivery_method, params, *args)
126+
mailer.constantize.with(params).public_send(mail_method, *args).send(delivery_method)
127+
end
128+
end
129+
124130
class MessageDelivery < ActionMailer::MessageDelivery # :nodoc:
125131
def initialize(mailer_class, action, params, *args)
126132
super(mailer_class, action, *args)
@@ -139,11 +145,19 @@ def enqueue_delivery(delivery_method, options = {})
139145
if processed?
140146
super
141147
else
142-
args = @mailer_class.name, @action.to_s, delivery_method.to_s, @params, *@args
143-
job = @mailer_class.delivery_job
148+
job = @mailer_class.delivery_job
149+
args = arguments_for(job, delivery_method)
144150
job.set(options).perform_later(*args)
145151
end
146152
end
153+
154+
def arguments_for(delivery_job, delivery_method)
155+
if delivery_job <= MailDeliveryJob
156+
[@mailer_class.name, @action.to_s, delivery_method.to_s, params: @params, args: @args]
157+
else
158+
[@mailer_class.name, @action.to_s, delivery_method.to_s, @params, *@args]
159+
end
160+
end
147161
end
148162
end
149163
end

‎actionmailer/lib/action_mailer/test_helper.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,9 +125,9 @@ def assert_enqueued_emails(number, &block)
125125
# end
126126
def assert_enqueued_email_with(mailer, method, args: nil, queue: "mailers", &block)
127127
args = if args.is_a?(Hash)
128-
[mailer.to_s, method.to_s, "deliver_now", args]
128+
[mailer.to_s, method.to_s, "deliver_now", params: args, args: []]
129129
else
130-
[mailer.to_s, method.to_s, "deliver_now", nil, *args]
130+
[mailer.to_s, method.to_s, "deliver_now", args: Array(args)]
131131
end
132132
assert_enqueued_with(job: mailer.delivery_job, args: args, queue: queue, &block)
133133
end

‎actionmailer/test/legacy_delivery_job_test.rb

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# frozen_string_literal: true
2+
3+
require "abstract_unit"
4+
require "active_job"
5+
require "mailers/params_mailer"
6+
require "mailers/delayed_mailer"
7+
8+
class LegacyDeliveryJobTest < ActiveSupport::TestCase
9+
include ActiveJob::TestHelper
10+
11+
class LegacyDeliveryJob < ActionMailer::DeliveryJob
12+
end
13+
14+
class LegacyParmeterizedDeliveryJob < ActionMailer::Parameterized::DeliveryJob
15+
end
16+
17+
setup do
18+
@previous_logger = ActiveJob::Base.logger
19+
ActiveJob::Base.logger = Logger.new(nil)
20+
21+
@previous_delivery_method = ActionMailer::Base.delivery_method
22+
ActionMailer::Base.delivery_method = :test
23+
24+
@previous_deliver_later_queue_name = ActionMailer::Base.deliver_later_queue_name
25+
ActionMailer::Base.deliver_later_queue_name = :test_queue
26+
end
27+
28+
teardown do
29+
ActiveJob::Base.logger = @previous_logger
30+
ParamsMailer.deliveries.clear
31+
32+
ActionMailer::Base.delivery_method = @previous_delivery_method
33+
ActionMailer::Base.deliver_later_queue_name = @previous_deliver_later_queue_name
34+
end
35+
36+
test "should send parameterized mail correctly" do
37+
mail = ParamsMailer.with(inviter: "david@basecamp.com", invitee: "jason@basecamp.com").invitation
38+
args = [
39+
"ParamsMailer",
40+
"invitation",
41+
"deliver_now",
42+
{ inviter: "david@basecamp.com", invitee: "jason@basecamp.com" },
43+
]
44+
45+
with_delivery_job(LegacyParmeterizedDeliveryJob) do
46+
assert_deprecated do
47+
assert_performed_with(job: LegacyParmeterizedDeliveryJob, args: args) do
48+
mail.deliver_later
49+
end
50+
end
51+
end
52+
end
53+
54+
test "should send mail correctly" do
55+
mail = DelayedMailer.test_message(1, 2, 3)
56+
args = [
57+
"DelayedMailer",
58+
"test_message",
59+
"deliver_now",
60+
1,
61+
2,
62+
3,
63+
]
64+
65+
with_delivery_job(LegacyDeliveryJob) do
66+
assert_deprecated do
67+
assert_performed_with(job: LegacyDeliveryJob, args: args) do
68+
mail.deliver_later
69+
end
70+
end
71+
end
72+
end
73+
74+
private
75+
76+
def with_delivery_job(job)
77+
old_params_delivery_job = ParamsMailer.delivery_job
78+
old_regular_delivery_job = DelayedMailer.delivery_job
79+
ParamsMailer.delivery_job = job
80+
DelayedMailer.delivery_job = job
81+
yield
82+
ensure
83+
ParamsMailer.delivery_job = old_params_delivery_job
84+
DelayedMailer.delivery_job = old_regular_delivery_job
85+
end
86+
end

‎actionmailer/test/message_delivery_test.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -64,34 +64,34 @@ def test_should_enqueue_and_run_correctly_in_activejob
6464
end
6565

6666
test "should enqueue the email with :deliver_now delivery method" do
67-
assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do
67+
assert_performed_with(job: ActionMailer::MailDeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]]) do
6868
@mail.deliver_later
6969
end
7070
end
7171

7272
test "should enqueue the email with :deliver_now! delivery method" do
73-
assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now!", nil, 1, 2, 3]) do
73+
assert_performed_with(job: ActionMailer::MailDeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now!", args: [1, 2, 3]]) do
7474
@mail.deliver_later!
7575
end
7676
end
7777

7878
test "should enqueue a delivery with a delay" do
7979
travel_to Time.new(2004, 11, 24, 01, 04, 44) do
80-
assert_performed_with(job: ActionMailer::DeliveryJob, at: Time.current + 10.minutes, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do
80+
assert_performed_with(job: ActionMailer::MailDeliveryJob, at: Time.current + 10.minutes, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]]) do
8181
@mail.deliver_later wait: 10.minutes
8282
end
8383
end
8484
end
8585

8686
test "should enqueue a delivery at a specific time" do
8787
later_time = Time.current + 1.hour
88-
assert_performed_with(job: ActionMailer::DeliveryJob, at: later_time, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do
88+
assert_performed_with(job: ActionMailer::MailDeliveryJob, at: later_time, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]]) do
8989
@mail.deliver_later wait_until: later_time
9090
end
9191
end
9292

9393
test "should enqueue the job on the correct queue" do
94-
assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3], queue: "test_queue") do
94+
assert_performed_with(job: ActionMailer::MailDeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]], queue: "test_queue") do
9595
@mail.deliver_later
9696
end
9797
end
@@ -100,17 +100,17 @@ def test_should_enqueue_and_run_correctly_in_activejob
100100
old_delivery_job = DelayedMailer.delivery_job
101101
DelayedMailer.delivery_job = DummyJob
102102

103-
assert_performed_with(job: DummyJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3]) do
103+
assert_performed_with(job: DummyJob, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]]) do
104104
@mail.deliver_later
105105
end
106106

107107
DelayedMailer.delivery_job = old_delivery_job
108108
end
109109

110-
class DummyJob < ActionMailer::DeliveryJob; end
110+
class DummyJob < ActionMailer::MailDeliveryJob; end
111111

112112
test "can override the queue when enqueuing mail" do
113-
assert_performed_with(job: ActionMailer::DeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", nil, 1, 2, 3], queue: "another_queue") do
113+
assert_performed_with(job: ActionMailer::MailDeliveryJob, args: ["DelayedMailer", "test_message", "deliver_now", args: [1, 2, 3]], queue: "another_queue") do
114114
@mail.deliver_later(queue: :another_queue)
115115
end
116116
end

‎actionmailer/test/parameterized_test.rb

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
class ParameterizedTest < ActiveSupport::TestCase
88
include ActiveJob::TestHelper
99

10-
class DummyDeliveryJob < ActionMailer::DeliveryJob
10+
class DummyDeliveryJob < ActionMailer::MailDeliveryJob
1111
end
1212

1313
setup do
@@ -42,9 +42,10 @@ class DummyDeliveryJob < ActionMailer::DeliveryJob
4242
"ParamsMailer",
4343
"invitation",
4444
"deliver_now",
45-
{ inviter: "david@basecamp.com", invitee: "jason@basecamp.com" },
45+
params: { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" },
46+
args: [],
4647
]
47-
assert_performed_with(job: ActionMailer::DeliveryJob, args: args) do
48+
assert_performed_with(job: ActionMailer::MailDeliveryJob, args: args) do
4849
@mail.deliver_later
4950
end
5051
end
@@ -68,7 +69,8 @@ class DummyDeliveryJob < ActionMailer::DeliveryJob
6869
"ParamsMailer",
6970
"invitation",
7071
"deliver_now",
71-
{ inviter: "david@basecamp.com", invitee: "jason@basecamp.com" },
72+
params: { inviter: "david@basecamp.com", invitee: "jason@basecamp.com" },
73+
args: [],
7274
]
7375

7476
with_delivery_job DummyDeliveryJob do

‎actionmailer/test/test_helper_test.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def test_parameter_args
2424
end
2525
end
2626

27-
class CustomDeliveryJob < ActionMailer::DeliveryJob
27+
class CustomDeliveryJob < ActionMailer::MailDeliveryJob
2828
end
2929

3030
class CustomDeliveryMailer < TestHelperMailer

0 commit comments

Comments
 (0)