Commit 55cb4bc9 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'pl-serialize-ac-parameters' into 'master'

Make `ActionContorller::Parameters` serializable for sidekiq jobs

See merge request gitlab-org/gitlab-ce!24864
parents 97041bb6 8dd3dc18
...@@ -23,7 +23,7 @@ module MailScheduler ...@@ -23,7 +23,7 @@ module MailScheduler
end end
def self.perform_async(*args) def self.perform_async(*args)
super(*ActiveJob::Arguments.serialize(args)) super(*Arguments.serialize(args))
end end
private private
...@@ -38,5 +38,34 @@ module MailScheduler ...@@ -38,5 +38,34 @@ module MailScheduler
end end
end 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
end end
---
title: Make `ActionController::Parameters` serializable for sidekiq jobs
merge_request: 24864
author:
type: fixed
...@@ -9,6 +9,10 @@ describe MailScheduler::NotificationServiceWorker do ...@@ -9,6 +9,10 @@ describe MailScheduler::NotificationServiceWorker do
ActiveJob::Arguments.serialize(args) ActiveJob::Arguments.serialize(args)
end end
def deserialize(args)
ActiveJob::Arguments.deserialize(args)
end
describe '#perform' do describe '#perform' do
it 'deserializes arguments from global IDs' do it 'deserializes arguments from global IDs' do
expect(worker.notification_service).to receive(method).with(key) expect(worker.notification_service).to receive(method).with(key)
...@@ -42,14 +46,49 @@ describe MailScheduler::NotificationServiceWorker do ...@@ -42,14 +46,49 @@ describe MailScheduler::NotificationServiceWorker do
end end
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 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.count).to eq(1)
expect(described_class.jobs.first).to include('args' => [method, *serialize(key)]) expect(described_class.jobs.first).to include('args' => [method, *serialize(key)])
end 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(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
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment