diff options
| -rw-r--r-- | Gemfile | 3 | ||||
| -rw-r--r-- | actionview/lib/action_view/buffers.rb | 4 | ||||
| -rw-r--r-- | activejob/Rakefile | 90 | ||||
| -rw-r--r-- | activejob/lib/active_job/arguments.rb | 22 | ||||
| -rw-r--r-- | activejob/test/cases/argument_serialization_test.rb | 76 | ||||
| -rw-r--r-- | activejob/test/cases/parameters_test.rb | 77 |
6 files changed, 124 insertions, 148 deletions
@@ -2,6 +2,9 @@ source 'https://rubygems.org' gemspec +# We need a newish Rake since Active Job sets its test tasks' descriptions. +gem 'rake', '>= 10.3' + # This needs to be with require false as it is # loaded after loading the test library to # ensure correct loading order diff --git a/actionview/lib/action_view/buffers.rb b/actionview/lib/action_view/buffers.rb index 361a0dccbe..50bc315700 100644 --- a/actionview/lib/action_view/buffers.rb +++ b/actionview/lib/action_view/buffers.rb @@ -13,10 +13,6 @@ module ActionView end alias :append= :<< - def safe_concat(value) - return self if value.nil? - super(value.to_s) - end alias :safe_append= :safe_concat end diff --git a/activejob/Rakefile b/activejob/Rakefile index e918428459..dadd0baf82 100644 --- a/activejob/Rakefile +++ b/activejob/Rakefile @@ -1,59 +1,45 @@ require 'rake/testtask' require 'rubygems/package_task' -dir = File.dirname(__FILE__) - -def run_without_aborting(*tasks) - errors = [] - - tasks.each do |task| - begin - Rake::Task[task].invoke - rescue Exception - errors << task - end - end - - abort "Errors running #{errors.join(', ')}" if errors.any? -end - -task default: :test - ACTIVEJOB_ADAPTERS = %w(inline delayed_job qu que queue_classic resque sidekiq sneakers sucker_punch backburner) ACTIVEJOB_ADAPTERS -= %w(queue_classic) if defined?(JRUBY_VERSION) -desc 'Run all adapter tests' -task :test do - tasks = ACTIVEJOB_ADAPTERS.map{|a| "test_#{a}" } - run_without_aborting(*tasks) -end +task default: :test +task test: 'test:default' namespace :test do + desc 'Run all adapter tests' + task :default do + run_without_aborting ACTIVEJOB_ADAPTERS.map { |a| "test:#{a}" } + end + desc 'Run all adapter tests in isolation' task :isolated do - tasks = ACTIVEJOB_ADAPTERS.map{|a| "isolated_test_#{a}" } - run_without_aborting(*tasks) + run_without_aborting ACTIVEJOB_ADAPTERS.map { |a| "test:isolated:#{a}" } end - desc 'Run all adapter integration tests' + desc 'Run integration tests for all adapters' task :integration do - tasks = ACTIVEJOB_ADAPTERS.map{|a| "integration_test_#{a}" } - run_without_aborting(*tasks) + run_without_aborting ACTIVEJOB_ADAPTERS.map { |a| "test:integration:#{a}" } + end + + task 'env:integration' do + ENV['AJ_INTEGRATION_TESTS'] = "1" end -end + ACTIVEJOB_ADAPTERS.each do |adapter| + task("env:#{adapter}") { ENV['AJADAPTER'] = adapter } -ACTIVEJOB_ADAPTERS.each do |adapter| - namespace :test do - Rake::TestTask.new(adapter => "#{adapter}:env") do |t| - t.description = "" + Rake::TestTask.new(adapter => "test:env:#{adapter}") do |t| + t.description = "Run adapter tests for #{adapter}" t.libs << 'test' t.test_files = FileList['test/cases/**/*_test.rb'] t.verbose = true end namespace :isolated do - task adapter => "#{adapter}:env" do + task adapter => "test:env:#{adapter}" do + dir = File.dirname(__FILE__) Dir.glob("#{dir}/test/cases/**/*_test.rb").all? do |file| sh(Gem.ruby, '-w', "-I#{dir}/lib", "-I#{dir}/test", file) end or raise 'Failures' @@ -61,42 +47,28 @@ ACTIVEJOB_ADAPTERS.each do |adapter| end namespace :integration do - Rake::TestTask.new(adapter => "#{adapter}:env") do |t| - t.description = "" + Rake::TestTask.new(adapter => ["test:env:#{adapter}", 'test:env:integration']) do |t| + t.description = "Run integration tests for #{adapter}" t.libs << 'test' t.test_files = FileList['test/integration/**/*_test.rb'] t.verbose = true end end end +end - namespace adapter do - task test: "test_#{adapter}" - task isolated_test: "isolated_test_#{adapter}" - - task(:env) { ENV['AJADAPTER'] = adapter } - - namespace :isolated do - task(:env) { ENV['AJADAPTER'] = adapter } - end +def run_without_aborting(tasks) + errors = [] - namespace :integration do - task(:env) do - ENV['AJADAPTER'] = adapter - ENV['AJ_INTEGRATION_TESTS'] = "1" - end + tasks.each do |task| + begin + Rake::Task[task].invoke + rescue Exception + errors << task end end - - desc "Run #{adapter} tests" - task "test_#{adapter}" => ["#{adapter}:env", "test:#{adapter}"] - - desc "Run #{adapter} tests in isolation" - task "isolated_test_#{adapter}" => ["#{adapter}:isolated:env", "test:isolated:#{adapter}"] - - desc "Run #{adapter} integration tests" - task "integration_test_#{adapter}" => ["#{adapter}:integration:env", "test:integration:#{adapter}"] + abort "Errors running #{errors.join(', ')}" if errors.any? end diff --git a/activejob/lib/active_job/arguments.rb b/activejob/lib/active_job/arguments.rb index 175a2f0956..099ea4d169 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 0000000000..5a46c5cdef --- /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 92f835af5d..0000000000 --- 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 |