Skip to content

Commit

Permalink
Deprecate usage of cursors not safe to serialize
Browse files Browse the repository at this point in the history
It is unsafe to use cursors which cannot be serialized using JSON.dump
and deserialized again using JSON.parse. Such cursors may result in a
different cursor being deserialized, or in serialization failing
entirely, which may result in incorrect behaviour in iterative jobs.

Starting in 2.0, usage of such cursors will raise an ArgumentError.
Until then, a deprecation warning will be logged.
  • Loading branch information
sambostock committed Aug 27, 2021
1 parent 350910b commit c148ad4
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 60 deletions.
2 changes: 2 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 Down
35 changes: 6 additions & 29 deletions lib/job-iteration/iteration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,6 @@ module JobIteration
module Iteration
extend ActiveSupport::Concern

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|
attr_accessor(
:cursor_position,
Expand Down Expand Up @@ -142,8 +120,7 @@ def iterate_with_enumerator(enumerator, arguments)
arguments = arguments.dup.freeze
found_record = 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 @@ -208,11 +185,11 @@ def build_enumerator(params, cursor:)
def assert_valid_cursor!(cursor)
return if serializable?(cursor)

raise CursorError.new(
"Cursor must be composed of objects capable of built-in (de)serialization: " \
"Strings, Integers, Floats, Arrays, Hashes, true, false, or nil.",
cursor: cursor,
)
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
2 changes: 1 addition & 1 deletion test/integration/integration_behaviour.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ module IntegrationBehaviour
end

test "unserializable corruption is prevented" do
skip "Deferred until 2.0.0"
skip "Breaking change deferred until 2.0" if Gem::Version.new(JobIteration::VERSION) < Gem::Version.new("2.0")
# Cursors are serialized as JSON, but not all objects are serializable.
# time = Time.at(0).utc # => 1970-01-01 00:00:00 UTC
# json = JSON.dump(time) # => "\"1970-01-01 00:00:00 UTC\""
Expand Down
81 changes: 51 additions & 30 deletions test/unit/iteration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ def each_iteration(*)
class InvalidCursorJob < ActiveJob::Base
include JobIteration::Iteration
def each_iteration(*)
return if Gem::Version.new(JobIteration::VERSION) < Gem::Version.new("2.0")
raise "Cursor invalid. This should never run!"
end
end
Expand Down Expand Up @@ -199,41 +200,37 @@ def foo
assert_includes(methods_added, :foo)
end

def test_jobs_using_time_cursor_will_raise
skip("Deferred until 2.0.0")
def test_jobs_using_time_cursor_is_deprecated
push(JobWithTimeCursor)
assert_raises_cursor_error { work_one_job }
assert_cursor_deprecation_warning { work_one_job }
end

def test_jobs_using_active_record_cursor_will_raise
skip("Deferred until 2.0.0")
def test_jobs_using_active_record_cursor_is_deprecated
refute_nil(Product.first)
push(JobWithActiveRecordCursor)
assert_raises_cursor_error { work_one_job }
assert_cursor_deprecation_warning { work_one_job }
end

def test_jobs_using_symbol_cursor_will_raise
skip("Deferred until 2.0.0")
def test_jobs_using_symbol_cursor_is_deprecated
push(JobWithSymbolCursor)
assert_raises_cursor_error { work_one_job }
assert_cursor_deprecation_warning { work_one_job }
end

def test_jobs_using_string_subclass_cursor_will_raise
skip("Deferred until 2.0.0")
def test_jobs_using_string_subclass_cursor_is_deprecated
push(JobWithStringSubclassCursor)
assert_raises_cursor_error { work_one_job }
assert_cursor_deprecation_warning { work_one_job }
end

def test_jobs_using_basic_object_cursor_will_raise
skip("Deferred until 2.0.0")
def test_jobs_using_basic_object_cursor_is_deprecated
push(JobWithBasicObjectCursor)
assert_raises_cursor_error { work_one_job }
assert_cursor_deprecation_warning { work_one_job }
end

def test_jobs_using_complex_but_serializable_cursor_will_not_raise
skip("Deferred until 2.0.0")
def test_jobs_using_complex_but_serializable_cursor_is_not_deprecated
push(JobWithComplexCursor)
work_one_job
assert_no_cursor_deprecation_warning do
work_one_job
end
end

def test_jobs_using_on_complete_have_accurate_total_time
Expand All @@ -244,21 +241,45 @@ def test_jobs_using_on_complete_have_accurate_total_time

private

def assert_raises_cursor_error(&block)
error = assert_raises(JobIteration::Iteration::CursorError, &block)
inspected_cursor = begin
error.cursor.inspect
rescue NoMethodError
Object.instance_method(:inspect).bind(error.cursor).call
end
assert_equal(
"Cursor must be composed of objects capable of built-in (de)serialization: " \
"Strings, Integers, Floats, Arrays, Hashes, true, false, or nil. " \
"(#{inspected_cursor})",
error.message,
def assert_cursor_deprecation_warning(&block)
job_class = ActiveJob::Base.queue_adapter.enqueued_jobs.first.fetch("job_class")
expected_message = <<~MESSAGE.chomp
DEPRECATION WARNING: The Enumerator returned by #{job_class}#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 #{JobIteration::Deprecation.deprecation_horizon} of #{JobIteration::Deprecation.gem_name}!
MESSAGE

warned = false
with_deprecation_behavior(
lambda do |message, *|
flunk("expected only one deprecation warning") if warned
warned = true
assert(
message.start_with?(expected_message),
"expected deprecation warning \n#{message.inspect}\n to start_with? \n#{expected_message.inspect}",
)
end,
&block
)

assert(warned, "expected deprecation warning")
end

def assert_no_cursor_deprecation_warning(&block)
with_deprecation_behavior(
-> (message, *) { flunk("Expected no deprecation warning: #{message}") },
&block
)
end

def with_deprecation_behavior(behavior)
original_behaviour = JobIteration::Deprecation.behavior
JobIteration::Deprecation.behavior = behavior
yield
ensure
JobIteration::Deprecation.behavior = original_behaviour
end

def push(job, *args)
job.perform_later(*args)
end
Expand Down

0 comments on commit c148ad4

Please sign in to comment.