Skip to content

Fix mailer argument deserialisation for 6.1 on Ruby 3.1 #2570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,3 @@ Layout/LineLength:
# Over time we'd like to get this down, but this is what we're at now.
Metrics/MethodLength:
Max: 43 # default: 10

Metrics/ClassLength:
Exclude:
- lib/rspec/rails/matchers/have_enqueued_mail.rb
6 changes: 4 additions & 2 deletions features/matchers/have_enqueued_mail_matcher.feature
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ Feature: have_enqueued_mail matcher
When I run `rspec spec/mailers/my_mailer_spec.rb`
Then the examples should all pass

@rails_post_6
Scenario: Parameterize the mailer
Given a file named "app/mailers/my_mailer.rb" with:
"""ruby
Expand All @@ -90,13 +91,14 @@ Feature: have_enqueued_mail matcher
# Works with named parameters
expect {
MyMailer.with(foo: 'bar').signup.deliver_later
}.to have_enqueued_mail(MyMailer, :signup).with(foo: 'bar')
}.to have_enqueued_mail(MyMailer, :signup).with(a_hash_including(params: {foo: 'bar'}))
end
end
"""
When I run `rspec spec/mailers/my_mailer_spec.rb`
Then the examples should all pass

@rails_post_6
Scenario: Parameterize and pass an argument to the mailer
Given a file named "app/mailers/my_mailer.rb" with:
"""ruby
Expand All @@ -120,7 +122,7 @@ Feature: have_enqueued_mail matcher
# Works also with both, named parameters match first argument
expect {
MyMailer.with(foo: 'bar').signup('user').deliver_later
}.to have_enqueued_mail(MyMailer, :signup).with({foo: 'bar'}, 'user')
}.to have_enqueued_mail(MyMailer, :signup).with(params: {foo: 'bar'}, args: ['user'])
end
end
"""
Expand Down
4 changes: 0 additions & 4 deletions lib/rspec/rails/feature_check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ def has_action_mailbox?
defined?(::ActionMailbox)
end

def ruby_3_1?
RUBY_VERSION >= "3.1"
end

def type_metatag(type)
"type: :#{type}"
end
Expand Down
67 changes: 31 additions & 36 deletions lib/rspec/rails/matchers/have_enqueued_mail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
require "rspec/mocks/argument_matchers"
require "rspec/rails/matchers/active_job"

# rubocop: disable Metrics/ClassLength
module RSpec
module Rails
module Matchers
Expand Down Expand Up @@ -76,7 +77,7 @@ def job_match?(job)
def arguments_match?(job)
@args =
if @mail_args.any?
base_mailer_args + process_arguments(job, @mail_args)
base_mailer_args + @mail_args
elsif @mailer_class && @method_name
base_mailer_args + [any_args]
elsif @mailer_class
Expand All @@ -88,38 +89,12 @@ def arguments_match?(job)
super(job)
end

def process_arguments(job, given_mail_args)
# Old matcher behavior working with all builtin classes but ActionMailer::MailDeliveryJob
return given_mail_args if use_given_mail_args?(job)

# If matching args starts with a hash and job instance has params match with them
if given_mail_args.first.is_a?(Hash) && job[:args][3]['params'].present?
[hash_including(params: given_mail_args[0], args: given_mail_args.drop(1))]
else
[hash_including(args: given_mail_args)]
end
end

def use_given_mail_args?(job)
return true if FeatureCheck.has_action_mailer_parameterized? && job[:job] <= ActionMailer::Parameterized::DeliveryJob
return false if FeatureCheck.ruby_3_1?

!(FeatureCheck.has_action_mailer_unified_delivery? && job[:job] <= ActionMailer::MailDeliveryJob)
end

def base_mailer_args
[mailer_class_name, @method_name.to_s, MAILER_JOB_METHOD]
end

def yield_mail_args(block)
proc do |*job_args|
mailer_args = job_args - base_mailer_args
if mailer_args.first.is_a?(Hash)
block.call(*mailer_args.first[:args])
else
block.call(*mailer_args)
end
end
proc { |*job_args| block.call(*(job_args - base_mailer_args)) }
end

