Skip to content

Commit

Permalink
Deprecate un(de)serializable cursor & add config
Browse files Browse the repository at this point in the history
Version 2.0 of the gem will forbid cursors that are not (de)serializable using
built in types. This is because adapters serialize to JSON, and we must ensure
this is possible in a lossless fashion.

Ahead of this change, we introduce a deprecation warning, as well as a
configuration which makes it possible to enable the strict behavior.
This configuration can be set globally, and/or at the job level.

For instance, it is possible to enable it globally, but disable it for a jobs
that have yet to be migrated, or alternatively to enable it for jobs that are
already compliant, but leave it disabled globally to allow for non-compliant
jobs.

In theory, any non-compliant jobs are "already broken", however it is possible
that the job works around the "bug" in (de)serialization. One known scenario is
where jobs complete rapidly enough to never actually require cursor
serialization, and as such never actually encounter the problem of
un(de)serializable cursors in production environments. However, this could
technically occur at any time.
  • Loading branch information
sambostock committed Jul 1, 2022
1 parent 41c5224 commit f8eb4f4
Show file tree
Hide file tree
Showing 4 changed files with 248 additions and 97 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

- [241](https://github.com/Shopify/job-iteration/pull/241) - Require Ruby 2.7+, dropping 2.6 support
- [241](https://github.com/Shopify/job-iteration/pull/241) - Require Rails 6.0+, dropping 5.2 support
- [80](https://github.com/Shopify/job-iteration/pull/80) - Deprecate un(de)serializable cursors
- [80](https://github.com/Shopify/job-iteration/pull/80) - Add `enforce_serializable_cursors` config

## v1.3.6 (Mar 9, 2022)

Expand Down
21 changes: 21 additions & 0 deletions lib/job-iteration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ module JobIteration

INTEGRATIONS = [:resque, :sidekiq]

Deprecation = ActiveSupport::Deprecation.new("2.0", "JobIteration")

extend self

# Use this to _always_ interrupt the job after it's been running for more than N seconds.
Expand All @@ -20,6 +22,25 @@ module JobIteration
# Defaults to nil which means that jobs will not be interrupted except on termination signal.
attr_accessor :max_job_runtime

# Set this to `true` to enforce that cursors be composed of objects capable
# of built-in (de)serialization: Strings, Integers, Floats, Arrays, Hashes,
# true, false, or nil.
#
# JobIteration.enforce_serializable_cursors = true
#
# For more granular control, this can also be configured per job class, and
# is inherited by child classes.
#
# class MyJob < ActiveJob::Base
# include JobIteration::Iteration
# self.enforce_serializable_cursors = false
# # ...
# end
#
# Note that non-enforcement is deprecated and enforcement will be mandatory
# in version 2.0, at which point this config will go away.
attr_accessor :enforce_serializable_cursors

# Used internally for hooking into job processing frameworks like Sidekiq and Resque.
attr_accessor :interruption_adapter

Expand Down
62 changes: 37 additions & 25 deletions lib/job-iteration/iteration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,17 @@ module Iteration
:total_time,
)

class CursorError < ArgumentError
attr_reader :cursor

def initialize(message, cursor:)
super(message)
@cursor = cursor
end

def message
"#{super} (#{inspected_cursor})"
end

private

def inspected_cursor
cursor.inspect
rescue NoMethodError
# For those brave enough to try to use BasicObject as cursor. Nice try.
Object.instance_method(:inspect).bind(cursor).call
end
end

included do |_base|
define_callbacks :start
define_callbacks :shutdown
define_callbacks :complete

class_attribute(
:enforce_serializable_cursors,
instance_writer: false,
instance_predicate: false,
default: JobIteration.enforce_serializable_cursors,
)
end

module ClassMethods
Expand Down Expand Up @@ -66,6 +51,28 @@ def ban_perform_definition
end
end

class CursorError < ArgumentError
attr_reader :cursor

def initialize(message, cursor:)
super(message)
@cursor = cursor
end

def message
"#{super} (#{inspected_cursor})"
end

private

def inspected_cursor
cursor.inspect
rescue NoMethodError
# For those brave enough to try to use BasicObject as cursor. Nice try.
Object.instance_method(:inspect).bind(cursor).call
end
end

def initialize(*arguments)
super
@job_iteration_retry_backoff = nil
Expand Down Expand Up @@ -149,8 +156,7 @@ def iterate_with_enumerator(enumerator, arguments)
@needs_reenqueue = false

enumerator.each do |object_from_enumerator, index|
# Deferred until 2.0.0
# assert_valid_cursor!(index)
assert_valid_cursor!(index)

record_unit_of_work do
found_record = true
Expand Down Expand Up @@ -218,7 +224,13 @@ def assert_valid_cursor!(cursor)
"Cursor must be composed of objects capable of built-in (de)serialization: " \
"Strings, Integers, Floats, Arrays, Hashes, true, false, or nil.",
cursor: cursor,
)
) if enforce_serializable_cursors

Deprecation.warn(<<~DEPRECATION_MESSAGE)
The Enumerator returned by #{self.class.name}#build_enumerator yielded a cursor which is unsafe to serialize.
Cursors must be composed of objects capable of built-in (de)serialization: Strings, Integers, Floats, Arrays, Hashes, true, false, or nil.
This will raise starting in version #{Deprecation.deprecation_horizon} of #{Deprecation.gem_name}!"
DEPRECATION_MESSAGE
end

def assert_implements_methods!
Expand Down
Loading

0 comments on commit f8eb4f4

Please sign in to comment.