Deprecate usage of cursors not safe to serialize

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.

Author: sambostock

lib/job-iteration.rb

@@ -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.

lib/job-iteration/iteration.rb

@@ -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,
@@ -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
@@ -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!

test/integration/integration_behaviour.rb

@@ -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\""

test/unit/iteration_test.rb

@@ -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
@@ -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
@@ -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