def check_active_job_adapter
Expand All @@ -145,22 +120,41 @@ def unmatching_mail_jobs_message
end

def mail_job_message(job)
mailer_method = job[:args][0..1].join('.')
mailer_args = deserialize_arguments(job)[3..-1]
mailer_args = mailer_args.first[:args] if unified_mail?(job)
job_args = deserialize_arguments(job)

mailer_method = job_args[0..1].join('.')
mailer_args = job_args[3..-1]

msg_parts = []
display_args = display_mailer_args(mailer_args)
msg_parts << "with #{display_args}" if display_args.any?
msg_parts << "with #{mailer_args}" if mailer_args.any?
msg_parts << "on queue #{job[:queue]}" if job[:queue] && job[:queue] != 'mailers'
msg_parts << "at #{Time.at(job[:at])}" if job[:at]

"#{mailer_method} #{msg_parts.join(', ')}".strip
end

def display_mailer_args(mailer_args)
return mailer_args unless mailer_args.first.is_a?(Hash) && mailer_args.first.key?(:args)
# Ruby 3.1 changed how params were serialized on Rails 6.1
# so we override the active job implementation and customise it here.
def deserialize_arguments(job)
args = super

return args unless Hash === args.last

hash = args.pop

mailer_args.first[:args]
if hash.key?("_aj_ruby2_keywords")
keywords = hash["_aj_ruby2_keywords"]

original_hash = keywords.each_with_object({}) { |keyword, new_hash| new_hash[keyword.to_sym] = hash[keyword] }

args + [original_hash]
elsif hash.key?(:args) && hash.key?(:params)
args + [hash]
elsif hash.key?(:args)
args + hash[:args]
else
args + [hash]
end
end

def legacy_mail?(job)
Expand Down Expand Up @@ -230,3 +224,4 @@ def have_enqueued_mail(mailer_class = nil, mail_method_name = nil)
end
end
end
# rubocop: enable Metrics/ClassLength
24 changes: 19 additions & 5 deletions spec/rspec/rails/matchers/have_enqueued_mail_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
require "action_mailer"
require "rspec/rails/matchers/have_enqueued_mail"

class GlobalIDArgument
include GlobalID::Identification
def id; 1; end
def to_global_id(options = {}); super(options.merge(app: 'rspec-rails')); end
end

class TestMailer < ActionMailer::Base
def test_email; end
def email_with_args(arg1, arg2); end
Expand Down Expand Up @@ -404,16 +410,24 @@ def self.name; "NonMailerJob"; end
}.to have_enqueued_mail(UnifiedMailer, :email_with_args).with(1, 2)
end

it "matches arguments when mailer is parameterized" do
it "passes with provided argument matchers" do
expect {
UnifiedMailer.with('foo' => 'bar').test_email.deliver_later
}.to have_enqueued_mail(UnifiedMailer, :test_email).with('foo' => 'bar')
end
}.to have_enqueued_mail(UnifiedMailer, :test_email).with(
a_hash_including(params: {'foo' => 'bar'})
)

it "matches arguments when mixing parameterized and non-parameterized emails" do
expect {
UnifiedMailer.with('foo' => 'bar').email_with_args(1, 2).deliver_later
}.to have_enqueued_mail(UnifiedMailer, :email_with_args).with({'foo' => 'bar'}, 1, 2)
}.to have_enqueued_mail(UnifiedMailer, :email_with_args).with(
a_hash_including(params: {'foo' => 'bar'}, args: [1, 2])
)
end

it "passes when given a global id serialised argument" do
expect {
UnifiedMailer.with(inquiry: GlobalIDArgument.new).test_email.deliver_later
}.to have_enqueued_email(UnifiedMailer, :test_email)
end

it "passes when using a mailer with `delivery_job` set to a sub class of `ActionMailer::DeliveryJob`" do
Expand Down