From 8dd3dc182e60224931034a692ec1be06a7d6e3df Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Tue, 5 Feb 2019 10:39:14 +0000 Subject: Make `ActionContorller::Parameters` serializable for sidekiq jobs --- .../mail_scheduler/notification_service_worker.rb | 31 +++++++++++++- .../unreleased/pl-serialize-ac-parameters.yml | 5 +++ .../notification_service_worker_spec.rb | 49 +++++++++++++++++++--- 3 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 changelogs/unreleased/pl-serialize-ac-parameters.yml diff --git a/app/workers/mail_scheduler/notification_service_worker.rb b/app/workers/mail_scheduler/notification_service_worker.rb index c8ccaf0c487..421fbf04e28 100644 --- a/app/workers/mail_scheduler/notification_service_worker.rb +++ b/app/workers/mail_scheduler/notification_service_worker.rb @@ -23,7 +23,7 @@ module MailScheduler end def self.perform_async(*args) - super(*ActiveJob::Arguments.serialize(args)) + super(*Arguments.serialize(args)) end private @@ -38,5 +38,34 @@ module MailScheduler end end end + + # Permit ActionController::Parameters for serializable Hash + # + # Port of + # https://github.com/rails/rails/commit/945fdd76925c9f615bf016717c4c8db2b2955357#diff-fc90ec41ef75be8b2259526fe1a8b663 + module Arguments + include ActiveJob::Arguments + extend self + + private + + def serialize_argument(argument) + case argument + when -> (arg) { arg.respond_to?(:permitted?) } + serialize_hash(argument.to_h).tap do |result| + result[WITH_INDIFFERENT_ACCESS_KEY] = serialize_argument(true) + end + else + super + end + end + end + + # Make sure we remove this patch starting with Rails 6.0. + if Rails.version.start_with?('6.0') + raise <<~MSG + Please remove the patch `Arguments` module and use `ActiveJob::Arguments` again. + MSG + end end end diff --git a/changelogs/unreleased/pl-serialize-ac-parameters.yml b/changelogs/unreleased/pl-serialize-ac-parameters.yml new file mode 100644 index 00000000000..aad222b5506 --- /dev/null +++ b/changelogs/unreleased/pl-serialize-ac-parameters.yml @@ -0,0 +1,5 @@ +--- +title: Make `ActionController::Parameters` serializable for sidekiq jobs +merge_request: 24864 +author: +type: fixed diff --git a/spec/workers/mail_scheduler/notification_service_worker_spec.rb b/spec/workers/mail_scheduler/notification_service_worker_spec.rb index 1033557ee88..5cfba01850c 100644 --- a/spec/workers/mail_scheduler/notification_service_worker_spec.rb +++ b/spec/workers/mail_scheduler/notification_service_worker_spec.rb @@ -9,6 +9,10 @@ describe MailScheduler::NotificationServiceWorker do ActiveJob::Arguments.serialize(args) end + def deserialize(args) + ActiveJob::Arguments.deserialize(args) + end + describe '#perform' do it 'deserializes arguments from global IDs' do expect(worker.notification_service).to receive(method).with(key) @@ -42,13 +46,48 @@ describe MailScheduler::NotificationServiceWorker do end end - describe '.perform_async' do + describe '.perform_async', :sidekiq do + around do |example| + Sidekiq::Testing.fake! { example.run } + end + it 'serializes arguments as global IDs when scheduling' do - Sidekiq::Testing.fake! do - described_class.perform_async(method, key) + described_class.perform_async(method, key) + + expect(described_class.jobs.count).to eq(1) + expect(described_class.jobs.first).to include('args' => [method, *serialize(key)]) + end + + context 'with ActiveController::Parameters' do + let(:parameters) { ActionController::Parameters.new(hash) } + + let(:hash) do + { + "nested" => { + "hash" => true + } + } + end + + context 'when permitted' do + before do + parameters.permit! + end + + it 'serializes as a serializable Hash' do + described_class.perform_async(method, parameters) - expect(described_class.jobs.count).to eq(1) - expect(described_class.jobs.first).to include('args' => [method, *serialize(key)]) + expect(described_class.jobs.count).to eq(1) + expect(deserialize(described_class.jobs.first['args'])) + .to eq([method, hash]) + end + end + + context 'when not permitted' do + it 'fails to serialize' do + expect { described_class.perform_async(method, parameters) } + .to raise_error(ActionController::UnfilteredParameters) + end end end end -- cgit v1.2.1