diff options
Diffstat (limited to 'activejob')
| -rw-r--r-- | activejob/CHANGELOG.md | 6 | ||||
| -rw-r--r-- | activejob/lib/active_job/arguments.rb | 49 | ||||
| -rw-r--r-- | activejob/lib/active_job/test_helper.rb | 14 | ||||
| -rw-r--r-- | activejob/test/cases/argument_serialization_test.rb | 39 | ||||
| -rw-r--r-- | activejob/test/cases/test_helper_test.rb | 39 | ||||
| -rw-r--r-- | activejob/test/jobs/kwargs_job.rb | 7 |
6 files changed, 133 insertions, 21 deletions
diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index 794d05d1b4..09c8f0800d 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,3 +1,9 @@ +* Allow keyword arguments to be used with Active Job. + + Fixes #18741. + + *Sean Griffin* + * Add `:only` option to `assert_enqueued_jobs`, to check the number of times a specific kind of job is enqueued. diff --git a/activejob/lib/active_job/arguments.rb b/activejob/lib/active_job/arguments.rb index 752be6898e..622c37098e 100644 --- a/activejob/lib/active_job/arguments.rb +++ b/activejob/lib/active_job/arguments.rb @@ -1,4 +1,4 @@ -require 'active_support/core_ext/hash/indifferent_access' +require 'active_support/core_ext/hash' module ActiveJob # Raised when an exception is raised during job arguments deserialization. @@ -44,7 +44,9 @@ module ActiveJob private GLOBALID_KEY = '_aj_globalid'.freeze - private_constant :GLOBALID_KEY + SYMBOL_KEYS_KEY = '_aj_symbol_keys'.freeze + WITH_INDIFFERENT_ACCESS_KEY = '_aj_hash_with_indifferent_access'.freeze + private_constant :GLOBALID_KEY, :SYMBOL_KEYS_KEY, :WITH_INDIFFERENT_ACCESS_KEY def serialize_argument(argument) case argument @@ -54,10 +56,15 @@ module ActiveJob { GLOBALID_KEY => argument.to_global_id.to_s } when Array argument.map { |arg| serialize_argument(arg) } + when ActiveSupport::HashWithIndifferentAccess + result = serialize_hash(argument) + result[WITH_INDIFFERENT_ACCESS_KEY] = serialize_argument(true) + result when Hash - argument.each_with_object({}) do |(key, value), hash| - hash[serialize_hash_key(key)] = serialize_argument(value) - end + symbol_keys = argument.each_key.grep(Symbol).map(&:to_s) + result = serialize_hash(argument) + result[SYMBOL_KEYS_KEY] = symbol_keys + result else raise SerializationError.new("Unsupported argument type: #{argument.class.name}") end @@ -75,7 +82,7 @@ module ActiveJob if serialized_global_id?(argument) deserialize_global_id argument else - deserialize_hash argument + deserialize_hash(argument) end else raise ArgumentError, "Can only deserialize primitive arguments: #{argument.inspect}" @@ -90,13 +97,27 @@ module ActiveJob GlobalID::Locator.locate hash[GLOBALID_KEY] end + def serialize_hash(argument) + argument.each_with_object({}) do |(key, value), hash| + hash[serialize_hash_key(key)] = serialize_argument(value) + end + end + def deserialize_hash(serialized_hash) - serialized_hash.each_with_object({}.with_indifferent_access) do |(key, value), hash| - hash[key] = deserialize_argument(value) + result = serialized_hash.transform_values { |v| deserialize_argument(v) } + if result.delete(WITH_INDIFFERENT_ACCESS_KEY) + result = result.with_indifferent_access + elsif symbol_keys = result.delete(SYMBOL_KEYS_KEY) + result = transform_symbol_keys(result, symbol_keys) end + result end - RESERVED_KEYS = [GLOBALID_KEY, GLOBALID_KEY.to_sym] + RESERVED_KEYS = [ + GLOBALID_KEY, GLOBALID_KEY.to_sym, + SYMBOL_KEYS_KEY, SYMBOL_KEYS_KEY.to_sym, + WITH_INDIFFERENT_ACCESS_KEY, WITH_INDIFFERENT_ACCESS_KEY.to_sym, + ] private_constant :RESERVED_KEYS def serialize_hash_key(key) @@ -109,5 +130,15 @@ module ActiveJob raise SerializationError.new("Only string and symbol hash keys may be serialized as job arguments, but #{key.inspect} is a #{key.class}") end end + + def transform_symbol_keys(hash, symbol_keys) + hash.transform_keys do |key| + if symbol_keys.include?(key) + key.to_sym + else + key + end + end + end end end diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index d18656e398..c544e8a10f 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -175,9 +175,10 @@ module ActiveJob original_enqueued_jobs = enqueued_jobs.dup clear_enqueued_jobs args.assert_valid_keys(:job, :args, :at, :queue) + serialized_args = serialize_args_for_assertion(args) yield matching_job = enqueued_jobs.any? do |job| - args.all? { |key, value| value == job[key] } + serialized_args.all? { |key, value| value == job[key] } end assert matching_job, "No enqueued job found with #{args}" ensure @@ -195,9 +196,10 @@ module ActiveJob original_performed_jobs = performed_jobs.dup clear_performed_jobs args.assert_valid_keys(:job, :args, :at, :queue) + serialized_args = serialize_args_for_assertion(args) perform_enqueued_jobs { yield } matching_job = performed_jobs.any? do |job| - args.all? { |key, value| value == job[key] } + serialized_args.all? { |key, value| value == job[key] } end assert matching_job, "No performed job found with #{args}" ensure @@ -239,6 +241,14 @@ module ActiveJob enqueued_jobs.size end end + + def serialize_args_for_assertion(args) + serialized_args = args.dup + if job_args = serialized_args.delete(:args) + serialized_args[:args] = ActiveJob::Arguments.serialize(job_args) + end + serialized_args + end end end end diff --git a/activejob/test/cases/argument_serialization_test.rb b/activejob/test/cases/argument_serialization_test.rb index dbe36fc572..8b9b62190f 100644 --- a/activejob/test/cases/argument_serialization_test.rb +++ b/activejob/test/cases/argument_serialization_test.rb @@ -2,6 +2,7 @@ require 'helper' require 'active_job/arguments' require 'models/person' require 'active_support/core_ext/hash/indifferent_access' +require 'jobs/kwargs_job' class ArgumentSerializationTest < ActiveSupport::TestCase setup do @@ -31,16 +32,26 @@ class ArgumentSerializationTest < ActiveSupport::TestCase end test 'should convert records to Global IDs' do - assert_arguments_roundtrip [@person], ['_aj_globalid' => @person.to_gid.to_s] + assert_arguments_roundtrip [@person] end test 'should dive deep into arrays and hashes' do - assert_arguments_roundtrip [3, [@person]], [3, ['_aj_globalid' => @person.to_gid.to_s]] - assert_arguments_roundtrip [{ 'a' => @person }], [{ 'a' => { '_aj_globalid' => @person.to_gid.to_s }}.with_indifferent_access] + assert_arguments_roundtrip [3, [@person]] + assert_arguments_roundtrip [{ 'a' => @person }] end - test 'should stringify symbol hash keys' do - assert_equal [ 'a' => 1 ], ActiveJob::Arguments.serialize([ a: 1 ]) + test 'should maintain string and symbol keys' do + assert_arguments_roundtrip([a: 1, "b" => 2]) + end + + test 'should maintain hash with indifferent access' do + symbol_key = { a: 1 } + string_key = { 'a' => 1 } + indifferent_access = { a: 1 }.with_indifferent_access + + assert_not_instance_of ActiveSupport::HashWithIndifferentAccess, perform_round_trip([symbol_key]).first + assert_not_instance_of ActiveSupport::HashWithIndifferentAccess, perform_round_trip([string_key]).first + assert_instance_of ActiveSupport::HashWithIndifferentAccess, perform_round_trip([indifferent_access]).first end test 'should disallow non-string/symbol hash keys' do @@ -71,14 +82,22 @@ class ArgumentSerializationTest < ActiveSupport::TestCase end end + test 'allows for keyword arguments' do + KwargsJob.perform_later(argument: 2) + + assert_equal "Job with argument: 2", JobBuffer.last_value + end + private def assert_arguments_unchanged(*args) - assert_arguments_roundtrip args, args + assert_arguments_roundtrip args + end + + def assert_arguments_roundtrip(args) + assert_equal args, perform_round_trip(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) + def perform_round_trip(args) + ActiveJob::Arguments.deserialize(ActiveJob::Arguments.serialize(args)) end end diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index f34638e7d8..0a23ae33c4 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -4,6 +4,7 @@ require 'active_support/core_ext/date' require 'jobs/hello_job' require 'jobs/logging_job' require 'jobs/nested_job' +require 'models/person' class EnqueuedJobsTest < ActiveJob::TestCase def test_assert_enqueued_jobs @@ -175,6 +176,25 @@ class EnqueuedJobsTest < ActiveJob::TestCase end end end + + def test_assert_enqueued_job_with_global_id_args + ricardo = Person.new(9) + assert_enqueued_with(job: HelloJob, args: [ricardo]) do + HelloJob.perform_later(ricardo) + end + end + + def test_assert_enqueued_job_failure_with_global_id_args + ricardo = Person.new(9) + wilma = Person.new(11) + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_enqueued_with(job: HelloJob, args: [wilma]) do + HelloJob.perform_later(ricardo) + end + end + + assert_equal "No enqueued job found with {:job=>HelloJob, :args=>[#{wilma.inspect}]}", error.message + end end class PerformedJobsTest < ActiveJob::TestCase @@ -282,4 +302,23 @@ class PerformedJobsTest < ActiveJob::TestCase end end end + + def test_assert_performed_job_with_global_id_args + ricardo = Person.new(9) + assert_performed_with(job: HelloJob, args: [ricardo]) do + HelloJob.perform_later(ricardo) + end + end + + def test_assert_performed_job_failure_with_global_id_args + ricardo = Person.new(9) + wilma = Person.new(11) + error = assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_with(job: HelloJob, args: [wilma]) do + HelloJob.perform_later(ricardo) + end + end + + assert_equal "No performed job found with {:job=>HelloJob, :args=>[#{wilma.inspect}]}", error.message + end end diff --git a/activejob/test/jobs/kwargs_job.rb b/activejob/test/jobs/kwargs_job.rb new file mode 100644 index 0000000000..2df17d15ae --- /dev/null +++ b/activejob/test/jobs/kwargs_job.rb @@ -0,0 +1,7 @@ +require_relative '../support/job_buffer' + +class KwargsJob < ActiveJob::Base + def perform(argument: 1) + JobBuffer.add("Job with argument: #{argument}") + end +end |