From 4ded7a46bbc8e5a06d4bdfcd3914f50eeb15ac22 Mon Sep 17 00:00:00 2001 From: Jon Rowe Date: Tue, 25 Jan 2022 09:40:18 +0000 Subject: [PATCH 1/3] Fix mailer argument deserialisation for 6.1 on Ruby 3.1 --- .rubocop_todo.yml | 4 -- lib/rspec/rails/feature_check.rb | 4 -- .../rails/matchers/have_enqueued_mail.rb | 67 +++++++++---------- .../rails/matchers/have_enqueued_mail_spec.rb | 12 ++-- 4 files changed, 38 insertions(+), 49 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 19abcfbf04..8221da2914 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -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 diff --git a/lib/rspec/rails/feature_check.rb b/lib/rspec/rails/feature_check.rb index a866acc79b..4a4d855d75 100644 --- a/lib/rspec/rails/feature_check.rb +++ b/lib/rspec/rails/feature_check.rb @@ -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 diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index 86aae6c5cc..918c618ec8 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -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 @@ -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 @@ -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 @@ -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({}) { |new_hash, keyword| 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) @@ -230,3 +224,4 @@ def have_enqueued_mail(mailer_class = nil, mail_method_name = nil) end end end +# rubocop: enable Metrics/ClassLength diff --git a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb index 9413be1aa0..3cdc80696d 100644 --- a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb +++ b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb @@ -404,16 +404,18 @@ 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 using a mailer with `delivery_job` set to a sub class of `ActionMailer::DeliveryJob`" do From 6fd11abcb832662371c94fd64ecae933fab409b8 Mon Sep 17 00:00:00 2001 From: Jon Rowe Date: Mon, 7 Mar 2022 10:40:31 +0000 Subject: [PATCH 2/3] Merge pull request #2578 from rspec/have-enqueued-mail-fix Fix have_enqueued_mail with global id arguments --- lib/rspec/rails/matchers/have_enqueued_mail.rb | 2 +- spec/rspec/rails/matchers/have_enqueued_mail_spec.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/rspec/rails/matchers/have_enqueued_mail.rb b/lib/rspec/rails/matchers/have_enqueued_mail.rb index 918c618ec8..1d2b16a203 100644 --- a/lib/rspec/rails/matchers/have_enqueued_mail.rb +++ b/lib/rspec/rails/matchers/have_enqueued_mail.rb @@ -145,7 +145,7 @@ def deserialize_arguments(job) if hash.key?("_aj_ruby2_keywords") keywords = hash["_aj_ruby2_keywords"] - original_hash = keywords.each_with_object({}) { |new_hash, keyword| new_hash[keyword.to_sym] = hash[keyword] } + 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) diff --git a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb index 3cdc80696d..13bc1d1faf 100644 --- a/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb +++ b/spec/rspec/rails/matchers/have_enqueued_mail_spec.rb @@ -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 @@ -418,6 +424,12 @@ def self.name; "NonMailerJob"; end ) 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 expect { UnifiedMailerWithDeliveryJobSubClass.test_email.deliver_later From b22a08b28d3a3f27730a97388bcffed881d68d6a Mon Sep 17 00:00:00 2001 From: Jon Rowe Date: Mon, 7 Mar 2022 17:15:54 +0000 Subject: [PATCH 3/3] Fix cucumber scenarios --- features/matchers/have_enqueued_mail_matcher.feature | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/features/matchers/have_enqueued_mail_matcher.feature b/features/matchers/have_enqueued_mail_matcher.feature index 8def19cf3c..dda6b46f9d 100644 --- a/features/matchers/have_enqueued_mail_matcher.feature +++ b/features/matchers/have_enqueued_mail_matcher.feature @@ -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 @@ -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 @@ -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 """