From b0f2f5e348a409d52f126c446951fa4397a56302 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Tue, 25 Sep 2018 00:54:05 +0300 Subject: Improve `enqueue_retry.active_job` message Since #33751 was added `enqueue_retry.active_job` instrumentation to the `retry_on` method, then #33897 moved the instrumentation to `retry_job` method in order to ensure that this method publish `enqueue_retry.active_job` notification too. See related discussion https://github.com/rails/rails/pull/33751#discussion_r214140008 Since `enqueue_retry.active_job` moved to `retry_job`, there is no guarantee that payload of `enqueue_retry.active_job` would have `:error`. See test `LoggingTest#test_enqueue_retry_logging_on_retry_job` as example of that case. Related to https://github.com/rails/rails/pull/33897#discussion_r219707024 I think we can improve notification of `enqueue_retry.active_job`: - If there is no `event.payload[:error]`, then publish like "Retrying RescueJob in 3 seconds." only. - If `event.payload[:wait]` is `nil`, then publish "Retrying RescueJob in 0 seconds." instead of "Retrying RescueJob in nil seconds." - If there is `event.payload[:error]`, then publish "Retrying RescueJob in 3 seconds, due to a DefaultsError.". - Change the type of the message from `error` to `info.` Also, this commit removes part of messages - "The original exception was #{ex.cause.inspect}." of `enqueue_retry.active_job`, `retry_stopped.active_job`, and `discard.active_job` since I haven't found it useful. Please let me know whether you agree with that? --- activejob/lib/active_job/logging.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/logging.rb b/activejob/lib/active_job/logging.rb index 0abee4ed98..416be83c24 100644 --- a/activejob/lib/active_job/logging.rb +++ b/activejob/lib/active_job/logging.rb @@ -93,8 +93,12 @@ module ActiveJob ex = event.payload[:error] wait = event.payload[:wait] - error do - "Retrying #{job.class} in #{wait.inspect} seconds, due to a #{ex&.class.inspect}. The original exception was #{ex&.cause.inspect}." + info do + if ex + "Retrying #{job.class} in #{wait.to_i} seconds, due to a #{ex.class}." + else + "Retrying #{job.class} in #{wait.to_i} seconds." + end end end @@ -103,7 +107,7 @@ module ActiveJob ex = event.payload[:error] error do - "Stopped retrying #{job.class} due to a #{ex.class}, which reoccurred on #{job.executions} attempts. The original exception was #{ex.cause.inspect}." + "Stopped retrying #{job.class} due to a #{ex.class}, which reoccurred on #{job.executions} attempts." end end @@ -112,7 +116,7 @@ module ActiveJob ex = event.payload[:error] error do - "Discarded #{job.class} due to a #{ex.class}. The original exception was #{ex.cause.inspect}." + "Discarded #{job.class} due to a #{ex.class}." end end -- cgit v1.2.3 From 3ebc22903433b97b38935778b955b53bc90447f6 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Tue, 9 Oct 2018 13:46:16 -0400 Subject: Include deserialized arguments in jobs returned by AJ test helpers `assert_enqueued_with` and `assert_performed_with` return a instantiated instance of the matching job for further assertion (#21010). Before this commit the `arguments` method on the returned instance returns a serialized version of the arguments. --- activejob/lib/active_job/test_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index 261b68d4f8..d1a1fd6292 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -594,8 +594,7 @@ module ActiveJob def flush_enqueued_jobs(only: nil, except: nil, queue: nil) enqueued_jobs_with(only: only, except: except, queue: queue) do |payload| - args = ActiveJob::Arguments.deserialize(payload[:args]) - instantiate_job(payload.merge(args: args)).perform_now + instantiate_job(payload).perform_now queue_adapter.performed_jobs << payload end end @@ -613,7 +612,8 @@ module ActiveJob end def instantiate_job(payload) - job = payload[:job].new(*payload[:args]) + args = ActiveJob::Arguments.deserialize(payload[:args]) + job = payload[:job].new(*args) job.scheduled_at = Time.at(payload[:at]) if payload.key?(:at) job.queue_name = payload[:queue] job -- cgit v1.2.3 From 0925d7340e719a1d82fb9faf81233cb33405de51 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Tue, 16 Oct 2018 00:26:18 +0300 Subject: Clarify docs of `ActiveJob::TestHelper` [ci skip] I found a few sentences that should be updated as well. See https://github.com/rails/rails/pull/33571#discussion_r209435886 Follow up #33571 --- activejob/lib/active_job/test_helper.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index 261b68d4f8..229855c5d9 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -197,7 +197,7 @@ module ActiveJob # assert_performed_jobs 2 # end # - # If a block is passed, that block should cause the specified number of + # If a block is passed, asserts that the block will cause the specified number of # jobs to be performed. # # def test_jobs_again @@ -279,7 +279,7 @@ module ActiveJob # end # end # - # If a block is passed, that block should not cause any job to be performed. + # If a block is passed, asserts that the block will not cause any job to be performed. # # def test_jobs_again # assert_no_performed_jobs do @@ -347,7 +347,7 @@ module ActiveJob # end # # - # If a block is passed, that block should cause the job to be + # If a block is passed, asserts that the block will cause the job to be # enqueued with the given arguments. # # def test_assert_enqueued_with -- cgit v1.2.3 From ee9fc120242b83c64bf77af1b72082ac76319332 Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Wed, 26 Sep 2018 17:25:20 +0100 Subject: Make AJ::Base#enqueue return false if the job wasn't enqueued --- activejob/lib/active_job/callbacks.rb | 3 +++ activejob/lib/active_job/enqueuing.rb | 13 ++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/callbacks.rb b/activejob/lib/active_job/callbacks.rb index 334b24fb3b..e0e48d3ef5 100644 --- a/activejob/lib/active_job/callbacks.rb +++ b/activejob/lib/active_job/callbacks.rb @@ -29,6 +29,9 @@ module ActiveJob included do define_callbacks :perform define_callbacks :enqueue + + class_attribute :return_false_on_aborted_enqueue, instance_accessor: false, instance_predicate: false + self.return_false_on_aborted_enqueue = false end # These methods will be included into any Active Job object, adding diff --git a/activejob/lib/active_job/enqueuing.rb b/activejob/lib/active_job/enqueuing.rb index 53cb98fc71..5be5618390 100644 --- a/activejob/lib/active_job/enqueuing.rb +++ b/activejob/lib/active_job/enqueuing.rb @@ -46,14 +46,25 @@ module ActiveJob self.scheduled_at = options[:wait_until].to_f if options[:wait_until] self.queue_name = self.class.queue_name_from_part(options[:queue]) if options[:queue] self.priority = options[:priority].to_i if options[:priority] + successfully_enqueued = false run_callbacks :enqueue do if scheduled_at self.class.queue_adapter.enqueue_at self, scheduled_at else self.class.queue_adapter.enqueue self end + successfully_enqueued = true + end + if successfully_enqueued + self + else + if self.class.return_false_on_aborted_enqueue + false + else + ActiveSupport::Deprecation.warn "this will return false, set config.active_job.return_false_on_aborted_enqueue = true to remove deprecation." + self + end end - self end end end -- cgit v1.2.3 From 1f7bd2a1711afd71a8a7c5537c677a95fc8978bd Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Tue, 30 Oct 2018 15:33:47 -0400 Subject: Restore HWIA support to AJ::Arguments.deserialize Restore HashWithIndifferentAccess support to ActiveJob::Arguments.deserialize. --- activejob/lib/active_job/arguments.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/arguments.rb b/activejob/lib/active_job/arguments.rb index 8dcf588f35..31347194d2 100644 --- a/activejob/lib/active_job/arguments.rb +++ b/activejob/lib/active_job/arguments.rb @@ -147,7 +147,10 @@ module ActiveJob end def transform_symbol_keys(hash, symbol_keys) - hash.transform_keys do |key| + # NOTE: HashWithIndifferentAccess#transform_keys always + # returns stringified keys with indifferent access + # so we call #to_h here to ensure keys are symbolized. + hash.to_h.transform_keys do |key| if symbol_keys.include?(key) key.to_sym else -- cgit v1.2.3 From b517347be1e229554093b061bd3fd7dbf9017f6a Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Sat, 3 Nov 2018 22:39:18 -0400 Subject: Allow using queue prefix with a default queue name Fixes #34366 Currently setting queue_name_prefix will not combine a prefix with the default_queue_name; it will only affect queue names set with `queue_as`. With this PR the prefix will affect the default_queue_name as well. Closes #19831 Currently setting default_queue_name doesn't actually affect the queue_name default (although default_queue_name does get used if you pass a falsey `part_name` to `queue_as`). This PR would get default_queue_name working as expected as well. Because the queue_name default is now a lambda wrapping the default_queue_name, rather than the default_queue_name itself, I had to update one test to use the instance method `#queue_name` (which `instance_exec`s the value) instead of the class method. I think this change is OK, since only the instance method is documented. There was a question about whether we want a `default_queue_name` configuration. If we want to get rid of it, I would also be happy to open a PR for that instead. It has been around for a while now, but it also hasn't really worked for a while now. r? @matthewd since you had an opinion about this before --- activejob/lib/active_job/queue_name.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/queue_name.rb b/activejob/lib/active_job/queue_name.rb index 9dc6bc7f2e..7bb1e35181 100644 --- a/activejob/lib/active_job/queue_name.rb +++ b/activejob/lib/active_job/queue_name.rb @@ -34,7 +34,7 @@ module ActiveJob end included do - class_attribute :queue_name, instance_accessor: false, default: default_queue_name + class_attribute :queue_name, instance_accessor: false, default: -> { self.class.default_queue_name } class_attribute :queue_name_delimiter, instance_accessor: false, default: "_" end -- cgit v1.2.3 From e899e228e4fb86ed2a63d19c9137059952e954f0 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 9 Nov 2018 20:05:29 +0900 Subject: Restore `private_constant` which is lost accidentally in #30941 https://github.com/rails/rails/pull/30941/files#diff-fc90ec41ef75be8b2259526fe1a8b663L53 --- activejob/lib/active_job/arguments.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/arguments.rb b/activejob/lib/active_job/arguments.rb index 31347194d2..a86a82c6e3 100644 --- a/activejob/lib/active_job/arguments.rb +++ b/activejob/lib/active_job/arguments.rb @@ -62,7 +62,7 @@ module ActiveJob OBJECT_SERIALIZER_KEY, OBJECT_SERIALIZER_KEY.to_sym, WITH_INDIFFERENT_ACCESS_KEY, WITH_INDIFFERENT_ACCESS_KEY.to_sym, ] - private_constant :RESERVED_KEYS + private_constant :RESERVED_KEYS, :GLOBALID_KEY, :SYMBOL_KEYS_KEY, :WITH_INDIFFERENT_ACCESS_KEY def serialize_argument(argument) case argument -- cgit v1.2.3 From 7c9e69dba1238441c9094983b334ee6d313e3820 Mon Sep 17 00:00:00 2001 From: Alberto Almagro Date: Fri, 9 Nov 2018 00:04:26 +0100 Subject: Document missing supported types [ci skip] This commit adds missing types to the supported types list, which was extended in #30941 --- activejob/lib/active_job/arguments.rb | 9 ++++++--- activejob/lib/active_job/enqueuing.rb | 10 ++++++---- 2 files changed, 12 insertions(+), 7 deletions(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/arguments.rb b/activejob/lib/active_job/arguments.rb index a86a82c6e3..38d5d0214e 100644 --- a/activejob/lib/active_job/arguments.rb +++ b/activejob/lib/active_job/arguments.rb @@ -14,11 +14,14 @@ module ActiveJob end # Raised when an unsupported argument type is set as a job argument. We - # currently support NilClass, Integer, Float, String, TrueClass, FalseClass, - # BigDecimal, and objects that can be represented as GlobalIDs (ex: Active Record). + # currently support String, Integer, Float, NilClass, TrueClass, FalseClass, + # BigDecimal, Symbol, Date, Time, DateTime, ActiveSupport::TimeWithZone, + # ActiveSupport::Duration, Hash, ActiveSupport::HashWithIndifferentAccess, + # Array or GlobalID::Identification instances, although this can be extended + # by adding custom serializers. # Raised if you set the key for a Hash something else than a string or # a symbol. Also raised when trying to serialize an object which can't be - # identified with a Global ID - such as an unpersisted Active Record model. + # identified with a GlobalID - such as an unpersisted Active Record model. class SerializationError < ArgumentError; end module Arguments diff --git a/activejob/lib/active_job/enqueuing.rb b/activejob/lib/active_job/enqueuing.rb index 53cb98fc71..b5b9f23c00 100644 --- a/activejob/lib/active_job/enqueuing.rb +++ b/activejob/lib/active_job/enqueuing.rb @@ -9,10 +9,12 @@ module ActiveJob # Includes the +perform_later+ method for job initialization. module ClassMethods - # Push a job onto the queue. The arguments must be legal JSON types - # (+string+, +int+, +float+, +nil+, +true+, +false+, +hash+ or +array+) or - # GlobalID::Identification instances. Arbitrary Ruby objects - # are not supported. + # Push a job onto the queue. By default the arguments must be either String, + # Integer, Float, NilClass, TrueClass, FalseClass, BigDecimal, Symbol, Date, + # Time, DateTime, ActiveSupport::TimeWithZone, ActiveSupport::Duration, + # Hash, ActiveSupport::HashWithIndifferentAccess, Array or + # GlobalID::Identification instances, although this can be extended by adding + # custom serializers. # # Returns an instance of the job class queued with arguments available in # Job#arguments. -- cgit v1.2.3 From 36ea629fae8fce2b478fde6dc9ebf980b6d676ea Mon Sep 17 00:00:00 2001 From: Alberto Almagro Date: Mon, 12 Nov 2018 20:44:59 +0100 Subject: Make `PERMITTED_TYPES` private The constant `PERMITTED_TYPES` is only used by the private method `serialize_argument` and it already has the `# :nodoc:` annotation as the other constants in the class. Complements e899e22 --- activejob/lib/active_job/arguments.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/arguments.rb b/activejob/lib/active_job/arguments.rb index 38d5d0214e..ffc57dae84 100644 --- a/activejob/lib/active_job/arguments.rb +++ b/activejob/lib/active_job/arguments.rb @@ -26,9 +26,6 @@ module ActiveJob module Arguments extend self - # :nodoc: - PERMITTED_TYPES = [ NilClass, String, Integer, Float, BigDecimal, TrueClass, FalseClass ] - # Serializes a set of arguments. Intrinsic types that can safely be # serialized without mutation are returned as-is. Arrays/Hashes are # serialized element by element. All other types are serialized using @@ -49,6 +46,8 @@ module ActiveJob private + # :nodoc: + PERMITTED_TYPES = [ NilClass, String, Integer, Float, BigDecimal, TrueClass, FalseClass ] # :nodoc: GLOBALID_KEY = "_aj_globalid" # :nodoc: @@ -65,7 +64,7 @@ module ActiveJob OBJECT_SERIALIZER_KEY, OBJECT_SERIALIZER_KEY.to_sym, WITH_INDIFFERENT_ACCESS_KEY, WITH_INDIFFERENT_ACCESS_KEY.to_sym, ] - private_constant :RESERVED_KEYS, :GLOBALID_KEY, :SYMBOL_KEYS_KEY, :WITH_INDIFFERENT_ACCESS_KEY + private_constant :PERMITTED_TYPES, :RESERVED_KEYS, :GLOBALID_KEY, :SYMBOL_KEYS_KEY, :WITH_INDIFFERENT_ACCESS_KEY def serialize_argument(argument) case argument -- cgit v1.2.3 From 308d637550f4b1ec7550df3fa6ab8db9fe7c81e8 Mon Sep 17 00:00:00 2001 From: jacobherrington Date: Sun, 18 Nov 2018 22:36:44 -0600 Subject: Change queueing to queuing in docs and comments [skip ci] My spellchecker flagged this as an incorrect spelling, upon further research it appears to be a point of contention in English. Either way might work. After further examination queuing is much more common in the Rails codebase so making this change will serve to standardize the spelling. --- activejob/lib/active_job/base.rb | 2 +- activejob/lib/active_job/callbacks.rb | 2 +- activejob/lib/active_job/core.rb | 2 +- activejob/lib/active_job/execution.rb | 2 +- activejob/lib/active_job/queue_adapters.rb | 6 +++--- 5 files changed, 7 insertions(+), 7 deletions(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/base.rb b/activejob/lib/active_job/base.rb index 95b1062701..ed41fac4b8 100644 --- a/activejob/lib/active_job/base.rb +++ b/activejob/lib/active_job/base.rb @@ -40,7 +40,7 @@ module ActiveJob #:nodoc: # Records that are passed in are serialized/deserialized using Global # ID. More information can be found in Arguments. # - # To enqueue a job to be performed as soon as the queueing system is free: + # To enqueue a job to be performed as soon as the queuing system is free: # # ProcessPhotoJob.perform_later(photo) # diff --git a/activejob/lib/active_job/callbacks.rb b/activejob/lib/active_job/callbacks.rb index 334b24fb3b..61317c7cfc 100644 --- a/activejob/lib/active_job/callbacks.rb +++ b/activejob/lib/active_job/callbacks.rb @@ -130,7 +130,7 @@ module ActiveJob set_callback(:enqueue, :after, *filters, &blk) end - # Defines a callback that will get called around the enqueueing + # Defines a callback that will get called around the enqueuing # of the job. # # class VideoProcessJob < ActiveJob::Base diff --git a/activejob/lib/active_job/core.rb b/activejob/lib/active_job/core.rb index 62bb5861bb..698153636b 100644 --- a/activejob/lib/active_job/core.rb +++ b/activejob/lib/active_job/core.rb @@ -78,7 +78,7 @@ module ActiveJob end # Returns a hash with the job data that can safely be passed to the - # queueing adapter. + # queuing adapter. def serialize { "job_class" => self.class.name, diff --git a/activejob/lib/active_job/execution.rb b/activejob/lib/active_job/execution.rb index f5a343311f..e96dbcd4c9 100644 --- a/activejob/lib/active_job/execution.rb +++ b/activejob/lib/active_job/execution.rb @@ -26,7 +26,7 @@ module ActiveJob end end - # Performs the job immediately. The job is not sent to the queueing adapter + # Performs the job immediately. The job is not sent to the queuing adapter # but directly executed by blocking the execution of others until it's finished. # # MyJob.new(*args).perform_now diff --git a/activejob/lib/active_job/queue_adapters.rb b/activejob/lib/active_job/queue_adapters.rb index 3e3a474fbb..525e79e302 100644 --- a/activejob/lib/active_job/queue_adapters.rb +++ b/activejob/lib/active_job/queue_adapters.rb @@ -3,7 +3,7 @@ module ActiveJob # == Active Job adapters # - # Active Job has adapters for the following queueing backends: + # Active Job has adapters for the following queuing backends: # # * {Backburner}[https://github.com/nesquena/backburner] # * {Delayed Job}[https://github.com/collectiveidea/delayed_job] @@ -52,7 +52,7 @@ module ActiveJob # # No: The adapter will run jobs at the next opportunity and cannot use perform_later. # - # N/A: The adapter does not support queueing. + # N/A: The adapter does not support queuing. # # NOTE: # queue_classic supports job scheduling since version 3.1. @@ -74,7 +74,7 @@ module ActiveJob # # No: Does not allow the priority of jobs to be configured. # - # N/A: The adapter does not support queueing, and therefore sorting them. + # N/A: The adapter does not support queuing, and therefore sorting them. # # ==== Timeout # -- cgit v1.2.3 From 945fdd76925c9f615bf016717c4c8db2b2955357 Mon Sep 17 00:00:00 2001 From: Bernie Chiu Date: Fri, 5 Oct 2018 15:26:02 +0800 Subject: Permit ActionController::Parameters for serializable Hash --- activejob/lib/active_job/arguments.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/arguments.rb b/activejob/lib/active_job/arguments.rb index ffc57dae84..fa58c50ed0 100644 --- a/activejob/lib/active_job/arguments.rb +++ b/activejob/lib/active_job/arguments.rb @@ -75,14 +75,14 @@ module ActiveJob when Array argument.map { |arg| serialize_argument(arg) } when ActiveSupport::HashWithIndifferentAccess - result = serialize_hash(argument) - result[WITH_INDIFFERENT_ACCESS_KEY] = serialize_argument(true) - result + serialize_indifferent_hash(argument) when Hash symbol_keys = argument.each_key.grep(Symbol).map(&:to_s) result = serialize_hash(argument) result[SYMBOL_KEYS_KEY] = symbol_keys result + when -> (arg) { arg.respond_to?(:permitted?) } + serialize_indifferent_hash(argument.to_h) else Serializers.serialize(argument) end @@ -148,6 +148,12 @@ module ActiveJob end end + def serialize_indifferent_hash(indifferent_hash) + result = serialize_hash(indifferent_hash) + result[WITH_INDIFFERENT_ACCESS_KEY] = serialize_argument(true) + result + end + def transform_symbol_keys(hash, symbol_keys) # NOTE: HashWithIndifferentAccess#transform_keys always # returns stringified keys with indifferent access -- cgit v1.2.3 From cdb16ac576198607916cde6d55fe14cb775a98c9 Mon Sep 17 00:00:00 2001 From: Edouard CHIN Date: Wed, 21 Nov 2018 22:43:14 +0100 Subject: Allow all ActiveJob assertion helper to accept Proc in their `only` kw: - That feature is useful to enqueue or assert that jobs got enqueued or performed based on dynamic conditions. We will be able to leverage that feature to fix all ActionMailer assertion helper issue when a Mailer define a custom delivery job (see next commit). --- .../lib/active_job/queue_adapters/test_adapter.rb | 10 +++++-- activejob/lib/active_job/test_helper.rb | 34 ++++++++++++++++++++-- 2 files changed, 40 insertions(+), 4 deletions(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/queue_adapters/test_adapter.rb b/activejob/lib/active_job/queue_adapters/test_adapter.rb index f73ad444ba..c134257ebc 100644 --- a/activejob/lib/active_job/queue_adapters/test_adapter.rb +++ b/activejob/lib/active_job/queue_adapters/test_adapter.rb @@ -65,11 +65,17 @@ module ActiveJob def filtered_job_class?(job) if filter - !Array(filter).include?(job.class) + !filter_as_proc(filter).call(job) elsif reject - Array(reject).include?(job.class) + filter_as_proc(reject).call(job) end end + + def filter_as_proc(filter) + return filter if filter.is_a?(Proc) + + ->(job) { Array(filter).include?(job.class) } + end end end end diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index 0deb68d0d2..e0a71f4da8 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -107,6 +107,9 @@ module ActiveJob # end # end # + # +:only+ and +:except+ options accepts Class, Array of Class or Proc. When passed a Proc, + # a hash containing the job's class and it's argument are passed as argument. + # # Asserts the number of times a job is enqueued to a specific queue by passing +:queue+ option. # # def test_logging_job @@ -163,6 +166,9 @@ module ActiveJob # end # end # + # +:only+ and +:except+ options accepts Class, Array of Class or Proc. When passed a Proc, + # a hash containing the job's class and it's argument are passed as argument. + # # Asserts that no jobs are enqueued to a specific queue by passing +:queue+ option # # def test_no_logging @@ -243,6 +249,18 @@ module ActiveJob # end # end # + # A proc may also be specified. When passed a Proc, the job's instance will be passed as argument. + # + # def test_hello_and_logging_jobs + # assert_nothing_raised do + # assert_performed_jobs(1, only: ->(job) { job.is_a?(HelloJob) }) do + # HelloJob.perform_later('jeremy') + # LoggingJob.perform_later('stewie') + # RescueJob.perform_later('david') + # end + # end + # end + # # If the +:queue+ option is specified, # then only the job(s) enqueued to a specific queue will be performed. # @@ -305,6 +323,9 @@ module ActiveJob # end # end # + # +:only+ and +:except+ options accepts Class, Array of Class or Proc. When passed a Proc, + # an instance of the job will be passed as argument. + # # If the +:queue+ option is specified, # then only the job(s) enqueued to a specific queue will not be performed. # @@ -505,6 +526,9 @@ module ActiveJob # assert_performed_jobs 1 # end # + # +:only+ and +:except+ options accepts Class, Array of Class or Proc. When passed a Proc, + # an instance of the job will be passed as argument. + # # If the +:queue+ option is specified, # then only the job(s) enqueued to a specific queue will be performed. # @@ -569,9 +593,9 @@ module ActiveJob job_class = job.fetch(:job) if only - next false unless Array(only).include?(job_class) + next false unless filter_as_proc(only).call(job) elsif except - next false if Array(except).include?(job_class) + next false if filter_as_proc(except).call(job) end if queue @@ -584,6 +608,12 @@ module ActiveJob end end + def filter_as_proc(filter) + return filter if filter.is_a?(Proc) + + ->(job) { Array(filter).include?(job.fetch(:job)) } + end + def enqueued_jobs_with(only: nil, except: nil, queue: nil, &block) jobs_with(enqueued_jobs, only: only, except: except, queue: queue, &block) end -- cgit v1.2.3 From 95d9c3b3d6828b8ce37591e68d4239ce8c18460b Mon Sep 17 00:00:00 2001 From: Alberto Almagro Date: Fri, 23 Nov 2018 20:31:14 +0100 Subject: Keep executions for each specific exception (#34352) * Keep executions for each specific declaration Fixes #34337 ActiveJob used the global executions counter to control the number of times a job should be retried. The problem with this approach was that in case a job raised different exceptions during its executions they weren't retried the number of times defined by their `attemps` number. **Example:** Having the following job: ```ruby class BuggyJob < ActiveJob::Base retry_on CustomException, attemps: 3 retry_on OtherException, attempts: 3 end ``` If the job raised `CustomException` in the first two executions and then it raised `OtherException`, the job wasn't retried anymore because the global executions counter was already indicating 3 attempts. With this patch each `retry_on` declaration has its specific counter so that the first two executions that raise `CustomException` don't affect the retries count that future exceptions may have. * Revert "clarifies documentation around the attempts arugment to retry_on" This reverts commit 86aa8f8c5631f77ed9a208e5107003c01512133e. --- activejob/lib/active_job/core.rb | 9 +++++++++ activejob/lib/active_job/exceptions.rb | 8 ++++---- 2 files changed, 13 insertions(+), 4 deletions(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/core.rb b/activejob/lib/active_job/core.rb index 698153636b..4ab62f89b0 100644 --- a/activejob/lib/active_job/core.rb +++ b/activejob/lib/active_job/core.rb @@ -28,6 +28,12 @@ module ActiveJob # Number of times this job has been executed (which increments on every retry, like after an exception). attr_accessor :executions + # Hash that contains the number of times this job handled errors for each specific retry_on declaration. + # Keys are the string representation of the exceptions listed in the retry_on declaration, + # while its associated value holds the number of executions where the corresponding retry_on + # declaration handled one of its listed exceptions. + attr_accessor :exception_executions + # I18n.locale to be used during the job. attr_accessor :locale @@ -75,6 +81,7 @@ module ActiveJob @queue_name = self.class.queue_name @priority = self.class.priority @executions = 0 + @exception_executions = Hash.new(0) end # Returns a hash with the job data that can safely be passed to the @@ -88,6 +95,7 @@ module ActiveJob "priority" => priority, "arguments" => serialize_arguments_if_needed(arguments), "executions" => executions, + "exception_executions" => exception_executions, "locale" => I18n.locale.to_s, "timezone" => Time.zone.try(:name) } @@ -126,6 +134,7 @@ module ActiveJob self.priority = job_data["priority"] self.serialized_arguments = job_data["arguments"] self.executions = job_data["executions"] + self.exception_executions = job_data["exception_executions"] self.locale = job_data["locale"] || I18n.locale.to_s self.timezone = job_data["timezone"] || Time.zone.try(:name) end diff --git a/activejob/lib/active_job/exceptions.rb b/activejob/lib/active_job/exceptions.rb index bc9e168971..53984a4e49 100644 --- a/activejob/lib/active_job/exceptions.rb +++ b/activejob/lib/active_job/exceptions.rb @@ -9,7 +9,6 @@ module ActiveJob module ClassMethods # Catch the exception and reschedule job for re-execution after so many seconds, for a specific number of attempts. - # The number of attempts includes the total executions of a job, not just the retried executions. # If the exception keeps getting raised beyond the specified number of attempts, the exception is allowed to # bubble up to the underlying queuing system, which may have its own retry mechanism or place it in a # holding queue for inspection. @@ -22,8 +21,7 @@ module ActiveJob # as a computing proc that the number of executions so far as an argument, or as a symbol reference of # :exponentially_longer, which applies the wait algorithm of (executions ** 4) + 2 # (first wait 3s, then 18s, then 83s, etc) - # * :attempts - Re-enqueues the job the specified number of times (default: 5 attempts), - # attempts here refers to the total number of times the job is executed, not just retried executions + # * :attempts - Re-enqueues the job the specified number of times (default: 5 attempts) # * :queue - Re-enqueues the job on a different queue # * :priority - Re-enqueues the job with a different priority # @@ -46,7 +44,9 @@ module ActiveJob # end def retry_on(*exceptions, wait: 3.seconds, attempts: 5, queue: nil, priority: nil) rescue_from(*exceptions) do |error| - if executions < attempts + exception_executions[exceptions.to_s] += 1 + + if exception_executions[exceptions.to_s] < attempts retry_job wait: determine_delay(wait), queue: queue, priority: priority, error: error else if block_given? -- cgit v1.2.3 From 72300f9742745f9535b06d45a9632e948ed7d79b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Wed, 5 Sep 2018 17:38:09 -0400 Subject: Do not deserialize GlobalID objects that were not generated by Active Job Trusting any GlobaID object when deserializing jobs can allow attackers to access information that should not be accessible to them. Fix CVE-2018-16476. --- activejob/lib/active_job/arguments.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/arguments.rb b/activejob/lib/active_job/arguments.rb index fa58c50ed0..92eb58aaaf 100644 --- a/activejob/lib/active_job/arguments.rb +++ b/activejob/lib/active_job/arguments.rb @@ -91,7 +91,7 @@ module ActiveJob def deserialize_argument(argument) case argument when String - GlobalID::Locator.locate(argument) || argument + argument when *PERMITTED_TYPES argument when Array -- cgit v1.2.3 From 884310fdd031ed8121944f9ea07c8b7723c4e6b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Wed, 5 Dec 2018 13:37:48 -0500 Subject: Improve deprecation message for enqueue returning false And make sure new applications in Rails 6.0 has this config enabled. Also, improve test coverage and add a CHANGELOG entry. --- activejob/lib/active_job/enqueuing.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'activejob/lib/active_job') diff --git a/activejob/lib/active_job/enqueuing.rb b/activejob/lib/active_job/enqueuing.rb index 58e6d3348d..ce118c1e8a 100644 --- a/activejob/lib/active_job/enqueuing.rb +++ b/activejob/lib/active_job/enqueuing.rb @@ -49,21 +49,29 @@ module ActiveJob self.queue_name = self.class.queue_name_from_part(options[:queue]) if options[:queue] self.priority = options[:priority].to_i if options[:priority] successfully_enqueued = false + run_callbacks :enqueue do if scheduled_at self.class.queue_adapter.enqueue_at self, scheduled_at else self.class.queue_adapter.enqueue self end + successfully_enqueued = true end + if successfully_enqueued self else if self.class.return_false_on_aborted_enqueue false else - ActiveSupport::Deprecation.warn "this will return false, set config.active_job.return_false_on_aborted_enqueue = true to remove deprecation." + ActiveSupport::Deprecation.warn( + "Rails 6.0 will return false when the enqueing is aborted. Make sure your code doesn't depend on it" \ + " returning the instance of the job and set `config.active_job.return_false_on_aborted_enqueue = true`" \ + " to remove the deprecations." + ) + self end end -- cgit v1.2.3