diff --git a/activejob/lib/active_job/arguments.rb b/activejob/lib/active_job/arguments.rb index 175a2f0956a..099ea4d169a 100644 --- a/activejob/lib/active_job/arguments.rb +++ b/activejob/lib/active_job/arguments.rb @@ -37,14 +37,16 @@ module ActiveJob private def serialize_argument(argument) case argument - when GlobalID::Identification - argument.to_global_id.to_s when *TYPE_WHITELIST argument + when GlobalID::Identification + argument.to_global_id.to_s when Array argument.map { |arg| serialize_argument(arg) } when Hash - Hash[ argument.map { |key, value| [ serialize_hash_key(key), serialize_argument(value) ] } ] + argument.each_with_object({}) do |(key, value), hash| + hash[serialize_hash_key(key)] = serialize_argument(value) + end else raise SerializationError.new("Unsupported argument type: #{argument.class.name}") end @@ -52,14 +54,18 @@ module ActiveJob def deserialize_argument(argument) case argument + when String + GlobalID::Locator.locate(argument) || argument + when *TYPE_WHITELIST + argument when Array argument.map { |arg| deserialize_argument(arg) } when Hash - Hash[ argument.map { |key, value| [ key, deserialize_argument(value) ] } ].with_indifferent_access - when String, GlobalID - GlobalID::Locator.locate(argument) || argument + argument.each_with_object({}.with_indifferent_access) do |(key, value), hash| + hash[key] = deserialize_argument(value) + end else - argument + raise ArgumentError, "Can only deserialize primitive arguments: #{argument.inspect}" end end @@ -68,7 +74,7 @@ module ActiveJob when String, Symbol key.to_s else - raise SerializationError.new("Unsupported hash key type: #{key.class.name}") + raise SerializationError.new("Only string and symbol hash keys may be serialized as job arguments, but #{key.inspect} is a #{key.class}") end end end diff --git a/activejob/test/cases/argument_serialization_test.rb b/activejob/test/cases/argument_serialization_test.rb new file mode 100644 index 00000000000..5a46c5cdef1 --- /dev/null +++ b/activejob/test/cases/argument_serialization_test.rb @@ -0,0 +1,76 @@ +require 'helper' +require 'active_job/arguments' +require 'models/person' +require 'active_support/core_ext/hash/indifferent_access' + +class ArgumentSerializationTest < ActiveSupport::TestCase + setup do + @person = Person.find('5') + end + + [ nil, 1, 1.0, 1_000_000_000_000_000_000_000, + 'a', true, false, + [ 1, 'a' ], + { 'a' => 1 } + ].each do |arg| + test "serializes #{arg.class} verbatim" do + assert_arguments_unchanged arg + end + end + + [ :a, Object.new, self, Person.find('5').to_gid ].each do |arg| + test "does not serialize #{arg.class}" do + assert_raises ActiveJob::SerializationError do + ActiveJob::Arguments.serialize [ arg ] + end + + assert_raises ActiveJob::DeserializationError do + ActiveJob::Arguments.deserialize [ arg ] + end + end + end + + test 'should convert records to Global IDs' do + assert_arguments_roundtrip [@person], [@person.to_gid.to_s] + end + + test 'should dive deep into arrays and hashes' do + assert_arguments_roundtrip [3, [@person]], [3, [@person.to_gid.to_s]] + assert_arguments_roundtrip [{ 'a' => @person }], [{ 'a' => @person.to_gid.to_s }.with_indifferent_access] + end + + test 'should stringify symbol hash keys' do + assert_equal [ 'a' => 1 ], ActiveJob::Arguments.serialize([ a: 1 ]) + end + + test 'should disallow non-string/symbol hash keys' do + assert_raises ActiveJob::SerializationError do + ActiveJob::Arguments.serialize [ { 1 => 2 } ] + end + + assert_raises ActiveJob::SerializationError do + ActiveJob::Arguments.serialize [ { :a => [{ 2 => 3 }] } ] + end + end + + test 'should not allow non-primitive objects' do + assert_raises ActiveJob::SerializationError do + ActiveJob::Arguments.serialize [Object.new] + end + + assert_raises ActiveJob::SerializationError do + ActiveJob::Arguments.serialize [1, [Object.new]] + end + end + + private + def assert_arguments_unchanged(*args) + assert_arguments_roundtrip args, args + end + + def assert_arguments_roundtrip(args, expected_serialized_args) + serialized = ActiveJob::Arguments.serialize(args) + assert_equal expected_serialized_args, serialized + assert_equal args, ActiveJob::Arguments.deserialize(serialized) + end +end diff --git a/activejob/test/cases/parameters_test.rb b/activejob/test/cases/parameters_test.rb deleted file mode 100644 index 92f835af5d0..00000000000 --- a/activejob/test/cases/parameters_test.rb +++ /dev/null @@ -1,77 +0,0 @@ -require 'helper' -require 'active_job/arguments' -require 'models/person' -require 'active_support/core_ext/hash/indifferent_access' - -class ParameterSerializationTest < ActiveSupport::TestCase - test 'should make no change to regular values' do - assert_equal [ 1, "something" ], ActiveJob::Arguments.serialize([ 1, "something" ]) - end - - test 'should not allow complex objects' do - assert_equal [ nil ], ActiveJob::Arguments.serialize([ nil ]) - assert_equal [ 1 ], ActiveJob::Arguments.serialize([ 1 ]) - assert_equal [ 1.0 ], ActiveJob::Arguments.serialize([ 1.0 ]) - assert_equal [ 'a' ], ActiveJob::Arguments.serialize([ 'a' ]) - assert_equal [ true ], ActiveJob::Arguments.serialize([ true ]) - assert_equal [ false ], ActiveJob::Arguments.serialize([ false ]) - assert_equal [ { "a" => 1, "b" => 2 } ], ActiveJob::Arguments.serialize([ { a: 1, "b" => 2 } ]) - assert_equal [ [ 1 ] ], ActiveJob::Arguments.serialize([ [ 1 ] ]) - assert_equal [ 1_000_000_000_000_000_000_000 ], ActiveJob::Arguments.serialize([ 1_000_000_000_000_000_000_000 ]) - - err = assert_raises ActiveJob::SerializationError do - ActiveJob::Arguments.serialize([ 1, self ]) - end - assert_equal "Unsupported argument type: #{self.class.name}", err.message - end - - test 'should dive deep into arrays or hashes' do - assert_equal [ { "a" => Person.find(5).to_gid.to_s }.with_indifferent_access ], ActiveJob::Arguments.serialize([ { a: Person.find(5) } ]) - assert_equal [ [ Person.find(5).to_gid.to_s ] ], ActiveJob::Arguments.serialize([ [ Person.find(5) ] ]) - end - - test 'should dive deep into arrays or hashes and raise exception on complex objects' do - err = assert_raises ActiveJob::SerializationError do - ActiveJob::Arguments.serialize([ 1, [self] ]) - end - assert_equal "Unsupported argument type: #{self.class.name}", err.message - end - - test 'shoud dive deep into hashes and allow raise exception on not string/symbol keys' do - err = assert_raises ActiveJob::SerializationError do - ActiveJob::Arguments.serialize([ [ { 1 => 2 } ] ]) - end - assert_equal "Unsupported hash key type: Fixnum", err.message - end - - test 'should serialize records with global id' do - assert_equal [ Person.find(5).to_gid.to_s ], ActiveJob::Arguments.serialize([ Person.find(5) ]) - end - - test 'should serialize values and records together' do - assert_equal [ 3, Person.find(5).to_gid.to_s ], ActiveJob::Arguments.serialize([ 3, Person.find(5) ]) - end -end - -class ParameterDeserializationTest < ActiveSupport::TestCase - test 'should make no change to regular values' do - assert_equal [ 1, "something" ], ActiveJob::Arguments.deserialize([ 1, "something" ]) - end - - test 'should deserialize records with global id' do - assert_equal [ Person.find(5) ], ActiveJob::Arguments.deserialize([ Person.find(5).to_gid ]) - end - - test 'should serialize values and records together' do - assert_equal [ 3, Person.find(5) ], ActiveJob::Arguments.deserialize([ 3, Person.find(5).to_gid ]) - end - - test 'should dive deep when deserialising arrays' do - assert_equal [ [ 3, Person.find(5) ] ], ActiveJob::Arguments.deserialize([ [ 3, Person.find(5).to_gid ] ]) - end - - test 'should dive deep when deserialising hashes' do - assert_equal [ { "5" => Person.find(5) } ], ActiveJob::Arguments.deserialize([ { "5" => Person.find(5).to_gid } ]) - end - -end